| Message ID | 20231107105025.1480561-3-billy_tsai@aspeedtech.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Support pwm/tach driver for aspeed ast26xx | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Tue, Nov 07, 2023 at 06:50:24PM +0800, Billy Tsai wrote: > 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@aspeedtech.com> > --- > .../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@aspeedtech.com> > + > +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. > + tach-ch = /bits/ 8 <0x0>; > + }; > + > + fan-1 { > + tach-ch = /bits/ 8 <0x1 0x2>; > + }; > + }; > -- > 2.25.1 >
> > 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@aspeedtech.com> > > --- > > .../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@aspeedtech.com> > > + > > +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 > >
On Mon, Nov 13, 2023 at 8:11 PM Billy Tsai <billy_tsai@aspeedtech.com> wrote: > > > > 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@aspeedtech.com> > > > --- > > > .../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@aspeedtech.com> > > > + > > > +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). I'm confused now. So what are the child nodes you have? You are defining the fan in 2 places? The "pwm-fan" driver supports a tach via an interrupt, so how would this work in your case? Rob
> > > > 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@aspeedtech.com> > > > > --- > > > > .../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@aspeedtech.com> > > > > + > > > > +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). > I'm confused now. So what are the child nodes you have? You are > defining the fan in 2 places? The "pwm-fan" driver supports a tach via > an interrupt, so how would this work in your case? Hi Rob, The tach interrupt for the pwm-fan is option. In our case, the dts just reuse the pwm control function of the pwm-fan, and the part of the tach monitor will be created by our fan child nodes. So the dts will like followings: // Use to declare the tach monitor for fan. &pwm_tach { fan-0 { tach-ch = /bits/ 8 <0x0>; }; fan-1 { tach-ch = /bits/ 8 <0x1>; }; ... } // Reuse the pwm-fan.c to control the behavior of the PWM for fan. fan0: pwm-fan0 { compatible = "pwm-fan"; pwms = <&pwm_tach 0 40000 0>; /* Target freq:25 kHz */ cooling-min-state = <0>; cooling-max-state = <3>; #cooling-cells = <2>; cooling-levels = <0 15 128 255>; }; fan1: pwm-fan1 { compatible = "pwm-fan"; pwms = <&pwm_tach 1 40000 0>; /* Target freq:25 kHz */ cooling-min-state = <0>; cooling-max-state = <3>; #cooling-cells = <2>; cooling-levels = <0 15 128 255>; }; Thanks
On Sun, Nov 26, 2023 at 11:45 PM Billy Tsai <billy_tsai@aspeedtech.com> wrote: > > > > > > 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@aspeedtech.com> > > > > > --- > > > > > .../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@aspeedtech.com> > > > > > + > > > > > +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). > > > I'm confused now. So what are the child nodes you have? You are > > defining the fan in 2 places? The "pwm-fan" driver supports a tach via > > an interrupt, so how would this work in your case? > > Hi Rob, > > The tach interrupt for the pwm-fan is option. In our case, the dts just reuse the pwm control function > of the pwm-fan, and the part of the tach monitor will be created by our fan child nodes. > So the dts will like followings: > > // Use to declare the tach monitor for fan. > &pwm_tach { > fan-0 { > tach-ch = /bits/ 8 <0x0>; > }; > fan-1 { > tach-ch = /bits/ 8 <0x1>; > }; > ... > } > > // Reuse the pwm-fan.c to control the behavior of the PWM for fan. > fan0: pwm-fan0 { > compatible = "pwm-fan"; > pwms = <&pwm_tach 0 40000 0>; /* Target freq:25 kHz */ > cooling-min-state = <0>; > cooling-max-state = <3>; > #cooling-cells = <2>; > cooling-levels = <0 15 128 255>; > }; No, you can't have a fan described by 2 different nodes. Can't you just merge everything from the pwm-fan0 node into the fan-0 node? Rob
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@aspeedtech.com> + +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 { + tach-ch = /bits/ 8 <0x0>; + }; + + fan-1 { + tach-ch = /bits/ 8 <0x1 0x2>; + }; + };
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@aspeedtech.com> --- .../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