| Message ID | 20220825092048.136973-1-shreeya.patel@collabora.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: light: tsl2583: Fix module unloading | expand |
Hi, On 8/25/22 12:20, Shreeya Patel wrote: > tsl2583 uses devm_iio_device_register() function and > calling iio_device_unregister() in remove breaks the > module unloading. > Fix this by removing call to iio_device_unregister() > from tsl2583_remove(). > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> Stable and fixes tags are missing > --- > drivers/iio/light/tsl2583.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/iio/light/tsl2583.c b/drivers/iio/light/tsl2583.c > index 82662dab87c0..36c25f79e6a6 100644 > --- a/drivers/iio/light/tsl2583.c > +++ b/drivers/iio/light/tsl2583.c > @@ -878,8 +878,6 @@ static int tsl2583_remove(struct i2c_client *client) > struct iio_dev *indio_dev = i2c_get_clientdata(client); > struct tsl2583_chip *chip = iio_priv(indio_dev); > > - iio_device_unregister(indio_dev); > - > pm_runtime_disable(&client->dev); > pm_runtime_set_suspended(&client->dev); > Driver removal sequence should be opposite to the registration order. Could be better not to use the devm in this case.
On Thu, 25 Aug 2022 21:53:09 +0300 Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > Hi, > > On 8/25/22 12:20, Shreeya Patel wrote: > > tsl2583 uses devm_iio_device_register() function and > > calling iio_device_unregister() in remove breaks the > > module unloading. > > Fix this by removing call to iio_device_unregister() > > from tsl2583_remove(). > > > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> > > Stable and fixes tags are missing Stable tags often added by maintainer when applying the patch rather than at submission: can take into account how they want to manage stable backports - some maintainers prefer to run a bunch of tests before asking for patches to be added to stable. I don't do that, but I don't always tag fixes for stable if I think there is some risk associated. Obviously this one is fine for stable material though and I would have tagged v2 whilst applying ;) > > > --- > > drivers/iio/light/tsl2583.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/iio/light/tsl2583.c b/drivers/iio/light/tsl2583.c > > index 82662dab87c0..36c25f79e6a6 100644 > > --- a/drivers/iio/light/tsl2583.c > > +++ b/drivers/iio/light/tsl2583.c > > @@ -878,8 +878,6 @@ static int tsl2583_remove(struct i2c_client *client) > > struct iio_dev *indio_dev = i2c_get_clientdata(client); > > struct tsl2583_chip *chip = iio_priv(indio_dev); > > > > - iio_device_unregister(indio_dev); > > - > > pm_runtime_disable(&client->dev); > > pm_runtime_set_suspended(&client->dev); > > > > Driver removal sequence should be opposite to the registration order. > Could be better not to use the devm in this case Agreed. Simplest fix is indeed to do what v2 does. Nicer tidy up after that is perhaps to switch to devm_pm_runtime_enable() which tidies up nicely on that and register a separate callback with devm_add_action_or_reset() to deal with the power down. However, I'm struggling to follow how the device is powered up in the first place (particularly if we disable runtime pm to make the flow simpler). I think the driver may only "work" today by merit of letting it autosuspend then resuming. Jonathan >
diff --git a/drivers/iio/light/tsl2583.c b/drivers/iio/light/tsl2583.c index 82662dab87c0..36c25f79e6a6 100644 --- a/drivers/iio/light/tsl2583.c +++ b/drivers/iio/light/tsl2583.c @@ -878,8 +878,6 @@ static int tsl2583_remove(struct i2c_client *client) struct iio_dev *indio_dev = i2c_get_clientdata(client); struct tsl2583_chip *chip = iio_priv(indio_dev); - iio_device_unregister(indio_dev); - pm_runtime_disable(&client->dev); pm_runtime_set_suspended(&client->dev);
tsl2583 uses devm_iio_device_register() function and calling iio_device_unregister() in remove breaks the module unloading. Fix this by removing call to iio_device_unregister() from tsl2583_remove(). Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> --- drivers/iio/light/tsl2583.c | 2 -- 1 file changed, 2 deletions(-)