Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 5/8] platform/x86: gpd pocket fan: Clean-up by using
 managed work init
To:     Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
        Matti Vaittinen <mazziesaccount@gmail.com>
Cc:     Mark Gross <mgross@linux.intel.com>, gregkh@linuxfoundation.org,
        linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org
References: <cover.1616506559.git.matti.vaittinen@fi.rohmeurope.com>
 <aa25a6781ba016772b045cd6e630da8c559a665d.1616506559.git.matti.vaittinen@fi.rohmeurope.com>
From:   Hans de Goede <hdegoede@redhat.com>
Message-ID: <3109f58a-91e4-09bd-d953-f43a4242c194@redhat.com>
Date:   Tue, 23 Mar 2021 16:30:38 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.0
MIME-Version: 1.0
In-Reply-To: <aa25a6781ba016772b045cd6e630da8c559a665d.1616506559.git.matti.vaittinen@fi.rohmeurope.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

HI,

On 3/23/21 2:57 PM, Matti Vaittinen wrote:
> Few drivers implement remove call-back only for ensuring a delayed
> work gets cancelled prior driver removal. Clean-up these by switching
> to use devm_delayed_work_autocancel() instead.
> 
> This change is compile-tested only. All testing is appreciated.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
> Changelog from RFCv2:
>  - RFC dropped. No functional changes.
> 
>  drivers/platform/x86/gpd-pocket-fan.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/platform/x86/gpd-pocket-fan.c b/drivers/platform/x86/gpd-pocket-fan.c
> index 5b516e4c2bfb..7a20f68ae206 100644
> --- a/drivers/platform/x86/gpd-pocket-fan.c
> +++ b/drivers/platform/x86/gpd-pocket-fan.c
> @@ -6,6 +6,7 @@
>   */
>  
>  #include <linux/acpi.h>
> +#include <linux/devm-helpers.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> @@ -124,7 +125,7 @@ static void gpd_pocket_fan_force_update(struct gpd_pocket_fan_data *fan)
>  static int gpd_pocket_fan_probe(struct platform_device *pdev)
>  {
>  	struct gpd_pocket_fan_data *fan;
> -	int i;
> +	int i, ret;
>  
>  	for (i = 0; i < ARRAY_SIZE(temp_limits); i++) {
>  		if (temp_limits[i] < 20000 || temp_limits[i] > 90000) {
> @@ -152,7 +153,10 @@ static int gpd_pocket_fan_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	fan->dev = &pdev->dev;
> -	INIT_DELAYED_WORK(&fan->work, gpd_pocket_fan_worker);
> +	ret = devm_delayed_work_autocancel(&pdev->dev, &fan->work,
> +					   gpd_pocket_fan_worker);
> +	if (ret)
> +		return ret;
>  
>  	/* Note this returns a "weak" reference which we don't need to free */
>  	fan->dts0 = thermal_zone_get_zone_by_name("soc_dts0");
> @@ -177,14 +181,6 @@ static int gpd_pocket_fan_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static int gpd_pocket_fan_remove(struct platform_device *pdev)
> -{
> -	struct gpd_pocket_fan_data *fan = platform_get_drvdata(pdev);
> -
> -	cancel_delayed_work_sync(&fan->work);
> -	return 0;
> -}
> -
>  #ifdef CONFIG_PM_SLEEP
>  static int gpd_pocket_fan_suspend(struct device *dev)
>  {
> @@ -215,7 +211,6 @@ MODULE_DEVICE_TABLE(acpi, gpd_pocket_fan_acpi_match);
>  
>  static struct platform_driver gpd_pocket_fan_driver = {
>  	.probe	= gpd_pocket_fan_probe,
> -	.remove	= gpd_pocket_fan_remove,
>  	.driver	= {
>  		.name			= "gpd_pocket_fan",
>  		.acpi_match_table	= gpd_pocket_fan_acpi_match,
>