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

[PATCH v3 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 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 yaml schema
  hwmon: emc2305: Add OF support
  hwmon: emc2305: Use devm_thermal_of_cooling_device_register

 .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++
 drivers/hwmon/emc2305.c                       | 36 ++++++----------
 2 files changed, 55 insertions(+), 24 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml

-- 
2.34.1

[PATCH v3 1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

[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>
---
 .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++
 1 file changed, 43 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..cac0075a65bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
@@ -0,0 +1,43 @@
+# 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:
+    enum:
+      - microchip,emc2301
+      - microchip,emc2302
+      - microchip,emc2303
+      - microchip,emc2305
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        emc2301: pwm@2f {
+            compatible = "microchip,emc2301";
+            reg = <0x2f>;
+        };
+    };
-- 
2.34.1

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

[florin.leotescu]

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

Introduce OF support for Microchip emc2301/2/3/5 pwm fan controller.

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

diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index 4d39fbd83769..a236f27c75bc 100644
--- a/drivers/hwmon/emc2305.c
+++ b/drivers/hwmon/emc2305.c
@@ -607,9 +607,19 @@ static void emc2305_remove(struct i2c_client *client)
 		emc2305_unset_tz(dev);
 }
 
+static const struct of_device_id of_emc2305_match_table[] = {
+	{ .compatible = "microchip,emc2301", },
+	{ .compatible = "microchip,emc2302", },
+	{ .compatible = "microchip,emc2303", },
+	{ .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 v3 3/3] hwmon: emc2305: Use devm_thermal_of_cooling_device_register

[florin.leotescu]

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

This prepares emc2305 driver to use configuration from Device Tree nodes.
Using devm_thermal_of_cooling_device_register also simplifies cleanup
procedure, so we are able to remove the emc2305_unset_tz and emc2305_remove
functions which are no longer used.

Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
---
 drivers/hwmon/emc2305.c | 28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index a236f27c75bc..5f3f06308826 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]);
@@ -337,21 +336,9 @@ static int emc2305_set_tz(struct device *dev)
 	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
 emc2305_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr, int channel)
 {
@@ -599,14 +586,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,emc2301", },
 	{ .compatible = "microchip,emc2302", },
@@ -622,7 +601,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 v3 1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

[Frank Li]

On Tue, Mar 04, 2025 at 06:21:34PM +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>
> ---
>  .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++
>  1 file changed, 43 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..cac0075a65bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> @@ -0,0 +1,43 @@
> +# 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>

who is it? look like not Microchip and hwmon maintainer.

> +
> +description: |

Needn't |

> +  Microchip EMC2301/2/3/5 pwm controller which supports up
> +  to five programmable fan control circuits
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,emc2301
> +      - microchip,emc2302
> +      - microchip,emc2303
> +      - microchip,emc2305

According to your driver code look like all compatible with microchip,emc2301

oneOf:
  - enum:
      - microchip,emc2301
  - items:
      - enum:
          - microchip,emc2302
          - microchip,emc2303
          - microchip,emc2305
      - const: microchip,emc2301

So your driver just need one "microchip,emc2301" compatible string.

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        emc2301: pwm@2f {
> +            compatible = "microchip,emc2301";
> +            reg = <0x2f>;

If only one reg:
You can go through trivial-devices.yaml

Frank

> +        };
> +    };
> --
> 2.34.1
>

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

[Frank Li]

On Tue, Mar 04, 2025 at 06:21:35PM +0200, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
>
> Introduce OF support for Microchip emc2301/2/3/5 pwm fan controller.
>
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/hwmon/emc2305.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
> index 4d39fbd83769..a236f27c75bc 100644
> --- a/drivers/hwmon/emc2305.c
> +++ b/drivers/hwmon/emc2305.c
> @@ -607,9 +607,19 @@ static void emc2305_remove(struct i2c_client *client)
>  		emc2305_unset_tz(dev);
>  }
>
> +static const struct of_device_id of_emc2305_match_table[] = {
> +	{ .compatible = "microchip,emc2301", },
> +	{ .compatible = "microchip,emc2302", },
> +	{ .compatible = "microchip,emc2303", },
> +	{ .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
>

Re: [PATCH v3 1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]

On Tue, Mar 04, 2025 at 06:21:34PM +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>
> ---
>  .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++
>  1 file changed, 43 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..cac0075a65bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> @@ -0,0 +1,43 @@
> +# 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:
> +    enum:
> +      - microchip,emc2301
> +      - microchip,emc2302
> +      - microchip,emc2303
> +      - microchip,emc2305

Driver has no match data etc, suggesting all of these are compatible.
Please pick one as a fallback compatible.

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg

Missing descriptions of the fans, no?
See max6639 for an example.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        emc2301: pwm@2f {

The emc2301 label here can be dropped, it is not used.
The nodename should be "fan-controller", not pwm here I guess.

Cheers,
Conor.

> +            compatible = "microchip,emc2301";
> +            reg = <0x2f>;
> +        };
> +    };
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

[Krzysztof Kozlowski]

On 04/03/2025 17:29, Conor Dooley wrote:
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - microchip,emc2301
>> +      - microchip,emc2302
>> +      - microchip,emc2303
>> +      - microchip,emc2305
> 
> Driver has no match data etc, suggesting all of these are compatible.
> Please pick one as a fallback compatible.
> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
> 
> Missing descriptions of the fans, no?
> See max6639 for an example.

Yes and missing $ref to fan-controller schema.

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

[Krzysztof Kozlowski]

On 04/03/2025 17:31, Krzysztof Kozlowski wrote:
> On 04/03/2025 17:29, Conor Dooley wrote:
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - microchip,emc2301
>>> +      - microchip,emc2302
>>> +      - microchip,emc2303
>>> +      - microchip,emc2305
>>
>> Driver has no match data etc, suggesting all of these are compatible.
>> Please pick one as a fallback compatible.
>>
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>
>> Missing descriptions of the fans, no?
>> See max6639 for an example.
> 
> Yes and missing $ref to fan-controller schema.
About which I asked at v2...

Best regards,
Krzysztof

Re: [PATCH v3 3/3] hwmon: emc2305: Use devm_thermal_of_cooling_device_register

[Frank Li]

On Tue, Mar 04, 2025 at 06:21:36PM +0200, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
>
> This prepares emc2305 driver to use configuration from Device Tree nodes.
> Using devm_thermal_of_cooling_device_register also simplifies cleanup
> procedure, so we are able to remove the emc2305_unset_tz and emc2305_remove
> functions which are no longer used.

Kernel doc suggest use imperative mode:

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.

nice cleanup

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
> ---
>  drivers/hwmon/emc2305.c | 28 +++-------------------------
>  1 file changed, 3 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
> index a236f27c75bc..5f3f06308826 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]);
> @@ -337,21 +336,9 @@ static int emc2305_set_tz(struct device *dev)
>  	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
>  emc2305_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr, int channel)
>  {
> @@ -599,14 +586,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,emc2301", },
>  	{ .compatible = "microchip,emc2302", },
> @@ -622,7 +601,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 v3 1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

[Krzysztof Kozlowski]

On 04/03/2025 17:21, 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>
> ---
>  .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++

I do not see improvements in the subject nor in missing $ref to common
schema for fans.

<form letter>
This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions
of patchset, under or above your Signed-off-by tag, unless patch changed
significantly (e.g. new properties added to the DT bindings). Tag is
"received", when provided in a message replied to you on the mailing
list. Tools like b4 can help here. However, there's no need to repost
patches *only* to add the tags. The upstream maintainer will do that for
tags received on the version they apply.

Please read:
https://elixir.bootlin.com/linux/v6.12-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.
</form letter>

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

[Daniel Baluta]

Hello Frank, Conor, Krzysztof,

Thanks a lot for your comments and help.

In this initial patchseries we only add minimal OF support and
introduce a minimal binding.

> How did you pickup the maintainer entry?

We got the author of the first commit for
See 0d8400c5a2ce ("hwmon: (emc2305) add support for EMC2301/2/3/5 RPM-based")

> If only one reg You can go through trivial-devices.yaml

This is not a trivial device as we will add some specific properties
with our next patches
(e.g polarity etc)

> Missing descriptions of the fans, no?
> missing $ref to fan-controller schema.

Do we need to add this now? As this is only the minimal binding
support. We want to add that
in a follow up patch when we go into specifics.

> The emc2301 label here can be dropped, it is not used.

Will fix.

> The nodename should be "fan-controller", not pwm here I guess.

Sure, will fix.

On Tue, Mar 4, 2025 at 6:56 PM Frank Li <Frank.li@nxp.com> wrote:
>
> On Tue, Mar 04, 2025 at 06:21:34PM +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>
> > ---
> >  .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++
> >  1 file changed, 43 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..cac0075a65bb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> > @@ -0,0 +1,43 @@
> > +# 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>
>
> who is it? look like not Microchip and hwmon maintainer.
>
> > +
> > +description: |
>
> Needn't |
>
> > +  Microchip EMC2301/2/3/5 pwm controller which supports up
> > +  to five programmable fan control circuits
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - microchip,emc2301
> > +      - microchip,emc2302
> > +      - microchip,emc2303
> > +      - microchip,emc2305
>
> According to your driver code look like all compatible with microchip,emc2301
>
> oneOf:
>   - enum:
>       - microchip,emc2301
>   - items:
>       - enum:
>           - microchip,emc2302
>           - microchip,emc2303
>           - microchip,emc2305
>       - const: microchip,emc2301
>
> So your driver just need one "microchip,emc2301" compatible string.
>
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        emc2301: pwm@2f {
> > +            compatible = "microchip,emc2301";
> > +            reg = <0x2f>;
>
> If only one reg:
> You can go through trivial-devices.yaml
>
> Frank
>
> > +        };
> > +    };
> > --
> > 2.34.1
> >
>

RE: [PATCH v3 1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

[Florin Leotescu (OSS)]

Hello Frank, Conor, Krzysztof,

Thanks a lot for your feedback, time and patience. 
We will address the changes in V4.

Regards,
Florin

-----Original Message-----
From: Daniel Baluta <daniel.baluta@gmail.com> 
Sent: Wednesday, March 5, 2025 2:48 PM
To: Frank Li <frank.li@nxp.com>; Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Florin Leotescu (OSS) <florin.leotescu@oss.nxp.com>; Jean Delvare <jdelvare@suse.com>; Guenter Roeck <linux@roeck-us.net>; Rob Herring <robh@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Michael Shych <michaelsh@nvidia.com>; linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Viorel Suman <viorel.suman@nxp.com>; Carlos Song <carlos.song@nxp.com>; linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev; festevam@gmail.com; Florin Leotescu <florin.leotescu@nxp.com>
Subject: Re: [PATCH v3 1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

Hello Frank, Conor, Krzysztof,

Thanks a lot for your comments and help.

In this initial patchseries we only add minimal OF support and introduce a minimal binding.

> How did you pickup the maintainer entry?

We got the author of the first commit for See 0d8400c5a2ce ("hwmon: (emc2305) add support for EMC2301/2/3/5 RPM-based")

> If only one reg You can go through trivial-devices.yaml

This is not a trivial device as we will add some specific properties with our next patches (e.g polarity etc)

> Missing descriptions of the fans, no?
> missing $ref to fan-controller schema.

Do we need to add this now? As this is only the minimal binding support. We want to add that in a follow up patch when we go into specifics.

> The emc2301 label here can be dropped, it is not used.

Will fix.

> The nodename should be "fan-controller", not pwm here I guess.

Sure, will fix.

On Tue, Mar 4, 2025 at 6:56 PM Frank Li <Frank.li@nxp.com> wrote:
>
> On Tue, Mar 04, 2025 at 06:21:34PM +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>
> > ---
> >  .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++
> >  1 file changed, 43 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..cac0075a65bb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> > @@ -0,0 +1,43 @@
> > +# 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>
>
> who is it? look like not Microchip and hwmon maintainer.
>
> > +
> > +description: |
>
> Needn't |
>
> > +  Microchip EMC2301/2/3/5 pwm controller which supports up  to five 
> > + programmable fan control circuits
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - microchip,emc2301
> > +      - microchip,emc2302
> > +      - microchip,emc2303
> > +      - microchip,emc2305
>
> According to your driver code look like all compatible with 
> microchip,emc2301
>
> oneOf:
>   - enum:
>       - microchip,emc2301
>   - items:
>       - enum:
>           - microchip,emc2302
>           - microchip,emc2303
>           - microchip,emc2305
>       - const: microchip,emc2301
>
> So your driver just need one "microchip,emc2301" compatible string.
>
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        emc2301: pwm@2f {
> > +            compatible = "microchip,emc2301";
> > +            reg = <0x2f>;
>
> If only one reg:
> You can go through trivial-devices.yaml
>
> Frank
>
> > +        };
> > +    };
> > --
> > 2.34.1
> >
>

Re: [PATCH v3 3/3] hwmon: emc2305: Use devm_thermal_of_cooling_device_register

[Guenter Roeck]

On 3/4/25 08:21, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
> 
> This prepares emc2305 driver to use configuration from Device Tree nodes.
> Using devm_thermal_of_cooling_device_register also simplifies cleanup
> procedure, so we are able to remove the emc2305_unset_tz and emc2305_remove
> functions which are no longer used.
> 
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
> ---
>   drivers/hwmon/emc2305.c | 28 +++-------------------------
>   1 file changed, 3 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
> index a236f27c75bc..5f3f06308826 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]);
> @@ -337,21 +336,9 @@ static int emc2305_set_tz(struct device *dev)
>   	return 0;
>   
>   thermal_cooling_device_register_fail:

This label is no longer necessary. Instead, any goto can be replaced with
an immediate return statement.

> -	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
>   emc2305_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr, int channel)
>   {
> @@ -599,14 +586,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,emc2301", },
>   	{ .compatible = "microchip,emc2302", },
> @@ -622,7 +601,6 @@ static struct i2c_driver emc2305_driver = {
>   		.of_match_table = of_emc2305_match_table,
>   	},
>   	.probe = emc2305_probe,
> -	.remove	  = emc2305_remove,
>   	.id_table = emc2305_ids,
>   };
>