| Message ID | 20210324125548.45983-9-aardelean@deviqon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | platform/x86: toshiba_acpi: move acpi add/remove to device-managed routines | expand |
On Wed, 24 Mar 2021 14:55:46 +0200 Alexandru Ardelean <aardelean@deviqon.com> wrote: > This change converts the wwan_rfkill object to be free'd automatically when > the parent refcount goes to zero. > There are 2 cleanup operations required: rfkill_unregister() and > rfkill_destroy(). Since they don't have any devm_ variants, they are hooked > via devm_add_action_or_reset(). > > The main reason to do this is to enforce ordering on cleanup, when the > Toshiba ACPI device is cleaned up. > > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com> Superficially looks fine to me though I don't know much about rfkill. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/platform/x86/toshiba_acpi.c | 40 ++++++++++++++++++++++------- > 1 file changed, 31 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c > index 12860ef60e4d..a1249f6dde9a 100644 > --- a/drivers/platform/x86/toshiba_acpi.c > +++ b/drivers/platform/x86/toshiba_acpi.c > @@ -2591,7 +2591,22 @@ static const struct rfkill_ops wwan_rfk_ops = { > .poll = toshiba_acpi_wwan_poll, > }; > > -static int toshiba_acpi_setup_wwan_rfkill(struct toshiba_acpi_dev *dev) > +static void toshiba_acpi_rfkill_destroy(void *data) > +{ > + struct rfkill *wwan_rfk = data; > + > + rfkill_destroy(wwan_rfk); > +} > + > +static void toshiba_acpi_rfkill_unreg(void *data) > +{ > + struct rfkill *wwan_rfk = data; > + > + rfkill_unregister(wwan_rfk); > +} > + > +static int toshiba_acpi_setup_wwan_rfkill(struct device *parent, > + struct toshiba_acpi_dev *dev) > { > int ret = toshiba_wireless_status(dev); > > @@ -2608,15 +2623,27 @@ static int toshiba_acpi_setup_wwan_rfkill(struct toshiba_acpi_dev *dev) > return -ENOMEM; > } > > + ret = devm_add_action_or_reset(parent, toshiba_acpi_rfkill_destroy, > + dev->wwan_rfk); > + if (ret) > + return ret; > + > rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch); > > ret = rfkill_register(dev->wwan_rfk); > if (ret) { > pr_err("Unable to register WWAN rfkill device\n"); > - rfkill_destroy(dev->wwan_rfk); > + return ret; > } > > - return ret; > + ret = devm_add_action_or_reset(parent, toshiba_acpi_rfkill_unreg, > + dev->wwan_rfk); > + if (ret) { > + dev->wwan_rfk = NULL; > + return ret; > + } > + > + return 0; > } > > /* > @@ -2996,11 +3023,6 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev) > sysfs_remove_group(&dev->acpi_dev->dev.kobj, > &toshiba_attr_group); > > - if (dev->wwan_rfk) { > - rfkill_unregister(dev->wwan_rfk); > - rfkill_destroy(dev->wwan_rfk); > - } > - > return 0; > } > > @@ -3189,7 +3211,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev) > > toshiba_wwan_available(dev); > if (dev->wwan_supported) > - toshiba_acpi_setup_wwan_rfkill(dev); > + toshiba_acpi_setup_wwan_rfkill(parent, dev); > > toshiba_cooling_method_available(dev); >
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c index 12860ef60e4d..a1249f6dde9a 100644 --- a/drivers/platform/x86/toshiba_acpi.c +++ b/drivers/platform/x86/toshiba_acpi.c @@ -2591,7 +2591,22 @@ static const struct rfkill_ops wwan_rfk_ops = { .poll = toshiba_acpi_wwan_poll, }; -static int toshiba_acpi_setup_wwan_rfkill(struct toshiba_acpi_dev *dev) +static void toshiba_acpi_rfkill_destroy(void *data) +{ + struct rfkill *wwan_rfk = data; + + rfkill_destroy(wwan_rfk); +} + +static void toshiba_acpi_rfkill_unreg(void *data) +{ + struct rfkill *wwan_rfk = data; + + rfkill_unregister(wwan_rfk); +} + +static int toshiba_acpi_setup_wwan_rfkill(struct device *parent, + struct toshiba_acpi_dev *dev) { int ret = toshiba_wireless_status(dev); @@ -2608,15 +2623,27 @@ static int toshiba_acpi_setup_wwan_rfkill(struct toshiba_acpi_dev *dev) return -ENOMEM; } + ret = devm_add_action_or_reset(parent, toshiba_acpi_rfkill_destroy, + dev->wwan_rfk); + if (ret) + return ret; + rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch); ret = rfkill_register(dev->wwan_rfk); if (ret) { pr_err("Unable to register WWAN rfkill device\n"); - rfkill_destroy(dev->wwan_rfk); + return ret; } - return ret; + ret = devm_add_action_or_reset(parent, toshiba_acpi_rfkill_unreg, + dev->wwan_rfk); + if (ret) { + dev->wwan_rfk = NULL; + return ret; + } + + return 0; } /* @@ -2996,11 +3023,6 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev) sysfs_remove_group(&dev->acpi_dev->dev.kobj, &toshiba_attr_group); - if (dev->wwan_rfk) { - rfkill_unregister(dev->wwan_rfk); - rfkill_destroy(dev->wwan_rfk); - } - return 0; } @@ -3189,7 +3211,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev) toshiba_wwan_available(dev); if (dev->wwan_supported) - toshiba_acpi_setup_wwan_rfkill(dev); + toshiba_acpi_setup_wwan_rfkill(parent, dev); toshiba_cooling_method_available(dev);
This change converts the wwan_rfkill object to be free'd automatically when the parent refcount goes to zero. There are 2 cleanup operations required: rfkill_unregister() and rfkill_destroy(). Since they don't have any devm_ variants, they are hooked via devm_add_action_or_reset(). The main reason to do this is to enforce ordering on cleanup, when the Toshiba ACPI device is cleaned up. Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com> --- drivers/platform/x86/toshiba_acpi.c | 40 ++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 9 deletions(-)