| Message ID | 20240108074348.735014-3-billy_tsai@aspeedtech.com |
|---|---|
| State | Changes Requested, archived |
| 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 Mon, Jan 08, 2024 at 03:43:47PM +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 { > + tach-ch = /bits/ 8 <0x0>; > + }; > + > + fan-1 { > + tach-ch = /bits/ 8 <0x1 0x2>; > + }; NAK on this based on how you are using pwm-fan in v10 discussion. See my comments there. 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 { > > + tach-ch = /bits/ 8 <0x0>; > > + }; > > + > > + fan-1 { > > + tach-ch = /bits/ 8 <0x1 0x2>; > > + }; > NAK on this based on how you are using pwm-fan in v10 discussion. See my > comments there. Okay, I will merge everything from the pwm-fan0 node into the fan-0 node and add the 'simple-bus' to the compatible string of the pwm_tach node. Thanks Billy Tsai
On 15/01/2024 08:05, Billy Tsai wrote: >>> +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>; >>> + }; > >> NAK on this based on how you are using pwm-fan in v10 discussion. See my >> comments there. > > Okay, I will merge everything from the pwm-fan0 node into the fan-0 node > and add the 'simple-bus' to the compatible string of the pwm_tach node. What simple-bus has anything to do with it? This is not a bus. Just to remind: we talk about bindings, not driver. Best regards, Krzysztof
> >>> +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>; > >>> + }; > > > >> NAK on this based on how you are using pwm-fan in v10 discussion. See my > >> comments there. > > > > Okay, I will merge everything from the pwm-fan0 node into the fan-0 node > > and add the 'simple-bus' to the compatible string of the pwm_tach node. > What simple-bus has anything to do with it? This is not a bus. Just to > remind: we talk about bindings, not driver. Hi Krzysztof, If I want to create a dt-binding to indicate that the child nodes should be treated as platform devices, which will be probed based on the compatible string, can I add "simple-bus" for our pwm_tach node like the following? pwm_tach: pwm-tach-controller@1e610000 { compatible = "aspeed,ast2600-pwm-tach", "simple-bus"; reg = <0x1e610000 0x100>; clocks = <&syscon ASPEED_CLK_AHB>; resets = <&syscon ASPEED_RESET_PWM>; #pwm-cells = <3>; fan-0 { tach-ch = /bits/ 8 <0x0>; compatible = "pwm-fan"; pwms = <&pwm_tach 0 40000 0>; }; fan-1 { tach-ch = /bits/ 8 <0x1 0x2>; compatible = "pwm-fan"; pwms = <&pwm_tach 1 40000 0>; }; }; Or do you have any other suggestions for describing this in the dt-bindings? Thanks Billy Tsai.
On 15/01/2024 09:43, Billy Tsai wrote: >>>>> +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>; >>>>> + }; >>> >>>> NAK on this based on how you are using pwm-fan in v10 discussion. See my >>>> comments there. >>> >>> Okay, I will merge everything from the pwm-fan0 node into the fan-0 node >>> and add the 'simple-bus' to the compatible string of the pwm_tach node. > >> What simple-bus has anything to do with it? This is not a bus. Just to >> remind: we talk about bindings, not driver. > > Hi Krzysztof, > > If I want to create a dt-binding to indicate that the child nodes > should be treated as platform devices, which will be probed based on the probed? Bindings do not probe. You ignored: "we talk about bindings, not driver." > compatible string, can I add "simple-bus" for our pwm_tach node like the > following? No, because this is not a bus. > pwm_tach: pwm-tach-controller@1e610000 { > compatible = "aspeed,ast2600-pwm-tach", "simple-bus"; > reg = <0x1e610000 0x100>; > clocks = <&syscon ASPEED_CLK_AHB>; > resets = <&syscon ASPEED_RESET_PWM>; > #pwm-cells = <3>; > > fan-0 { > tach-ch = /bits/ 8 <0x0>; > compatible = "pwm-fan"; > pwms = <&pwm_tach 0 40000 0>; > }; > > fan-1 { > tach-ch = /bits/ 8 <0x1 0x2>; > compatible = "pwm-fan"; > pwms = <&pwm_tach 1 40000 0>; > }; > }; > Or do you have any other suggestions for describing this in the dt-bindings? There is no need to describe it in the bindings. The existing compatible describes it sufficiently. Your pwms now duplicate the tach-ch... I don't understand what you want to achieve here in terms of hardware description (again, please steer away from talking about Linux drivers and probing, it's not related). Best regards, Krzysztof
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