All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] platform/x86: wmi: Make remove callback return void
@ 2021-03-01 16:04 Uwe Kleine-König
  2021-03-04 14:14 ` Hans de Goede
  2021-03-05 15:04 ` Pali Rohár
  0 siblings, 2 replies; 3+ messages in thread
From: Uwe Kleine-König @ 2021-03-01 16:04 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross
  Cc: Mario Limonciello, Divya Bharathi, Prasanth Ksr, Matthew Garrett,
	Pali Rohár, Jithu Joseph, Maurice Ma, platform-driver-x86,
	kernel

The driver core ignores the return value of struct bus_type::remove()
(and so wmi_dev_remove()) because there is only little that can be done.

To simplify the quest to make this function return void, let struct
wmi_driver::remove() return void, too. All implementers of this callback
return 0 already and this way it should be obvious to driver authors
that returning an error code is a bad idea.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/platform/x86/dell/dell-smbios-wmi.c                  | 3 +--
 drivers/platform/x86/dell/dell-wmi-descriptor.c              | 3 +--
 .../platform/x86/dell/dell-wmi-sysman/biosattr-interface.c   | 3 +--
 .../x86/dell/dell-wmi-sysman/passwordattr-interface.c        | 3 +--
 drivers/platform/x86/dell/dell-wmi.c                         | 3 +--
 drivers/platform/x86/intel-wmi-sbl-fw-update.c               | 3 +--
 drivers/platform/x86/intel-wmi-thunderbolt.c                 | 3 +--
 drivers/platform/x86/wmi-bmof.c                              | 3 +--
 drivers/platform/x86/wmi.c                                   | 5 ++---
 include/linux/wmi.h                                          | 2 +-
 10 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
index 27a298b7c541..a1753485159c 100644
--- a/drivers/platform/x86/dell/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
@@ -205,7 +205,7 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
 	return ret;
 }
 
-static int dell_smbios_wmi_remove(struct wmi_device *wdev)
+static void dell_smbios_wmi_remove(struct wmi_device *wdev)
 {
 	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
 	int count;
@@ -218,7 +218,6 @@ static int dell_smbios_wmi_remove(struct wmi_device *wdev)
 	count = get_order(priv->req_buf_size);
 	free_pages((unsigned long)priv->buf, count);
 	mutex_unlock(&call_mutex);
-	return 0;
 }
 
 static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c b/drivers/platform/x86/dell/dell-wmi-descriptor.c
index a068900ae8a1..3c4af7c08bb1 100644
--- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
+++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
@@ -174,14 +174,13 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
 	return ret;
 }
 
-static int dell_wmi_descriptor_remove(struct wmi_device *wdev)
+static void dell_wmi_descriptor_remove(struct wmi_device *wdev)
 {
 	struct descriptor_priv *priv = dev_get_drvdata(&wdev->dev);
 
 	mutex_lock(&list_mutex);
 	list_del(&priv->list);
 	mutex_unlock(&list_mutex);
-	return 0;
 }
 
 static const struct wmi_device_id dell_wmi_descriptor_id_table[] = {
diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c b/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c
index f95d8ddace5a..c2dd2de6bc20 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c
@@ -152,12 +152,11 @@ static int bios_attr_set_interface_probe(struct wmi_device *wdev, const void *co
 	return 0;
 }
 
-static int bios_attr_set_interface_remove(struct wmi_device *wdev)
+static void bios_attr_set_interface_remove(struct wmi_device *wdev)
 {
 	mutex_lock(&wmi_priv.mutex);
 	wmi_priv.bios_attr_wdev = NULL;
 	mutex_unlock(&wmi_priv.mutex);
-	return 0;
 }
 
 static const struct wmi_device_id bios_attr_set_interface_id_table[] = {
diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c b/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c
index 5780b4d94759..339a082d6c18 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c
@@ -119,12 +119,11 @@ static int bios_attr_pass_interface_probe(struct wmi_device *wdev, const void *c
 	return 0;
 }
 
-static int bios_attr_pass_interface_remove(struct wmi_device *wdev)
+static void bios_attr_pass_interface_remove(struct wmi_device *wdev)
 {
 	mutex_lock(&wmi_priv.mutex);
 	wmi_priv.password_attr_wdev = NULL;
 	mutex_unlock(&wmi_priv.mutex);
-	return 0;
 }
 
 static const struct wmi_device_id bios_attr_pass_interface_id_table[] = {
diff --git a/drivers/platform/x86/dell/dell-wmi.c b/drivers/platform/x86/dell/dell-wmi.c
index bbdb3e860892..5e1b7f897df5 100644
--- a/drivers/platform/x86/dell/dell-wmi.c
+++ b/drivers/platform/x86/dell/dell-wmi.c
@@ -714,10 +714,9 @@ static int dell_wmi_probe(struct wmi_device *wdev, const void *context)
 	return dell_wmi_input_setup(wdev);
 }
 
-static int dell_wmi_remove(struct wmi_device *wdev)
+static void dell_wmi_remove(struct wmi_device *wdev)
 {
 	dell_wmi_input_destroy(wdev);
-	return 0;
 }
 static const struct wmi_device_id dell_wmi_id_table[] = {
 	{ .guid_string = DELL_EVENT_GUID },
diff --git a/drivers/platform/x86/intel-wmi-sbl-fw-update.c b/drivers/platform/x86/intel-wmi-sbl-fw-update.c
index ea87fa0786e8..3c86e0108a24 100644
--- a/drivers/platform/x86/intel-wmi-sbl-fw-update.c
+++ b/drivers/platform/x86/intel-wmi-sbl-fw-update.c
@@ -117,10 +117,9 @@ static int intel_wmi_sbl_fw_update_probe(struct wmi_device *wdev,
 	return 0;
 }
 
-static int intel_wmi_sbl_fw_update_remove(struct wmi_device *wdev)
+static void intel_wmi_sbl_fw_update_remove(struct wmi_device *wdev)
 {
 	dev_info(&wdev->dev, "Slim Bootloader signaling driver removed\n");
-	return 0;
 }
 
 static const struct wmi_device_id intel_wmi_sbl_id_table[] = {
diff --git a/drivers/platform/x86/intel-wmi-thunderbolt.c b/drivers/platform/x86/intel-wmi-thunderbolt.c
index 974c22a7ff61..4ae87060d18b 100644
--- a/drivers/platform/x86/intel-wmi-thunderbolt.c
+++ b/drivers/platform/x86/intel-wmi-thunderbolt.c
@@ -66,11 +66,10 @@ static int intel_wmi_thunderbolt_probe(struct wmi_device *wdev,
 	return ret;
 }
 
-static int intel_wmi_thunderbolt_remove(struct wmi_device *wdev)
+static void intel_wmi_thunderbolt_remove(struct wmi_device *wdev)
 {
 	sysfs_remove_group(&wdev->dev.kobj, &tbt_attribute_group);
 	kobject_uevent(&wdev->dev.kobj, KOBJ_CHANGE);
-	return 0;
 }
 
 static const struct wmi_device_id intel_wmi_thunderbolt_id_table[] = {
diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
index 66b434d6307f..80137afb9753 100644
--- a/drivers/platform/x86/wmi-bmof.c
+++ b/drivers/platform/x86/wmi-bmof.c
@@ -86,13 +86,12 @@ static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
 	return ret;
 }
 
-static int wmi_bmof_remove(struct wmi_device *wdev)
+static void wmi_bmof_remove(struct wmi_device *wdev)
 {
 	struct bmof_priv *priv = dev_get_drvdata(&wdev->dev);
 
 	sysfs_remove_bin_file(&wdev->dev.kobj, &priv->bmof_bin_attr);
 	kfree(priv->bmofdata);
-	return 0;
 }
 
 static const struct wmi_device_id wmi_bmof_id_table[] = {
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index c669676ea8e8..aa9bd2ee7390 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -986,7 +986,6 @@ static int wmi_dev_remove(struct device *dev)
 	struct wmi_block *wblock = dev_to_wblock(dev);
 	struct wmi_driver *wdriver =
 		container_of(dev->driver, struct wmi_driver, driver);
-	int ret = 0;
 
 	if (wdriver->filter_callback) {
 		misc_deregister(&wblock->char_dev);
@@ -995,12 +994,12 @@ static int wmi_dev_remove(struct device *dev)
 	}
 
 	if (wdriver->remove)
-		ret = wdriver->remove(dev_to_wdev(dev));
+		wdriver->remove(dev_to_wdev(dev));
 
 	if (ACPI_FAILURE(wmi_method_enable(wblock, 0)))
 		dev_warn(dev, "failed to disable device\n");
 
-	return ret;
+	return 0;
 }
 
 static struct class wmi_bus_class = {
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index 8ef7e7faea1e..2cb3913c1f50 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -37,7 +37,7 @@ struct wmi_driver {
 	const struct wmi_device_id *id_table;
 
 	int (*probe)(struct wmi_device *wdev, const void *context);
-	int (*remove)(struct wmi_device *wdev);
+	void (*remove)(struct wmi_device *wdev);
 	void (*notify)(struct wmi_device *device, union acpi_object *data);
 	long (*filter_callback)(struct wmi_device *wdev, unsigned int cmd,
 				struct wmi_ioctl_buffer *arg);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] platform/x86: wmi: Make remove callback return void
  2021-03-01 16:04 [PATCH] platform/x86: wmi: Make remove callback return void Uwe Kleine-König
@ 2021-03-04 14:14 ` Hans de Goede
  2021-03-05 15:04 ` Pali Rohár
  1 sibling, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2021-03-04 14:14 UTC (permalink / raw)
  To: Uwe Kleine-König, Mark Gross
  Cc: Mario Limonciello, Divya Bharathi, Prasanth Ksr, Matthew Garrett,
	Pali Rohár, Jithu Joseph, Maurice Ma, platform-driver-x86,
	kernel

Hi,

On 3/1/21 5:04 PM, Uwe Kleine-König wrote:
> The driver core ignores the return value of struct bus_type::remove()
> (and so wmi_dev_remove()) because there is only little that can be done.
> 
> To simplify the quest to make this function return void, let struct
> wmi_driver::remove() return void, too. All implementers of this callback
> return 0 already and this way it should be obvious to driver authors
> that returning an error code is a bad idea.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans


> ---
>  drivers/platform/x86/dell/dell-smbios-wmi.c                  | 3 +--
>  drivers/platform/x86/dell/dell-wmi-descriptor.c              | 3 +--
>  .../platform/x86/dell/dell-wmi-sysman/biosattr-interface.c   | 3 +--
>  .../x86/dell/dell-wmi-sysman/passwordattr-interface.c        | 3 +--
>  drivers/platform/x86/dell/dell-wmi.c                         | 3 +--
>  drivers/platform/x86/intel-wmi-sbl-fw-update.c               | 3 +--
>  drivers/platform/x86/intel-wmi-thunderbolt.c                 | 3 +--
>  drivers/platform/x86/wmi-bmof.c                              | 3 +--
>  drivers/platform/x86/wmi.c                                   | 5 ++---
>  include/linux/wmi.h                                          | 2 +-
>  10 files changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
> index 27a298b7c541..a1753485159c 100644
> --- a/drivers/platform/x86/dell/dell-smbios-wmi.c
> +++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
> @@ -205,7 +205,7 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
>  	return ret;
>  }
>  
> -static int dell_smbios_wmi_remove(struct wmi_device *wdev)
> +static void dell_smbios_wmi_remove(struct wmi_device *wdev)
>  {
>  	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
>  	int count;
> @@ -218,7 +218,6 @@ static int dell_smbios_wmi_remove(struct wmi_device *wdev)
>  	count = get_order(priv->req_buf_size);
>  	free_pages((unsigned long)priv->buf, count);
>  	mutex_unlock(&call_mutex);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
> diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> index a068900ae8a1..3c4af7c08bb1 100644
> --- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
> +++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> @@ -174,14 +174,13 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>  	return ret;
>  }
>  
> -static int dell_wmi_descriptor_remove(struct wmi_device *wdev)
> +static void dell_wmi_descriptor_remove(struct wmi_device *wdev)
>  {
>  	struct descriptor_priv *priv = dev_get_drvdata(&wdev->dev);
>  
>  	mutex_lock(&list_mutex);
>  	list_del(&priv->list);
>  	mutex_unlock(&list_mutex);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id dell_wmi_descriptor_id_table[] = {
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c b/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c
> index f95d8ddace5a..c2dd2de6bc20 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c
> @@ -152,12 +152,11 @@ static int bios_attr_set_interface_probe(struct wmi_device *wdev, const void *co
>  	return 0;
>  }
>  
> -static int bios_attr_set_interface_remove(struct wmi_device *wdev)
> +static void bios_attr_set_interface_remove(struct wmi_device *wdev)
>  {
>  	mutex_lock(&wmi_priv.mutex);
>  	wmi_priv.bios_attr_wdev = NULL;
>  	mutex_unlock(&wmi_priv.mutex);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id bios_attr_set_interface_id_table[] = {
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c b/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c
> index 5780b4d94759..339a082d6c18 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c
> @@ -119,12 +119,11 @@ static int bios_attr_pass_interface_probe(struct wmi_device *wdev, const void *c
>  	return 0;
>  }
>  
> -static int bios_attr_pass_interface_remove(struct wmi_device *wdev)
> +static void bios_attr_pass_interface_remove(struct wmi_device *wdev)
>  {
>  	mutex_lock(&wmi_priv.mutex);
>  	wmi_priv.password_attr_wdev = NULL;
>  	mutex_unlock(&wmi_priv.mutex);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id bios_attr_pass_interface_id_table[] = {
> diff --git a/drivers/platform/x86/dell/dell-wmi.c b/drivers/platform/x86/dell/dell-wmi.c
> index bbdb3e860892..5e1b7f897df5 100644
> --- a/drivers/platform/x86/dell/dell-wmi.c
> +++ b/drivers/platform/x86/dell/dell-wmi.c
> @@ -714,10 +714,9 @@ static int dell_wmi_probe(struct wmi_device *wdev, const void *context)
>  	return dell_wmi_input_setup(wdev);
>  }
>  
> -static int dell_wmi_remove(struct wmi_device *wdev)
> +static void dell_wmi_remove(struct wmi_device *wdev)
>  {
>  	dell_wmi_input_destroy(wdev);
> -	return 0;
>  }
>  static const struct wmi_device_id dell_wmi_id_table[] = {
>  	{ .guid_string = DELL_EVENT_GUID },
> diff --git a/drivers/platform/x86/intel-wmi-sbl-fw-update.c b/drivers/platform/x86/intel-wmi-sbl-fw-update.c
> index ea87fa0786e8..3c86e0108a24 100644
> --- a/drivers/platform/x86/intel-wmi-sbl-fw-update.c
> +++ b/drivers/platform/x86/intel-wmi-sbl-fw-update.c
> @@ -117,10 +117,9 @@ static int intel_wmi_sbl_fw_update_probe(struct wmi_device *wdev,
>  	return 0;
>  }
>  
> -static int intel_wmi_sbl_fw_update_remove(struct wmi_device *wdev)
> +static void intel_wmi_sbl_fw_update_remove(struct wmi_device *wdev)
>  {
>  	dev_info(&wdev->dev, "Slim Bootloader signaling driver removed\n");
> -	return 0;
>  }
>  
>  static const struct wmi_device_id intel_wmi_sbl_id_table[] = {
> diff --git a/drivers/platform/x86/intel-wmi-thunderbolt.c b/drivers/platform/x86/intel-wmi-thunderbolt.c
> index 974c22a7ff61..4ae87060d18b 100644
> --- a/drivers/platform/x86/intel-wmi-thunderbolt.c
> +++ b/drivers/platform/x86/intel-wmi-thunderbolt.c
> @@ -66,11 +66,10 @@ static int intel_wmi_thunderbolt_probe(struct wmi_device *wdev,
>  	return ret;
>  }
>  
> -static int intel_wmi_thunderbolt_remove(struct wmi_device *wdev)
> +static void intel_wmi_thunderbolt_remove(struct wmi_device *wdev)
>  {
>  	sysfs_remove_group(&wdev->dev.kobj, &tbt_attribute_group);
>  	kobject_uevent(&wdev->dev.kobj, KOBJ_CHANGE);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id intel_wmi_thunderbolt_id_table[] = {
> diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
> index 66b434d6307f..80137afb9753 100644
> --- a/drivers/platform/x86/wmi-bmof.c
> +++ b/drivers/platform/x86/wmi-bmof.c
> @@ -86,13 +86,12 @@ static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
>  	return ret;
>  }
>  
> -static int wmi_bmof_remove(struct wmi_device *wdev)
> +static void wmi_bmof_remove(struct wmi_device *wdev)
>  {
>  	struct bmof_priv *priv = dev_get_drvdata(&wdev->dev);
>  
>  	sysfs_remove_bin_file(&wdev->dev.kobj, &priv->bmof_bin_attr);
>  	kfree(priv->bmofdata);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id wmi_bmof_id_table[] = {
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index c669676ea8e8..aa9bd2ee7390 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -986,7 +986,6 @@ static int wmi_dev_remove(struct device *dev)
>  	struct wmi_block *wblock = dev_to_wblock(dev);
>  	struct wmi_driver *wdriver =
>  		container_of(dev->driver, struct wmi_driver, driver);
> -	int ret = 0;
>  
>  	if (wdriver->filter_callback) {
>  		misc_deregister(&wblock->char_dev);
> @@ -995,12 +994,12 @@ static int wmi_dev_remove(struct device *dev)
>  	}
>  
>  	if (wdriver->remove)
> -		ret = wdriver->remove(dev_to_wdev(dev));
> +		wdriver->remove(dev_to_wdev(dev));
>  
>  	if (ACPI_FAILURE(wmi_method_enable(wblock, 0)))
>  		dev_warn(dev, "failed to disable device\n");
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static struct class wmi_bus_class = {
> diff --git a/include/linux/wmi.h b/include/linux/wmi.h
> index 8ef7e7faea1e..2cb3913c1f50 100644
> --- a/include/linux/wmi.h
> +++ b/include/linux/wmi.h
> @@ -37,7 +37,7 @@ struct wmi_driver {
>  	const struct wmi_device_id *id_table;
>  
>  	int (*probe)(struct wmi_device *wdev, const void *context);
> -	int (*remove)(struct wmi_device *wdev);
> +	void (*remove)(struct wmi_device *wdev);
>  	void (*notify)(struct wmi_device *device, union acpi_object *data);
>  	long (*filter_callback)(struct wmi_device *wdev, unsigned int cmd,
>  				struct wmi_ioctl_buffer *arg);
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] platform/x86: wmi: Make remove callback return void
  2021-03-01 16:04 [PATCH] platform/x86: wmi: Make remove callback return void Uwe Kleine-König
  2021-03-04 14:14 ` Hans de Goede
@ 2021-03-05 15:04 ` Pali Rohár
  1 sibling, 0 replies; 3+ messages in thread
From: Pali Rohár @ 2021-03-05 15:04 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Hans de Goede, Mark Gross, Mario Limonciello, Divya Bharathi,
	Prasanth Ksr, Matthew Garrett, Jithu Joseph, Maurice Ma,
	platform-driver-x86, kernel

On Monday 01 March 2021 17:04:04 Uwe Kleine-König wrote:
> The driver core ignores the return value of struct bus_type::remove()
> (and so wmi_dev_remove()) because there is only little that can be done.
> 
> To simplify the quest to make this function return void, let struct
> wmi_driver::remove() return void, too. All implementers of this callback
> return 0 already and this way it should be obvious to driver authors
> that returning an error code is a bad idea.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Looks good,

Reviewed-by: Pali Rohár <pali@kernel.org>

> ---
>  drivers/platform/x86/dell/dell-smbios-wmi.c                  | 3 +--
>  drivers/platform/x86/dell/dell-wmi-descriptor.c              | 3 +--
>  .../platform/x86/dell/dell-wmi-sysman/biosattr-interface.c   | 3 +--
>  .../x86/dell/dell-wmi-sysman/passwordattr-interface.c        | 3 +--
>  drivers/platform/x86/dell/dell-wmi.c                         | 3 +--
>  drivers/platform/x86/intel-wmi-sbl-fw-update.c               | 3 +--
>  drivers/platform/x86/intel-wmi-thunderbolt.c                 | 3 +--
>  drivers/platform/x86/wmi-bmof.c                              | 3 +--
>  drivers/platform/x86/wmi.c                                   | 5 ++---
>  include/linux/wmi.h                                          | 2 +-
>  10 files changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
> index 27a298b7c541..a1753485159c 100644
> --- a/drivers/platform/x86/dell/dell-smbios-wmi.c
> +++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
> @@ -205,7 +205,7 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
>  	return ret;
>  }
>  
> -static int dell_smbios_wmi_remove(struct wmi_device *wdev)
> +static void dell_smbios_wmi_remove(struct wmi_device *wdev)
>  {
>  	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
>  	int count;
> @@ -218,7 +218,6 @@ static int dell_smbios_wmi_remove(struct wmi_device *wdev)
>  	count = get_order(priv->req_buf_size);
>  	free_pages((unsigned long)priv->buf, count);
>  	mutex_unlock(&call_mutex);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
> diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> index a068900ae8a1..3c4af7c08bb1 100644
> --- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
> +++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
> @@ -174,14 +174,13 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>  	return ret;
>  }
>  
> -static int dell_wmi_descriptor_remove(struct wmi_device *wdev)
> +static void dell_wmi_descriptor_remove(struct wmi_device *wdev)
>  {
>  	struct descriptor_priv *priv = dev_get_drvdata(&wdev->dev);
>  
>  	mutex_lock(&list_mutex);
>  	list_del(&priv->list);
>  	mutex_unlock(&list_mutex);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id dell_wmi_descriptor_id_table[] = {
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c b/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c
> index f95d8ddace5a..c2dd2de6bc20 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c
> @@ -152,12 +152,11 @@ static int bios_attr_set_interface_probe(struct wmi_device *wdev, const void *co
>  	return 0;
>  }
>  
> -static int bios_attr_set_interface_remove(struct wmi_device *wdev)
> +static void bios_attr_set_interface_remove(struct wmi_device *wdev)
>  {
>  	mutex_lock(&wmi_priv.mutex);
>  	wmi_priv.bios_attr_wdev = NULL;
>  	mutex_unlock(&wmi_priv.mutex);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id bios_attr_set_interface_id_table[] = {
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c b/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c
> index 5780b4d94759..339a082d6c18 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/passwordattr-interface.c
> @@ -119,12 +119,11 @@ static int bios_attr_pass_interface_probe(struct wmi_device *wdev, const void *c
>  	return 0;
>  }
>  
> -static int bios_attr_pass_interface_remove(struct wmi_device *wdev)
> +static void bios_attr_pass_interface_remove(struct wmi_device *wdev)
>  {
>  	mutex_lock(&wmi_priv.mutex);
>  	wmi_priv.password_attr_wdev = NULL;
>  	mutex_unlock(&wmi_priv.mutex);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id bios_attr_pass_interface_id_table[] = {
> diff --git a/drivers/platform/x86/dell/dell-wmi.c b/drivers/platform/x86/dell/dell-wmi.c
> index bbdb3e860892..5e1b7f897df5 100644
> --- a/drivers/platform/x86/dell/dell-wmi.c
> +++ b/drivers/platform/x86/dell/dell-wmi.c
> @@ -714,10 +714,9 @@ static int dell_wmi_probe(struct wmi_device *wdev, const void *context)
>  	return dell_wmi_input_setup(wdev);
>  }
>  
> -static int dell_wmi_remove(struct wmi_device *wdev)
> +static void dell_wmi_remove(struct wmi_device *wdev)
>  {
>  	dell_wmi_input_destroy(wdev);
> -	return 0;
>  }
>  static const struct wmi_device_id dell_wmi_id_table[] = {
>  	{ .guid_string = DELL_EVENT_GUID },
> diff --git a/drivers/platform/x86/intel-wmi-sbl-fw-update.c b/drivers/platform/x86/intel-wmi-sbl-fw-update.c
> index ea87fa0786e8..3c86e0108a24 100644
> --- a/drivers/platform/x86/intel-wmi-sbl-fw-update.c
> +++ b/drivers/platform/x86/intel-wmi-sbl-fw-update.c
> @@ -117,10 +117,9 @@ static int intel_wmi_sbl_fw_update_probe(struct wmi_device *wdev,
>  	return 0;
>  }
>  
> -static int intel_wmi_sbl_fw_update_remove(struct wmi_device *wdev)
> +static void intel_wmi_sbl_fw_update_remove(struct wmi_device *wdev)
>  {
>  	dev_info(&wdev->dev, "Slim Bootloader signaling driver removed\n");
> -	return 0;
>  }
>  
>  static const struct wmi_device_id intel_wmi_sbl_id_table[] = {
> diff --git a/drivers/platform/x86/intel-wmi-thunderbolt.c b/drivers/platform/x86/intel-wmi-thunderbolt.c
> index 974c22a7ff61..4ae87060d18b 100644
> --- a/drivers/platform/x86/intel-wmi-thunderbolt.c
> +++ b/drivers/platform/x86/intel-wmi-thunderbolt.c
> @@ -66,11 +66,10 @@ static int intel_wmi_thunderbolt_probe(struct wmi_device *wdev,
>  	return ret;
>  }
>  
> -static int intel_wmi_thunderbolt_remove(struct wmi_device *wdev)
> +static void intel_wmi_thunderbolt_remove(struct wmi_device *wdev)
>  {
>  	sysfs_remove_group(&wdev->dev.kobj, &tbt_attribute_group);
>  	kobject_uevent(&wdev->dev.kobj, KOBJ_CHANGE);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id intel_wmi_thunderbolt_id_table[] = {
> diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
> index 66b434d6307f..80137afb9753 100644
> --- a/drivers/platform/x86/wmi-bmof.c
> +++ b/drivers/platform/x86/wmi-bmof.c
> @@ -86,13 +86,12 @@ static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
>  	return ret;
>  }
>  
> -static int wmi_bmof_remove(struct wmi_device *wdev)
> +static void wmi_bmof_remove(struct wmi_device *wdev)
>  {
>  	struct bmof_priv *priv = dev_get_drvdata(&wdev->dev);
>  
>  	sysfs_remove_bin_file(&wdev->dev.kobj, &priv->bmof_bin_attr);
>  	kfree(priv->bmofdata);
> -	return 0;
>  }
>  
>  static const struct wmi_device_id wmi_bmof_id_table[] = {
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index c669676ea8e8..aa9bd2ee7390 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -986,7 +986,6 @@ static int wmi_dev_remove(struct device *dev)
>  	struct wmi_block *wblock = dev_to_wblock(dev);
>  	struct wmi_driver *wdriver =
>  		container_of(dev->driver, struct wmi_driver, driver);
> -	int ret = 0;
>  
>  	if (wdriver->filter_callback) {
>  		misc_deregister(&wblock->char_dev);
> @@ -995,12 +994,12 @@ static int wmi_dev_remove(struct device *dev)
>  	}
>  
>  	if (wdriver->remove)
> -		ret = wdriver->remove(dev_to_wdev(dev));
> +		wdriver->remove(dev_to_wdev(dev));
>  
>  	if (ACPI_FAILURE(wmi_method_enable(wblock, 0)))
>  		dev_warn(dev, "failed to disable device\n");
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static struct class wmi_bus_class = {
> diff --git a/include/linux/wmi.h b/include/linux/wmi.h
> index 8ef7e7faea1e..2cb3913c1f50 100644
> --- a/include/linux/wmi.h
> +++ b/include/linux/wmi.h
> @@ -37,7 +37,7 @@ struct wmi_driver {
>  	const struct wmi_device_id *id_table;
>  
>  	int (*probe)(struct wmi_device *wdev, const void *context);
> -	int (*remove)(struct wmi_device *wdev);
> +	void (*remove)(struct wmi_device *wdev);
>  	void (*notify)(struct wmi_device *device, union acpi_object *data);
>  	long (*filter_callback)(struct wmi_device *wdev, unsigned int cmd,
>  				struct wmi_ioctl_buffer *arg);
> -- 
> 2.30.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-05 15:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01 16:04 [PATCH] platform/x86: wmi: Make remove callback return void Uwe Kleine-König
2021-03-04 14:14 ` Hans de Goede
2021-03-05 15:04 ` Pali Rohár

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.