Re: [PATCH RESEND v10 2/3] dt-bindings: hwmon: Support Aspeed g6 PWM TACH Control

[Billy Tsai]

> > Document the compatible for aspeed,ast2600-pwm-tach device, which can
> > support up to 16 PWM outputs and 16 fan tach input.
> >
> > Signed-off-by: Billy Tsai <billy_tsai@xxxxxxxxxxxxxx>
> > ---
> >  .../bindings/hwmon/aspeed,g6-pwm-tach.yaml    | 69 +++++++++++++++++++
> >  1 file changed, 69 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed,g6-pwm-tach.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/aspeed,g6-pwm-tach.yaml b/Documentation/devicetree/bindings/hwmon/aspeed,g6-pwm-tach.yaml
> > new file mode 100644
> > index 000000000000..c615fb10705c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/aspeed,g6-pwm-tach.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2023 Aspeed, Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/aspeed,g6-pwm-tach.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ASPEED G6 PWM and Fan Tach controller
> > +
> > +maintainers:
> > +  - Billy Tsai <billy_tsai@xxxxxxxxxxxxxx>
> > +
> > +description: |
> > +  The ASPEED PWM controller can support up to 16 PWM outputs.
> > +  The ASPEED Fan Tacho controller can support up to 16 fan tach input.
> > +  They are independent hardware blocks, which are different from the
> > +  previous version of the ASPEED chip.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - aspeed,ast2600-pwm-tach
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  "#pwm-cells":
> > +    const: 3
> > +
> > +patternProperties:
> > +  "^fan-[0-9]+$":
> > +    $ref: fan-common.yaml#
> > +    unevaluatedProperties: false
> > +    required:
> > +      - tach-ch
> > +
> > +required:
> > +  - reg
> > +  - clocks
> > +  - resets
> > +  - "#pwm-cells"
> > +  - compatible
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/aspeed-clock.h>
> > +    pwm_tach: pwm-tach-controller@1e610000 {
> > +      compatible = "aspeed,ast2600-pwm-tach";
> > +      reg = <0x1e610000 0x100>;
> > +      clocks = <&syscon ASPEED_CLK_AHB>;
> > +      resets = <&syscon ASPEED_RESET_PWM>;
> > +      #pwm-cells = <3>;
> > +
> > +      fan-0 {

> I assume there's a PWM connection here? How do you know which PWM? You
> said the tach channel is independent, so it is not that.

> It should not be 0 from 'fan-0' because that's just a meaningless index.

> You either need 'pwms' here or you can use 'reg' and the reg value is
> the PWM channel.

Hi Rob, this binding is used to export the PWM provider and the Fan monitor (i.e., Tach).
If the user wants to add the PWM connection for the fan, it can be done as follows:

fan0: pwm-fan0 {
        compatible = "pwm-fan";
        pwms = <&pwm_tach 0 40000 0>; 
        cooling-min-state = <0>;
        cooling-max-state = <3>;
        #cooling-cells = <2>;
        cooling-levels = <0 15 128 255>;
};

This will reuse the existing PWM fan driver (e.g., pwm-fan.c).

Thanks

Billy

> > +        tach-ch = /bits/ 8 <0x0>;
> > +      };
> > +
> > +      fan-1 {
> > +        tach-ch = /bits/ 8 <0x1 0x2>;
> > +      };
> > +    };
> > --
> > 2.25.1
> >