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

[Changhuang Liang]

Hi, Conor

Thanks for the review.

> 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.yam
> > l
> >
> b/Documentation/devicetree/bindings/hwmon/starfive,jhb100-fan-tach.yam
> > l
> > 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?
> 
> 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
> 

Perhaps I can refer to aspeed,g6-pwm-tach.yaml and change "^fan@[0-9a-f]+$" to 
"^fan-[0-9]+$", which would remove the reg property. In fact, the driver does not 
use reg either.

Our fan-tach controller does not include PWM. The JHB100 SoC will have a separate 
PWM controller. (This controller uses the same IP as the JH7110 SoC, but there are 
some differences in driver implementation.)

The JHB100 has 8 PWM channels and 16 fan tach channels.

So currently we expect the Device Tree to be configured like this:

	pwm0: pwm {
		compatible = "starfive,jhb100-pwm";
	};
	
	pwm1: pwm {
		compatible = "starfive,jhb100-pwm";
	};
	
	pwm2: pwm {
		compatible = "starfive,jhb100-pwm";
	};
	
	pwm3: pwm {
		compatible = "starfive,jhb100-pwm";
	};
	
	pwm4: pwm {
		compatible = "starfive,jhb100-pwm";
	};
	
	pwm5: pwm {
		compatible = "starfive,jhb100-pwm";
	};
	
	pwm6: pwm {
		compatible = "starfive,jhb100-pwm";
	};
	
	pwm7: pwm {
		compatible = "starfive,jhb100-pwm";
	};

	fan0: pwm-fan0 {
		compatible = "pwm-fan";
		pwms = <&pwm0 0 40000 0>;
	};

	fan1: pwm-fan1 {
		compatible = "pwm-fan";
		pwms = <&pwm1 0 40000 0>;
	};

	fan2: pwm-fan2 {
		compatible = "pwm-fan";
		pwms = <&pwm2 0 40000 0>;
	};

	fan3: pwm-fan3 {
		compatible = "pwm-fan";
		pwms = <&pwm3 0 40000 0>;
	};

	fan4: pwm-fan4 {
		compatible = "pwm-fan";
		pwms = <&pwm4 0 40000 0>;
	};

	fan5: pwm-fan5 {
		compatible = "pwm-fan";
		pwms = <&pwm5 0 40000 0>;
	};

	fan6: pwm-fan6 {
		compatible = "pwm-fan";
		pwms = <&pwm6 0 40000 0>;
	};

	fan7: pwm-fan7 {
		compatible = "pwm-fan";
		pwms = <&pwm7 0 40000 0>;
	};

fan-controller {
	compatible = "starfive,jhb100-fan-tach";

	fan@0 {
		tach-ch = <0x0>, <0x8>;
	};
	fan@1 {
		tach-ch = <0x1>, <0x9>;
	};
	fan@2 {
		tach-ch = <0x2>, <0xa>;
	};
	fan@3 {
		tach-ch = <0x3>, <0xb>;
	};
	fan@4 {
		tach-ch = <0x4>, <0xc>;
	};
	fan@5 {
		tach-ch = <0x5>, <0xd>;
	};
	fan@6 {
		tach-ch = <0x6>, <0xe>;
	};
	fan@7 {
		tach-ch = <0x7>, <0xf>;
	};
};


> 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
> >

Best Regards,
Changhuang