[PATCH v1 0/3] Add D1/T113s thermal sensor controller support

[PATCH v1 0/3] Add D1/T113s thermal sensor controller support

[Maksim Kiselev]

This series adds support for Allwinner D1/T113s thermal sensor controller.
THIS controller is similar to the one on H6, but with only one sensor and
uses a different scale and offset values.

Maxim Kiselev (3):
  thermal: sun8i: Add D1/T113s THS controller support
  riscv: dts: allwinner: d1: Add thermal sensor and thermal zone
  dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

 .../thermal/allwinner,sun8i-a83t-ths.yaml     | 20 +++++++++++++-
 .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 26 +++++++++++++++++++
 drivers/thermal/sun8i_thermal.c               | 13 ++++++++++
 3 files changed, 58 insertions(+), 1 deletion(-)

-- 
2.39.2

[PATCH v1 1/3] thermal: sun8i: Add D1/T113s THS controller support

[Maksim Kiselev]

From: Maxim Kiselev <bigunclemax@gmail.com>

This patch adds a thermal sensor controller support for the D1/T113s,
which is similar to the one on H6, but with only one sensor and
different scale and offset values.

Signed-off-by: Maxim Kiselev <bigunclemax@gmail.com>
Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
---
 drivers/thermal/sun8i_thermal.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
index 793ddce72132..01cc4e130892 100644
--- a/drivers/thermal/sun8i_thermal.c
+++ b/drivers/thermal/sun8i_thermal.c
@@ -628,6 +628,18 @@ static const struct ths_thermal_chip sun50i_h6_ths = {
 	.calc_temp = sun8i_ths_calc_temp,
 };
 
+static const struct ths_thermal_chip sun20i_d1_ths = {
+	.sensor_num = 1,
+	.has_bus_clk_reset = true,
+	.offset = 188552,
+	.scale = 673,
+	.temp_data_base = SUN50I_H6_THS_TEMP_DATA,
+	.calibrate = sun50i_h6_ths_calibrate,
+	.init = sun50i_h6_thermal_init,
+	.irq_ack = sun50i_h6_irq_ack,
+	.calc_temp = sun8i_ths_calc_temp,
+};
+
 static const struct of_device_id of_ths_match[] = {
 	{ .compatible = "allwinner,sun8i-a83t-ths", .data = &sun8i_a83t_ths },
 	{ .compatible = "allwinner,sun8i-h3-ths", .data = &sun8i_h3_ths },
@@ -636,6 +648,7 @@ static const struct of_device_id of_ths_match[] = {
 	{ .compatible = "allwinner,sun50i-a100-ths", .data = &sun50i_a100_ths },
 	{ .compatible = "allwinner,sun50i-h5-ths", .data = &sun50i_h5_ths },
 	{ .compatible = "allwinner,sun50i-h6-ths", .data = &sun50i_h6_ths },
+	{ .compatible = "allwinner,sun20i-d1-ths", .data = &sun20i_d1_ths },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, of_ths_match);
-- 
2.39.2

[PATCH v1 2/3] riscv: dts: allwinner: d1: Add thermal sensor and thermal zone

[Maksim Kiselev]

From: Maxim Kiselev <bigunclemax@gmail.com>

This patch adds a thermal sensor controller node for the D1/T113s.
Also it adds a THS calibration data cell and thermal zone.

Signed-off-by: Maxim Kiselev <bigunclemax@gmail.com>
Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
---
 .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
index 922e8e0e2c09..b893f3325554 100644
--- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
+++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
@@ -10,6 +10,7 @@
 #include <dt-bindings/reset/sun8i-de2.h>
 #include <dt-bindings/reset/sun20i-d1-ccu.h>
 #include <dt-bindings/reset/sun20i-d1-r-ccu.h>
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	#address-cells = <1>;
@@ -138,6 +139,19 @@ ccu: clock-controller@2001000 {
 			#reset-cells = <1>;
 		};
 
+		ths: thermal-sensor@2009400 {
+			compatible = "allwinner,sun20i-d1-ths";
+			reg = <0x02009400 0x400>;
+			interrupts = <SOC_PERIPHERAL_IRQ(58) IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_THS>;
+			clock-names = "bus";
+			resets = <&ccu RST_BUS_THS>;
+			nvmem-cells = <&ths_calibration>;
+			nvmem-cell-names = "calibration";
+			status = "disabled";
+			#thermal-sensor-cells = <0>;
+		};
+
 		dmic: dmic@2031000 {
 			compatible = "allwinner,sun20i-d1-dmic",
 				     "allwinner,sun50i-h6-dmic";
@@ -365,6 +379,10 @@ sid: efuse@3006000 {
 			reg = <0x3006000 0x1000>;
 			#address-cells = <1>;
 			#size-cells = <1>;
+
+			ths_calibration: thermal-sensor-calibration@14 {
+				reg = <0x14 0x4>;
+			};
 		};
 
 		crypto: crypto@3040000 {
@@ -843,4 +861,12 @@ rtc: rtc@7090000 {
 			#clock-cells = <1>;
 		};
 	};
+
+	thermal-zones {
+		cpu_thermal: cpu-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&ths 0>;
+		};
+	};
 };
-- 
2.39.2

[PATCH v1 3/3] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

[Maksim Kiselev]

From: Maxim Kiselev <bigunclemax@gmail.com>

Add a binding for D1/T113s thermal sensor controller.

Signed-off-by: Maxim Kiselev <bigunclemax@gmail.com>
Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
---
 .../thermal/allwinner,sun8i-a83t-ths.yaml     | 20 ++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
index fbd4212285e2..001faa37fc27 100644
--- a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
+++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
@@ -16,6 +16,7 @@ properties:
       - allwinner,sun8i-a83t-ths
       - allwinner,sun8i-h3-ths
       - allwinner,sun8i-r40-ths
+      - allwinner,sun20i-d1-ths
       - allwinner,sun50i-a64-ths
       - allwinner,sun50i-a100-ths
       - allwinner,sun50i-h5-ths
@@ -61,6 +62,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - allwinner,sun20i-d1-ths
               - allwinner,sun50i-a100-ths
               - allwinner,sun50i-h6-ths
 
@@ -84,7 +86,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: allwinner,sun8i-h3-ths
+            enum:
+              - allwinner,sun8i-h3-ths
+              - allwinner,sun20i-d1-ths
 
     then:
       properties:
@@ -103,6 +107,7 @@ allOf:
             enum:
               - allwinner,sun8i-h3-ths
               - allwinner,sun8i-r40-ths
+              - allwinner,sun20i-d1-ths
               - allwinner,sun50i-a64-ths
               - allwinner,sun50i-a100-ths
               - allwinner,sun50i-h5-ths
@@ -159,4 +164,17 @@ examples:
          #thermal-sensor-cells = <1>;
     };
 
+  - |
+    thermal-sensor@2009400 {
+          compatible = "allwinner,sun20i-d1-ths";
+          reg = <0x02009400 0x400>;
+          interrupts = <0 90 0>;
+          clocks = <&ccu 0>;
+          clock-names = "bus";
+          resets = <&ccu 2>;
+          nvmem-cells = <&ths_calibration>;
+          nvmem-cell-names = "calibration";
+          #thermal-sensor-cells = <0>;
+    };
+
 ...
-- 
2.39.2

Re: [PATCH v1 3/3] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2693 bytes --]

On Sat, Jun 10, 2023 at 05:17:34PM +0300, Maksim Kiselev wrote:
> From: Maxim Kiselev <bigunclemax@gmail.com>
> 
> Add a binding for D1/T113s thermal sensor controller.
> 
> Signed-off-by: Maxim Kiselev <bigunclemax@gmail.com>
> Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>

Why do you have 2 sign-offs that are both you w/ different spellings?
Content looks fine to me, other than one commit below.

> ---
>  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 20 ++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> index fbd4212285e2..001faa37fc27 100644
> --- a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> @@ -16,6 +16,7 @@ properties:
>        - allwinner,sun8i-a83t-ths
>        - allwinner,sun8i-h3-ths
>        - allwinner,sun8i-r40-ths
> +      - allwinner,sun20i-d1-ths
>        - allwinner,sun50i-a64-ths
>        - allwinner,sun50i-a100-ths
>        - allwinner,sun50i-h5-ths
> @@ -61,6 +62,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - allwinner,sun20i-d1-ths
>                - allwinner,sun50i-a100-ths
>                - allwinner,sun50i-h6-ths
>  
> @@ -84,7 +86,9 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: allwinner,sun8i-h3-ths
> +            enum:
> +              - allwinner,sun8i-h3-ths
> +              - allwinner,sun20i-d1-ths
>  
>      then:
>        properties:
> @@ -103,6 +107,7 @@ allOf:
>              enum:
>                - allwinner,sun8i-h3-ths
>                - allwinner,sun8i-r40-ths
> +              - allwinner,sun20i-d1-ths
>                - allwinner,sun50i-a64-ths
>                - allwinner,sun50i-a100-ths
>                - allwinner,sun50i-h5-ths
> @@ -159,4 +164,17 @@ examples:
>           #thermal-sensor-cells = <1>;
>      };
>  
> +  - |
> +    thermal-sensor@2009400 {
> +          compatible = "allwinner,sun20i-d1-ths";
> +          reg = <0x02009400 0x400>;
> +          interrupts = <0 90 0>;
> +          clocks = <&ccu 0>;
> +          clock-names = "bus";
> +          resets = <&ccu 2>;
> +          nvmem-cells = <&ths_calibration>;
> +          nvmem-cell-names = "calibration";
> +          #thermal-sensor-cells = <0>;

Is this calibration required, or optional, for the d1?

Cheers,
Conor.

> +    };
> +
>  ...
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 3/3] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

[Maxim Kiselev]

сб, 10 июн. 2023 г. в 17:52, Conor Dooley <conor@kernel.org>:

...

>
> Signed-off-by: Maxim Kiselev <bigunclemax@gmail.com>
> Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
>
>> Why do you have 2 sign-offs that are both you w/ different spellings?

Sorry for that. I accidentally signed it twice.

...

> +  - |
> +    thermal-sensor@2009400 {
> +          compatible = "allwinner,sun20i-d1-ths";
> +          reg = <0x02009400 0x400>;
> +          interrupts = <0 90 0>;
> +          clocks = <&ccu 0>;
> +          clock-names = "bus";
> +          resets = <&ccu 2>;
> +          nvmem-cells = <&ths_calibration>;
> +          nvmem-cell-names = "calibration";
> +          #thermal-sensor-cells = <0>;
>
>> Is this calibration required, or optional, for the d1?

Calibration data for the D1 is optional as for the H6.
(But without it, the sensor data will be inaccurate).

Re: [PATCH v1 3/3] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1151 bytes --]

On Sat, Jun 10, 2023 at 06:30:48PM +0300, Maxim Kiselev wrote:
> сб, 10 июн. 2023 г. в 17:52, Conor Dooley <conor@kernel.org>:
> 
> ...
> 
> >
> > Signed-off-by: Maxim Kiselev <bigunclemax@gmail.com>
> > Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> >
> >> Why do you have 2 sign-offs that are both you w/ different spellings?
> 
> Sorry for that. I accidentally signed it twice.
> 
> ...
> 
> > +  - |
> > +    thermal-sensor@2009400 {
> > +          compatible = "allwinner,sun20i-d1-ths";
> > +          reg = <0x02009400 0x400>;
> > +          interrupts = <0 90 0>;
> > +          clocks = <&ccu 0>;
> > +          clock-names = "bus";
> > +          resets = <&ccu 2>;
> > +          nvmem-cells = <&ths_calibration>;
> > +          nvmem-cell-names = "calibration";
> > +          #thermal-sensor-cells = <0>;
> >
> >> Is this calibration required, or optional, for the d1?
> 
> Calibration data for the D1 is optional as for the H6.
> (But without it, the sensor data will be inaccurate).

Okay. Modulo the signoff:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 3/3] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1502 bytes --]

On Sat, Jun 10, 2023 at 06:59:29PM +0100, Conor Dooley wrote:
> On Sat, Jun 10, 2023 at 06:30:48PM +0300, Maxim Kiselev wrote:
> > сб, 10 июн. 2023 г. в 17:52, Conor Dooley <conor@kernel.org>:
> > 
> > ...
> > 
> > >
> > > Signed-off-by: Maxim Kiselev <bigunclemax@gmail.com>
> > > Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> > >
> > >> Why do you have 2 sign-offs that are both you w/ different spellings?
> > 
> > Sorry for that. I accidentally signed it twice.
> > 
> > ...
> > 
> > > +  - |
> > > +    thermal-sensor@2009400 {
> > > +          compatible = "allwinner,sun20i-d1-ths";
> > > +          reg = <0x02009400 0x400>;
> > > +          interrupts = <0 90 0>;
> > > +          clocks = <&ccu 0>;
> > > +          clock-names = "bus";
> > > +          resets = <&ccu 2>;
> > > +          nvmem-cells = <&ths_calibration>;
> > > +          nvmem-cell-names = "calibration";
> > > +          #thermal-sensor-cells = <0>;
> > >
> > >> Is this calibration required, or optional, for the d1?
> > 
> > Calibration data for the D1 is optional as for the H6.
> > (But without it, the sensor data will be inaccurate).
> 
> Okay. Modulo the signoff:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Wait, that's not what I meant to say... Clearly turned my brain off for
a moment. If it is not going to function correctly without the
calibration data, why not make the calibration data required for the
d1's ths?

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 0/3] Add D1/T113s thermal sensor controller support

[Jernej Škrabec]

Dne sobota, 10. junij 2023 ob 16:17:31 CEST je Maksim Kiselev napisal(a):
> This series adds support for Allwinner D1/T113s thermal sensor controller.
> THIS controller is similar to the one on H6, but with only one sensor and
> uses a different scale and offset values.
> 
> Maxim Kiselev (3):
>   thermal: sun8i: Add D1/T113s THS controller support
>   riscv: dts: allwinner: d1: Add thermal sensor and thermal zone
>   dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

As mentioned elsewhere, please fix SoB tags. Also binding patch should
come before driver changes.

Best regards,
Jernej

> 
>  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 20 +++++++++++++-
>  .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 26 +++++++++++++++++++
>  drivers/thermal/sun8i_thermal.c               | 13 ++++++++++
>  3 files changed, 58 insertions(+), 1 deletion(-)
> 
> 

Re: [PATCH v1 3/3] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

[Maxim Kiselev]

сб, 10 июн. 2023 г. в 21:01, Conor Dooley <conor@kernel.org>:

...

> > > > +  - |
> > > > +    thermal-sensor@2009400 {
> > > > +          compatible = "allwinner,sun20i-d1-ths";
> > > > +          reg = <0x02009400 0x400>;
> > > > +          interrupts = <0 90 0>;
> > > > +          clocks = <&ccu 0>;
> > > > +          clock-names = "bus";
> > > > +          resets = <&ccu 2>;
> > > > +          nvmem-cells = <&ths_calibration>;
> > > > +          nvmem-cell-names = "calibration";
> > > > +          #thermal-sensor-cells = <0>;
> > > >
> > > >> Is this calibration required, or optional, for the d1?
> > >
> > > Calibration data for the D1 is optional as for the H6.
> > > (But without it, the sensor data will be inaccurate).
> >
> > Okay. Modulo the signoff:
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>
> Wait, that's not what I meant to say... Clearly turned my brain off for
> a moment. If it is not going to function correctly without the
> calibration data, why not make the calibration data required for the
> d1's ths?

I don't really know why, but the calibration data is optional for
other controllers.
So I just did it the same way.
Here is explanation comment about it from the driver:
        /*
         * Even if the external calibration data stored in sid is
         * not accessible, the THS hardware can still work, although
         * the data won't be so accurate.
         *
         * The default value of calibration register is 0x800 for
         * every sensor, and the calibration value is usually 0x7xx
         * or 0x8xx, so they won't be away from the default value
         * for a lot.
         *
         * So here we do not return error if the calibration data is
         * not available, except the probe needs deferring.
         */

Re: [PATCH v1 3/3] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2244 bytes --]

On Sat, Jun 10, 2023 at 11:31:03PM +0300, Maxim Kiselev wrote:
> сб, 10 июн. 2023 г. в 21:01, Conor Dooley <conor@kernel.org>:
> 
> ...
> 
> > > > > +  - |
> > > > > +    thermal-sensor@2009400 {
> > > > > +          compatible = "allwinner,sun20i-d1-ths";
> > > > > +          reg = <0x02009400 0x400>;
> > > > > +          interrupts = <0 90 0>;
> > > > > +          clocks = <&ccu 0>;
> > > > > +          clock-names = "bus";
> > > > > +          resets = <&ccu 2>;
> > > > > +          nvmem-cells = <&ths_calibration>;
> > > > > +          nvmem-cell-names = "calibration";
> > > > > +          #thermal-sensor-cells = <0>;
> > > > >
> > > > >> Is this calibration required, or optional, for the d1?
> > > >
> > > > Calibration data for the D1 is optional as for the H6.
> > > > (But without it, the sensor data will be inaccurate).
> > >
> > > Okay. Modulo the signoff:
> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> >
> > Wait, that's not what I meant to say... Clearly turned my brain off for
> > a moment. If it is not going to function correctly without the
> > calibration data, why not make the calibration data required for the
> > d1's ths?
> 
> I don't really know why, but the calibration data is optional for
> other controllers.
> So I just did it the same way.
> Here is explanation comment about it from the driver:
>         /*
>          * Even if the external calibration data stored in sid is
>          * not accessible, the THS hardware can still work, although
>          * the data won't be so accurate.
>          *
>          * The default value of calibration register is 0x800 for
>          * every sensor, and the calibration value is usually 0x7xx
>          * or 0x8xx, so they won't be away from the default value
>          * for a lot.
>          *
>          * So here we do not return error if the calibration data is
>          * not available, except the probe needs deferring.
>          */

Wait, so if there is no calibration data then this thing will just defer
probing, ad infinitum? Looks like everything other than the a33 has the
cells in the dts, and the software seems to expect the cells.
What am I missing?



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 3/3] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

[Maxim Kiselev]

вс, 11 июн. 2023 г. в 01:14, Conor Dooley <conor@kernel.org>:
>
> On Sat, Jun 10, 2023 at 11:31:03PM +0300, Maxim Kiselev wrote:
> > сб, 10 июн. 2023 г. в 21:01, Conor Dooley <conor@kernel.org>:
> >
> > ...
> >
> > > > > > +  - |
> > > > > > +    thermal-sensor@2009400 {
> > > > > > +          compatible = "allwinner,sun20i-d1-ths";
> > > > > > +          reg = <0x02009400 0x400>;
> > > > > > +          interrupts = <0 90 0>;
> > > > > > +          clocks = <&ccu 0>;
> > > > > > +          clock-names = "bus";
> > > > > > +          resets = <&ccu 2>;
> > > > > > +          nvmem-cells = <&ths_calibration>;
> > > > > > +          nvmem-cell-names = "calibration";
> > > > > > +          #thermal-sensor-cells = <0>;
> > > > > >
> > > > > >> Is this calibration required, or optional, for the d1?
> > > > >
> > > > > Calibration data for the D1 is optional as for the H6.
> > > > > (But without it, the sensor data will be inaccurate).
> > > >
> > > > Okay. Modulo the signoff:
> > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > >
> > > Wait, that's not what I meant to say... Clearly turned my brain off for
> > > a moment. If it is not going to function correctly without the
> > > calibration data, why not make the calibration data required for the
> > > d1's ths?
> >
> > I don't really know why, but the calibration data is optional for
> > other controllers.
> > So I just did it the same way.
> > Here is explanation comment about it from the driver:
> >         /*
> >          * Even if the external calibration data stored in sid is
> >          * not accessible, the THS hardware can still work, although
> >          * the data won't be so accurate.
> >          *
> >          * The default value of calibration register is 0x800 for
> >          * every sensor, and the calibration value is usually 0x7xx
> >          * or 0x8xx, so they won't be away from the default value
> >          * for a lot.
> >          *
> >          * So here we do not return error if the calibration data is
> >          * not available, except the probe needs deferring.
> >          */
>
> Wait, so if there is no calibration data then this thing will just defer
> probing, ad infinitum? Looks like everything other than the a33 has the
> cells in the dts, and the software seems to expect the cells.
> What am I missing?

Not quite. If we don't have a calibration cell in the dts, then we
just skip the calibration.
We will only get EPROBE_DEFER if we have calibration in the dts.
Here is the whole function code:

static int sun8i_ths_calibrate(struct ths_device *tmdev)
{
    struct nvmem_cell *calcell;
    struct device *dev = tmdev->dev;
    u16 *caldata;
    size_t callen;
    int ret = 0;

    calcell = devm_nvmem_cell_get(dev, "calibration");
    if (IS_ERR(calcell)) {
        if (PTR_ERR(calcell) == -EPROBE_DEFER)
            return -EPROBE_DEFER;
        /*
         * Even if the external calibration data stored in sid is
         * not accessible, the THS hardware can still work, although
         * the data won't be so accurate.
         *
         * The default value of calibration register is 0x800 for
         * every sensor, and the calibration value is usually 0x7xx
         * or 0x8xx, so they won't be away from the default value
         * for a lot.
         *
         * So here we do not return error if the calibration data is
         * not available, except the probe needs deferring.
         */
        goto out;
    }

    caldata = nvmem_cell_read(calcell, &callen);
    if (IS_ERR(caldata)) {
        ret = PTR_ERR(caldata);
        goto out;
    }

    tmdev->chip->calibrate(tmdev, caldata, callen);

    kfree(caldata);
out:
    return ret;
}

Re: [PATCH v1 3/3] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2802 bytes --]

On Sun, Jun 11, 2023 at 10:53:56AM +0300, Maxim Kiselev wrote:
> вс, 11 июн. 2023 г. в 01:14, Conor Dooley <conor@kernel.org>:
> >
> > On Sat, Jun 10, 2023 at 11:31:03PM +0300, Maxim Kiselev wrote:
> > > сб, 10 июн. 2023 г. в 21:01, Conor Dooley <conor@kernel.org>:
> > >
> > > ...
> > >
> > > > > > > +  - |
> > > > > > > +    thermal-sensor@2009400 {
> > > > > > > +          compatible = "allwinner,sun20i-d1-ths";
> > > > > > > +          reg = <0x02009400 0x400>;
> > > > > > > +          interrupts = <0 90 0>;
> > > > > > > +          clocks = <&ccu 0>;
> > > > > > > +          clock-names = "bus";
> > > > > > > +          resets = <&ccu 2>;
> > > > > > > +          nvmem-cells = <&ths_calibration>;
> > > > > > > +          nvmem-cell-names = "calibration";
> > > > > > > +          #thermal-sensor-cells = <0>;
> > > > > > >
> > > > > > >> Is this calibration required, or optional, for the d1?
> > > > > >
> > > > > > Calibration data for the D1 is optional as for the H6.
> > > > > > (But without it, the sensor data will be inaccurate).
> > > > >
> > > > > Okay. Modulo the signoff:
> > > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > >
> > > > Wait, that's not what I meant to say... Clearly turned my brain off for
> > > > a moment. If it is not going to function correctly without the
> > > > calibration data, why not make the calibration data required for the
> > > > d1's ths?
> > >
> > > I don't really know why, but the calibration data is optional for
> > > other controllers.
> > > So I just did it the same way.
> > > Here is explanation comment about it from the driver:
> > >         /*
> > >          * Even if the external calibration data stored in sid is
> > >          * not accessible, the THS hardware can still work, although
> > >          * the data won't be so accurate.
> > >          *
> > >          * The default value of calibration register is 0x800 for
> > >          * every sensor, and the calibration value is usually 0x7xx
> > >          * or 0x8xx, so they won't be away from the default value
> > >          * for a lot.
> > >          *
> > >          * So here we do not return error if the calibration data is
> > >          * not available, except the probe needs deferring.
> > >          */
> >
> > Wait, so if there is no calibration data then this thing will just defer
> > probing, ad infinitum? Looks like everything other than the a33 has the
> > cells in the dts, and the software seems to expect the cells.
> > What am I missing?
> 
> Not quite. If we don't have a calibration cell in the dts, then we
> just skip the calibration.
> We will only get EPROBE_DEFER if we have calibration in the dts.

Ah that makes a lot more sense, thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]