| Message ID | 20250318085444.3459380-2-florin.leotescu@oss.nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add OF support for Microchip emc2305 fan controller | expand |
On 3/18/25 01:54, florin.leotescu@oss.nxp.com wrote: > From: Florin Leotescu <florin.leotescu@nxp.com> > > Introduce yaml schema for Microchip emc2305 pwm fan controller. > > Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com> > Reviewed-by: Frank Li <Frank.Li@nxp.com> > --- > .../bindings/hwmon/microchip,emc2305.yaml | 113 ++++++++++++++++++ > 1 file changed, 113 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > new file mode 100644 > index 000000000000..e61ef97e63af > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > @@ -0,0 +1,113 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > + > +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip EMC2305 SMBus compliant PWM fan controller > + > +maintainers: > + - Michael Shych <michaelsh@nvidia.com> > + > +description: > + Microchip EMC2301/2/3/5 pwm controller which supports > + up to five programmable fan control circuits. > + > +properties: > + compatible: > + oneOf: > + - enum: > + - microchip,emc2305 > + - items: > + - enum: > + - microchip,emc2303 > + - microchip,emc2302 > + - microchip,emc2301 > + - const: microchip,emc2305 > + > + reg: > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + '#pwm-cells': > + const: 3 > + description: | > + Number of cells in a PWM specifier. > + - cell 0: The PWM frequency > + - cell 1: The PWM polarity: 0 or PWM_POLARITY_INVERTED > + - cell 2: The PWM output config: > + - 0 (Open-Drain) > + - 1 (Push-Pull) > + > + > +patternProperties: > + '^fan@[0-4]$': > + $ref: fan-common.yaml# > + unevaluatedProperties: false > + properties: > + reg: > + description: > + The fan number used to determine the associated PWM channel. > + > + required: > + - reg > + - pwms Is it necessary to make 'pwms' mandatory ? The current code works just fine with defaults. Thanks, Guenter
On Tue, Mar 18, 2025 at 5:22 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 3/18/25 01:54, florin.leotescu@oss.nxp.com wrote: > > From: Florin Leotescu <florin.leotescu@nxp.com> > > > > Introduce yaml schema for Microchip emc2305 pwm fan controller. > > > > Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com> > > Reviewed-by: Frank Li <Frank.Li@nxp.com> > > --- > > .../bindings/hwmon/microchip,emc2305.yaml | 113 ++++++++++++++++++ > > 1 file changed, 113 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > > > > diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > > new file mode 100644 > > index 000000000000..e61ef97e63af > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > > @@ -0,0 +1,113 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > + > > +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Microchip EMC2305 SMBus compliant PWM fan controller > > + > > +maintainers: > > + - Michael Shych <michaelsh@nvidia.com> > > + > > +description: > > + Microchip EMC2301/2/3/5 pwm controller which supports > > + up to five programmable fan control circuits. > > + > > +properties: > > + compatible: > > + oneOf: > > + - enum: > > + - microchip,emc2305 > > + - items: > > + - enum: > > + - microchip,emc2303 > > + - microchip,emc2302 > > + - microchip,emc2301 > > + - const: microchip,emc2305 > > + > > + reg: > > + maxItems: 1 > > + > > + '#address-cells': > > + const: 1 > > + > > + '#size-cells': > > + const: 0 > > + > > + '#pwm-cells': > > + const: 3 > > + description: | > > + Number of cells in a PWM specifier. > > + - cell 0: The PWM frequency > > + - cell 1: The PWM polarity: 0 or PWM_POLARITY_INVERTED > > + - cell 2: The PWM output config: > > + - 0 (Open-Drain) > > + - 1 (Push-Pull) > > + > > + > > +patternProperties: > > + '^fan@[0-4]$': > > + $ref: fan-common.yaml# > > + unevaluatedProperties: false > > + properties: > > + reg: > > + description: > > + The fan number used to determine the associated PWM channel. > > + > > + required: > > + - reg > > + - pwms > > Is it necessary to make 'pwms' mandatory ? The current code works > just fine with defaults. The code adding OF support is added just in the next patch, so the current code isn't event probed when trying to use dts. Or am I missing something? +static const struct of_device_id of_emc2305_match_table[] = { + { .compatible = "microchip,emc2305", }, + {}, +}; +MODULE_DEVICE_TABLE(of, of_emc2305_match_table); I have no strong opinion on this we could make pwms optional.
On 3/18/25 08:33, Daniel Baluta wrote: > On Tue, Mar 18, 2025 at 5:22 PM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 3/18/25 01:54, florin.leotescu@oss.nxp.com wrote: >>> From: Florin Leotescu <florin.leotescu@nxp.com> >>> >>> Introduce yaml schema for Microchip emc2305 pwm fan controller. >>> >>> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com> >>> Reviewed-by: Frank Li <Frank.Li@nxp.com> >>> --- >>> .../bindings/hwmon/microchip,emc2305.yaml | 113 ++++++++++++++++++ >>> 1 file changed, 113 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >>> new file mode 100644 >>> index 000000000000..e61ef97e63af >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >>> @@ -0,0 +1,113 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> + >>> +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Microchip EMC2305 SMBus compliant PWM fan controller >>> + >>> +maintainers: >>> + - Michael Shych <michaelsh@nvidia.com> >>> + >>> +description: >>> + Microchip EMC2301/2/3/5 pwm controller which supports >>> + up to five programmable fan control circuits. >>> + >>> +properties: >>> + compatible: >>> + oneOf: >>> + - enum: >>> + - microchip,emc2305 >>> + - items: >>> + - enum: >>> + - microchip,emc2303 >>> + - microchip,emc2302 >>> + - microchip,emc2301 >>> + - const: microchip,emc2305 >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + '#address-cells': >>> + const: 1 >>> + >>> + '#size-cells': >>> + const: 0 >>> + >>> + '#pwm-cells': >>> + const: 3 >>> + description: | >>> + Number of cells in a PWM specifier. >>> + - cell 0: The PWM frequency >>> + - cell 1: The PWM polarity: 0 or PWM_POLARITY_INVERTED >>> + - cell 2: The PWM output config: >>> + - 0 (Open-Drain) >>> + - 1 (Push-Pull) >>> + >>> + >>> +patternProperties: >>> + '^fan@[0-4]$': >>> + $ref: fan-common.yaml# >>> + unevaluatedProperties: false >>> + properties: >>> + reg: >>> + description: >>> + The fan number used to determine the associated PWM channel. >>> + >>> + required: >>> + - reg >>> + - pwms >> >> Is it necessary to make 'pwms' mandatory ? The current code works >> just fine with defaults. > > The code adding OF support is added just in the next patch, so the > current code isn't event > probed when trying to use dts. > > Or am I missing something? > The patch introducing devicetree support to the driver doesn't evaluate the pwm property. That makes it quite obvious that, from driver perspective, it isn't needed. I don't immediately see why it would add value to _force_ users to provide pwm frequency, polarity, and the output configuration if the defaults work just fine. Guenter
> >> Is it necessary to make 'pwms' mandatory ? The current code works > >> just fine with defaults. > > > > The code adding OF support is added just in the next patch, so the > > current code isn't event > > probed when trying to use dts. > > > > Or am I missing something? > > > > The patch introducing devicetree support to the driver doesn't evaluate > the pwm property. That makes it quite obvious that, from driver perspective, > it isn't needed. I don't immediately see why it would add value to _force_ > users to provide pwm frequency, polarity, and the output configuration > if the defaults work just fine. > Got your point now! Thanks.
On Tue, Mar 18, 2025 at 10:54:42AM +0200, florin.leotescu@oss.nxp.com wrote: > From: Florin Leotescu <florin.leotescu@nxp.com> > > Introduce yaml schema for Microchip emc2305 pwm fan controller. > > Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com> > Reviewed-by: Frank Li <Frank.Li@nxp.com> > --- > .../bindings/hwmon/microchip,emc2305.yaml | 113 ++++++++++++++++++ > 1 file changed, 113 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > new file mode 100644 > index 000000000000..e61ef97e63af > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml > @@ -0,0 +1,113 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > + No blank line here. Use existing code as template. If you find such code, share so we can fix it. > +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip EMC2305 SMBus compliant PWM fan controller > + > +maintainers: > + - Michael Shych <michaelsh@nvidia.com> > + > +description: > + Microchip EMC2301/2/3/5 pwm controller which supports > + up to five programmable fan control circuits. > + > +properties: > + compatible: > + oneOf: > + - enum: > + - microchip,emc2305 > + - items: > + - enum: > + - microchip,emc2303 > + - microchip,emc2302 > + - microchip,emc2301 > + - const: microchip,emc2305 > + > + reg: > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + '#pwm-cells': > + const: 3 > + description: | > + Number of cells in a PWM specifier. > + - cell 0: The PWM frequency > + - cell 1: The PWM polarity: 0 or PWM_POLARITY_INVERTED > + - cell 2: The PWM output config: > + - 0 (Open-Drain) > + - 1 (Push-Pull) > + > + Just one blank line Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Tue, Mar 18, 2025 at 09:37:29AM -0700, Guenter Roeck wrote: > > > > +patternProperties: > > > > + '^fan@[0-4]$': > > > > + $ref: fan-common.yaml# > > > > + unevaluatedProperties: false > > > > + properties: > > > > + reg: > > > > + description: > > > > + The fan number used to determine the associated PWM channel. > > > > + > > > > + required: > > > > + - reg > > > > + - pwms > > > > > > Is it necessary to make 'pwms' mandatory ? The current code works > > > just fine with defaults. > > > > The code adding OF support is added just in the next patch, so the > > current code isn't event > > probed when trying to use dts. > > > > Or am I missing something? > > > > The patch introducing devicetree support to the driver doesn't evaluate > the pwm property. That makes it quite obvious that, from driver perspective, > it isn't needed. I don't immediately see why it would add value to _force_ > users to provide pwm frequency, polarity, and the output configuration > if the defaults work just fine. Indeed. PWM source is actually fixed and rest of pwm argument could be fine with defaults. Both options are fine with me, so keep my tag. Best regards, Krzysztof
>>On Tue, Mar 18, 2025 at 10:54:42AM +0200, florin.leotescu@oss.nxp.com wrote: >> From: Florin Leotescu <florin.leotescu@nxp.com> >> >> Introduce yaml schema for Microchip emc2305 pwm fan controller. >> >> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com> >> Reviewed-by: Frank Li <Frank.Li@nxp.com> >> --- >> .../bindings/hwmon/microchip,emc2305.yaml | 113 ++++++++++++++++++ >> 1 file changed, 113 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >> b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >> new file mode 100644 >> index 000000000000..e61ef97e63af >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml >> @@ -0,0 +1,113 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 >> +--- >> + > No blank line here. Use existing code as template. If you find such code, share so we can fix it. Thanks! I will remove it. Identified similar blank line on the following dt-binding files: 1 adi,ad741x.yaml 2 adi,adm1275.yaml 3 adi,ltc2991.yaml 4 maxim,max20730.yaml 5 maxim,max6639.yaml 6 maxim,max6650.yaml 7 nuvoton,nct6775.yaml 8 nuvoton,nct7363.yaml 9 nuvoton,nct7802.yaml 10 ti,adc128d818.yaml 11 ti,ads7828.yaml 12 ti,ina2xx.yaml 13 ti,lm87.yaml 14 ti,tmp513.yaml 15 ti,tps23861.yaml 16 winbond,w83781d.yaml >> +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Microchip EMC2305 SMBus compliant PWM fan controller >> + >> +maintainers: >> + - Michael Shych <michaelsh@nvidia.com> >> + >> +description: >> + Microchip EMC2301/2/3/5 pwm controller which supports >> + up to five programmable fan control circuits. >> + >> +properties: >> + compatible: >> + oneOf: >> + - enum: >> + - microchip,emc2305 >> + - items: >> + - enum: >> + - microchip,emc2303 >> + - microchip,emc2302 >> + - microchip,emc2301 >> + - const: microchip,emc2305 >> + >> + reg: >> + maxItems: 1 >> + >> + '#address-cells': >> + const: 1 >> + >> + '#size-cells': >> + const: 0 >> + >> + '#pwm-cells': >> + const: 3 >> + description: | >> + Number of cells in a PWM specifier. >> + - cell 0: The PWM frequency >> + - cell 1: The PWM polarity: 0 or PWM_POLARITY_INVERTED >> + - cell 2: The PWM output config: >> + - 0 (Open-Drain) >> + - 1 (Push-Pull) >> + >> + > Just one blank line Ok, I'll remove it. Thank you! > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Florin
On 19/03/2025 12:54, Florin Leotescu (OSS) wrote: >> No blank line here. Use existing code as template. If you find such code, share so we can fix it. > > Thanks! I will remove it. > > Identified similar blank line on the following dt-binding files: > > 1 adi,ad741x.yaml > 2 adi,adm1275.yaml > 3 adi,ltc2991.yaml > 4 maxim,max20730.yaml > 5 maxim,max6639.yaml > 6 maxim,max6650.yaml > 7 nuvoton,nct6775.yaml > 8 nuvoton,nct7363.yaml > 9 nuvoton,nct7802.yaml > 10 ti,adc128d818.yaml > 11 ti,ads7828.yaml > 12 ti,ina2xx.yaml > 13 ti,lm87.yaml > 14 ti,tmp513.yaml > 15 ti,tps23861.yaml > 16 winbond,w83781d.yaml Oh, that's quite a lot. Thanks, I'll fix them. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml new file mode 100644 index 000000000000..e61ef97e63af --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml @@ -0,0 +1,113 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- + +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip EMC2305 SMBus compliant PWM fan controller + +maintainers: + - Michael Shych <michaelsh@nvidia.com> + +description: + Microchip EMC2301/2/3/5 pwm controller which supports + up to five programmable fan control circuits. + +properties: + compatible: + oneOf: + - enum: + - microchip,emc2305 + - items: + - enum: + - microchip,emc2303 + - microchip,emc2302 + - microchip,emc2301 + - const: microchip,emc2305 + + reg: + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + '#pwm-cells': + const: 3 + description: | + Number of cells in a PWM specifier. + - cell 0: The PWM frequency + - cell 1: The PWM polarity: 0 or PWM_POLARITY_INVERTED + - cell 2: The PWM output config: + - 0 (Open-Drain) + - 1 (Push-Pull) + + +patternProperties: + '^fan@[0-4]$': + $ref: fan-common.yaml# + unevaluatedProperties: false + properties: + reg: + description: + The fan number used to determine the associated PWM channel. + + required: + - reg + - pwms + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/pwm/pwm.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + + fan_controller: fan-controller@2f { + compatible = "microchip,emc2305"; + reg = <0x2f>; + #address-cells = <1>; + #size-cells = <0>; + #pwm-cells = <3>; + + fan@0 { + reg = <0x0>; + pwms = <&fan_controller 26000 PWM_POLARITY_INVERTED 1>; + #cooling-cells = <2>; + }; + + fan@1 { + reg = <0x1>; + pwms = <&fan_controller 26000 0 1>; + #cooling-cells = <2>; + }; + + fan@2 { + reg = <0x2>; + pwms = <&fan_controller 26000 0 1>; + #cooling-cells = <2>; + }; + + fan@3 { + reg = <0x3>; + pwms = <&fan_controller 26000 0 1>; + #cooling-cells = <2>; + }; + + fan@4 { + reg = <0x4>; + pwms = <&fan_controller 26000 0 1>; + #cooling-cells = <2>; + }; + }; + }; +...