* [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues
[not found] <cover.1698743706.git.zhoubinbin@loongson.cn>
@ 2023-10-31 11:05 ` Binbin Zhou
2023-10-31 16:58 ` Conor Dooley
2023-10-31 11:05 ` [PATCH 2/2] drivers/thermal/loongson2_thermal: Fix incorrect PTR_ERR() judgment Binbin Zhou
1 sibling, 1 reply; 7+ messages in thread
From: Binbin Zhou @ 2023-10-31 11:05 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Rafael J . Wysocki, Daniel Lezcano,
Amit Kucheria, Zhang Rui, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Huacai Chen, loongson-kernel, linux-pm, devicetree, Yinbo Zhu,
WANG Xuerui, loongarch, Binbin Zhou
Add the missing 'thermal-sensor-cells' property which is required for
every thermal sensor as it's used when using phandles.
And add the thermal-sensor.yaml reference.
Fixes: 72684d99a854 ("thermal: dt-bindings: add loongson-2 thermal")
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
.../bindings/thermal/loongson,ls2k-thermal.yaml | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
index 7538469997f9..b634f57cd011 100644
--- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
@@ -10,6 +10,9 @@ maintainers:
- zhanghongchen <zhanghongchen@loongson.cn>
- Yinbo Zhu <zhuyinbo@loongson.cn>
+allOf:
+ - $ref: /schemas/thermal/thermal-sensor.yaml#
+
properties:
compatible:
oneOf:
@@ -26,12 +29,16 @@ properties:
interrupts:
maxItems: 1
+ '#thermal-sensor-cells':
+ const: 1
+
required:
- compatible
- reg
- interrupts
+ - '#thermal-sensor-cells'
-additionalProperties: false
+unevaluatedProperties: false
examples:
- |
@@ -41,4 +48,5 @@ examples:
reg = <0x1fe01500 0x30>;
interrupt-parent = <&liointc0>;
interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+ #thermal-sensor-cells = <1>;
};
--
2.39.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] drivers/thermal/loongson2_thermal: Fix incorrect PTR_ERR() judgment
[not found] <cover.1698743706.git.zhoubinbin@loongson.cn>
2023-10-31 11:05 ` [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues Binbin Zhou
@ 2023-10-31 11:05 ` Binbin Zhou
1 sibling, 0 replies; 7+ messages in thread
From: Binbin Zhou @ 2023-10-31 11:05 UTC (permalink / raw)
To: Binbin Zhou, Huacai Chen, Rafael J . Wysocki, Daniel Lezcano,
Amit Kucheria, Zhang Rui, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Huacai Chen, loongson-kernel, linux-pm, devicetree, Yinbo Zhu,
WANG Xuerui, loongarch, Binbin Zhou
PTR_ERR() returns -ENODEV when thermal-zones are undefined, and we need
-ENODEV as the right value for comparison.
Otherwise, tz->type is NULL when thermal-zones is undefined, resulting
in the following error:
[ 12.290030] CPU 1 Unable to handle kernel paging request at virtual address fffffffffffffff1, era == 900000000355f410, ra == 90000000031579b8
[ 12.302877] Oops[#1]:
[ 12.305190] CPU: 1 PID: 181 Comm: systemd-udevd Not tainted 6.6.0-rc7+ #5385
[ 12.312304] pc 900000000355f410 ra 90000000031579b8 tp 90000001069e8000 sp 90000001069eba10
[ 12.320739] a0 0000000000000000 a1 fffffffffffffff1 a2 0000000000000014 a3 0000000000000001
[ 12.329173] a4 90000001069eb990 a5 0000000000000001 a6 0000000000001001 a7 900000010003431c
[ 12.337606] t0 fffffffffffffff1 t1 54567fd5da9b4fd4 t2 900000010614ec40 t3 00000000000dc901
[ 12.346041] t4 0000000000000000 t5 0000000000000004 t6 900000010614ee20 t7 900000000d00b790
[ 12.354472] t8 00000000000dc901 u0 54567fd5da9b4fd4 s9 900000000402ae10 s0 900000010614ec40
[ 12.362916] s1 90000000039fced0 s2 ffffffffffffffed s3 ffffffffffffffed s4 9000000003acc000
[ 12.362931] s5 0000000000000004 s6 fffffffffffff000 s7 0000000000000490 s8 90000001028b2ec8
[ 12.362938] ra: 90000000031579b8 thermal_add_hwmon_sysfs+0x258/0x300
[ 12.386411] ERA: 900000000355f410 strscpy+0xf0/0x160
[ 12.391626] CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
[ 12.397898] PRMD: 00000004 (PPLV0 +PIE -PWE)
[ 12.403678] EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
[ 12.409859] ECFG: 00071c1c (LIE=2-4,10-12 VS=7)
[ 12.415882] ESTAT: 00010000 [PIL] (IS= ECode=1 EsubCode=0)
[ 12.415907] BADV: fffffffffffffff1
[ 12.415911] PRID: 0014a000 (Loongson-64bit, Loongson-2K1000)
[ 12.415917] Modules linked in: loongson2_thermal(+) vfat fat uio_pdrv_genirq uio fuse zram zsmalloc
[ 12.415950] Process systemd-udevd (pid: 181, threadinfo=00000000358b9718, task=00000000ace72fe3)
[ 12.415961] Stack : 0000000000000dc0 54567fd5da9b4fd4 900000000402ae10 9000000002df9358
[ 12.415982] ffffffffffffffed 0000000000000004 9000000107a10aa8 90000001002a3410
[ 12.415999] ffffffffffffffed ffffffffffffffed 9000000107a11268 9000000003157ab0
[ 12.416016] 9000000107a10aa8 ffffff80020fc0c8 90000001002a3410 ffffffffffffffed
[ 12.416032] 0000000000000024 ffffff80020cc1e8 900000000402b2a0 9000000003acc000
[ 12.416048] 90000001002a3410 0000000000000000 ffffff80020f4030 90000001002a3410
[ 12.416065] 0000000000000000 9000000002df6808 90000001002a3410 0000000000000000
[ 12.416081] ffffff80020f4030 0000000000000000 90000001002a3410 9000000002df2ba8
[ 12.416097] 00000000000000b4 90000001002a34f4 90000001002a3410 0000000000000002
[ 12.416114] ffffff80020f4030 fffffffffffffff0 90000001002a3410 9000000002df2f30
[ 12.416131] ...
[ 12.416138] Call Trace:
[ 12.416142] [<900000000355f410>] strscpy+0xf0/0x160
[ 12.416167] [<90000000031579b8>] thermal_add_hwmon_sysfs+0x258/0x300
[ 12.416183] [<9000000003157ab0>] devm_thermal_add_hwmon_sysfs+0x50/0xe0
[ 12.416200] [<ffffff80020cc1e8>] loongson2_thermal_probe+0x128/0x200 [loongson2_thermal]
[ 12.416232] [<9000000002df6808>] platform_probe+0x68/0x140
[ 12.416249] [<9000000002df2ba8>] really_probe+0xc8/0x3c0
[ 12.416269] [<9000000002df2f30>] __driver_probe_device+0x90/0x180
[ 12.416286] [<9000000002df3058>] driver_probe_device+0x38/0x160
[ 12.416302] [<9000000002df33a8>] __driver_attach+0xa8/0x200
[ 12.416314] [<9000000002deffec>] bus_for_each_dev+0x8c/0x120
[ 12.416330] [<9000000002df198c>] bus_add_driver+0x10c/0x2a0
[ 12.416346] [<9000000002df46b4>] driver_register+0x74/0x160
[ 12.416358] [<90000000022201a4>] do_one_initcall+0x84/0x220
[ 12.416372] [<90000000022f3ab8>] do_init_module+0x58/0x2c0
[ 12.416386] [<90000000022f6538>] init_module_from_file+0x98/0x100
[ 12.416399] [<90000000022f67f0>] sys_finit_module+0x230/0x3c0
[ 12.416412] [<900000000358f7c8>] do_syscall+0x88/0xc0
[ 12.416431] [<900000000222137c>] handle_syscall+0xbc/0x158
Fixes: e7e3a7c35791 ("thermal/drivers/loongson-2: Add thermal management support")
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
drivers/thermal/loongson2_thermal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/loongson2_thermal.c b/drivers/thermal/loongson2_thermal.c
index 133098dc0854..99ca0c7bc41c 100644
--- a/drivers/thermal/loongson2_thermal.c
+++ b/drivers/thermal/loongson2_thermal.c
@@ -127,7 +127,7 @@ static int loongson2_thermal_probe(struct platform_device *pdev)
if (!IS_ERR(tzd))
break;
- if (PTR_ERR(tzd) != ENODEV)
+ if (PTR_ERR(tzd) != -ENODEV)
continue;
return dev_err_probe(dev, PTR_ERR(tzd), "failed to register");
--
2.39.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues
2023-10-31 11:05 ` [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues Binbin Zhou
@ 2023-10-31 16:58 ` Conor Dooley
2023-11-01 1:38 ` Binbin Zhou
0 siblings, 1 reply; 7+ messages in thread
From: Conor Dooley @ 2023-10-31 16:58 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Rafael J . Wysocki, Daniel Lezcano,
Amit Kucheria, Zhang Rui, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Huacai Chen, loongson-kernel, linux-pm, devicetree,
Yinbo Zhu, WANG Xuerui, loongarch
[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]
On Tue, Oct 31, 2023 at 07:05:49PM +0800, Binbin Zhou wrote:
> Add the missing 'thermal-sensor-cells' property which is required for
> every thermal sensor as it's used when using phandles.
> And add the thermal-sensor.yaml reference.
>
> Fixes: 72684d99a854 ("thermal: dt-bindings: add loongson-2 thermal")
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
> .../bindings/thermal/loongson,ls2k-thermal.yaml | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> index 7538469997f9..b634f57cd011 100644
> --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> @@ -10,6 +10,9 @@ maintainers:
> - zhanghongchen <zhanghongchen@loongson.cn>
> - Yinbo Zhu <zhuyinbo@loongson.cn>
>
> +allOf:
> + - $ref: /schemas/thermal/thermal-sensor.yaml#
> +
> properties:
> compatible:
> oneOf:
> @@ -26,12 +29,16 @@ properties:
> interrupts:
> maxItems: 1
>
> + '#thermal-sensor-cells':
> + const: 1
> +
> required:
> - compatible
> - reg
> - interrupts
> + - '#thermal-sensor-cells'
Why does it need to be a required property now though?
Adding new required properties is technically an ABI break.
Cheers,
Conor.
>
> -additionalProperties: false
> +unevaluatedProperties: false
>
> examples:
> - |
> @@ -41,4 +48,5 @@ examples:
> reg = <0x1fe01500 0x30>;
> interrupt-parent = <&liointc0>;
> interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> + #thermal-sensor-cells = <1>;
> };
> --
> 2.39.3
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues
2023-10-31 16:58 ` Conor Dooley
@ 2023-11-01 1:38 ` Binbin Zhou
2023-11-01 7:59 ` Conor Dooley
0 siblings, 1 reply; 7+ messages in thread
From: Binbin Zhou @ 2023-11-01 1:38 UTC (permalink / raw)
To: Conor Dooley
Cc: Binbin Zhou, Huacai Chen, Rafael J . Wysocki, Daniel Lezcano,
Amit Kucheria, Zhang Rui, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Huacai Chen, loongson-kernel, linux-pm, devicetree,
Yinbo Zhu, WANG Xuerui, loongarch
On Tue, Oct 31, 2023 at 10:58 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Oct 31, 2023 at 07:05:49PM +0800, Binbin Zhou wrote:
> > Add the missing 'thermal-sensor-cells' property which is required for
> > every thermal sensor as it's used when using phandles.
> > And add the thermal-sensor.yaml reference.
> >
> > Fixes: 72684d99a854 ("thermal: dt-bindings: add loongson-2 thermal")
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> > .../bindings/thermal/loongson,ls2k-thermal.yaml | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > index 7538469997f9..b634f57cd011 100644
> > --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > @@ -10,6 +10,9 @@ maintainers:
> > - zhanghongchen <zhanghongchen@loongson.cn>
> > - Yinbo Zhu <zhuyinbo@loongson.cn>
> >
> > +allOf:
> > + - $ref: /schemas/thermal/thermal-sensor.yaml#
> > +
> > properties:
> > compatible:
> > oneOf:
> > @@ -26,12 +29,16 @@ properties:
> > interrupts:
> > maxItems: 1
> >
> > + '#thermal-sensor-cells':
> > + const: 1
> > +
> > required:
> > - compatible
> > - reg
> > - interrupts
> > + - '#thermal-sensor-cells'
>
> Why does it need to be a required property now though?
> Adding new required properties is technically an ABI break.
Hi Conor:
I don't think it makes sense to have a separate thermal sensor
definition, it needs thermal-zones to describe specific behaviors,
e.g. cpu-thermal, so we need '#thermal-sensor-cells' to specify the
reference.
And the Loongson-2K1000 has 4 sets of control registers, we need to
specify the id when referencing it.
Thanks.
Binbin
>
> Cheers,
> Conor.
>
> >
> > -additionalProperties: false
> > +unevaluatedProperties: false
> >
> > examples:
> > - |
> > @@ -41,4 +48,5 @@ examples:
> > reg = <0x1fe01500 0x30>;
> > interrupt-parent = <&liointc0>;
> > interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> > + #thermal-sensor-cells = <1>;
> > };
> > --
> > 2.39.3
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues
2023-11-01 1:38 ` Binbin Zhou
@ 2023-11-01 7:59 ` Conor Dooley
2023-11-15 6:50 ` Binbin Zhou
0 siblings, 1 reply; 7+ messages in thread
From: Conor Dooley @ 2023-11-01 7:59 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Rafael J . Wysocki, Daniel Lezcano,
Amit Kucheria, Zhang Rui, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Huacai Chen, loongson-kernel, linux-pm, devicetree,
Yinbo Zhu, WANG Xuerui, loongarch
[-- Attachment #1: Type: text/plain, Size: 2164 bytes --]
On Wed, Nov 01, 2023 at 07:38:39AM +0600, Binbin Zhou wrote:
> On Tue, Oct 31, 2023 at 10:58 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Tue, Oct 31, 2023 at 07:05:49PM +0800, Binbin Zhou wrote:
> > > Add the missing 'thermal-sensor-cells' property which is required for
> > > every thermal sensor as it's used when using phandles.
> > > And add the thermal-sensor.yaml reference.
> > >
> > > Fixes: 72684d99a854 ("thermal: dt-bindings: add loongson-2 thermal")
> > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > ---
> > > .../bindings/thermal/loongson,ls2k-thermal.yaml | 10 +++++++++-
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > index 7538469997f9..b634f57cd011 100644
> > > --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > @@ -10,6 +10,9 @@ maintainers:
> > > - zhanghongchen <zhanghongchen@loongson.cn>
> > > - Yinbo Zhu <zhuyinbo@loongson.cn>
> > >
> > > +allOf:
> > > + - $ref: /schemas/thermal/thermal-sensor.yaml#
> > > +
> > > properties:
> > > compatible:
> > > oneOf:
> > > @@ -26,12 +29,16 @@ properties:
> > > interrupts:
> > > maxItems: 1
> > >
> > > + '#thermal-sensor-cells':
> > > + const: 1
> > > +
> > > required:
> > > - compatible
> > > - reg
> > > - interrupts
> > > + - '#thermal-sensor-cells'
> >
> > Why does it need to be a required property now though?
> > Adding new required properties is technically an ABI break.
>
> Hi Conor:
>
> I don't think it makes sense to have a separate thermal sensor
> definition, it needs thermal-zones to describe specific behaviors,
> e.g. cpu-thermal, so we need '#thermal-sensor-cells' to specify the
> reference.
> And the Loongson-2K1000 has 4 sets of control registers, we need to
> specify the id when referencing it.
Unfortunately, none of this is an answer to my question.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues
2023-11-01 7:59 ` Conor Dooley
@ 2023-11-15 6:50 ` Binbin Zhou
2023-11-15 14:54 ` Conor Dooley
0 siblings, 1 reply; 7+ messages in thread
From: Binbin Zhou @ 2023-11-15 6:50 UTC (permalink / raw)
To: Conor Dooley
Cc: Binbin Zhou, Huacai Chen, Rafael J . Wysocki, Daniel Lezcano,
Amit Kucheria, Zhang Rui, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Huacai Chen, loongson-kernel, linux-pm, devicetree,
Yinbo Zhu, WANG Xuerui, loongarch
On Wed, Nov 1, 2023 at 1:59 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Wed, Nov 01, 2023 at 07:38:39AM +0600, Binbin Zhou wrote:
> > On Tue, Oct 31, 2023 at 10:58 PM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Tue, Oct 31, 2023 at 07:05:49PM +0800, Binbin Zhou wrote:
> > > > Add the missing 'thermal-sensor-cells' property which is required for
> > > > every thermal sensor as it's used when using phandles.
> > > > And add the thermal-sensor.yaml reference.
> > > >
> > > > Fixes: 72684d99a854 ("thermal: dt-bindings: add loongson-2 thermal")
> > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > ---
> > > > .../bindings/thermal/loongson,ls2k-thermal.yaml | 10 +++++++++-
> > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > > index 7538469997f9..b634f57cd011 100644
> > > > --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > > +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > > @@ -10,6 +10,9 @@ maintainers:
> > > > - zhanghongchen <zhanghongchen@loongson.cn>
> > > > - Yinbo Zhu <zhuyinbo@loongson.cn>
> > > >
> > > > +allOf:
> > > > + - $ref: /schemas/thermal/thermal-sensor.yaml#
> > > > +
> > > > properties:
> > > > compatible:
> > > > oneOf:
> > > > @@ -26,12 +29,16 @@ properties:
> > > > interrupts:
> > > > maxItems: 1
> > > >
> > > > + '#thermal-sensor-cells':
> > > > + const: 1
> > > > +
> > > > required:
> > > > - compatible
> > > > - reg
> > > > - interrupts
> > > > + - '#thermal-sensor-cells'
> > >
> > > Why does it need to be a required property now though?
> > > Adding new required properties is technically an ABI break.
> >
> > Hi Conor:
> >
> > I don't think it makes sense to have a separate thermal sensor
> > definition, it needs thermal-zones to describe specific behaviors,
> > e.g. cpu-thermal, so we need '#thermal-sensor-cells' to specify the
> > reference.
> > And the Loongson-2K1000 has 4 sets of control registers, we need to
> > specify the id when referencing it.
>
> Unfortunately, none of this is an answer to my question.
Hi Conor:
Sorry for my late reply.
Over the past few days, I've been communicating offline with Yinbo
(the driver author) about the use of the '#thermal-sensor-cells'
attribute. He retested the attribute and determined that it is
'required'.
We can see that the '#thermal-sensor-cells' attribute in the
dt-binding was dropped between the V12 patchset[1] and the V13
patchset[2]. Yinbo may have misunderstood Daniel's comment and removed
the '#thermal-sensor-cells' attribute from the dt-binding. But the
attribute was carelessly still left in the dts file, resulting in the
issue not being found during functional validation.
Indeed, re-adding the '#thermal-sensor-cells' attribute as "required"
is technically an ABI breakage, but the driver does not work properly
under the current dt-binding rules.
Also, the driver is only valid under LoongArch and will have no effect
on other architectures.
[1]:https://lore.kernel.org/all/20221114024709.7975-2-zhuyinbo@loongson.cn/
[2]:https://lore.kernel.org/all/20230221095355.9799-2-zhuyinbo@loongson.cn/
Thanks.
Binbin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues
2023-11-15 6:50 ` Binbin Zhou
@ 2023-11-15 14:54 ` Conor Dooley
0 siblings, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2023-11-15 14:54 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Rafael J . Wysocki, Daniel Lezcano,
Amit Kucheria, Zhang Rui, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Huacai Chen, loongson-kernel, linux-pm, devicetree,
Yinbo Zhu, WANG Xuerui, loongarch
[-- Attachment #1: Type: text/plain, Size: 3757 bytes --]
On Wed, Nov 15, 2023 at 12:50:41PM +0600, Binbin Zhou wrote:
> On Wed, Nov 1, 2023 at 1:59 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Wed, Nov 01, 2023 at 07:38:39AM +0600, Binbin Zhou wrote:
> > > On Tue, Oct 31, 2023 at 10:58 PM Conor Dooley <conor@kernel.org> wrote:
> > > >
> > > > On Tue, Oct 31, 2023 at 07:05:49PM +0800, Binbin Zhou wrote:
> > > > > Add the missing 'thermal-sensor-cells' property which is required for
> > > > > every thermal sensor as it's used when using phandles.
> > > > > And add the thermal-sensor.yaml reference.
> > > > >
> > > > > Fixes: 72684d99a854 ("thermal: dt-bindings: add loongson-2 thermal")
> > > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > > ---
> > > > > .../bindings/thermal/loongson,ls2k-thermal.yaml | 10 +++++++++-
> > > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > > > index 7538469997f9..b634f57cd011 100644
> > > > > --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > > > +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> > > > > @@ -10,6 +10,9 @@ maintainers:
> > > > > - zhanghongchen <zhanghongchen@loongson.cn>
> > > > > - Yinbo Zhu <zhuyinbo@loongson.cn>
> > > > >
> > > > > +allOf:
> > > > > + - $ref: /schemas/thermal/thermal-sensor.yaml#
> > > > > +
> > > > > properties:
> > > > > compatible:
> > > > > oneOf:
> > > > > @@ -26,12 +29,16 @@ properties:
> > > > > interrupts:
> > > > > maxItems: 1
> > > > >
> > > > > + '#thermal-sensor-cells':
> > > > > + const: 1
> > > > > +
> > > > > required:
> > > > > - compatible
> > > > > - reg
> > > > > - interrupts
> > > > > + - '#thermal-sensor-cells'
> > > >
> > > > Why does it need to be a required property now though?
> > > > Adding new required properties is technically an ABI break.
> > >
> > > Hi Conor:
> > >
> > > I don't think it makes sense to have a separate thermal sensor
> > > definition, it needs thermal-zones to describe specific behaviors,
> > > e.g. cpu-thermal, so we need '#thermal-sensor-cells' to specify the
> > > reference.
> > > And the Loongson-2K1000 has 4 sets of control registers, we need to
> > > specify the id when referencing it.
> >
> > Unfortunately, none of this is an answer to my question.
>
> Hi Conor:
>
> Sorry for my late reply.
>
> Over the past few days, I've been communicating offline with Yinbo
> (the driver author) about the use of the '#thermal-sensor-cells'
> attribute. He retested the attribute and determined that it is
> 'required'.
>
> We can see that the '#thermal-sensor-cells' attribute in the
> dt-binding was dropped between the V12 patchset[1] and the V13
> patchset[2]. Yinbo may have misunderstood Daniel's comment and removed
> the '#thermal-sensor-cells' attribute from the dt-binding. But the
> attribute was carelessly still left in the dts file, resulting in the
> issue not being found during functional validation.
>
> Indeed, re-adding the '#thermal-sensor-cells' attribute as "required"
> is technically an ABI breakage, but the driver does not work properly
> under the current dt-binding rules.
I was going to say that you should add some comment about this in the
commit message, but at the end of the day - you're not much of a thermal
sensor without having thermal-sensor-cells, so I think your commit
message is actually fine.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
and I probably should have recognised that earlier.
Thanks,
Conor
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-11-15 14:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1698743706.git.zhoubinbin@loongson.cn>
2023-10-31 11:05 ` [PATCH 1/2] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues Binbin Zhou
2023-10-31 16:58 ` Conor Dooley
2023-11-01 1:38 ` Binbin Zhou
2023-11-01 7:59 ` Conor Dooley
2023-11-15 6:50 ` Binbin Zhou
2023-11-15 14:54 ` Conor Dooley
2023-10-31 11:05 ` [PATCH 2/2] drivers/thermal/loongson2_thermal: Fix incorrect PTR_ERR() judgment Binbin Zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).