Re: [PATCH 2/2] ARM: dts: st: spear: fix dtbs warning on spear thermal sensor

From: Gopi Krishna Menon

Date: Tue Mar 24 2026 - 09:38:47 EST


On Tue, Mar 24, 2026 at 11:01:05AM +0100, Krzysztof Kozlowski wrote:

> On 24/03/2026 11:00, Daniel Baluta wrote:
> > On 3/24/26 11:26, Krzysztof Kozlowski wrote:
> >> On Mon, Mar 23, 2026 at 07:08:09PM +0530, Gopi Krishna Menon wrote:
> >>> Running DTBS checks on st/spear1340-evb.dtb results in the following
> >>> warning:
> >>>
> >>> thermal@e07008c4 (st,thermal-spear1340): Unevaluated properties are not allowed ('thermal_flags' was unexpected)
> >>> from schema $id: http://devicetree.org/schemas/thermal/st,thermal-spear1340.yaml
> >> How is it possible if there is no such file?
> >>
> >> Did you just add new warning in patch #1 and then claim in patch #2 that
> >> you fix it?
> >>
> >> You completely miss the point why this change is needed: how could the
> >> DTS work before? It could not. And that should be your justification for
> >> the patch, with explanation why it could not work.
> >
> > Correct me if I'm wrong but I think there was a hidden bug here 
> >
> > drivers/thermal/spear_thermal.c:spear_thermal_probe:
> >
> >       if (!np || !of_property_read_u32(np, "st,thermal-flags", &val)) {
> > »       »       dev_err(&pdev->dev, "Failed: DT Pdata not passed\n");                                                                                                                         
> > »       »       return -EINVAL;
> > »       }
> >
> > So, the driver was checking for the correct property as pointed by 
>
> Yes
>
> >
> > bindings/thermal/spear-thermal.txt but the dts was using the wrong
>
> No, DTS had two properties - correct one and incorrect.
>

Yup, spear13xx is included by spear1310.dtsi and spear1340.dtsi both of
which have st,thermal-flags correctly defined. When working on this
patch, after converting the binding, I ran dtbs check and upon seeing
dtbs warning immediately fixed it thinking it was complaining because
st,thermal-flags (thermal_flags) was written incorrectly. (Which is not
incorrect as st,thermal-flags was there in the final node but there was one
additional property named thermal_flags as well which is not allowed as
UnevaluatedProperty is set to false).

Also I dont think we have to override the st,thermal-flags property in
spear1310.dtsi as the flags value is same in spear13xx.dtsi and
spear1310.dtsi (0x7000).

> >
> > property name: arch/arm/boot/dts/st/spear13xx.dtsi » » » thermal@e07008c4 { » » » » compatible = "st,thermal-spear1340"; » » » » reg = <0xe07008c4 0x4>; » » » » thermal_flags = <0x7000>; » » » }; And because this check is wrong:
> >
> >       if (!np || !of_property_read_u32(np, "st,thermal-flags", &val)) {
> >
> > people really didn't notice it.
> >
> > The check should be:
> >
> >       if (!np || of_property_read_u32(np, "st,thermal-flags", &val)) {
> > »       »       dev_err(&pdev->dev, "Failed: DT Pdata not passed\n");                                                                                                                         
> > »       »       return -EINVAL;
> > »       }
> >
> > So, this actual patch has uncovered a bug! 
>
> Yes. Driver also has bug, so probably was never working. The point is
> whatever commit is doing, the dtbs_check warning is not the
> justification, because it was introduced by this patchset.
>

Please correct me if I am wrong Krzysztof but should i send a seperate patch
fixing the thermal_flags property (Not connected to these converted
bindings)?.

Also, is it necessary to set additionalProperties to true? I am not able to
figure out if that is needed.

> Best regards,
> Krzysztof

Thanks,
Gopi Krishna Menon