[PATCH v3 0/2] hwmon: (amc6821) Add PWM polarity configuration with OF

[PATCH v3 0/2] hwmon: (amc6821) Add PWM polarity configuration with OF

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Add support for configuring the PWM polarity of the amc6821 fan controller.

Francesco Dolcini (2):
  dt-bindings: hwmon: amc6821: add fan and PWM output
  hwmon: (amc6821) Add PWM polarity configuration with OF

 .../devicetree/bindings/hwmon/ti,amc6821.yaml | 14 +++++-
 drivers/hwmon/amc6821.c                       | 50 +++++++++++++++++--
 2 files changed, 58 insertions(+), 6 deletions(-)

-- 
2.39.5

[PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Add properties to describe the fan and the PWM controller output.

Link: https://www.ti.com/lit/gpn/amc6821
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v3:
 - explicitly describe the fan, use standard PWM and FAN bindings
 - pwm.yaml cannot be referenced, because of the $nodename pattern that is
   enforced there
v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
 - no changes
v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
---
 .../devicetree/bindings/hwmon/ti,amc6821.yaml      | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
index 5d33f1a23d03..94aca9c378e6 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
@@ -28,6 +28,13 @@ properties:
   i2c-mux:
     type: object
 
+  fan:
+    $ref: fan-common.yaml#
+    unevaluatedProperties: false
+
+  "#pwm-cells":
+    const: 3
+
 required:
   - compatible
   - reg
@@ -50,9 +57,14 @@ examples:
         #address-cells = <1>;
         #size-cells = <0>;
 
-        fan@18 {
+        fan_controller: fan@18 {
             compatible = "ti,amc6821";
             reg = <0x18>;
+            #pwm-cells = <3>;
+
+            fan {
+                pwms = <&fan_controller 0 40000 0>;
+            };
         };
     };
 
-- 
2.39.5

Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output

[Guenter Roeck]

On 3/31/25 08:52, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Add properties to describe the fan and the PWM controller output.
> 
> Link: https://www.ti.com/lit/gpn/amc6821
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
> v3:
>   - explicitly describe the fan, use standard PWM and FAN bindings
>   - pwm.yaml cannot be referenced, because of the $nodename pattern that is
>     enforced there
> v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
>   - no changes
> v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> ---
>   .../devicetree/bindings/hwmon/ti,amc6821.yaml      | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> index 5d33f1a23d03..94aca9c378e6 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> @@ -28,6 +28,13 @@ properties:
>     i2c-mux:
>       type: object
>   
> +  fan:
> +    $ref: fan-common.yaml#
> +    unevaluatedProperties: false
> +
> +  "#pwm-cells":
> +    const: 3
> +
>   required:
>     - compatible
>     - reg
> @@ -50,9 +57,14 @@ examples:
>           #address-cells = <1>;
>           #size-cells = <0>;
>   
> -        fan@18 {
> +        fan_controller: fan@18 {
>               compatible = "ti,amc6821";
>               reg = <0x18>;
> +            #pwm-cells = <3>;
> +
> +            fan {
> +                pwms = <&fan_controller 0 40000 0>;

There is no explanation for the parameters. I guess that the first is the index and the
second is the frequency. The index is not used and can be dropped (there is just a single
channel), and the frequency only makes sense if it can actually be configured.

Either case, the fields need to be documented.

Thanks,
Guenter

> +            };
>           };
>       };
>   

Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output

[Francesco Dolcini]

Hello Guenter,


On Mon, Mar 31, 2025 at 09:01:08AM -0700, Guenter Roeck wrote:
> On 3/31/25 08:52, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > Add properties to describe the fan and the PWM controller output.
> > 
> > Link: https://www.ti.com/lit/gpn/amc6821
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> > v3:
> >   - explicitly describe the fan, use standard PWM and FAN bindings
> >   - pwm.yaml cannot be referenced, because of the $nodename pattern that is
> >     enforced there
> > v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
> >   - no changes
> > v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> > ---
> >   .../devicetree/bindings/hwmon/ti,amc6821.yaml      | 14 +++++++++++++-
> >   1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > index 5d33f1a23d03..94aca9c378e6 100644
> > --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > @@ -28,6 +28,13 @@ properties:
> >     i2c-mux:
> >       type: object
> > +  fan:
> > +    $ref: fan-common.yaml#
> > +    unevaluatedProperties: false
> > +
> > +  "#pwm-cells":
> > +    const: 3
> > +
> >   required:
> >     - compatible
> >     - reg
> > @@ -50,9 +57,14 @@ examples:
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> > -        fan@18 {
> > +        fan_controller: fan@18 {
> >               compatible = "ti,amc6821";
> >               reg = <0x18>;
> > +            #pwm-cells = <3>;
> > +
> > +            fan {
> > +                pwms = <&fan_controller 0 40000 0>;
> 
> There is no explanation for the parameters. I guess that the first is the index and the
> second is the frequency. The index is not used and can be dropped (there is just a single
> channel), and the frequency only makes sense if it can actually be configured.

The PWM period can be configured, the HW supports it, so I think is a
good idea to properly describe the HW now that I am modifying the
binding and not have the situation in which we need it in 1 month.

The instance is just one, it's correct, but there is an assumption
pretty much everywhere that these 3 #cells are
 - PWM instance
 - PWM period in ns
 - PWM flags

I decided to keep the first cell there, with the PWM instance, for this
reason, even as you correctly wrote it would not be required here.

With that said, looking at existing binding, the description is almost
never present.

I would proceed this way for v4:
 - keep the first cell as PWM instance
 - add the description

Francesco

Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output

[Guenter Roeck]

On 3/31/25 09:16, Francesco Dolcini wrote:
> Hello Guenter,
> 
> 
> On Mon, Mar 31, 2025 at 09:01:08AM -0700, Guenter Roeck wrote:
>> On 3/31/25 08:52, Francesco Dolcini wrote:
>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>
>>> Add properties to describe the fan and the PWM controller output.
>>>
>>> Link: https://www.ti.com/lit/gpn/amc6821
>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>> ---
>>> v3:
>>>    - explicitly describe the fan, use standard PWM and FAN bindings
>>>    - pwm.yaml cannot be referenced, because of the $nodename pattern that is
>>>      enforced there
>>> v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
>>>    - no changes
>>> v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
>>> ---
>>>    .../devicetree/bindings/hwmon/ti,amc6821.yaml      | 14 +++++++++++++-
>>>    1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> index 5d33f1a23d03..94aca9c378e6 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> @@ -28,6 +28,13 @@ properties:
>>>      i2c-mux:
>>>        type: object
>>> +  fan:
>>> +    $ref: fan-common.yaml#
>>> +    unevaluatedProperties: false
>>> +
>>> +  "#pwm-cells":
>>> +    const: 3
>>> +
>>>    required:
>>>      - compatible
>>>      - reg
>>> @@ -50,9 +57,14 @@ examples:
>>>            #address-cells = <1>;
>>>            #size-cells = <0>;
>>> -        fan@18 {
>>> +        fan_controller: fan@18 {
>>>                compatible = "ti,amc6821";
>>>                reg = <0x18>;
>>> +            #pwm-cells = <3>;
>>> +
>>> +            fan {
>>> +                pwms = <&fan_controller 0 40000 0>;
>>
>> There is no explanation for the parameters. I guess that the first is the index and the
>> second is the frequency. The index is not used and can be dropped (there is just a single
>> channel), and the frequency only makes sense if it can actually be configured.
> 
> The PWM period can be configured, the HW supports it, so I think is a
> good idea to properly describe the HW now that I am modifying the
> binding and not have the situation in which we need it in 1 month.
> 
> The instance is just one, it's correct, but there is an assumption
> pretty much everywhere that these 3 #cells are
>   - PWM instance
>   - PWM period in ns
>   - PWM flags
> 
> I decided to keep the first cell there, with the PWM instance, for this
> reason, even as you correctly wrote it would not be required here.
> 
> With that said, looking at existing binding, the description is almost
> never present.
> 

That is not a valid argument.

> I would proceed this way for v4:
>   - keep the first cell as PWM instance
>   - add the description
> 

We just had another driver where we dropped the pwm instance number because
it is unnecessary. See commit a25633a00906. I very much prefer to keep it that
way if it is not needed. Everything else is just confusing. It is bad enough
that the sub-node is needed. We should not make it worse.

Thanks,
Guenter

Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output

[Krzysztof Kozlowski]

On Mon, Mar 31, 2025 at 05:52:28PM +0200, Francesco Dolcini wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Add properties to describe the fan and the PWM controller output.
> 
> Link: https://www.ti.com/lit/gpn/amc6821
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
> v3:
>  - explicitly describe the fan, use standard PWM and FAN bindings
>  - pwm.yaml cannot be referenced, because of the $nodename pattern that is
>    enforced there
> v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
>  - no changes
> v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> ---
>  .../devicetree/bindings/hwmon/ti,amc6821.yaml      | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> index 5d33f1a23d03..94aca9c378e6 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> @@ -28,6 +28,13 @@ properties:
>    i2c-mux:
>      type: object
>  
> +  fan:
> +    $ref: fan-common.yaml#
> +    unevaluatedProperties: false

Why do you need the child, instead of referencing fan-common in the top
level?

Best regards,
Krzysztof

Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output

[Francesco Dolcini]

On Tue, Apr 01, 2025 at 08:13:14AM +0200, Krzysztof Kozlowski wrote:
> On Mon, Mar 31, 2025 at 05:52:28PM +0200, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > Add properties to describe the fan and the PWM controller output.
> > 
> > Link: https://www.ti.com/lit/gpn/amc6821
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> > v3:
> >  - explicitly describe the fan, use standard PWM and FAN bindings
> >  - pwm.yaml cannot be referenced, because of the $nodename pattern that is
> >    enforced there
> > v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
> >  - no changes
> > v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> > ---
> >  .../devicetree/bindings/hwmon/ti,amc6821.yaml      | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > index 5d33f1a23d03..94aca9c378e6 100644
> > --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > @@ -28,6 +28,13 @@ properties:
> >    i2c-mux:
> >      type: object
> >  
> > +  fan:
> > +    $ref: fan-common.yaml#
> > +    unevaluatedProperties: false
> 
> Why do you need the child, instead of referencing fan-common in the top
> level?

Two small reasons.

First is that the amc6821 is a fan controller, and the fan is just
connected to it. So having the fan as a child seemed the right way to
describe it, and this is done like that in other hwmon binding.

.. but now that you asked I tried to move the fan-common to the top
level and it's not working.

I added

  allOf:
    - $ref: fan-common.yaml#

at top level, removed the fan child, and moved the pwms up one level in
the example

    i2c {
        #address-cells = <1>;
        #size-cells = <0>;

        fan_controller: fan@18 {
            compatible = "ti,amc6821";
            reg = <0x18>;
            #pwm-cells = <2>;
            pwms = <&fan_controller 40000 0>;
        };
    };

and the binding checker is not fine with it

$ make dt_binding_check DT_SCHEMA_FILES=ti,amc6821.yaml
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  CHKDT   ./Documentation/devicetree/bindings
  LINT    ./Documentation/devicetree/bindings
  DTEX    Documentation/devicetree/bindings/hwmon/ti,amc6821.example.dts
  DTC [C] Documentation/devicetree/bindings/hwmon/ti,amc6821.example.dtb
/home/francesco/Toradex/sources/linux/Documentation/devicetree/bindings/hwmon/ti,amc6821.example.dtb: fan@18: 'pwms' does not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/hwmon/ti,amc6821.yaml#


Any suggestion?

Francesco

Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output

[Guenter Roeck]

On 4/1/25 00:43, Francesco Dolcini wrote:
> On Tue, Apr 01, 2025 at 08:13:14AM +0200, Krzysztof Kozlowski wrote:
>> On Mon, Mar 31, 2025 at 05:52:28PM +0200, Francesco Dolcini wrote:
>>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>>
>>> Add properties to describe the fan and the PWM controller output.
>>>
>>> Link: https://www.ti.com/lit/gpn/amc6821
>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>> ---
>>> v3:
>>>   - explicitly describe the fan, use standard PWM and FAN bindings
>>>   - pwm.yaml cannot be referenced, because of the $nodename pattern that is
>>>     enforced there
>>> v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
>>>   - no changes
>>> v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
>>> ---
>>>   .../devicetree/bindings/hwmon/ti,amc6821.yaml      | 14 +++++++++++++-
>>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> index 5d33f1a23d03..94aca9c378e6 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
>>> @@ -28,6 +28,13 @@ properties:
>>>     i2c-mux:
>>>       type: object
>>>   
>>> +  fan:
>>> +    $ref: fan-common.yaml#
>>> +    unevaluatedProperties: false
>>
>> Why do you need the child, instead of referencing fan-common in the top
>> level?
> 
> Two small reasons.
> 
> First is that the amc6821 is a fan controller, and the fan is just
> connected to it. So having the fan as a child seemed the right way to
> describe it, and this is done like that in other hwmon binding.
> 
> .. but now that you asked I tried to move the fan-common to the top
> level and it's not working.
> 
> I added
> 
>    allOf:
>      - $ref: fan-common.yaml#
> 
> at top level, removed the fan child, and moved the pwms up one level in
> the example
> 
>      i2c {
>          #address-cells = <1>;
>          #size-cells = <0>;
> 
>          fan_controller: fan@18 {
>              compatible = "ti,amc6821";
>              reg = <0x18>;
>              #pwm-cells = <2>;
>              pwms = <&fan_controller 40000 0>;

devicetree wants to see #pwm-cells one level above pwms. Obviously you can't
put it under the i2c node, so you need the sub-level. Maybe there is a way
around it, but if so I don't know what it might be.

Guenter

Re: [PATCH v3 1/2] dt-bindings: hwmon: amc6821: add fan and PWM output

[Rob Herring]

On Tue, Apr 01, 2025 at 08:13:14AM +0200, Krzysztof Kozlowski wrote:
> On Mon, Mar 31, 2025 at 05:52:28PM +0200, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > Add properties to describe the fan and the PWM controller output.
> > 
> > Link: https://www.ti.com/lit/gpn/amc6821
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> > v3:
> >  - explicitly describe the fan, use standard PWM and FAN bindings
> >  - pwm.yaml cannot be referenced, because of the $nodename pattern that is
> >    enforced there
> > v2: https://lore.kernel.org/all/20250224180801.128685-2-francesco@dolcini.it/
> >  - no changes
> > v1: https://lore.kernel.org/all/20250218165633.106867-2-francesco@dolcini.it/
> > ---
> >  .../devicetree/bindings/hwmon/ti,amc6821.yaml      | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > index 5d33f1a23d03..94aca9c378e6 100644
> > --- a/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/ti,amc6821.yaml
> > @@ -28,6 +28,13 @@ properties:
> >    i2c-mux:
> >      type: object
> >  
> > +  fan:
> > +    $ref: fan-common.yaml#
> > +    unevaluatedProperties: false
> 
> Why do you need the child, instead of referencing fan-common in the top
> level?

Because shockingly a fan controller and a fan are 2 different pieces of 
h/w. It took *years* for this advancement to come to fruition.

Rob

[PATCH v3 2/2] hwmon: (amc6821) Add PWM polarity configuration with OF

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Add support to configure the PWM-Out pin polarity based on the device
tree. The binding would allow also to configure the PWM period, this is
currently not implemented by the driver.

The driver has a module option to set the PWM polarity (normal=0,
inverted=1), when specified it always takes the precedence over the DT.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v3:
 - configure PWM polarity using standard PWM DT properties
v2: https://lore.kernel.org/lkml/20250224180801.128685-3-francesco@dolcini.it/
 - pwminv module parameter takes always the precedence over the DT property
v1: https://lore.kernel.org/all/20250218165633.106867-3-francesco@dolcini.it/
---
 drivers/hwmon/amc6821.c | 50 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c
index 1e3c6acd8974..9ad4e9b63138 100644
--- a/drivers/hwmon/amc6821.c
+++ b/drivers/hwmon/amc6821.c
@@ -23,9 +23,12 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of_platform.h>
+#include <linux/pwm.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
 
+#include <dt-bindings/pwm/pwm.h>
+
 /*
  * Addresses to scan.
  */
@@ -37,7 +40,7 @@ static const unsigned short normal_i2c[] = {0x18, 0x19, 0x1a, 0x2c, 0x2d, 0x2e,
  * Insmod parameters
  */
 
-static int pwminv;	/*Inverted PWM output. */
+static int pwminv = -1; /*Inverted PWM output. */
 module_param(pwminv, int, 0444);
 
 static int init = 1; /*Power-on initialization.*/
@@ -845,9 +848,43 @@ static int amc6821_detect(struct i2c_client *client, struct i2c_board_info *info
 	return 0;
 }
 
-static int amc6821_init_client(struct amc6821_data *data)
+static enum pwm_polarity amc6821_pwm_polarity(struct i2c_client *client)
+{
+	enum pwm_polarity polarity = PWM_POLARITY_NORMAL;
+	struct of_phandle_args args;
+	struct device_node *fan_np;
+
+	/*
+	 * For backward compatibility, the pwminv module parameter takes
+	 * always the precedence over any other device description
+	 */
+	if (pwminv == 0)
+		return PWM_POLARITY_NORMAL;
+	if (pwminv > 0)
+		return PWM_POLARITY_INVERSED;
+
+	fan_np = of_get_child_by_name(client->dev.of_node, "fan");
+	if (!fan_np)
+		return PWM_POLARITY_NORMAL;
+
+	if (of_parse_phandle_with_args(fan_np, "pwms", "#pwm-cells", 0, &args))
+		goto out;
+	of_node_put(args.np);
+
+	if (args.args_count != 3)
+		goto out;
+
+	if (args.args[2] & PWM_POLARITY_INVERTED)
+		polarity = PWM_POLARITY_INVERSED;
+out:
+	of_node_put(fan_np);
+	return polarity;
+}
+
+static int amc6821_init_client(struct i2c_client *client, struct amc6821_data *data)
 {
 	struct regmap *regmap = data->regmap;
+	u32 regval;
 	int err;
 
 	if (init) {
@@ -864,11 +901,14 @@ static int amc6821_init_client(struct amc6821_data *data)
 		if (err)
 			return err;
 
+		regval = AMC6821_CONF1_START;
+		if (amc6821_pwm_polarity(client) == PWM_POLARITY_INVERSED)
+			regval |= AMC6821_CONF1_PWMINV;
+
 		err = regmap_update_bits(regmap, AMC6821_REG_CONF1,
 					 AMC6821_CONF1_THERMOVIE | AMC6821_CONF1_FANIE |
 					 AMC6821_CONF1_START | AMC6821_CONF1_PWMINV,
-					 AMC6821_CONF1_START |
-					 (pwminv ? AMC6821_CONF1_PWMINV : 0));
+					 regval);
 		if (err)
 			return err;
 	}
@@ -916,7 +956,7 @@ static int amc6821_probe(struct i2c_client *client)
 				     "Failed to initialize regmap\n");
 	data->regmap = regmap;
 
-	err = amc6821_init_client(data);
+	err = amc6821_init_client(client, data);
 	if (err)
 		return err;
 
-- 
2.39.5