[v8,0/7] add thermal sensor driver for A64, A83T, H3, H5, H6, R40
mbox series

Message ID 20191219172823.1652600-1-anarsoul@gmail.com
Headers show
Series
  • add thermal sensor driver for A64, A83T, H3, H5, H6, R40
Related show

Message

Vasily Khoruzhick Dec. 19, 2019, 5:28 p.m. UTC
This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
H6 and R40 SoCs.

v8:
	- [vasily] Address more Maxime's comments for dt-schema
	- [vasily] Add myself to MAINTAINERS for the driver and schema
	- [vasily] Round calibration data size to word boundary for H6 and A64
	- [vasily] Change offset for A64 since it reports too low temp otherwise.
	           Likely conversion formula in user manual is not correct.

v7:
	- [vasily] Address Maxime's comments for dt-schema
	- [vasily] Move common part of H3 and H5 dts into sunxi-h3-h5.dtsi
	- [vasily] Add Maxime's a-b to the driver patch 

v6:
	- [ondrej, vasily] Squash all driver related changes into a
			   single patch
	- [ondrej] Rename calib -> calibration
	- [ondrej] Fix thermal zone registration check
	- [ondrej] Lower rate of sensor data interrupts to 4/sec/sensor
	- [ondrej] Rework scale/offset values, H6 calibration
	- [ondrej] Explicitly set mod clock to 24 MHz
	- [ondrej] Set undocumented bits in CTRL0 for H6
	- [ondrej] Add support for A83T
	- [ondrej] Add dts changes for A83T, H3, H5, H6
	- [vasily] Add dts changes for A64
	- [vasily] Address Maxime's comments for YAML scheme
	- [vasily] Make .calc_temp callback mandatory
	- [vasily] Set .max_register in regmap config, so regs can be
		   inspected using debugfs

Ondrej Jirman (4):
  ARM: dts: sun8i-a83t: Add thermal sensor and thermal zones
  ARM: dts: sun8i-h3: Add thermal sensor and thermal zones
  arm64: dts: allwinner: h5: Add thermal sensor and thermal zones
  arm64: dts: allwinner: h6: Add thermal sensor and thermal zones

Vasily Khoruzhick (1):
  arm64: dts: allwinner: a64: Add thermal sensors and thermal zones

Yangtao Li (2):
  thermal: sun8i: add thermal driver for H6/H5/H3/A64/A83T/R40
  dt-bindings: thermal: add YAML schema for sun8i-thermal driver
    bindings

 .../thermal/allwinner,sun8i-a83t-ths.yaml     | 160 +++++
 MAINTAINERS                                   |   8 +
 arch/arm/boot/dts/sun8i-a83t.dtsi             |  36 +
 arch/arm/boot/dts/sun8i-h3.dtsi               |  20 +
 arch/arm/boot/dts/sunxi-h3-h5.dtsi            |   6 +
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  42 ++
 arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  26 +
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 +
 drivers/thermal/Kconfig                       |  14 +
 drivers/thermal/Makefile                      |   1 +
 drivers/thermal/sun8i_thermal.c               | 639 ++++++++++++++++++
 11 files changed, 985 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
 create mode 100644 drivers/thermal/sun8i_thermal.c

Comments

Maxime Ripard Dec. 19, 2019, 5:33 p.m. UTC | #1
Hi,

On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> H6 and R40 SoCs.

Thanks again for working on this.

I'll merge the DT patches when the driver will have been merged.

Maxime
Corentin Labbe Dec. 24, 2019, 1:11 p.m. UTC | #2
On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> H6 and R40 SoCs.
> 
> v8:
> 	- [vasily] Address more Maxime's comments for dt-schema
> 	- [vasily] Add myself to MAINTAINERS for the driver and schema
> 	- [vasily] Round calibration data size to word boundary for H6 and A64
> 	- [vasily] Change offset for A64 since it reports too low temp otherwise.
> 	           Likely conversion formula in user manual is not correct.
> 
> v7:
> 	- [vasily] Address Maxime's comments for dt-schema
> 	- [vasily] Move common part of H3 and H5 dts into sunxi-h3-h5.dtsi
> 	- [vasily] Add Maxime's a-b to the driver patch 
> 
> v6:
> 	- [ondrej, vasily] Squash all driver related changes into a
> 			   single patch
> 	- [ondrej] Rename calib -> calibration
> 	- [ondrej] Fix thermal zone registration check
> 	- [ondrej] Lower rate of sensor data interrupts to 4/sec/sensor
> 	- [ondrej] Rework scale/offset values, H6 calibration
> 	- [ondrej] Explicitly set mod clock to 24 MHz
> 	- [ondrej] Set undocumented bits in CTRL0 for H6
> 	- [ondrej] Add support for A83T
> 	- [ondrej] Add dts changes for A83T, H3, H5, H6
> 	- [vasily] Add dts changes for A64
> 	- [vasily] Address Maxime's comments for YAML scheme
> 	- [vasily] Make .calc_temp callback mandatory
> 	- [vasily] Set .max_register in regmap config, so regs can be
> 		   inspected using debugfs
> 
> Ondrej Jirman (4):
>   ARM: dts: sun8i-a83t: Add thermal sensor and thermal zones
>   ARM: dts: sun8i-h3: Add thermal sensor and thermal zones
>   arm64: dts: allwinner: h5: Add thermal sensor and thermal zones
>   arm64: dts: allwinner: h6: Add thermal sensor and thermal zones
> 
> Vasily Khoruzhick (1):
>   arm64: dts: allwinner: a64: Add thermal sensors and thermal zones
> 
> Yangtao Li (2):
>   thermal: sun8i: add thermal driver for H6/H5/H3/A64/A83T/R40
>   dt-bindings: thermal: add YAML schema for sun8i-thermal driver
>     bindings
> 
>  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 160 +++++
>  MAINTAINERS                                   |   8 +
>  arch/arm/boot/dts/sun8i-a83t.dtsi             |  36 +
>  arch/arm/boot/dts/sun8i-h3.dtsi               |  20 +
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |   6 +
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  42 ++
>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  26 +
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 +
>  drivers/thermal/Kconfig                       |  14 +
>  drivers/thermal/Makefile                      |   1 +
>  drivers/thermal/sun8i_thermal.c               | 639 ++++++++++++++++++
>  11 files changed, 985 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
>  create mode 100644 drivers/thermal/sun8i_thermal.c
> 
> -- 
> 2.24.1
> 

Hello

Thanks for your work.

Tested-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Tested-on: sun50i-h6-pine-h64-model-b
Tested-on: sun50i-h6-pine-h64
Tested-on: sun8i-a83t-bananapi-m3
Tested-on: sun8i-h2-plus-orangepi-zero
Tested-on: sun8i-h2-plus-orangepi-r1
Tested-on: sun8i-h2-plus-libretech-all-h3-cc
Tested-on: sun8i-h3-libretech-all-h3-cc
Tested-on: sun50i-h5-libretech-all-h3-cc
Tested-on: sun50i-a64-pine64-plus

Note that your patch serie support R40 but you do not have any R40 DT patch.
If you need testing, do not hesitate to ask.

Regards
Yangtao Li Dec. 24, 2019, 2:36 p.m. UTC | #3
On Tue, Dec 24, 2019 at 9:11 PM Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
>
> On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> > This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> > H6 and R40 SoCs.
> >
> > v8:
> >       - [vasily] Address more Maxime's comments for dt-schema
> >       - [vasily] Add myself to MAINTAINERS for the driver and schema
> >       - [vasily] Round calibration data size to word boundary for H6 and A64
> >       - [vasily] Change offset for A64 since it reports too low temp otherwise.
> >                  Likely conversion formula in user manual is not correct.
> >
> > v7:
> >       - [vasily] Address Maxime's comments for dt-schema
> >       - [vasily] Move common part of H3 and H5 dts into sunxi-h3-h5.dtsi
> >       - [vasily] Add Maxime's a-b to the driver patch
> >
> > v6:
> >       - [ondrej, vasily] Squash all driver related changes into a
> >                          single patch
> >       - [ondrej] Rename calib -> calibration
> >       - [ondrej] Fix thermal zone registration check
> >       - [ondrej] Lower rate of sensor data interrupts to 4/sec/sensor
> >       - [ondrej] Rework scale/offset values, H6 calibration
> >       - [ondrej] Explicitly set mod clock to 24 MHz
> >       - [ondrej] Set undocumented bits in CTRL0 for H6
> >       - [ondrej] Add support for A83T
> >       - [ondrej] Add dts changes for A83T, H3, H5, H6
> >       - [vasily] Add dts changes for A64
> >       - [vasily] Address Maxime's comments for YAML scheme
> >       - [vasily] Make .calc_temp callback mandatory
> >       - [vasily] Set .max_register in regmap config, so regs can be
> >                  inspected using debugfs
> >
> > Ondrej Jirman (4):
> >   ARM: dts: sun8i-a83t: Add thermal sensor and thermal zones
> >   ARM: dts: sun8i-h3: Add thermal sensor and thermal zones
> >   arm64: dts: allwinner: h5: Add thermal sensor and thermal zones
> >   arm64: dts: allwinner: h6: Add thermal sensor and thermal zones
> >
> > Vasily Khoruzhick (1):
> >   arm64: dts: allwinner: a64: Add thermal sensors and thermal zones
> >
> > Yangtao Li (2):
> >   thermal: sun8i: add thermal driver for H6/H5/H3/A64/A83T/R40
> >   dt-bindings: thermal: add YAML schema for sun8i-thermal driver
> >     bindings
> >
> >  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 160 +++++
> >  MAINTAINERS                                   |   8 +
> >  arch/arm/boot/dts/sun8i-a83t.dtsi             |  36 +
> >  arch/arm/boot/dts/sun8i-h3.dtsi               |  20 +
> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |   6 +
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  42 ++
> >  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  26 +
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 +
> >  drivers/thermal/Kconfig                       |  14 +
> >  drivers/thermal/Makefile                      |   1 +
> >  drivers/thermal/sun8i_thermal.c               | 639 ++++++++++++++++++
> >  11 files changed, 985 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> >  create mode 100644 drivers/thermal/sun8i_thermal.c
> >
> > --
> > 2.24.1
> >
>
> Hello
>
> Thanks for your work.
>
> Tested-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> Tested-on: sun50i-h6-pine-h64-model-b
> Tested-on: sun50i-h6-pine-h64
> Tested-on: sun8i-a83t-bananapi-m3
> Tested-on: sun8i-h2-plus-orangepi-zero
> Tested-on: sun8i-h2-plus-orangepi-r1
> Tested-on: sun8i-h2-plus-libretech-all-h3-cc
> Tested-on: sun8i-h3-libretech-all-h3-cc
> Tested-on: sun50i-h5-libretech-all-h3-cc
> Tested-on: sun50i-a64-pine64-plus
>
> Note that your patch serie support R40 but you do not have any R40 DT patch.
> If you need testing, do not hesitate to ask.

How about this?  No one has yet added the R40's SID, and I'm not sure about
the available size of the sid. So the current therm sensor is not calibrated,
hope this is available.

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index 421dfbbfd7ee..8ccda5cb873f 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -46,6 +46,7 @@
 #include <dt-bindings/clock/sun8i-r40-ccu.h>
 #include <dt-bindings/reset/sun8i-r40-ccu.h>
 #include <dt-bindings/reset/sun8i-de2.h>
+#include <dt-bindings/thermal/thermal.h>

 / {
        #address-cells = <1>;
@@ -109,6 +110,22 @@
                status = "disabled";
        };

+       thermal-zones {
+               cpu_thermal: cpu0-thermal {
+                       /* milliseconds */
+                       polling-delay-passive = <0>;
+                       polling-delay = <0>;
+                       thermal-sensors = <&ths 0>;
+               };
+
+               gpu_thermal: gpu-thermal {
+                       /* milliseconds */
+                       polling-delay-passive = <0>;
+                       polling-delay = <0>;
+                       thermal-sensors = <&ths 1>;
+               };
+       };
+
        soc {
                compatible = "simple-bus";
                #address-cells = <1>;
@@ -421,6 +438,17 @@
                        clocks = <&osc24M>;
                };

+               ths: thermal-sensor@1c24c00 {
+                       compatible = "allwinner,sun8i-r40-ths";
+                       reg = <0x01c24c00 0x100>;
+                       clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+                       clock-names = "bus", "mod";
+                       interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+                       resets = <&ccu RST_BUS_THS>;
+                       /* TODO: add nvmem-cells for calibration */
+                       #thermal-sensor-cells = <1>;
+               };
+
                uart0: serial@1c28000 {
                        compatible = "snps,dw-apb-uart";
                        reg = <0x01c28000 0x400>;
diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
index 23a5f4aa4be4..c5661d7c3e20 100644
--- a/drivers/thermal/sun8i_thermal.c
+++ b/drivers/thermal/sun8i_thermal.c
@@ -565,7 +565,7 @@ static const struct ths_thermal_chip sun8i_h3_ths = {
 };

 static const struct ths_thermal_chip sun8i_r40_ths = {
-       .sensor_num = 3,
+       .sensor_num = 2,
        .offset = 251086,
        .scale = 1130,
        .has_mod_clk = true,

MBR,
Yangtao

>
> Regards
Daniel Lezcano Dec. 24, 2019, 6:30 p.m. UTC | #4
On 19/12/2019 18:33, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
>> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
>> H6 and R40 SoCs.
> 
> Thanks again for working on this.
> 
> I'll merge the DT patches when the driver will have been merged.

I've applied patches 1 & 2.

They are in the testing branch and will go to the linux-next branch as
soon as the kernelci passes.
Maxime Ripard Dec. 26, 2019, 9:27 a.m. UTC | #5
On Tue, Dec 24, 2019 at 07:30:55PM +0100, Daniel Lezcano wrote:
> On 19/12/2019 18:33, Maxime Ripard wrote:
> > Hi,
> >
> > On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> >> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> >> H6 and R40 SoCs.
> >
> > Thanks again for working on this.
> >
> > I'll merge the DT patches when the driver will have been merged.
>
> I've applied patches 1 & 2.
>
> They are in the testing branch and will go to the linux-next branch as
> soon as the kernelci passes.

I just merged all the other patches (except the patch 6, for the H6,
as requested by Vasily on IRC).

Thanks to everyone involved!
Maxime
Ondřej Jirman Dec. 26, 2019, 10:37 a.m. UTC | #6
On Thu, Dec 26, 2019 at 10:27:51AM +0100, Maxime Ripard wrote:
> On Tue, Dec 24, 2019 at 07:30:55PM +0100, Daniel Lezcano wrote:
> > On 19/12/2019 18:33, Maxime Ripard wrote:
> > > Hi,
> > >
> > > On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> > >> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> > >> H6 and R40 SoCs.
> > >
> > > Thanks again for working on this.
> > >
> > > I'll merge the DT patches when the driver will have been merged.
> >
> > I've applied patches 1 & 2.
> >
> > They are in the testing branch and will go to the linux-next branch as
> > soon as the kernelci passes.
> 
> I just merged all the other patches (except the patch 6, for the H6,
> as requested by Vasily on IRC).

Hello,

I think you can apply H6 patch. This was just some misunderstanding
and H6 is working.

thnaks and regards,
	o.

> Thanks to everyone involved!
> Maxime



> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Corentin Labbe Dec. 26, 2019, 10:44 a.m. UTC | #7
On Thu, Dec 26, 2019 at 10:27:51AM +0100, Maxime Ripard wrote:
> On Tue, Dec 24, 2019 at 07:30:55PM +0100, Daniel Lezcano wrote:
> > On 19/12/2019 18:33, Maxime Ripard wrote:
> > > Hi,
> > >
> > > On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> > >> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> > >> H6 and R40 SoCs.
> > >
> > > Thanks again for working on this.
> > >
> > > I'll merge the DT patches when the driver will have been merged.
> >
> > I've applied patches 1 & 2.
> >
> > They are in the testing branch and will go to the linux-next branch as
> > soon as the kernelci passes.
> 
> I just merged all the other patches (except the patch 6, for the H6,
> as requested by Vasily on IRC).
> 

Hello

Vasily asked to not apply H6 due to my test failling on h6 and since he didnt own any H6 hw.
But it was failling due my fault (a failling build).

So the patchset work perfect on H6 (opi1+, opi3, pineH64 both model A and B) as reported by my answer to this thread.

Regards
Yangtao Li Dec. 26, 2019, 12:47 p.m. UTC | #8
On Thu, Dec 26, 2019 at 6:44 PM Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
>
> On Thu, Dec 26, 2019 at 10:27:51AM +0100, Maxime Ripard wrote:
> > On Tue, Dec 24, 2019 at 07:30:55PM +0100, Daniel Lezcano wrote:
> > > On 19/12/2019 18:33, Maxime Ripard wrote:
> > > > Hi,
> > > >
> > > > On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> > > >> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> > > >> H6 and R40 SoCs.
> > > >
> > > > Thanks again for working on this.
> > > >
> > > > I'll merge the DT patches when the driver will have been merged.
> > >
> > > I've applied patches 1 & 2.
> > >
> > > They are in the testing branch and will go to the linux-next branch as
> > > soon as the kernelci passes.
> >
> > I just merged all the other patches (except the patch 6, for the H6,
> > as requested by Vasily on IRC).
> >
>
> Hello
>
> Vasily asked to not apply H6 due to my test failling on h6 and since he didnt own any H6 hw.
> But it was failling due my fault (a failling build).
>
> So the patchset work perfect on H6 (opi1+, opi3, pineH64 both model A and B) as reported by my answer to this thread.

HI Corentin,

Although it is not calibrated, it should work on the R40. Can you give
my patch a try?

Thx,
Yangtao

>
> Regards
Corentin Labbe Dec. 26, 2019, 8:26 p.m. UTC | #9
On Thu, Dec 26, 2019 at 08:47:47PM +0800, Frank Lee wrote:
> On Thu, Dec 26, 2019 at 6:44 PM Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> >
> > On Thu, Dec 26, 2019 at 10:27:51AM +0100, Maxime Ripard wrote:
> > > On Tue, Dec 24, 2019 at 07:30:55PM +0100, Daniel Lezcano wrote:
> > > > On 19/12/2019 18:33, Maxime Ripard wrote:
> > > > > Hi,
> > > > >
> > > > > On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> > > > >> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> > > > >> H6 and R40 SoCs.
> > > > >
> > > > > Thanks again for working on this.
> > > > >
> > > > > I'll merge the DT patches when the driver will have been merged.
> > > >
> > > > I've applied patches 1 & 2.
> > > >
> > > > They are in the testing branch and will go to the linux-next branch as
> > > > soon as the kernelci passes.
> > >
> > > I just merged all the other patches (except the patch 6, for the H6,
> > > as requested by Vasily on IRC).
> > >
> >
> > Hello
> >
> > Vasily asked to not apply H6 due to my test failling on h6 and since he didnt own any H6 hw.
> > But it was failling due my fault (a failling build).
> >
> > So the patchset work perfect on H6 (opi1+, opi3, pineH64 both model A and B) as reported by my answer to this thread.
> 
> HI Corentin,
> 
> Although it is not calibrated, it should work on the R40. Can you give
> my patch a try?
> 

Hello

It is planned, I will report the result.

Regards
Maxime Ripard Dec. 27, 2019, 3:37 p.m. UTC | #10
On Thu, Dec 26, 2019 at 11:37:39AM +0100, Ondřej Jirman wrote:
> On Thu, Dec 26, 2019 at 10:27:51AM +0100, Maxime Ripard wrote:
> > On Tue, Dec 24, 2019 at 07:30:55PM +0100, Daniel Lezcano wrote:
> > > On 19/12/2019 18:33, Maxime Ripard wrote:
> > > > Hi,
> > > >
> > > > On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> > > >> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> > > >> H6 and R40 SoCs.
> > > >
> > > > Thanks again for working on this.
> > > >
> > > > I'll merge the DT patches when the driver will have been merged.
> > >
> > > I've applied patches 1 & 2.
> > >
> > > They are in the testing branch and will go to the linux-next branch as
> > > soon as the kernelci passes.
> >
> > I just merged all the other patches (except the patch 6, for the H6,
> > as requested by Vasily on IRC).
>
> Hello,
>
> I think you can apply H6 patch. This was just some misunderstanding
> and H6 is working.

Done, thanks!
Maxime
Corentin Labbe Dec. 29, 2019, 9:18 p.m. UTC | #11
On Thu, Dec 26, 2019 at 08:47:47PM +0800, Frank Lee wrote:
> On Thu, Dec 26, 2019 at 6:44 PM Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> >
> > On Thu, Dec 26, 2019 at 10:27:51AM +0100, Maxime Ripard wrote:
> > > On Tue, Dec 24, 2019 at 07:30:55PM +0100, Daniel Lezcano wrote:
> > > > On 19/12/2019 18:33, Maxime Ripard wrote:
> > > > > Hi,
> > > > >
> > > > > On Thu, Dec 19, 2019 at 09:28:16AM -0800, Vasily Khoruzhick wrote:
> > > > >> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> > > > >> H6 and R40 SoCs.
> > > > >
> > > > > Thanks again for working on this.
> > > > >
> > > > > I'll merge the DT patches when the driver will have been merged.
> > > >
> > > > I've applied patches 1 & 2.
> > > >
> > > > They are in the testing branch and will go to the linux-next branch as
> > > > soon as the kernelci passes.
> > >
> > > I just merged all the other patches (except the patch 6, for the H6,
> > > as requested by Vasily on IRC).
> > >
> >
> > Hello
> >
> > Vasily asked to not apply H6 due to my test failling on h6 and since he didnt own any H6 hw.
> > But it was failling due my fault (a failling build).
> >
> > So the patchset work perfect on H6 (opi1+, opi3, pineH64 both model A and B) as reported by my answer to this thread.
> 
> HI Corentin,
> 
> Although it is not calibrated, it should work on the R40. Can you give
> my patch a try?
> 
> Thx,
> Yangtao
> 

Hello

It works:
uname -a
Linux zlad 5.5.0-rc2-next-20191220+ #196 SMP PREEMPT Sun Dec 29 22:08:05 CET 2019 armv7l ARMv7 Processor rev 5 (v7l) Allwinner sun8i Family GNU/Linux
cat /sys/devices/virtual/thermal/thermal_zone0/temp
30736
cat /sys/devices/virtual/thermal/thermal_zone0/type 
cpu0-thermal
cat /sys/devices/virtual/thermal/thermal_zone1/temp
29380
cat /sys/devices/virtual/thermal/thermal_zone1/type 
gpu-thermal

So you could add to your patch:
Tested-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Tested-on: sun8i-r40-bananapi-m2-ultra

thanks
Regards
Amit Kucheria Feb. 6, 2020, 2:13 p.m. UTC | #12
Hi Vasily,

For this entire series, the DTS files don't contain any trip points.
Did I miss some other series?

At a minimum, you should add some "hot" or "critical" trip points
since then don't require a cooling-map with throttling actions. If you
have "passive" trip points, then you need to provide cooling-maps.

Since this series has been merged, could you please follow up with a
fixup series to add the trip points?

Regards,
Amit
p.s. We should catch all this automatically, I'll send out yaml
bindings for the thermal framework soon that should catch this stuff.

On Thu, Dec 19, 2019 at 10:58 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>
> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> H6 and R40 SoCs.
>
> v8:
>         - [vasily] Address more Maxime's comments for dt-schema
>         - [vasily] Add myself to MAINTAINERS for the driver and schema
>         - [vasily] Round calibration data size to word boundary for H6 and A64
>         - [vasily] Change offset for A64 since it reports too low temp otherwise.
>                    Likely conversion formula in user manual is not correct.
>
> v7:
>         - [vasily] Address Maxime's comments for dt-schema
>         - [vasily] Move common part of H3 and H5 dts into sunxi-h3-h5.dtsi
>         - [vasily] Add Maxime's a-b to the driver patch
>
> v6:
>         - [ondrej, vasily] Squash all driver related changes into a
>                            single patch
>         - [ondrej] Rename calib -> calibration
>         - [ondrej] Fix thermal zone registration check
>         - [ondrej] Lower rate of sensor data interrupts to 4/sec/sensor
>         - [ondrej] Rework scale/offset values, H6 calibration
>         - [ondrej] Explicitly set mod clock to 24 MHz
>         - [ondrej] Set undocumented bits in CTRL0 for H6
>         - [ondrej] Add support for A83T
>         - [ondrej] Add dts changes for A83T, H3, H5, H6
>         - [vasily] Add dts changes for A64
>         - [vasily] Address Maxime's comments for YAML scheme
>         - [vasily] Make .calc_temp callback mandatory
>         - [vasily] Set .max_register in regmap config, so regs can be
>                    inspected using debugfs
>
> Ondrej Jirman (4):
>   ARM: dts: sun8i-a83t: Add thermal sensor and thermal zones
>   ARM: dts: sun8i-h3: Add thermal sensor and thermal zones
>   arm64: dts: allwinner: h5: Add thermal sensor and thermal zones
>   arm64: dts: allwinner: h6: Add thermal sensor and thermal zones
>
> Vasily Khoruzhick (1):
>   arm64: dts: allwinner: a64: Add thermal sensors and thermal zones
>
> Yangtao Li (2):
>   thermal: sun8i: add thermal driver for H6/H5/H3/A64/A83T/R40
>   dt-bindings: thermal: add YAML schema for sun8i-thermal driver
>     bindings
>
>  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 160 +++++
>  MAINTAINERS                                   |   8 +
>  arch/arm/boot/dts/sun8i-a83t.dtsi             |  36 +
>  arch/arm/boot/dts/sun8i-h3.dtsi               |  20 +
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |   6 +
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  42 ++
>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  26 +
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 +
>  drivers/thermal/Kconfig                       |  14 +
>  drivers/thermal/Makefile                      |   1 +
>  drivers/thermal/sun8i_thermal.c               | 639 ++++++++++++++++++
>  11 files changed, 985 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
>  create mode 100644 drivers/thermal/sun8i_thermal.c
>
> --
> 2.24.1
>
Ondřej Jirman Feb. 6, 2020, 3:57 p.m. UTC | #13
Hi Amit,

On Thu, Feb 06, 2020 at 07:43:59PM +0530, Amit Kucheria wrote:
> Hi Vasily,
> 
> For this entire series, the DTS files don't contain any trip points.
> Did I miss some other series?
> 
> At a minimum, you should add some "hot" or "critical" trip points
> since then don't require a cooling-map with throttling actions. If you
> have "passive" trip points, then you need to provide cooling-maps.
> 
> Since this series has been merged, could you please follow up with a
> fixup series to add the trip points?

I don't think lack of trip points causes runtime issues. Or does it? I planned
to send update with some trip points and cooling maps for 5.7 merge window.
Is this acceptable?

If not, I can send a patch that adds:

+ trips {
+         cpu-very-hot {
+                 temperature = <100000>;
+                 hysteresis = <0>;
+                 type = "critical";
+         };
+ };

and 

+ trips {
+         gpu-very-hot {
+                 temperature = <100000>;
+                 hysteresis = <0>;
+                 type = "critical";
+         };
+ };

everywhere where appropriate. Though that will make rebase of out of
tree patches that already have a more complicated setup to be sent for the next
merge window a bit tedious.

thank you,
	Ondrej

> Regards,
> Amit
> p.s. We should catch all this automatically, I'll send out yaml
> bindings for the thermal framework soon that should catch this stuff.
> 
> On Thu, Dec 19, 2019 at 10:58 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> >
> > This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> > H6 and R40 SoCs.
> >
> > v8:
> >         - [vasily] Address more Maxime's comments for dt-schema
> >         - [vasily] Add myself to MAINTAINERS for the driver and schema
> >         - [vasily] Round calibration data size to word boundary for H6 and A64
> >         - [vasily] Change offset for A64 since it reports too low temp otherwise.
> >                    Likely conversion formula in user manual is not correct.
> >
> > v7:
> >         - [vasily] Address Maxime's comments for dt-schema
> >         - [vasily] Move common part of H3 and H5 dts into sunxi-h3-h5.dtsi
> >         - [vasily] Add Maxime's a-b to the driver patch
> >
> > v6:
> >         - [ondrej, vasily] Squash all driver related changes into a
> >                            single patch
> >         - [ondrej] Rename calib -> calibration
> >         - [ondrej] Fix thermal zone registration check
> >         - [ondrej] Lower rate of sensor data interrupts to 4/sec/sensor
> >         - [ondrej] Rework scale/offset values, H6 calibration
> >         - [ondrej] Explicitly set mod clock to 24 MHz
> >         - [ondrej] Set undocumented bits in CTRL0 for H6
> >         - [ondrej] Add support for A83T
> >         - [ondrej] Add dts changes for A83T, H3, H5, H6
> >         - [vasily] Add dts changes for A64
> >         - [vasily] Address Maxime's comments for YAML scheme
> >         - [vasily] Make .calc_temp callback mandatory
> >         - [vasily] Set .max_register in regmap config, so regs can be
> >                    inspected using debugfs
> >
> > Ondrej Jirman (4):
> >   ARM: dts: sun8i-a83t: Add thermal sensor and thermal zones
> >   ARM: dts: sun8i-h3: Add thermal sensor and thermal zones
> >   arm64: dts: allwinner: h5: Add thermal sensor and thermal zones
> >   arm64: dts: allwinner: h6: Add thermal sensor and thermal zones
> >
> > Vasily Khoruzhick (1):
> >   arm64: dts: allwinner: a64: Add thermal sensors and thermal zones
> >
> > Yangtao Li (2):
> >   thermal: sun8i: add thermal driver for H6/H5/H3/A64/A83T/R40
> >   dt-bindings: thermal: add YAML schema for sun8i-thermal driver
> >     bindings
> >
> >  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 160 +++++
> >  MAINTAINERS                                   |   8 +
> >  arch/arm/boot/dts/sun8i-a83t.dtsi             |  36 +
> >  arch/arm/boot/dts/sun8i-h3.dtsi               |  20 +
> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |   6 +
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  42 ++
> >  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  26 +
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 +
> >  drivers/thermal/Kconfig                       |  14 +
> >  drivers/thermal/Makefile                      |   1 +
> >  drivers/thermal/sun8i_thermal.c               | 639 ++++++++++++++++++
> >  11 files changed, 985 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> >  create mode 100644 drivers/thermal/sun8i_thermal.c
> >
> > --
> > 2.24.1
> >
Daniel Lezcano Feb. 6, 2020, 4:46 p.m. UTC | #14
Hi Amit,

On 06/02/2020 15:13, Amit Kucheria wrote:
> Hi Vasily,
> 
> For this entire series, the DTS files don't contain any trip points.
> Did I miss some other series?
> 
> At a minimum, you should add some "hot" or "critical" trip points
> since then don't require a cooling-map with throttling actions. If you
> have "passive" trip points, then you need to provide cooling-maps.

Except I'm misunderstanding the bindings, a thermal zone must define
these required properties:

- polling-delay
- polling-delay-passive
- thermal-sensors
- trips
- cooling-maps


> Since this series has been merged, could you please follow up with a
> fixup series to add the trip points?
> 
> Regards,
> Amit
> p.s. We should catch all this automatically, I'll send out yaml
> bindings for the thermal framework soon that should catch this stuff.

+1

There was a small discussion about converting the binding to a schema:

https://www.spinics.net/lists/devicetree/msg332424.html



> On Thu, Dec 19, 2019 at 10:58 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>>
>> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
>> H6 and R40 SoCs.
>>
>> v8:
>>         - [vasily] Address more Maxime's comments for dt-schema
>>         - [vasily] Add myself to MAINTAINERS for the driver and schema
>>         - [vasily] Round calibration data size to word boundary for H6 and A64
>>         - [vasily] Change offset for A64 since it reports too low temp otherwise.
>>                    Likely conversion formula in user manual is not correct.
>>
>> v7:
>>         - [vasily] Address Maxime's comments for dt-schema
>>         - [vasily] Move common part of H3 and H5 dts into sunxi-h3-h5.dtsi
>>         - [vasily] Add Maxime's a-b to the driver patch
>>
>> v6:
>>         - [ondrej, vasily] Squash all driver related changes into a
>>                            single patch
>>         - [ondrej] Rename calib -> calibration
>>         - [ondrej] Fix thermal zone registration check
>>         - [ondrej] Lower rate of sensor data interrupts to 4/sec/sensor
>>         - [ondrej] Rework scale/offset values, H6 calibration
>>         - [ondrej] Explicitly set mod clock to 24 MHz
>>         - [ondrej] Set undocumented bits in CTRL0 for H6
>>         - [ondrej] Add support for A83T
>>         - [ondrej] Add dts changes for A83T, H3, H5, H6
>>         - [vasily] Add dts changes for A64
>>         - [vasily] Address Maxime's comments for YAML scheme
>>         - [vasily] Make .calc_temp callback mandatory
>>         - [vasily] Set .max_register in regmap config, so regs can be
>>                    inspected using debugfs
>>
>> Ondrej Jirman (4):
>>   ARM: dts: sun8i-a83t: Add thermal sensor and thermal zones
>>   ARM: dts: sun8i-h3: Add thermal sensor and thermal zones
>>   arm64: dts: allwinner: h5: Add thermal sensor and thermal zones
>>   arm64: dts: allwinner: h6: Add thermal sensor and thermal zones
>>
>> Vasily Khoruzhick (1):
>>   arm64: dts: allwinner: a64: Add thermal sensors and thermal zones
>>
>> Yangtao Li (2):
>>   thermal: sun8i: add thermal driver for H6/H5/H3/A64/A83T/R40
>>   dt-bindings: thermal: add YAML schema for sun8i-thermal driver
>>     bindings
>>
>>  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 160 +++++
>>  MAINTAINERS                                   |   8 +
>>  arch/arm/boot/dts/sun8i-a83t.dtsi             |  36 +
>>  arch/arm/boot/dts/sun8i-h3.dtsi               |  20 +
>>  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |   6 +
>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  42 ++
>>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  26 +
>>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 +
>>  drivers/thermal/Kconfig                       |  14 +
>>  drivers/thermal/Makefile                      |   1 +
>>  drivers/thermal/sun8i_thermal.c               | 639 ++++++++++++++++++
>>  11 files changed, 985 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
>>  create mode 100644 drivers/thermal/sun8i_thermal.c
>>
>> --
>> 2.24.1
>>
Amit Kucheria Feb. 6, 2020, 7:23 p.m. UTC | #15
On Thu, Feb 6, 2020 at 10:16 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
>
> Hi Amit,
>
> On 06/02/2020 15:13, Amit Kucheria wrote:
> > Hi Vasily,
> >
> > For this entire series, the DTS files don't contain any trip points.
> > Did I miss some other series?
> >
> > At a minimum, you should add some "hot" or "critical" trip points
> > since then don't require a cooling-map with throttling actions. If you
> > have "passive" trip points, then you need to provide cooling-maps.
>
> Except I'm misunderstanding the bindings, a thermal zone must define
> these required properties:
>
> - polling-delay
> - polling-delay-passive
> - thermal-sensors
> - trips
> - cooling-maps

Right, except for the cooling-maps. Those are exempted if there is the
trip type is not passive. That is my understanding of the existing
bindings.

Trip type critical triggers a shutdown and trip type hot only triggers
a notification - see thermal_core.c:handle_critical_trips(). So we
only need cooling maps for passive trip types.

> > Since this series has been merged, could you please follow up with a
> > fixup series to add the trip points?
> >
> > Regards,
> > Amit
> > p.s. We should catch all this automatically, I'll send out yaml
> > bindings for the thermal framework soon that should catch this stuff.
>
> +1
>
> There was a small discussion about converting the binding to a schema:
>
> https://www.spinics.net/lists/devicetree/msg332424.html


Aah, I missed that. I started working on something last week that
looks similar to your discussion. Pushed a WIP branch here[1], it
looks like I had a similar idea on how to split the bindings. Hope to
finish this up tomorrow for an RFC.

Regards,
Amit

[1] https://github.com/idlethread/linux/commits/up/thermal/yaml-conversion-v1

> > On Thu, Dec 19, 2019 at 10:58 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> >>
> >> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> >> H6 and R40 SoCs.
> >>
> >> v8:
> >>         - [vasily] Address more Maxime's comments for dt-schema
> >>         - [vasily] Add myself to MAINTAINERS for the driver and schema
> >>         - [vasily] Round calibration data size to word boundary for H6 and A64
> >>         - [vasily] Change offset for A64 since it reports too low temp otherwise.
> >>                    Likely conversion formula in user manual is not correct.
> >>
> >> v7:
> >>         - [vasily] Address Maxime's comments for dt-schema
> >>         - [vasily] Move common part of H3 and H5 dts into sunxi-h3-h5.dtsi
> >>         - [vasily] Add Maxime's a-b to the driver patch
> >>
> >> v6:
> >>         - [ondrej, vasily] Squash all driver related changes into a
> >>                            single patch
> >>         - [ondrej] Rename calib -> calibration
> >>         - [ondrej] Fix thermal zone registration check
> >>         - [ondrej] Lower rate of sensor data interrupts to 4/sec/sensor
> >>         - [ondrej] Rework scale/offset values, H6 calibration
> >>         - [ondrej] Explicitly set mod clock to 24 MHz
> >>         - [ondrej] Set undocumented bits in CTRL0 for H6
> >>         - [ondrej] Add support for A83T
> >>         - [ondrej] Add dts changes for A83T, H3, H5, H6
> >>         - [vasily] Add dts changes for A64
> >>         - [vasily] Address Maxime's comments for YAML scheme
> >>         - [vasily] Make .calc_temp callback mandatory
> >>         - [vasily] Set .max_register in regmap config, so regs can be
> >>                    inspected using debugfs
> >>
> >> Ondrej Jirman (4):
> >>   ARM: dts: sun8i-a83t: Add thermal sensor and thermal zones
> >>   ARM: dts: sun8i-h3: Add thermal sensor and thermal zones
> >>   arm64: dts: allwinner: h5: Add thermal sensor and thermal zones
> >>   arm64: dts: allwinner: h6: Add thermal sensor and thermal zones
> >>
> >> Vasily Khoruzhick (1):
> >>   arm64: dts: allwinner: a64: Add thermal sensors and thermal zones
> >>
> >> Yangtao Li (2):
> >>   thermal: sun8i: add thermal driver for H6/H5/H3/A64/A83T/R40
> >>   dt-bindings: thermal: add YAML schema for sun8i-thermal driver
> >>     bindings
> >>
> >>  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 160 +++++
> >>  MAINTAINERS                                   |   8 +
> >>  arch/arm/boot/dts/sun8i-a83t.dtsi             |  36 +
> >>  arch/arm/boot/dts/sun8i-h3.dtsi               |  20 +
> >>  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |   6 +
> >>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  42 ++
> >>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  26 +
> >>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 +
> >>  drivers/thermal/Kconfig                       |  14 +
> >>  drivers/thermal/Makefile                      |   1 +
> >>  drivers/thermal/sun8i_thermal.c               | 639 ++++++++++++++++++
> >>  11 files changed, 985 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> >>  create mode 100644 drivers/thermal/sun8i_thermal.c
> >>
> >> --
> >> 2.24.1
> >>
>
>
> --
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
Amit Kucheria Feb. 6, 2020, 7:26 p.m. UTC | #16
On Thu, Feb 6, 2020 at 9:27 PM Ondřej Jirman <megous@megous.com> wrote:
>
> Hi Amit,
>
> On Thu, Feb 06, 2020 at 07:43:59PM +0530, Amit Kucheria wrote:
> > Hi Vasily,
> >
> > For this entire series, the DTS files don't contain any trip points.
> > Did I miss some other series?
> >
> > At a minimum, you should add some "hot" or "critical" trip points
> > since then don't require a cooling-map with throttling actions. If you
> > have "passive" trip points, then you need to provide cooling-maps.
> >
> > Since this series has been merged, could you please follow up with a
> > fixup series to add the trip points?
>
> I don't think lack of trip points causes runtime issues. Or does it? I planned
> to send update with some trip points and cooling maps for 5.7 merge window.
> Is this acceptable?

Yes, I think that would be fine.

> If not, I can send a patch that adds:
>
> + trips {
> +         cpu-very-hot {
> +                 temperature = <100000>;
> +                 hysteresis = <0>;
> +                 type = "critical";
> +         };
> + };
>
> and
>
> + trips {
> +         gpu-very-hot {
> +                 temperature = <100000>;
> +                 hysteresis = <0>;
> +                 type = "critical";
> +         };
> + };
>
> everywhere where appropriate. Though that will make rebase of out of
> tree patches that already have a more complicated setup to be sent for the next
> merge window a bit tedious.

Right, don't do that.

> thank you,
>         Ondrej
>
> > Regards,
> > Amit
> > p.s. We should catch all this automatically, I'll send out yaml
> > bindings for the thermal framework soon that should catch this stuff.
Vasily Khoruzhick Feb. 7, 2020, 12:49 a.m. UTC | #17
On Thu, Feb 6, 2020 at 6:14 AM Amit Kucheria
<amit.kucheria@verdurent.com> wrote:
>
> Hi Vasily,
>
> For this entire series, the DTS files don't contain any trip points.
> Did I miss some other series?

See https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/commit/?h=sunxi/dt-for-5.6&id=e1c3804a177418fe14d95f0c4ccba5ae66f73d82
for A64

> At a minimum, you should add some "hot" or "critical" trip points
> since then don't require a cooling-map with throttling actions. If you
> have "passive" trip points, then you need to provide cooling-maps.
>
> Since this series has been merged, could you please follow up with a
> fixup series to add the trip points?

A64 has already made it into linux-next, I believe there's other
series in flight at least for H6 from Yangtao Li and for H5 from
Chen-Yu Tsai

> Regards,
> Amit
> p.s. We should catch all this automatically, I'll send out yaml
> bindings for the thermal framework soon that should catch this stuff.
>
> On Thu, Dec 19, 2019 at 10:58 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> >
> > This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
> > H6 and R40 SoCs.
> >
> > v8:
> >         - [vasily] Address more Maxime's comments for dt-schema
> >         - [vasily] Add myself to MAINTAINERS for the driver and schema
> >         - [vasily] Round calibration data size to word boundary for H6 and A64
> >         - [vasily] Change offset for A64 since it reports too low temp otherwise.
> >                    Likely conversion formula in user manual is not correct.
> >
> > v7:
> >         - [vasily] Address Maxime's comments for dt-schema
> >         - [vasily] Move common part of H3 and H5 dts into sunxi-h3-h5.dtsi
> >         - [vasily] Add Maxime's a-b to the driver patch
> >
> > v6:
> >         - [ondrej, vasily] Squash all driver related changes into a
> >                            single patch
> >         - [ondrej] Rename calib -> calibration
> >         - [ondrej] Fix thermal zone registration check
> >         - [ondrej] Lower rate of sensor data interrupts to 4/sec/sensor
> >         - [ondrej] Rework scale/offset values, H6 calibration
> >         - [ondrej] Explicitly set mod clock to 24 MHz
> >         - [ondrej] Set undocumented bits in CTRL0 for H6
> >         - [ondrej] Add support for A83T
> >         - [ondrej] Add dts changes for A83T, H3, H5, H6
> >         - [vasily] Add dts changes for A64
> >         - [vasily] Address Maxime's comments for YAML scheme
> >         - [vasily] Make .calc_temp callback mandatory
> >         - [vasily] Set .max_register in regmap config, so regs can be
> >                    inspected using debugfs
> >
> > Ondrej Jirman (4):
> >   ARM: dts: sun8i-a83t: Add thermal sensor and thermal zones
> >   ARM: dts: sun8i-h3: Add thermal sensor and thermal zones
> >   arm64: dts: allwinner: h5: Add thermal sensor and thermal zones
> >   arm64: dts: allwinner: h6: Add thermal sensor and thermal zones
> >
> > Vasily Khoruzhick (1):
> >   arm64: dts: allwinner: a64: Add thermal sensors and thermal zones
> >
> > Yangtao Li (2):
> >   thermal: sun8i: add thermal driver for H6/H5/H3/A64/A83T/R40
> >   dt-bindings: thermal: add YAML schema for sun8i-thermal driver
> >     bindings
> >
> >  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 160 +++++
> >  MAINTAINERS                                   |   8 +
> >  arch/arm/boot/dts/sun8i-a83t.dtsi             |  36 +
> >  arch/arm/boot/dts/sun8i-h3.dtsi               |  20 +
> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |   6 +
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  42 ++
> >  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  26 +
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 +
> >  drivers/thermal/Kconfig                       |  14 +
> >  drivers/thermal/Makefile                      |   1 +
> >  drivers/thermal/sun8i_thermal.c               | 639 ++++++++++++++++++
> >  11 files changed, 985 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> >  create mode 100644 drivers/thermal/sun8i_thermal.c
> >
> > --
> > 2.24.1
> >
Daniel Lezcano Feb. 7, 2020, 9:31 a.m. UTC | #18
On 06/02/2020 20:23, Amit Kucheria wrote:
> On Thu, Feb 6, 2020 at 10:16 PM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>>
>>
>> Hi Amit,
>>
>> On 06/02/2020 15:13, Amit Kucheria wrote:
>>> Hi Vasily,
>>>
>>> For this entire series, the DTS files don't contain any trip points.
>>> Did I miss some other series?
>>>
>>> At a minimum, you should add some "hot" or "critical" trip points
>>> since then don't require a cooling-map with throttling actions. If you
>>> have "passive" trip points, then you need to provide cooling-maps.
>>
>> Except I'm misunderstanding the bindings, a thermal zone must define
>> these required properties:
>>
>> - polling-delay
>> - polling-delay-passive
>> - thermal-sensors
>> - trips
>> - cooling-maps
> 
> Right, except for the cooling-maps. Those are exempted if there is the
> trip type is not passive. That is my understanding of the existing
> bindings.

The binding is ambiguous.

For me it states the cooling maps must be defined as it is a required
node of the thermal zone.

We may not have an active or passive cooling device for the thermal
zone, thus we can not comply with the dt binding and strictly speaking
we shouldn't add this thermal zone.

But the logic of having a 'hot' or a 'critical' trip point without a
cooling device is correct.

As we move this binding to a schema, we shall clarify the cooling-maps
is required if there are active or passive trip points otherwise it is
optional.


> Trip type critical triggers a shutdown and trip type hot only triggers
> a notification - see thermal_core.c:handle_critical_trips(). So we
> only need cooling maps for passive trip types.
> 
>>> Since this series has been merged, could you please follow up with a
>>> fixup series to add the trip points?
>>>
>>> Regards,
>>> Amit
>>> p.s. We should catch all this automatically, I'll send out yaml
>>> bindings for the thermal framework soon that should catch this stuff.
>>
>> +1
>>
>> There was a small discussion about converting the binding to a schema:
>>
>> https://www.spinics.net/lists/devicetree/msg332424.html
> 
> 
> Aah, I missed that. I started working on something last week that
> looks similar to your discussion. Pushed a WIP branch here[1], it
> looks like I had a similar idea on how to split the bindings. Hope to
> finish this up tomorrow for an RFC.

Great, thanks for taking care of that.


> [1] https://github.com/idlethread/linux/commits/up/thermal/yaml-conversion-v1
> 
>>> On Thu, Dec 19, 2019 at 10:58 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>>>>
>>>> This patchset adds driver for thermal sensor in A64, A83T, H3, H5,
>>>> H6 and R40 SoCs.
>>>>
>>>> v8:
>>>>         - [vasily] Address more Maxime's comments for dt-schema
>>>>         - [vasily] Add myself to MAINTAINERS for the driver and schema
>>>>         - [vasily] Round calibration data size to word boundary for H6 and A64
>>>>         - [vasily] Change offset for A64 since it reports too low temp otherwise.
>>>>                    Likely conversion formula in user manual is not correct.
>>>>
>>>> v7:
>>>>         - [vasily] Address Maxime's comments for dt-schema
>>>>         - [vasily] Move common part of H3 and H5 dts into sunxi-h3-h5.dtsi
>>>>         - [vasily] Add Maxime's a-b to the driver patch
>>>>
>>>> v6:
>>>>         - [ondrej, vasily] Squash all driver related changes into a
>>>>                            single patch
>>>>         - [ondrej] Rename calib -> calibration
>>>>         - [ondrej] Fix thermal zone registration check
>>>>         - [ondrej] Lower rate of sensor data interrupts to 4/sec/sensor
>>>>         - [ondrej] Rework scale/offset values, H6 calibration
>>>>         - [ondrej] Explicitly set mod clock to 24 MHz
>>>>         - [ondrej] Set undocumented bits in CTRL0 for H6
>>>>         - [ondrej] Add support for A83T
>>>>         - [ondrej] Add dts changes for A83T, H3, H5, H6
>>>>         - [vasily] Add dts changes for A64
>>>>         - [vasily] Address Maxime's comments for YAML scheme
>>>>         - [vasily] Make .calc_temp callback mandatory
>>>>         - [vasily] Set .max_register in regmap config, so regs can be
>>>>                    inspected using debugfs
>>>>
>>>> Ondrej Jirman (4):
>>>>   ARM: dts: sun8i-a83t: Add thermal sensor and thermal zones
>>>>   ARM: dts: sun8i-h3: Add thermal sensor and thermal zones
>>>>   arm64: dts: allwinner: h5: Add thermal sensor and thermal zones
>>>>   arm64: dts: allwinner: h6: Add thermal sensor and thermal zones
>>>>
>>>> Vasily Khoruzhick (1):
>>>>   arm64: dts: allwinner: a64: Add thermal sensors and thermal zones
>>>>
>>>> Yangtao Li (2):
>>>>   thermal: sun8i: add thermal driver for H6/H5/H3/A64/A83T/R40
>>>>   dt-bindings: thermal: add YAML schema for sun8i-thermal driver
>>>>     bindings
>>>>
>>>>  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 160 +++++
>>>>  MAINTAINERS                                   |   8 +
>>>>  arch/arm/boot/dts/sun8i-a83t.dtsi             |  36 +
>>>>  arch/arm/boot/dts/sun8i-h3.dtsi               |  20 +
>>>>  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |   6 +
>>>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  42 ++
>>>>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  26 +
>>>>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  33 +
>>>>  drivers/thermal/Kconfig                       |  14 +
>>>>  drivers/thermal/Makefile                      |   1 +
>>>>  drivers/thermal/sun8i_thermal.c               | 639 ++++++++++++++++++
>>>>  11 files changed, 985 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
>>>>  create mode 100644 drivers/thermal/sun8i_thermal.c
>>>>
>>>> --
>>>> 2.24.1
>>>>
>>
>>
>> --
>>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
>>