Re: [PATCH v4 1/2] dt-bindings: hwmon: Add starfive,jhb100-fan-tach

[Guenter Roeck]

On 8/31/26 09:32, Conor Dooley wrote:
> On Sat, Aug 29, 2026 at 06:19:40PM -0700, Changhuang Liang wrote:
> > Add compatible "starfive,jhb100-fan-tach" for StarFive JHB100 Fan-Tach
> > controller.
> >
> > Signed-off-by: Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx>
> > ---
> >   .../hwmon/starfive,jhb100-fan-tach.yaml       | 93 +++++++++++++++++++
> >   1 file changed, 93 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/hwmon/starfive,jhb100-fan-tach.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/starfive,jhb100-fan-tach.yaml b/Documentation/devicetree/bindings/hwmon/starfive,jhb100-fan-tach.yaml
> > new file mode 100644
> > index 000000000000..1dcd4932768f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/starfive,jhb100-fan-tach.yaml
> > @@ -0,0 +1,93 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/starfive,jhb100-fan-tach.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: StarFive JHB100 Fan-Tach controller
> > +
> > +maintainers:
> > +  - Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx>
> > +
> > +description:
> > +  The StarFive Fan-Tach controller can support up to 16 fan tach input.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - starfive,jhb100-fan-tach
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +patternProperties:
> > +  "^fan@[0-9a-f]+$":
> > +    $ref: fan-common.yaml#
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        description:
> > +          PWM channel index. The driver allows two fans to share the same
> > +          PWM channel, or each fan to use a dedicated channel.
>
> Doesn't matter what the driver can do, the description should describe
> what the hardware supports.
> pw-bot: changes-requested
>
> Does this fan-tach controller provide the PWMs?
> If so (although Guenter may correct me), I think the fan-tach controller
> needs to be.
>
> If you don't do that, I think you're going to run into problems with
> having multiple nodes with the same unit address when two fans share a
> pwm?

Depends on how the controller works. Normally fan controllers have a feedback
mechanism which can be either temperature -> pwm output or fan speed -> pwm output.
I don't know how this controller works, specifically if the fan speed inputs are
internally associated with pwm outputs or if they are completely independent
of each other. If it is the latter, aspeed,g6-pwm-tach.yaml would probably
be a good baseline.

> I think what you're supposed to do is drop "reg" and replace it with
> "pwms", but once again Guenter may correct me there.
> e.g. aspeed,g6-pwm-tach.yaml

Sorry, when it comes to devicetree properties, I am completely clueless.
Usually the devicetree and pwm maintainers insist that all fan controllers
are modeled as pwm controllers because they have pwm outputs. My approach
is "use whatever bindings are acceptable by DT maintainers".

Guenter

> Cheers,
> Conor.
>
> > +        maximum: 15
> > +
> > +      tach-ch:
> > +        description:
> > +          The tachometer channel(s) wired to the fan(s) driven by this PWM
> > +          channel. Integer values in the range 0-15.
> > +        minItems: 1
> > +        maxItems: 2
> > +        items:
> > +          maximum: 15
> > +
> > +    required:
> > +      - reg
> > +      - tach-ch
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - resets
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    fan-controller@11be0000 {
> > +        compatible = "starfive,jhb100-fan-tach";
> > +        reg = <0x11be0000 0x400>;
> > +        interrupts = <128>;
> > +        clocks = <&per2crg 33>;
> > +        resets = <&per2crg 5>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        fan@0 {
> > +            reg = <0x0>;
> > +            tach-ch = /bits/ 8 <0x0 0x8>;
> > +            pulses-per-revolution = <2>;
> > +        };
> > +        fan@1 {
> > +            reg = <0x1>;
> > +            tach-ch = /bits/ 8 <0x1>;
> > +            pulses-per-revolution = <2>;
> > +        };
> > +    };
> > --
> > 2.25.1