Re: [hwmon-next PATCH v3 1/4] dt-bindings: hwmon: adi,adt7470

[Guenter Roeck]

On 7/29/26 23:17, Krzysztof Kozlowski wrote:
> On 29/07/2026 06:45, Luiz Angelo Daros de Luca wrote:
> > Hi Krzysztof,
> >
> > Thanks for the review.
> >
> > > On Tue, Jul 28, 2026 at 12:07:09AM -0300, Luiz Angelo Daros de Luca wrote:
> > > > The Analog Devices ADT7470 is a multichannel temperature sensor and
> > > > PWM fan controller. It supports monitoring up to 10 external
> > > > temperature sensors and controlling up to 4 fans.
> > > >
> > > > Add the device tree binding documentation for it. This includes
> > > > support for the thermal framework by defining the "#thermal-sensor-cells"
> > > > property, and models the fan control lines as PWM channels by
> > > > defining the "#pwm-cells" property.
> > > >
> > > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@xxxxxxxxx>
> > > > ---
> > > >   .../devicetree/bindings/hwmon/adi,adt7470.yaml     | 55 ++++++++++++++++++++++
> > > >   1 file changed, 55 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
> > > > new file mode 100644
> > > > index 000000000000..bb2375a4a490
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/hwmon/adi,adt7470.yaml
> > > > @@ -0,0 +1,55 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/hwmon/adi,adt7470.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Analog Devices ADT7470 hwmon sensor
> > > > +
> > > > +maintainers:
> > > > +  - Luiz Angelo Daros de Luca <luizluca@xxxxxxxxx>
> > > > +
> > > > +description:
> > > > +  Multi-channel temperature monitor and PWM fan controller.
> > > > +  It supports monitoring up to 10 external temperature sensors and
> > > > +  controlling up to four fans. The fan control lines are modeled
> > > > +  as standard PWM channels.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: adi,adt7470
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  "#pwm-cells":
> > > > +    const: 3
> > > > +    description:
> > > > +      See Documentation/devicetree/bindings/pwm/pwm.yaml for the
> > > > +      generic PWM binding description.
> > >
> > > Drop description.
> > >
> > > You OTOH, miss ref to pwm schema. Referencing here in description should
> > > raise you questions... And then use unevaluatedProperties.
> >
> > I will drop the description and change to `unevaluatedProperties: false`.
> >
> > Regarding the reference to the PWM schema, adding `$ref:
> > /schemas/pwm/pwm.yaml#` enforces the `$nodename` to match
> > `^pwm(@.*|-([0-9a-f]+))?$`. Since the ADT7470 is primarily a hardware
> > monitor, its node is typically named `sensor@...`,
> > `fan-controller@...` or `hwmon@...`. Importing `pwm.yaml` causes
> > `dtbs_check` to fail on existing node names because of this mismatch.
>
> If it is a fan-controller then it should use common fan schema, because
> you might be missing several properties.
>
> Device having PWM outputs alone does not make it a fan controller, though.

It is a fan controller. The logic used here is the opposite:
It has pwm outputs to control the fans, therefore it should be modeled as
pwm controller and use pwm controller bindings (to clarify, this is not
my logic, but both DT and PWM maintainers insist on it).

Also, to clarify: I do not like the idea of registering pwm outputs with
the pwm subsystem and require the pwm-fan driver to actually control the
fan. I prefer the mechanism used by, for example, then emc2305 driver,
which registers cooling devices with the thermal subsystem directly
and uses of_parse_phandle_with_args() to extract the pwm configuration.

However, the patch author claims that the pwm outputs are used for
non-fan-control purposes (such as controlling LEDs), making the two-driver
approach necessary.

Guenter