mbox series

[0/9] arm64: dts: renesas: Thermal binding validation

Message ID 20211104224033.3997504-1-kieran.bingham+renesas@ideasonboard.com (mailing list archive)
Headers show
Series arm64: dts: renesas: Thermal binding validation | expand

Message

Kieran Bingham Nov. 4, 2021, 10:40 p.m. UTC
The thermal sensor bindings were not matched correctly against the
expected naming scheme.

r8a77980.dtsi also used a different naming scheme compared to the other
related platforms.

This series cleans up the dtsi files for the CPU target thermal sensors,
allowing the validation to run.

Enabling this validation shows up a new validation failure:

linux/arch/arm64/boot/dts/renesas/r8a77951-ulcb-kf.dt.yaml: thermal-zones: sensor3-thermal:cooling-maps:map0:contribution:0:0: 1024 is greater than the maximum of 100
	From schema: Documentation/devicetree/bindings/thermal/thermal-zones.yaml

This validation error appears to be pervasive across all of these
bindings, but changing that will be more invasive and require someone to
perform dedicated testing with the thermal drivers to ensure that the
updates to the ranges do not cause unexpected side effects.

Kieran Bingham (9):
  arm64: dts: renesas: r8a774a1: Fix thermal bindings
  arm64: dts: renesas: r8a774b1: Fix thermal bindings
  arm64: dts: renesas: r8a774e1: Fix thermal bindings
  arm64: dts: renesas: r8a77951: Fix thermal bindings
  arm64: dts: renesas: r8a77960: Fix thermal bindings
  arm64: dts: renesas: r8a77961: Fix thermal bindings
  arm64: dts: renesas: r8a77965: Fix thermal bindings
  arm64: dts: renesas: r8a77980: Fix thermal bindings
  arm64: dts: renesas: r8a779a0: Fix thermal bindings

 arch/arm64/boot/dts/renesas/r8a774a1.dtsi |  6 +++---
 arch/arm64/boot/dts/renesas/r8a774b1.dtsi |  6 +++---
 arch/arm64/boot/dts/renesas/r8a774e1.dtsi |  6 +++---
 arch/arm64/boot/dts/renesas/r8a77951.dtsi |  6 +++---
 arch/arm64/boot/dts/renesas/r8a77960.dtsi |  6 +++---
 arch/arm64/boot/dts/renesas/r8a77961.dtsi |  6 +++---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi |  6 +++---
 arch/arm64/boot/dts/renesas/r8a77980.dtsi |  4 ++--
 arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 10 +++++-----
 9 files changed, 28 insertions(+), 28 deletions(-)

Comments

Geert Uytterhoeven Nov. 9, 2021, 8:29 a.m. UTC | #1
Hi Kieran,

On Thu, Nov 4, 2021 at 11:40 PM Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
> The thermal sensor bindings were not matched correctly against the
> expected naming scheme.
>
> r8a77980.dtsi also used a different naming scheme compared to the other
> related platforms.

It lacked the labels, which you added for consistency.
Is there any point in providing them, as there are no users? Or should
they be removed instead?

> This series cleans up the dtsi files for the CPU target thermal sensors,
> allowing the validation to run.
>
> Enabling this validation shows up a new validation failure:
>
> linux/arch/arm64/boot/dts/renesas/r8a77951-ulcb-kf.dt.yaml: thermal-zones: sensor3-thermal:cooling-maps:map0:contribution:0:0: 1024 is greater than the maximum of 100
>         From schema: Documentation/devicetree/bindings/thermal/thermal-zones.yaml
>
> This validation error appears to be pervasive across all of these
> bindings, but changing that will be more invasive and require someone to
> perform dedicated testing with the thermal drivers to ensure that the
> updates to the ranges do not cause unexpected side effects.

Niklas?

> Kieran Bingham (9):
>   arm64: dts: renesas: r8a774a1: Fix thermal bindings
>   arm64: dts: renesas: r8a774b1: Fix thermal bindings
>   arm64: dts: renesas: r8a774e1: Fix thermal bindings
>   arm64: dts: renesas: r8a77951: Fix thermal bindings
>   arm64: dts: renesas: r8a77960: Fix thermal bindings
>   arm64: dts: renesas: r8a77961: Fix thermal bindings
>   arm64: dts: renesas: r8a77965: Fix thermal bindings
>   arm64: dts: renesas: r8a77980: Fix thermal bindings
>   arm64: dts: renesas: r8a779a0: Fix thermal bindings

For the whole series:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Niklas Söderlund Nov. 9, 2021, 8:43 a.m. UTC | #2
Hello,

On 2021-11-09 09:29:01 +0100, Geert Uytterhoeven wrote:
> Hi Kieran,
> 
> On Thu, Nov 4, 2021 at 11:40 PM Kieran Bingham
> <kieran.bingham+renesas@ideasonboard.com> wrote:
> > The thermal sensor bindings were not matched correctly against the
> > expected naming scheme.
> >
> > r8a77980.dtsi also used a different naming scheme compared to the other
> > related platforms.
> 
> It lacked the labels, which you added for consistency.
> Is there any point in providing them, as there are no users? Or should
> they be removed instead?
> 
> > This series cleans up the dtsi files for the CPU target thermal sensors,
> > allowing the validation to run.
> >
> > Enabling this validation shows up a new validation failure:
> >
> > linux/arch/arm64/boot/dts/renesas/r8a77951-ulcb-kf.dt.yaml: thermal-zones: sensor3-thermal:cooling-maps:map0:contribution:0:0: 1024 is greater than the maximum of 100
> >         From schema: Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> >
> > This validation error appears to be pervasive across all of these
> > bindings, but changing that will be more invasive and require someone to
> > perform dedicated testing with the thermal drivers to ensure that the
> > updates to the ranges do not cause unexpected side effects.
> 
> Niklas?

I will have a look. The thermal driver is the one driver where I have 
automated CI test running.

> 
> > Kieran Bingham (9):
> >   arm64: dts: renesas: r8a774a1: Fix thermal bindings
> >   arm64: dts: renesas: r8a774b1: Fix thermal bindings
> >   arm64: dts: renesas: r8a774e1: Fix thermal bindings
> >   arm64: dts: renesas: r8a77951: Fix thermal bindings
> >   arm64: dts: renesas: r8a77960: Fix thermal bindings
> >   arm64: dts: renesas: r8a77961: Fix thermal bindings
> >   arm64: dts: renesas: r8a77965: Fix thermal bindings
> >   arm64: dts: renesas: r8a77980: Fix thermal bindings
> >   arm64: dts: renesas: r8a779a0: Fix thermal bindings
> 
> For the whole series:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Niklas Söderlund Nov. 9, 2021, 9:09 a.m. UTC | #3
On 2021-11-09 09:43:33 +0100, Niklas Söderlund wrote:
> > > linux/arch/arm64/boot/dts/renesas/r8a77951-ulcb-kf.dt.yaml: 
> > > thermal-zones: sensor3-thermal:cooling-maps:map0:contribution:0:0: 
> > > 1024 is greater than the maximum of 100
> > >         From schema: Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> > >
> > > This validation error appears to be pervasive across all of these
> > > bindings, but changing that will be more invasive and require someone to
> > > perform dedicated testing with the thermal drivers to ensure that the
> > > updates to the ranges do not cause unexpected side effects.
> > 
> > Niklas?
> 
> I will have a look. The thermal driver is the one driver where I have 
> automated CI test running.

So the core of the issue is that the definition of the property changed 
in the txt to yaml conversion. The original definition was,

  Optional property:
  - contribution:         The cooling contribution to the thermal zone of the
    Type: unsigned        referred cooling device at the referred trip point.
    Size: one cell        The contribution is a ratio of the sum
			    of all cooling contributions within a thermal zone.

While the  new binding states,

  contribution:
    $ref: /schemas/types.yaml#/definitions/uint32
    minimum: 0
    maximum: 100 
    description:
      The percentage contribution of the cooling devices at the 
      specific trip temperature referenced in this map 
      to this thermal zone

Looking at the real world usage of this only 2 out of 17 platforms sets 
a contribution value less or equal to 100. I will send a patch to fix 
the bindings.
Geert Uytterhoeven Nov. 30, 2021, 4:45 p.m. UTC | #4
Hi Niklas,

On Tue, Nov 9, 2021 at 10:09 AM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> On 2021-11-09 09:43:33 +0100, Niklas Söderlund wrote:
> > > > linux/arch/arm64/boot/dts/renesas/r8a77951-ulcb-kf.dt.yaml:
> > > > thermal-zones: sensor3-thermal:cooling-maps:map0:contribution:0:0:
> > > > 1024 is greater than the maximum of 100
> > > >         From schema: Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> > > >
> > > > This validation error appears to be pervasive across all of these
> > > > bindings, but changing that will be more invasive and require someone to
> > > > perform dedicated testing with the thermal drivers to ensure that the
> > > > updates to the ranges do not cause unexpected side effects.
> > >
> > > Niklas?
> >
> > I will have a look. The thermal driver is the one driver where I have
> > automated CI test running.
>
> So the core of the issue is that the definition of the property changed
> in the txt to yaml conversion. The original definition was,
>
>   Optional property:
>   - contribution:         The cooling contribution to the thermal zone of the
>     Type: unsigned        referred cooling device at the referred trip point.
>     Size: one cell        The contribution is a ratio of the sum
>                             of all cooling contributions within a thermal zone.
>
> While the  new binding states,
>
>   contribution:
>     $ref: /schemas/types.yaml#/definitions/uint32
>     minimum: 0
>     maximum: 100
>     description:
>       The percentage contribution of the cooling devices at the
>       specific trip temperature referenced in this map
>       to this thermal zone
>
> Looking at the real world usage of this only 2 out of 17 platforms sets
> a contribution value less or equal to 100. I will send a patch to fix
> the bindings.

Given Rob said he applied your patch[1], does that mean this series
is good to be applied?
Thanks!

[1] https://lore.kernel.org/all/YaU4XuiaJgEjGCdQ@robh.at.kernel.org/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Niklas Söderlund Nov. 30, 2021, 4:49 p.m. UTC | #5
Hi Geert,

On 2021-11-30 17:45:11 +0100, Geert Uytterhoeven wrote:
> Given Rob said he applied your patch[1], does that mean this series
> is good to be applied?
> Thanks!
> 
> [1] https://lore.kernel.org/all/YaU4XuiaJgEjGCdQ@robh.at.kernel.org/

Yes, with that patch applied this change won't generate any (new)
warnings from DT :-)