diff mbox series

Input: elan_i2c - convert to use dev_groups

Message ID 20220729135816.2306989-1-gregkh@linuxfoundation.org
State Superseded
Headers show
Series Input: elan_i2c - convert to use dev_groups | expand

Commit Message

Greg KH July 29, 2022, 1:58 p.m. UTC
The driver core supports the ability to handle the creation and removal
of device-specific sysfs files in a race-free manner.  Take advantage of
that by converting this driver to use this by moving the sysfs
attributes into a group and assigning the dev_groups pointer to it.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "jingle.wu" <jingle.wu@emc.com.tw>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/mouse/elan_i2c_core.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Comments

Dmitry Torokhov July 29, 2022, 6:43 p.m. UTC | #1
Hi Greg,

On Fri, Jul 29, 2022 at 03:58:16PM +0200, Greg Kroah-Hartman wrote:
> The driver core supports the ability to handle the creation and removal
> of device-specific sysfs files in a race-free manner.  Take advantage of
> that by converting this driver to use this by moving the sysfs
> attributes into a group and assigning the dev_groups pointer to it.

I obviously like this patch as the less boilerplate code the better, but
I am not sure what race are you talking about in the commit message?
Could you please elaborate?

Thanks!

> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "jingle.wu" <jingle.wu@emc.com.tw>
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/input/mouse/elan_i2c_core.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index e1758d5ffe42..d4eb59b55bf1 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -1311,12 +1311,6 @@ static int elan_probe(struct i2c_client *client,
>  		return error;
>  	}
>  
> -	error = devm_device_add_groups(dev, elan_sysfs_groups);
> -	if (error) {
> -		dev_err(dev, "failed to create sysfs attributes: %d\n", error);
> -		return error;
> -	}
> -
>  	error = input_register_device(data->input);
>  	if (error) {
>  		dev_err(dev, "failed to register input device: %d\n", error);
> @@ -1442,6 +1436,7 @@ static struct i2c_driver elan_driver = {
>  		.acpi_match_table = ACPI_PTR(elan_acpi_id),
>  		.of_match_table = of_match_ptr(elan_of_match),
>  		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +		.dev_groups = elan_sysfs_groups,
>  	},
>  	.probe		= elan_probe,
>  	.id_table	= elan_id,
> -- 
> 2.37.1
>
Greg KH July 30, 2022, 7:37 a.m. UTC | #2
On Fri, Jul 29, 2022 at 11:43:03AM -0700, Dmitry Torokhov wrote:
> Hi Greg,
> 
> On Fri, Jul 29, 2022 at 03:58:16PM +0200, Greg Kroah-Hartman wrote:
> > The driver core supports the ability to handle the creation and removal
> > of device-specific sysfs files in a race-free manner.  Take advantage of
> > that by converting this driver to use this by moving the sysfs
> > attributes into a group and assigning the dev_groups pointer to it.
> 
> I obviously like this patch as the less boilerplate code the better, but
> I am not sure what race are you talking about in the commit message?
> Could you please elaborate?

The race is normally because the device is announced to userspace before
the attributes are added.  But you are right, the race is still the same
here, the attributes are added after probe succeeds, so it's the same
time, so this changelog text is wrong, sorry about that.

I'll rewrite this to just say it's cleaner and simpler with this change
and resend it in a few days.

thanks,

greg k-h
Greg KH Aug. 2, 2022, 4:48 p.m. UTC | #3
On Fri, Jul 29, 2022 at 03:58:16PM +0200, Greg Kroah-Hartman wrote:
> The driver core supports the ability to handle the creation and removal
> of device-specific sysfs files in a race-free manner.  Take advantage of
> that by converting this driver to use this by moving the sysfs
> attributes into a group and assigning the dev_groups pointer to it.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "jingle.wu" <jingle.wu@emc.com.tw>
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/input/mouse/elan_i2c_core.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)

v2 is now here:
	https://lore.kernel.org/r/20220802162854.3015369-1-gregkh@linuxfoundation.org
diff mbox series

Patch

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index e1758d5ffe42..d4eb59b55bf1 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -1311,12 +1311,6 @@  static int elan_probe(struct i2c_client *client,
 		return error;
 	}
 
-	error = devm_device_add_groups(dev, elan_sysfs_groups);
-	if (error) {
-		dev_err(dev, "failed to create sysfs attributes: %d\n", error);
-		return error;
-	}
-
 	error = input_register_device(data->input);
 	if (error) {
 		dev_err(dev, "failed to register input device: %d\n", error);
@@ -1442,6 +1436,7 @@  static struct i2c_driver elan_driver = {
 		.acpi_match_table = ACPI_PTR(elan_acpi_id),
 		.of_match_table = of_match_ptr(elan_of_match),
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.dev_groups = elan_sysfs_groups,
 	},
 	.probe		= elan_probe,
 	.id_table	= elan_id,