Re: [PATCH 09/10] platform/x86: toshiba_acpi: use device-managed for sysfs removal

From: Jonathan Cameron
Date: Mon Mar 29 2021 - 11:10:08 EST


On Wed, 24 Mar 2021 14:55:47 +0200
Alexandru Ardelean <aardelean@xxxxxxxxxxx> wrote:

> This change moves the creation of the Toshiba ACPI group to be
> automatically removed when the parent refcount goes to zero.
>
> The main reason to do this, is to also enforce that the order of removal is
> mirroring the order of initialization.
>
> Signed-off-by: Alexandru Ardelean <aardelean@xxxxxxxxxxx>
Hmm. The manual handling of the sysfs_create_group() is unfortunate (as opposed
to just setting a groups pointer) but there doesn't seem to be an easy way to fix
that with the current architecture. Ah well

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>

> ---
> drivers/platform/x86/toshiba_acpi.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index a1249f6dde9a..8e8917979047 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -200,7 +200,6 @@ struct toshiba_acpi_dev {
> unsigned int usb_three_supported:1;
> unsigned int wwan_supported:1;
> unsigned int cooling_method_supported:1;
> - unsigned int sysfs_created:1;
> unsigned int special_functions;
>
> bool kbd_event_generated;
> @@ -3019,10 +3018,6 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
>
> remove_toshiba_proc_entries(dev);
>
> - if (dev->sysfs_created)
> - sysfs_remove_group(&dev->acpi_dev->dev.kobj,
> - &toshiba_attr_group);
> -
> return 0;
> }
>
> @@ -3049,6 +3044,13 @@ static void toshiba_acpi_misc_deregister(void *data)
> misc_deregister(miscdev);
> }
>
> +static void toshiba_acpi_sysfs_remove(void *data)
> +{
> + struct kobject *kobj = data;
> +
> + sysfs_remove_group(kobj, &toshiba_attr_group);
> +}
> +
> static int toshiba_acpi_add(struct acpi_device *acpi_dev)
> {
> struct device *parent = &acpi_dev->dev;
> @@ -3219,21 +3221,20 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>
> ret = sysfs_create_group(&dev->acpi_dev->dev.kobj,
> &toshiba_attr_group);
> - if (ret) {
> - dev->sysfs_created = 0;
> - goto error;
> - }
> - dev->sysfs_created = !ret;
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(parent,
> + toshiba_acpi_sysfs_remove,
> + &dev->acpi_dev->dev.kobj);
> + if (ret)
> + return ret;
>
> create_toshiba_proc_entries(dev);
>
> toshiba_acpi = dev;
>
> return 0;
> -
> -error:
> - toshiba_acpi_remove(acpi_dev);
> - return ret;
> }
>
> static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)