[PATCH 0/6] thermal: Introduce devm_thermal_of_cooling_device_register

[PATCH 0/6] thermal: Introduce devm_thermal_of_cooling_device_register

[Guenter Roeck]

thermal_of_cooling_device_register() and thermal_cooling_device_register()
are typically called from driver probe functions, and
thermal_cooling_device_unregister() is called from remove functions. This
makes both a perfect candidate for device managed functions.

Introduce devm_thermal_of_cooling_device_register(). This function can
also be used to replace thermal_cooling_device_register() by passing a NULL
pointer as device node. The new function requires both struct device *
and struct device_node * as parameters since the struct device_node *
parameter is not always identical to dev->of_node.

Don't introduce a device managed remove function since it is not needed
at this point.

The patch series introduces the new function and then converts various
hwmon drivers to use it.

[PATCH 1/6] thermal: Introduce devm_thermal_of_cooling_device_register

[Guenter Roeck]

thermal_of_cooling_device_register() and thermal_cooling_device_register()
are typically called from driver probe functions, and
thermal_cooling_device_unregister() is called from remove functions. This
makes both a perfect candidate for device managed functions.

Introduce devm_thermal_of_cooling_device_register(). This function can
also be used to replace thermal_cooling_device_register() by passing a NULL
pointer as device node. The new function requires both struct device *
and struct device_node * as parameters since the struct device_node *
parameter is not always identical to dev->of_node.

Don't introduce a device managed remove function since it is not needed
at this point.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/thermal/thermal_core.c | 49 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/thermal.h        |  5 +++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 6590bb5cb688..e0b530603db6 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1046,6 +1046,55 @@ thermal_of_cooling_device_register(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(thermal_of_cooling_device_register);
 
+static void thermal_cooling_device_release(struct device *dev, void *res)
+{
+	thermal_cooling_device_unregister(
+				*(struct thermal_cooling_device **)res);
+}
+
+/**
+ * devm_thermal_of_cooling_device_register() - register an OF thermal cooling
+ *					       device
+ * @dev:	a valid struct device pointer of a sensor device.
+ * @np:		a pointer to a device tree node.
+ * @type:	the thermal cooling device type.
+ * @devdata:	device private data.
+ * @ops:	standard thermal cooling devices callbacks.
+ *
+ * This function will register a cooling device with device tree node reference.
+ * This interface function adds a new thermal cooling device (fan/processor/...)
+ * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
+ * to all the thermal zone devices registered at the same time.
+ *
+ * Return: a pointer to the created struct thermal_cooling_device or an
+ * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
+ */
+struct thermal_cooling_device *
+devm_thermal_of_cooling_device_register(struct device *dev,
+				struct device_node *np,
+				char *type, void *devdata,
+				const struct thermal_cooling_device_ops *ops)
+{
+	struct thermal_cooling_device **ptr, *tcd;
+
+	ptr = devres_alloc(thermal_cooling_device_release, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	tcd = __thermal_cooling_device_register(np, type, devdata, ops);
+	if (IS_ERR(tcd)) {
+		devres_free(ptr);
+		return tcd;
+	}
+
+	*ptr = tcd;
+	devres_add(dev, ptr);
+
+	return tcd;
+}
+EXPORT_SYMBOL_GPL(devm_thermal_of_cooling_device_register);
+
 static void __unbind(struct thermal_zone_device *tz, int mask,
 		     struct thermal_cooling_device *cdev)
 {
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5f4705f46c2f..43cf4fdd71d4 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -447,6 +447,11 @@ struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
 struct thermal_cooling_device *
 thermal_of_cooling_device_register(struct device_node *np, char *, void *,
 				   const struct thermal_cooling_device_ops *);
+struct thermal_cooling_device *
+devm_thermal_of_cooling_device_register(struct device *dev,
+				struct device_node *np,
+				char *type, void *devdata,
+				const struct thermal_cooling_device_ops *ops);
 void thermal_cooling_device_unregister(struct thermal_cooling_device *);
 struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
 int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
-- 
2.7.4

[PATCH 1/6] thermal: Introduce devm_thermal_of_cooling_device_register

[Guenter Roeck]

On Thu, Apr 18, 2019 at 12:58:15PM -0700, Guenter Roeck wrote:
> thermal_of_cooling_device_register() and thermal_cooling_device_register()
> are typically called from driver probe functions, and
> thermal_cooling_device_unregister() is called from remove functions. This
> makes both a perfect candidate for device managed functions.
> 
> Introduce devm_thermal_of_cooling_device_register(). This function can
> also be used to replace thermal_cooling_device_register() by passing a NULL
> pointer as device node. The new function requires both struct device *
> and struct device_node * as parameters since the struct device_node *
> parameter is not always identical to dev->of_node.
> 
> Don't introduce a device managed remove function since it is not needed
> at this point.
> 

Any feedback / thoughts / comments ?

Thanks,
Guenter

[PATCH 1/6] thermal: Introduce devm_thermal_of_cooling_device_register

[Daniel Lezcano]

On 01/05/2019 18:48, Guenter Roeck wrote:
> On Thu, Apr 18, 2019 at 12:58:15PM -0700, Guenter Roeck wrote:
>> thermal_of_cooling_device_register() and thermal_cooling_device_register()
>> are typically called from driver probe functions, and
>> thermal_cooling_device_unregister() is called from remove functions. This
>> makes both a perfect candidate for device managed functions.
>>
>> Introduce devm_thermal_of_cooling_device_register(). This function can
>> also be used to replace thermal_cooling_device_register() by passing a NULL
>> pointer as device node. The new function requires both struct device *
>> and struct device_node * as parameters since the struct device_node *
>> parameter is not always identical to dev->of_node.
>>
>> Don't introduce a device managed remove function since it is not needed
>> at this point.
>>
> 
> Any feedback / thoughts / comments ?

Hi Guenter,

I have comments about your patch but I need some time to double check in
the current code how the 'of' and 'devm' are implemented.


-- 
 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 1/6] thermal: Introduce devm_thermal_of_cooling_device_register

[Eduardo Valentin]

Hello Guenter,

On Thu, Apr 18, 2019 at 12:58:15PM -0700, Guenter Roeck wrote:
> thermal_of_cooling_device_register() and thermal_cooling_device_register()
> are typically called from driver probe functions, and
> thermal_cooling_device_unregister() is called from remove functions. This
> makes both a perfect candidate for device managed functions.
> 
> Introduce devm_thermal_of_cooling_device_register(). This function can
> also be used to replace thermal_cooling_device_register() by passing a NULL
> pointer as device node. The new function requires both struct device *
> and struct device_node * as parameters since the struct device_node *
> parameter is not always identical to dev->of_node.
> 
> Don't introduce a device managed remove function since it is not needed
> at this point.

I don't have any objection on adding this API. Only a minor thing below:


> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/thermal/thermal_core.c | 49 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/thermal.h        |  5 +++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 6590bb5cb688..e0b530603db6 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1046,6 +1046,55 @@ thermal_of_cooling_device_register(struct device_node *np,
>  }
>  EXPORT_SYMBOL_GPL(thermal_of_cooling_device_register);
>  
> +static void thermal_cooling_device_release(struct device *dev, void *res)
> +{
> +	thermal_cooling_device_unregister(
> +				*(struct thermal_cooling_device **)res);
> +}
> +
> +/**
> + * devm_thermal_of_cooling_device_register() - register an OF thermal cooling
> + *					       device
> + * @dev:	a valid struct device pointer of a sensor device.
> + * @np:		a pointer to a device tree node.
> + * @type:	the thermal cooling device type.
> + * @devdata:	device private data.
> + * @ops:	standard thermal cooling devices callbacks.
> + *
> + * This function will register a cooling device with device tree node reference.
> + * This interface function adds a new thermal cooling device (fan/processor/...)
> + * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
> + * to all the thermal zone devices registered at the same time.
> + *
> + * Return: a pointer to the created struct thermal_cooling_device or an
> + * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
> + */
> +struct thermal_cooling_device *
> +devm_thermal_of_cooling_device_register(struct device *dev,
> +				struct device_node *np,
> +				char *type, void *devdata,
> +				const struct thermal_cooling_device_ops *ops)
> +{
> +	struct thermal_cooling_device **ptr, *tcd;
> +
> +	ptr = devres_alloc(thermal_cooling_device_release, sizeof(*ptr),
> +			   GFP_KERNEL);
> +	if (!ptr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	tcd = __thermal_cooling_device_register(np, type, devdata, ops);
> +	if (IS_ERR(tcd)) {
> +		devres_free(ptr);
> +		return tcd;
> +	}
> +
> +	*ptr = tcd;
> +	devres_add(dev, ptr);
> +
> +	return tcd;
> +}
> +EXPORT_SYMBOL_GPL(devm_thermal_of_cooling_device_register);
> +
>  static void __unbind(struct thermal_zone_device *tz, int mask,
>  		     struct thermal_cooling_device *cdev)
>  {
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 5f4705f46c2f..43cf4fdd71d4 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -447,6 +447,11 @@ struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
>  struct thermal_cooling_device *
>  thermal_of_cooling_device_register(struct device_node *np, char *, void *,
>  				   const struct thermal_cooling_device_ops *);
> +struct thermal_cooling_device *
> +devm_thermal_of_cooling_device_register(struct device *dev,
> +				struct device_node *np,
> +				char *type, void *devdata,
> +				const struct thermal_cooling_device_ops *ops);

We need to stub this in case thermal is not selected.

>  void thermal_cooling_device_unregister(struct thermal_cooling_device *);
>  struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
>  int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);

Something like:


diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 43cf4fd..9b1b365 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -508,6 +508,14 @@ static inline struct thermal_cooling_device *
 thermal_of_cooling_device_register(struct device_node *np,
        char *type, void *devdata, const struct thermal_cooling_device_ops *ops)
 { return ERR_PTR(-ENODEV); }
+struct thermal_cooling_device *
+devm_thermal_of_cooling_device_register(struct device *dev,
+                               struct device_node *np,
+                               char *type, void *devdata,
+                               const struct thermal_cooling_device_ops *ops)
+{
+       return ERR_PTR(-ENODEV);
+}
 static inline void thermal_cooling_device_unregister(
        struct thermal_cooling_device *cdev)
 { }
~


If you want I can amend this to your patch and apply it.

Also, do you prefer me to collect only this patch and you would collect hwmon changes,
or are you ok if I collect all the series?

[PATCH 1/6] thermal: Introduce devm_thermal_of_cooling_device_register

[Guenter Roeck]

Hi Eduardo,

On 5/11/19 12:04 PM, Eduardo Valentin wrote:
> Hello Guenter,
> 
> On Thu, Apr 18, 2019 at 12:58:15PM -0700, Guenter Roeck wrote:
>> thermal_of_cooling_device_register() and thermal_cooling_device_register()
>> are typically called from driver probe functions, and
>> thermal_cooling_device_unregister() is called from remove functions. This
>> makes both a perfect candidate for device managed functions.
>>
>> Introduce devm_thermal_of_cooling_device_register(). This function can
>> also be used to replace thermal_cooling_device_register() by passing a NULL
>> pointer as device node. The new function requires both struct device *
>> and struct device_node * as parameters since the struct device_node *
>> parameter is not always identical to dev->of_node.
>>
>> Don't introduce a device managed remove function since it is not needed
>> at this point.
> 
> I don't have any objection on adding this API. Only a minor thing below:
> 
> 
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   drivers/thermal/thermal_core.c | 49 ++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/thermal.h        |  5 +++++
>>   2 files changed, 54 insertions(+)
>>
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index 6590bb5cb688..e0b530603db6 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -1046,6 +1046,55 @@ thermal_of_cooling_device_register(struct device_node *np,
>>   }
>>   EXPORT_SYMBOL_GPL(thermal_of_cooling_device_register);
>>   
>> +static void thermal_cooling_device_release(struct device *dev, void *res)
>> +{
>> +	thermal_cooling_device_unregister(
>> +				*(struct thermal_cooling_device **)res);
>> +}
>> +
>> +/**
>> + * devm_thermal_of_cooling_device_register() - register an OF thermal cooling
>> + *					       device
>> + * @dev:	a valid struct device pointer of a sensor device.
>> + * @np:		a pointer to a device tree node.
>> + * @type:	the thermal cooling device type.
>> + * @devdata:	device private data.
>> + * @ops:	standard thermal cooling devices callbacks.
>> + *
>> + * This function will register a cooling device with device tree node reference.
>> + * This interface function adds a new thermal cooling device (fan/processor/...)
>> + * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
>> + * to all the thermal zone devices registered at the same time.
>> + *
>> + * Return: a pointer to the created struct thermal_cooling_device or an
>> + * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
>> + */
>> +struct thermal_cooling_device *
>> +devm_thermal_of_cooling_device_register(struct device *dev,
>> +				struct device_node *np,
>> +				char *type, void *devdata,
>> +				const struct thermal_cooling_device_ops *ops)
>> +{
>> +	struct thermal_cooling_device **ptr, *tcd;
>> +
>> +	ptr = devres_alloc(thermal_cooling_device_release, sizeof(*ptr),
>> +			   GFP_KERNEL);
>> +	if (!ptr)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	tcd = __thermal_cooling_device_register(np, type, devdata, ops);
>> +	if (IS_ERR(tcd)) {
>> +		devres_free(ptr);
>> +		return tcd;
>> +	}
>> +
>> +	*ptr = tcd;
>> +	devres_add(dev, ptr);
>> +
>> +	return tcd;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_thermal_of_cooling_device_register);
>> +
>>   static void __unbind(struct thermal_zone_device *tz, int mask,
>>   		     struct thermal_cooling_device *cdev)
>>   {
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 5f4705f46c2f..43cf4fdd71d4 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -447,6 +447,11 @@ struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
>>   struct thermal_cooling_device *
>>   thermal_of_cooling_device_register(struct device_node *np, char *, void *,
>>   				   const struct thermal_cooling_device_ops *);
>> +struct thermal_cooling_device *
>> +devm_thermal_of_cooling_device_register(struct device *dev,
>> +				struct device_node *np,
>> +				char *type, void *devdata,
>> +				const struct thermal_cooling_device_ops *ops);
> 
> We need to stub this in case thermal is not selected.
> 

Yes. Sorry, that completely slipped my mind.

>>   void thermal_cooling_device_unregister(struct thermal_cooling_device *);
>>   struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
>>   int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
> 
> Something like:
> 
> 
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 43cf4fd..9b1b365 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -508,6 +508,14 @@ static inline struct thermal_cooling_device *
>   thermal_of_cooling_device_register(struct device_node *np,
>          char *type, void *devdata, const struct thermal_cooling_device_ops *ops)
>   { return ERR_PTR(-ENODEV); }
> +struct thermal_cooling_device *
> +devm_thermal_of_cooling_device_register(struct device *dev,
> +                               struct device_node *np,
> +                               char *type, void *devdata,
> +                               const struct thermal_cooling_device_ops *ops)
> +{
> +       return ERR_PTR(-ENODEV);
> +}
>   static inline void thermal_cooling_device_unregister(
>          struct thermal_cooling_device *cdev)
>   { }
> ~
> 
> 
> If you want I can amend this to your patch and apply it.
> 
Please do.

> Also, do you prefer me to collect only this patch and you would collect hwmon changes,
> or are you ok if I collect all the series?
> 

Please go ahead and collect the entire series.

Thanks,
Guenter

[PATCH 2/6] hwmon: (aspeed-pwm-tacho) Use devm_thermal_of_cooling_device_register

[Guenter Roeck]

Use devm_thermal_of_cooling_device_register() to register the cooling
device. As a side effect, this fixes a driver bug:
thermal_cooling_device_unregister() was not called on removal.

Fixes: f198907d2ff6d ("hwmon: (aspeed-pwm-tacho) cooling device support.")
Cc: Mykola Kostenok <c_mykolak@mellanox.com>
Cc: Joel Stanley <joel@jms.id.au>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/aspeed-pwm-tacho.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
index c4dd6301e7c8..0daf0b32aa4a 100644
--- a/drivers/hwmon/aspeed-pwm-tacho.c
+++ b/drivers/hwmon/aspeed-pwm-tacho.c
@@ -830,10 +830,8 @@ static int aspeed_create_pwm_cooling(struct device *dev,
 	}
 	snprintf(cdev->name, MAX_CDEV_NAME_LEN, "%pOFn%d", child, pwm_port);
 
-	cdev->tcdev = thermal_of_cooling_device_register(child,
-							 cdev->name,
-							 cdev,
-							 &aspeed_pwm_cool_ops);
+	cdev->tcdev = devm_thermal_of_cooling_device_register(dev, child,
+					cdev->name, cdev, &aspeed_pwm_cool_ops);
 	if (IS_ERR(cdev->tcdev))
 		return PTR_ERR(cdev->tcdev);
 
-- 
2.7.4

[PATCH 2/6] hwmon: (aspeed-pwm-tacho) Use devm_thermal_of_cooling_device_register

[Patrick Venture]

On Thu, Apr 18, 2019 at 12:58 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> Use devm_thermal_of_cooling_device_register() to register the cooling
> device. As a side effect, this fixes a driver bug:
> thermal_cooling_device_unregister() was not called on removal.
>
> Fixes: f198907d2ff6d ("hwmon: (aspeed-pwm-tacho) cooling device support.")
> Cc: Mykola Kostenok <c_mykolak@mellanox.com>
> Cc: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Patrick Venture <venture@google.com>

> ---
>  drivers/hwmon/aspeed-pwm-tacho.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
> index c4dd6301e7c8..0daf0b32aa4a 100644
> --- a/drivers/hwmon/aspeed-pwm-tacho.c
> +++ b/drivers/hwmon/aspeed-pwm-tacho.c
> @@ -830,10 +830,8 @@ static int aspeed_create_pwm_cooling(struct device *dev,
>         }
>         snprintf(cdev->name, MAX_CDEV_NAME_LEN, "%pOFn%d", child, pwm_port);
>
> -       cdev->tcdev = thermal_of_cooling_device_register(child,
> -                                                        cdev->name,
> -                                                        cdev,
> -                                                        &aspeed_pwm_cool_ops);
> +       cdev->tcdev = devm_thermal_of_cooling_device_register(dev, child,
> +                                       cdev->name, cdev, &aspeed_pwm_cool_ops);
>         if (IS_ERR(cdev->tcdev))
>                 return PTR_ERR(cdev->tcdev);
>
> --
> 2.7.4
>

[PATCH 3/6] hwmon: (gpio-fan) Use devm_thermal_of_cooling_device_register

[Guenter Roeck]

Call devm_thermal_of_cooling_device_register() to register the cooling
device. Also use devm_add_action_or_reset() to stop the fan on device
removal. This fixes a race condition since the fan was stopped before
the hwmon device was removed.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/gpio-fan.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index f1bf67aca9e8..3f6e5b4e3997 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -498,6 +498,11 @@ static const struct of_device_id of_gpio_fan_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_gpio_fan_match);
 
+static void gpio_fan_stop(void *data)
+{
+	set_fan_speed(data, 0);
+}
+
 static int gpio_fan_probe(struct platform_device *pdev)
 {
 	int err;
@@ -532,6 +537,7 @@ static int gpio_fan_probe(struct platform_device *pdev)
 		err = fan_ctrl_init(fan_data);
 		if (err)
 			return err;
+		devm_add_action_or_reset(dev, gpio_fan_stop, fan_data);
 	}
 
 	/* Make this driver part of hwmon class. */
@@ -543,32 +549,20 @@ static int gpio_fan_probe(struct platform_device *pdev)
 		return PTR_ERR(fan_data->hwmon_dev);
 
 	/* Optional cooling device register for Device tree platforms */
-	fan_data->cdev = thermal_of_cooling_device_register(np,
-							    "gpio-fan",
-							    fan_data,
-							    &gpio_fan_cool_ops);
+	fan_data->cdev = devm_thermal_of_cooling_device_register(dev, np,
+				"gpio-fan", fan_data, &gpio_fan_cool_ops);
 
 	dev_info(dev, "GPIO fan initialized\n");
 
 	return 0;
 }
 
-static int gpio_fan_remove(struct platform_device *pdev)
+static void gpio_fan_shutdown(struct platform_device *pdev)
 {
 	struct gpio_fan_data *fan_data = platform_get_drvdata(pdev);
 
-	if (!IS_ERR(fan_data->cdev))
-		thermal_cooling_device_unregister(fan_data->cdev);
-
 	if (fan_data->gpios)
 		set_fan_speed(fan_data, 0);
-
-	return 0;
-}
-
-static void gpio_fan_shutdown(struct platform_device *pdev)
-{
-	gpio_fan_remove(pdev);
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -602,7 +596,6 @@ static SIMPLE_DEV_PM_OPS(gpio_fan_pm, gpio_fan_suspend, gpio_fan_resume);
 
 static struct platform_driver gpio_fan_driver = {
 	.probe		= gpio_fan_probe,
-	.remove		= gpio_fan_remove,
 	.shutdown	= gpio_fan_shutdown,
 	.driver	= {
 		.name	= "gpio-fan",
-- 
2.7.4

[PATCH 4/6] hwmon: (mlxreg-fan) Use devm_thermal_of_cooling_device_register

[Guenter Roeck]

Call devm_thermal_of_cooling_device_register() to register the cooling
device. Also introduce struct device *dev = &pdev->dev; to make the code
easier to read.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/mlxreg-fan.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
index db8c6de0b6a0..a14347ea0d77 100644
--- a/drivers/hwmon/mlxreg-fan.c
+++ b/drivers/hwmon/mlxreg-fan.c
@@ -420,42 +420,42 @@ static int mlxreg_fan_config(struct mlxreg_fan *fan,
 static int mlxreg_fan_probe(struct platform_device *pdev)
 {
 	struct mlxreg_core_platform_data *pdata;
+	struct device *dev = &pdev->dev;
 	struct mlxreg_fan *fan;
 	struct device *hwm;
 	int err;
 
-	pdata = dev_get_platdata(&pdev->dev);
+	pdata = dev_get_platdata(dev);
 	if (!pdata) {
-		dev_err(&pdev->dev, "Failed to get platform data.\n");
+		dev_err(dev, "Failed to get platform data.\n");
 		return -EINVAL;
 	}
 
-	fan = devm_kzalloc(&pdev->dev, sizeof(*fan), GFP_KERNEL);
+	fan = devm_kzalloc(dev, sizeof(*fan), GFP_KERNEL);
 	if (!fan)
 		return -ENOMEM;
 
-	fan->dev = &pdev->dev;
+	fan->dev = dev;
 	fan->regmap = pdata->regmap;
-	platform_set_drvdata(pdev, fan);
 
 	err = mlxreg_fan_config(fan, pdata);
 	if (err)
 		return err;
 
-	hwm = devm_hwmon_device_register_with_info(&pdev->dev, "mlxreg_fan",
+	hwm = devm_hwmon_device_register_with_info(dev, "mlxreg_fan",
 						   fan,
 						   &mlxreg_fan_hwmon_chip_info,
 						   NULL);
 	if (IS_ERR(hwm)) {
-		dev_err(&pdev->dev, "Failed to register hwmon device\n");
+		dev_err(dev, "Failed to register hwmon device\n");
 		return PTR_ERR(hwm);
 	}
 
 	if (IS_REACHABLE(CONFIG_THERMAL)) {
-		fan->cdev = thermal_cooling_device_register("mlxreg_fan", fan,
-						&mlxreg_fan_cooling_ops);
+		fan->cdev = devm_thermal_of_cooling_device_register(dev,
+			NULL, "mlxreg_fan", fan, &mlxreg_fan_cooling_ops);
 		if (IS_ERR(fan->cdev)) {
-			dev_err(&pdev->dev, "Failed to register cooling device\n");
+			dev_err(dev, "Failed to register cooling device\n");
 			return PTR_ERR(fan->cdev);
 		}
 	}
@@ -463,22 +463,11 @@ static int mlxreg_fan_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int mlxreg_fan_remove(struct platform_device *pdev)
-{
-	struct mlxreg_fan *fan = platform_get_drvdata(pdev);
-
-	if (IS_REACHABLE(CONFIG_THERMAL))
-		thermal_cooling_device_unregister(fan->cdev);
-
-	return 0;
-}
-
 static struct platform_driver mlxreg_fan_driver = {
 	.driver = {
 	    .name = "mlxreg-fan",
 	},
 	.probe = mlxreg_fan_probe,
-	.remove = mlxreg_fan_remove,
 };
 
 module_platform_driver(mlxreg_fan_driver);
-- 
2.7.4

[PATCH 5/6] hwmon: (npcm750-pwm-fan) Use devm_thermal_of_cooling_device_register

[Guenter Roeck]

Use devm_thermal_of_cooling_device_register() to register the cooling
device. As a side effect, this fixes a driver bug:
thermal_cooling_device_unregister() was not called on device removal.

Fixes: f1fd4a4db777 ("hwmon: Add NPCM7xx PWM and Fan driver")
Cc: Tomer Maimon <tmaimon77@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/npcm750-pwm-fan.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/npcm750-pwm-fan.c b/drivers/hwmon/npcm750-pwm-fan.c
index b3b907bdfb63..f24cc00caba9 100644
--- a/drivers/hwmon/npcm750-pwm-fan.c
+++ b/drivers/hwmon/npcm750-pwm-fan.c
@@ -864,10 +864,8 @@ static int npcm7xx_create_pwm_cooling(struct device *dev,
 	snprintf(cdev->name, THERMAL_NAME_LENGTH, "%pOFn%d", child,
 		 pwm_port);
 
-	cdev->tcdev = thermal_of_cooling_device_register(child,
-							 cdev->name,
-							 cdev,
-							 &npcm7xx_pwm_cool_ops);
+	cdev->tcdev = devm_thermal_of_cooling_device_register(dev, child,
+				cdev->name, cdev, &npcm7xx_pwm_cool_ops);
 	if (IS_ERR(cdev->tcdev))
 		return PTR_ERR(cdev->tcdev);
 
-- 
2.7.4

[PATCH 6/6] hwmon: (pwm-fan) Use devm_thermal_of_cooling_device_register

[Guenter Roeck]

Use devm_thermal_of_cooling_device_register() to register the cooling
device. Also use devm_add_action_or_reset() to stop the fan on device
removal, and to disable the pwm. Introduce a local 'dev' variable in
the probe function to make the code easier to read.

As a side effect, this fixes a bug seen if pwm_fan_of_get_cooling_data()
returned an error. In that situation, the pwm was not disabled, and
the fan was not stopped. Using devm functions also ensures that the
pwm is disabled and that the fan is stopped only after the hwmon device
has been unregistered.

Cc: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pwm-fan.c | 73 ++++++++++++++++++++-----------------------------
 1 file changed, 29 insertions(+), 44 deletions(-)

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 167221c7628a..0243ba70107e 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -207,33 +207,44 @@ static int pwm_fan_of_get_cooling_data(struct device *dev,
 	return 0;
 }
 
+static void pwm_fan_regulator_disable(void *data)
+{
+	regulator_disable(data);
+}
+
+static void pwm_fan_pwm_disable(void *data)
+{
+	pwm_disable(data);
+}
+
 static int pwm_fan_probe(struct platform_device *pdev)
 {
 	struct thermal_cooling_device *cdev;
+	struct device *dev = &pdev->dev;
 	struct pwm_fan_ctx *ctx;
 	struct device *hwmon;
 	int ret;
 	struct pwm_state state = { };
 
-	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 
 	mutex_init(&ctx->lock);
 
-	ctx->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
+	ctx->pwm = devm_of_pwm_get(dev, dev->of_node, NULL);
 	if (IS_ERR(ctx->pwm)) {
 		ret = PTR_ERR(ctx->pwm);
 
 		if (ret != -EPROBE_DEFER)
-			dev_err(&pdev->dev, "Could not get PWM: %d\n", ret);
+			dev_err(dev, "Could not get PWM: %d\n", ret);
 
 		return ret;
 	}
 
 	platform_set_drvdata(pdev, ctx);
 
-	ctx->reg_en = devm_regulator_get_optional(&pdev->dev, "fan");
+	ctx->reg_en = devm_regulator_get_optional(dev, "fan");
 	if (IS_ERR(ctx->reg_en)) {
 		if (PTR_ERR(ctx->reg_en) != -ENODEV)
 			return PTR_ERR(ctx->reg_en);
@@ -242,10 +253,11 @@ static int pwm_fan_probe(struct platform_device *pdev)
 	} else {
 		ret = regulator_enable(ctx->reg_en);
 		if (ret) {
-			dev_err(&pdev->dev,
-				"Failed to enable fan supply: %d\n", ret);
+			dev_err(dev, "Failed to enable fan supply: %d\n", ret);
 			return ret;
 		}
+		devm_add_action_or_reset(dev, pwm_fan_regulator_disable,
+					 ctx->reg_en);
 	}
 
 	ctx->pwm_value = MAX_PWM;
@@ -257,62 +269,36 @@ static int pwm_fan_probe(struct platform_device *pdev)
 
 	ret = pwm_apply_state(ctx->pwm, &state);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to configure PWM\n");
-		goto err_reg_disable;
+		dev_err(dev, "Failed to configure PWM\n");
+		return ret;
 	}
+	devm_add_action_or_reset(dev, pwm_fan_pwm_disable, ctx->pwm);
 
-	hwmon = devm_hwmon_device_register_with_groups(&pdev->dev, "pwmfan",
+	hwmon = devm_hwmon_device_register_with_groups(dev, "pwmfan",
 						       ctx, pwm_fan_groups);
 	if (IS_ERR(hwmon)) {
-		dev_err(&pdev->dev, "Failed to register hwmon device\n");
-		ret = PTR_ERR(hwmon);
-		goto err_pwm_disable;
+		dev_err(dev, "Failed to register hwmon device\n");
+		return PTR_ERR(hwmon);
 	}
 
-	ret = pwm_fan_of_get_cooling_data(&pdev->dev, ctx);
+	ret = pwm_fan_of_get_cooling_data(dev, ctx);
 	if (ret)
 		return ret;
 
 	ctx->pwm_fan_state = ctx->pwm_fan_max_state;
 	if (IS_ENABLED(CONFIG_THERMAL)) {
-		cdev = thermal_of_cooling_device_register(pdev->dev.of_node,
-							  "pwm-fan", ctx,
-							  &pwm_fan_cooling_ops);
+		cdev = devm_thermal_of_cooling_device_register(dev,
+			dev->of_node, "pwm-fan", ctx, &pwm_fan_cooling_ops);
 		if (IS_ERR(cdev)) {
-			dev_err(&pdev->dev,
+			dev_err(dev,
 				"Failed to register pwm-fan as cooling device");
-			ret = PTR_ERR(cdev);
-			goto err_pwm_disable;
+			return PTR_ERR(cdev);
 		}
 		ctx->cdev = cdev;
 		thermal_cdev_update(cdev);
 	}
 
 	return 0;
-
-err_pwm_disable:
-	state.enabled = false;
-	pwm_apply_state(ctx->pwm, &state);
-
-err_reg_disable:
-	if (ctx->reg_en)
-		regulator_disable(ctx->reg_en);
-
-	return ret;
-}
-
-static int pwm_fan_remove(struct platform_device *pdev)
-{
-	struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev);
-
-	thermal_cooling_device_unregister(ctx->cdev);
-	if (ctx->pwm_value)
-		pwm_disable(ctx->pwm);
-
-	if (ctx->reg_en)
-		regulator_disable(ctx->reg_en);
-
-	return 0;
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -380,7 +366,6 @@ MODULE_DEVICE_TABLE(of, of_pwm_fan_match);
 
 static struct platform_driver pwm_fan_driver = {
 	.probe		= pwm_fan_probe,
-	.remove		= pwm_fan_remove,
 	.driver	= {
 		.name		= "pwm-fan",
 		.pm		= &pwm_fan_pm,
-- 
2.7.4

[PATCH 6/6] hwmon: (pwm-fan) Use devm_thermal_of_cooling_device_register

[Marek Szyprowski]

Dear All,

On 2019-04-18 21:58, Guenter Roeck wrote:
> Use devm_thermal_of_cooling_device_register() to register the cooling
> device. Also use devm_add_action_or_reset() to stop the fan on device
> removal, and to disable the pwm. Introduce a local 'dev' variable in
> the probe function to make the code easier to read.
>
> As a side effect, this fixes a bug seen if pwm_fan_of_get_cooling_data()
> returned an error. In that situation, the pwm was not disabled, and
> the fan was not stopped. Using devm functions also ensures that the
> pwm is disabled and that the fan is stopped only after the hwmon device
> has been unregistered.
>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>


I've noticed the following lockdep warning after this commit during CPU 
hotplug tests on TM2e board (ARM64 Exynos5433). It looks like a false 
positive, but it would be nice to annotate it somehow in the code to 
make lockdep happy:

--->8---

IRQ 8: no longer affine to CPU5
CPU5: shutdown
IRQ 9: no longer affine to CPU6
CPU6: shutdown

======================================================
WARNING: possible circular locking dependency detected
5.2.0-rc1+ #6093 Not tainted
------------------------------------------------------
cpuhp/7/43 is trying to acquire lock:
00000000d1a60be3 (thermal_list_lock){+.+.}, at: 
thermal_cooling_device_unregister+0x34/0x1c0

but task is already holding lock:
00000000a6a56c92 (&policy->rwsem){++++}, at: cpufreq_offline+0x68/0x228

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #3 (&policy->rwsem){++++}:
 ?????? down_write+0x48/0x98
 ?????? cpufreq_cpu_acquire+0x20/0x58
 ?????? cpufreq_update_policy+0x28/0xc0
 ?????? cpufreq_set_cur_state+0x44/0x70
 ?????? thermal_cdev_update+0x7c/0x240
 ?????? step_wise_throttle+0x4c/0x88
 ?????? handle_thermal_trip+0xc0/0x348
 ?????? thermal_zone_device_update.part.7+0x6c/0x250
 ?????? thermal_zone_device_update+0x28/0x38
 ?????? exynos_tmu_work+0x28/0x70
 ?????? process_one_work+0x298/0x6c0
 ?????? worker_thread+0x48/0x430
 ?????? kthread+0x100/0x130
 ?????? ret_from_fork+0x10/0x18

-> #2 (&cdev->lock){+.+.}:
 ?????? __mutex_lock+0x90/0x890
 ?????? mutex_lock_nested+0x1c/0x28
 ?????? thermal_zone_bind_cooling_device+0x2cc/0x3e0
 ?????? of_thermal_bind+0x9c/0xf8
 ?????? __thermal_cooling_device_register+0x1a4/0x388
 ?????? thermal_of_cooling_device_register+0xc/0x18
 ?????? __cpufreq_cooling_register+0x360/0x410
 ?????? of_cpufreq_cooling_register+0x84/0xf8
 ?????? cpufreq_online+0x414/0x720
 ?????? cpufreq_add_dev+0x78/0x88
 ?????? subsys_interface_register+0xa4/0xf8
 ?????? cpufreq_register_driver+0x140/0x1e0
 ?????? dt_cpufreq_probe+0xb0/0x130
 ?????? platform_drv_probe+0x50/0xa8
 ?????? really_probe+0x1b0/0x2a0
 ?????? driver_probe_device+0x58/0x100
 ?????? __device_attach_driver+0x90/0xc0
 ?????? bus_for_each_drv+0x70/0xc8
 ?????? __device_attach+0xdc/0x140
 ?????? device_initial_probe+0x10/0x18
 ?????? bus_probe_device+0x94/0xa0
 ?????? device_add+0x39c/0x5c8
 ?????? platform_device_add+0x110/0x248
 ?????? platform_device_register_full+0x134/0x178
 ?????? cpufreq_dt_platdev_init+0x114/0x14c
 ?????? do_one_initcall+0x84/0x430
 ?????? kernel_init_freeable+0x440/0x534
 ?????? kernel_init+0x10/0x108
 ?????? ret_from_fork+0x10/0x18

-> #1 (&tz->lock){+.+.}:
 ?????? __mutex_lock+0x90/0x890
 ?????? mutex_lock_nested+0x1c/0x28
 ?????? thermal_zone_bind_cooling_device+0x2b8/0x3e0
 ?????? of_thermal_bind+0x9c/0xf8
 ?????? __thermal_cooling_device_register+0x1a4/0x388
 ?????? thermal_of_cooling_device_register+0xc/0x18
 ?????? __cpufreq_cooling_register+0x360/0x410
 ?????? of_cpufreq_cooling_register+0x84/0xf8
 ?????? cpufreq_online+0x414/0x720
 ?????? cpufreq_add_dev+0x78/0x88
 ?????? subsys_interface_register+0xa4/0xf8
 ?????? cpufreq_register_driver+0x140/0x1e0
 ?????? dt_cpufreq_probe+0xb0/0x130
 ?????? platform_drv_probe+0x50/0xa8
 ?????? really_probe+0x1b0/0x2a0
 ?????? driver_probe_device+0x58/0x100
 ?????? __device_attach_driver+0x90/0xc0
 ?????? bus_for_each_drv+0x70/0xc8
 ?????? __device_attach+0xdc/0x140
 ?????? device_initial_probe+0x10/0x18
 ?????? bus_probe_device+0x94/0xa0
 ?????? device_add+0x39c/0x5c8
 ?????? platform_device_add+0x110/0x248
 ?????? platform_device_register_full+0x134/0x178
 ?????? cpufreq_dt_platdev_init+0x114/0x14c
 ?????? do_one_initcall+0x84/0x430
 ?????? kernel_init_freeable+0x440/0x534
 ?????? kernel_init+0x10/0x108
 ?????? ret_from_fork+0x10/0x18

-> #0 (thermal_list_lock){+.+.}:
 ?????? lock_acquire+0xdc/0x260
 ?????? __mutex_lock+0x90/0x890
 ?????? mutex_lock_nested+0x1c/0x28
 ?????? thermal_cooling_device_unregister+0x34/0x1c0
 ?????? cpufreq_cooling_unregister+0x78/0xd0
 ?????? cpufreq_offline+0x200/0x228
 ?????? cpuhp_cpufreq_offline+0xc/0x18
 ?????? cpuhp_invoke_callback+0xd0/0xcb0
 ?????? cpuhp_thread_fun+0x1e8/0x258
 ?????? smpboot_thread_fn+0x1b4/0x2d0
 ?????? kthread+0x100/0x130
 ?????? ret_from_fork+0x10/0x18

other info that might help us debug this:

Chain exists of:
 ? thermal_list_lock --> &cdev->lock --> &policy->rwsem

 ?Possible unsafe locking scenario:

 ?????? CPU0??????????????????? CPU1
 ?????? ----??????????????????? ----
 ? lock(&policy->rwsem);
 ?????????????????????????????? lock(&cdev->lock);
 ?????????????????????????????? lock(&policy->rwsem);
 ? lock(thermal_list_lock);

 ?*** DEADLOCK ***

3 locks held by cpuhp/7/43:
 ?#0: 00000000ae30cc2b (cpu_hotplug_lock.rw_sem){++++}, at: 
cpuhp_thread_fun+0x34/0x258
 ?#1: 00000000a0e2460a (cpuhp_state-down){+.+.}, at: 
cpuhp_thread_fun+0x178/0x258
 ?#2: 00000000a6a56c92 (&policy->rwsem){++++}, at: 
cpufreq_offline+0x68/0x228

stack backtrace:
CPU: 7 PID: 43 Comm: cpuhp/7 Not tainted 5.2.0-rc1+ #6093
Hardware name: Samsung TM2E board (DT)
Call trace:
 ?dump_backtrace+0x0/0x158
 ?show_stack+0x14/0x20
 ?dump_stack+0xc8/0x114
 ?print_circular_bug+0x1cc/0x2d8
 ?__lock_acquire+0x18f4/0x20f8
 ?lock_acquire+0xdc/0x260
 ?__mutex_lock+0x90/0x890
 ?mutex_lock_nested+0x1c/0x28
 ?thermal_cooling_device_unregister+0x34/0x1c0
 ?cpufreq_cooling_unregister+0x78/0xd0
 ?cpufreq_offline+0x200/0x228
 ?cpuhp_cpufreq_offline+0xc/0x18
 ?cpuhp_invoke_callback+0xd0/0xcb0
 ?cpuhp_thread_fun+0x1e8/0x258
 ?smpboot_thread_fn+0x1b4/0x2d0
 ?kthread+0x100/0x130
 ?ret_from_fork+0x10/0x18
IRQ 10: no longer affine to CPU7

--->8---

> ---
>   drivers/hwmon/pwm-fan.c | 73 ++++++++++++++++++++-----------------------------
>   1 file changed, 29 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 167221c7628a..0243ba70107e 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -207,33 +207,44 @@ static int pwm_fan_of_get_cooling_data(struct device *dev,
>   	return 0;
>   }
>   
> +static void pwm_fan_regulator_disable(void *data)
> +{
> +	regulator_disable(data);
> +}
> +
> +static void pwm_fan_pwm_disable(void *data)
> +{
> +	pwm_disable(data);
> +}
> +
>   static int pwm_fan_probe(struct platform_device *pdev)
>   {
>   	struct thermal_cooling_device *cdev;
> +	struct device *dev = &pdev->dev;
>   	struct pwm_fan_ctx *ctx;
>   	struct device *hwmon;
>   	int ret;
>   	struct pwm_state state = { };
>   
> -	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>   	if (!ctx)
>   		return -ENOMEM;
>   
>   	mutex_init(&ctx->lock);
>   
> -	ctx->pwm = devm_of_pwm_get(&pdev->dev, pdev->dev.of_node, NULL);
> +	ctx->pwm = devm_of_pwm_get(dev, dev->of_node, NULL);
>   	if (IS_ERR(ctx->pwm)) {
>   		ret = PTR_ERR(ctx->pwm);
>   
>   		if (ret != -EPROBE_DEFER)
> -			dev_err(&pdev->dev, "Could not get PWM: %d\n", ret);
> +			dev_err(dev, "Could not get PWM: %d\n", ret);
>   
>   		return ret;
>   	}
>   
>   	platform_set_drvdata(pdev, ctx);
>   
> -	ctx->reg_en = devm_regulator_get_optional(&pdev->dev, "fan");
> +	ctx->reg_en = devm_regulator_get_optional(dev, "fan");
>   	if (IS_ERR(ctx->reg_en)) {
>   		if (PTR_ERR(ctx->reg_en) != -ENODEV)
>   			return PTR_ERR(ctx->reg_en);
> @@ -242,10 +253,11 @@ static int pwm_fan_probe(struct platform_device *pdev)
>   	} else {
>   		ret = regulator_enable(ctx->reg_en);
>   		if (ret) {
> -			dev_err(&pdev->dev,
> -				"Failed to enable fan supply: %d\n", ret);
> +			dev_err(dev, "Failed to enable fan supply: %d\n", ret);
>   			return ret;
>   		}
> +		devm_add_action_or_reset(dev, pwm_fan_regulator_disable,
> +					 ctx->reg_en);
>   	}
>   
>   	ctx->pwm_value = MAX_PWM;
> @@ -257,62 +269,36 @@ static int pwm_fan_probe(struct platform_device *pdev)
>   
>   	ret = pwm_apply_state(ctx->pwm, &state);
>   	if (ret) {
> -		dev_err(&pdev->dev, "Failed to configure PWM\n");
> -		goto err_reg_disable;
> +		dev_err(dev, "Failed to configure PWM\n");
> +		return ret;
>   	}
> +	devm_add_action_or_reset(dev, pwm_fan_pwm_disable, ctx->pwm);
>   
> -	hwmon = devm_hwmon_device_register_with_groups(&pdev->dev, "pwmfan",
> +	hwmon = devm_hwmon_device_register_with_groups(dev, "pwmfan",
>   						       ctx, pwm_fan_groups);
>   	if (IS_ERR(hwmon)) {
> -		dev_err(&pdev->dev, "Failed to register hwmon device\n");
> -		ret = PTR_ERR(hwmon);
> -		goto err_pwm_disable;
> +		dev_err(dev, "Failed to register hwmon device\n");
> +		return PTR_ERR(hwmon);
>   	}
>   
> -	ret = pwm_fan_of_get_cooling_data(&pdev->dev, ctx);
> +	ret = pwm_fan_of_get_cooling_data(dev, ctx);
>   	if (ret)
>   		return ret;
>   
>   	ctx->pwm_fan_state = ctx->pwm_fan_max_state;
>   	if (IS_ENABLED(CONFIG_THERMAL)) {
> -		cdev = thermal_of_cooling_device_register(pdev->dev.of_node,
> -							  "pwm-fan", ctx,
> -							  &pwm_fan_cooling_ops);
> +		cdev = devm_thermal_of_cooling_device_register(dev,
> +			dev->of_node, "pwm-fan", ctx, &pwm_fan_cooling_ops);
>   		if (IS_ERR(cdev)) {
> -			dev_err(&pdev->dev,
> +			dev_err(dev,
>   				"Failed to register pwm-fan as cooling device");
> -			ret = PTR_ERR(cdev);
> -			goto err_pwm_disable;
> +			return PTR_ERR(cdev);
>   		}
>   		ctx->cdev = cdev;
>   		thermal_cdev_update(cdev);
>   	}
>   
>   	return 0;
> -
> -err_pwm_disable:
> -	state.enabled = false;
> -	pwm_apply_state(ctx->pwm, &state);
> -
> -err_reg_disable:
> -	if (ctx->reg_en)
> -		regulator_disable(ctx->reg_en);
> -
> -	return ret;
> -}
> -
> -static int pwm_fan_remove(struct platform_device *pdev)
> -{
> -	struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev);
> -
> -	thermal_cooling_device_unregister(ctx->cdev);
> -	if (ctx->pwm_value)
> -		pwm_disable(ctx->pwm);
> -
> -	if (ctx->reg_en)
> -		regulator_disable(ctx->reg_en);
> -
> -	return 0;
>   }
>   
>   #ifdef CONFIG_PM_SLEEP
> @@ -380,7 +366,6 @@ MODULE_DEVICE_TABLE(of, of_pwm_fan_match);
>   
>   static struct platform_driver pwm_fan_driver = {
>   	.probe		= pwm_fan_probe,
> -	.remove		= pwm_fan_remove,
>   	.driver	= {
>   		.name		= "pwm-fan",
>   		.pm		= &pwm_fan_pm,

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland