[PATCH v5 0/3] Add OF support for Microchip emc2305 fan controller

[PATCH v5 0/3] Add OF support for Microchip emc2305 fan controller

[florin.leotescu]

From: Florin Leotescu <florin.leotescu@nxp.com>

This patch series add initial OF support for Microchip emc2305 fan controller.

Changes since v4:
- Corrected quotes to have a consistent definition.
- Corrected DTS coding style as recommended by Krzysztof Kozlowski.
- Updated pwms phandle to include pwm frequency, pwm polarity and pwm output,
  as recommended by Guenter Roeck and skipped the pwm channel as recommended by Krzysztof Kozlowski.
- Documented the pwms args in #pwm-cells description.
- Updated pwms phandle in dt-binding example to include pwm frequency and 
  use PWM_POLARITY_INVERTED macro to inverse the PWM polarity. 

Changes since v3:
- Removed thermal_cooling_device_register_fail label since is no longer needed
  and replaced goto with imediate return, as recommended by Guenter Roeck. 
- Redefined compatible property in dt-binding, as recommended by Frank Li.
- Updated driver of compatible list.
- Renamed node name in the dt-binding example, as recommended by Connor Dooley.
- Corrected dt-binding commit subject.
- Added dt-binding fan descriptions and $ref to fan schema, 
  as Connor Dooley & Krzysztof Kozlowski recommended.
  Used similar as on maxim,max6639 and nuvoton,nct7363. 
- Updated dt-binding example.

Changes since v2:
- Removed the introduction of new properties for now and we only 
  enable basic OF support.
- Link to previous version: 
  https://lore.kernel.org/linux-arm-kernel/20250219133221.2641041-3-florin.leotescu@oss.nxp.com/T/

Florin Leotescu (3):
  dt-bindings: hwmon: Add Microchip emc2305 support
  hwmon: emc2305: Add OF support
  hwmon: emc2305: Use devm_thermal_of_cooling_device_register

 .../bindings/hwmon/microchip,emc2305.yaml     | 113 ++++++++++++++++++
 drivers/hwmon/emc2305.c                       |  38 ++----
 2 files changed, 123 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml

-- 
2.34.1

[PATCH v5 1/3] dt-bindings: hwmon: Add Microchip emc2305 support

[florin.leotescu]

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
+
+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>;
+            };
+        };
+    };
+...
-- 
2.34.1

[PATCH v5 2/3] hwmon: emc2305: Add OF support

[florin.leotescu]

From: Florin Leotescu <florin.leotescu@nxp.com>

Introduce OF support for Microchip emc2305 pwm fan controller.

Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/hwmon/emc2305.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index 4d39fbd83769..f8a4c76fcadd 100644
--- a/drivers/hwmon/emc2305.c
+++ b/drivers/hwmon/emc2305.c
@@ -607,9 +607,16 @@ static void emc2305_remove(struct i2c_client *client)
 		emc2305_unset_tz(dev);
 }
 
+static const struct of_device_id of_emc2305_match_table[] = {
+	{ .compatible = "microchip,emc2305", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_emc2305_match_table);
+
 static struct i2c_driver emc2305_driver = {
 	.driver = {
 		.name = "emc2305",
+		.of_match_table = of_emc2305_match_table,
 	},
 	.probe = emc2305_probe,
 	.remove	  = emc2305_remove,
-- 
2.34.1

[PATCH v5 3/3] hwmon: emc2305: Use devm_thermal_of_cooling_device_register

[florin.leotescu]

From: Florin Leotescu <florin.leotescu@nxp.com>

Prepare the emc2305 driver to use configuration from Device Tree nodes.
Switch to devm_thermal_of_cooling_device_register to simplify the
cleanup procedure, allowing the removal of emc2305_unset_tz and
emc2305_remove, which are no longer needed.

Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/hwmon/emc2305.c | 33 ++++-----------------------------
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index f8a4c76fcadd..234c54956a4b 100644
--- a/drivers/hwmon/emc2305.c
+++ b/drivers/hwmon/emc2305.c
@@ -112,8 +112,6 @@ static char *emc2305_fan_name[] = {
 	"emc2305_fan5",
 };
 
-static void emc2305_unset_tz(struct device *dev);
-
 static int emc2305_get_max_channel(const struct emc2305_data *data)
 {
 	return data->pwm_num;
@@ -293,8 +291,9 @@ static int emc2305_set_single_tz(struct device *dev, int idx)
 	pwm = data->pwm_min[cdev_idx];
 
 	data->cdev_data[cdev_idx].cdev =
-		thermal_cooling_device_register(emc2305_fan_name[idx], data,
-						&emc2305_cooling_ops);
+		devm_thermal_of_cooling_device_register(dev, dev->of_node,
+							emc2305_fan_name[idx], data,
+							&emc2305_cooling_ops);
 
 	if (IS_ERR(data->cdev_data[cdev_idx].cdev)) {
 		dev_err(dev, "Failed to register cooling device %s\n", emc2305_fan_name[idx]);
@@ -332,24 +331,9 @@ static int emc2305_set_tz(struct device *dev)
 	for (i = 0; i < data->pwm_num; i++) {
 		ret = emc2305_set_single_tz(dev, i + 1);
 		if (ret)
-			goto thermal_cooling_device_register_fail;
+			return ret;
 	}
 	return 0;
-
-thermal_cooling_device_register_fail:
-	emc2305_unset_tz(dev);
-	return ret;
-}
-
-static void emc2305_unset_tz(struct device *dev)
-{
-	struct emc2305_data *data = dev_get_drvdata(dev);
-	int i;
-
-	/* Unregister cooling device. */
-	for (i = 0; i < EMC2305_PWM_MAX; i++)
-		if (data->cdev_data[i].cdev)
-			thermal_cooling_device_unregister(data->cdev_data[i].cdev);
 }
 
 static umode_t
@@ -599,14 +583,6 @@ static int emc2305_probe(struct i2c_client *client)
 	return 0;
 }
 
-static void emc2305_remove(struct i2c_client *client)
-{
-	struct device *dev = &client->dev;
-
-	if (IS_REACHABLE(CONFIG_THERMAL))
-		emc2305_unset_tz(dev);
-}
-
 static const struct of_device_id of_emc2305_match_table[] = {
 	{ .compatible = "microchip,emc2305", },
 	{},
@@ -619,7 +595,6 @@ static struct i2c_driver emc2305_driver = {
 		.of_match_table = of_emc2305_match_table,
 	},
 	.probe = emc2305_probe,
-	.remove	  = emc2305_remove,
 	.id_table = emc2305_ids,
 };
 
-- 
2.34.1

Re: [PATCH v5 1/3] dt-bindings: hwmon: Add Microchip emc2305 support

[Guenter Roeck]

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

Re: [PATCH v5 1/3] dt-bindings: hwmon: Add Microchip emc2305 support

[Daniel Baluta]

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.

Re: [PATCH v5 1/3] dt-bindings: hwmon: Add Microchip emc2305 support

[Guenter Roeck]

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

Re: [PATCH v5 1/3] dt-bindings: hwmon: Add Microchip emc2305 support

[Daniel Baluta]

> >> 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.

Re: [PATCH v5 1/3] dt-bindings: hwmon: Add Microchip emc2305 support

[Krzysztof Kozlowski]

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

Re: [PATCH v5 1/3] dt-bindings: hwmon: Add Microchip emc2305 support

[Krzysztof Kozlowski]

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

RE: [PATCH v5 1/3] dt-bindings: hwmon: Add Microchip emc2305 support

[Florin Leotescu (OSS)]

>>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

Re: [PATCH v5 1/3] dt-bindings: hwmon: Add Microchip emc2305 support

[Krzysztof Kozlowski]

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