| Message ID | 20210523162315.1965869-6-jic23@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: accel: mma9551/mma9553 Cleanup and update | expand |
On Sun, 23 May 2021 at 19:24, Jonathan Cameron <jic23@kernel.org> wrote: > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > The error handling in here left runtime pm enabled, and didn't do the > same steps as occurred in remove. Moving over to fully devm_ managed > makes it harder to get this stuff wrong, so let's do that. > Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/iio/accel/mma9553.c | 70 ++++++++++++++++++++----------------- > 1 file changed, 37 insertions(+), 33 deletions(-) > > diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c > index dc2a3316c1a3..14a9d5fedc06 100644 > --- a/drivers/iio/accel/mma9553.c > +++ b/drivers/iio/accel/mma9553.c > @@ -375,6 +375,15 @@ static int mma9553_conf_gpio(struct mma9553_data *data) > return 0; > } > > +static void mma9553_disable_cb(void *_data) > +{ > + struct mma9553_data *data = _data; > + > + mutex_lock(&data->mutex); > + mma9551_set_device_state(data->client, false); > + mutex_unlock(&data->mutex); > +} > + > static int mma9553_init(struct mma9553_data *data) > { > int ret; > @@ -430,7 +439,11 @@ static int mma9553_init(struct mma9553_data *data) > return ret; > } > > - return mma9551_set_device_state(data->client, true); > + ret = mma9551_set_device_state(data->client, true); > + if (ret) > + return ret; > + > + return devm_add_action_or_reset(&data->client->dev, mma9553_disable_cb, data); > } > > static int mma9553_read_status_word(struct mma9553_data *data, u16 reg, > @@ -1062,6 +1075,16 @@ static irqreturn_t mma9553_event_handler(int irq, void *private) > return IRQ_HANDLED; > } > > +static void mma9553_rpm_set_susp(void *d) > +{ > + pm_runtime_set_suspended(d); > +} > + > +static void mma9553_rpm_disable(void *d) > +{ > + pm_runtime_disable(d); > +} > + > static int mma9553_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -1105,48 +1128,30 @@ static int mma9553_probe(struct i2c_client *client, > if (ret < 0) { > dev_err(&client->dev, "request irq %d failed\n", > client->irq); > - goto out_poweroff; > + return ret; > } > } > > ret = pm_runtime_set_active(&client->dev); > if (ret < 0) > - goto out_poweroff; > + return ret; > + > + ret = devm_add_action_or_reset(&client->dev, mma9553_rpm_set_susp, > + &client->dev); > + if (ret) > + return ret; > > pm_runtime_enable(&client->dev); > + ret = devm_add_action_or_reset(&client->dev, mma9553_rpm_disable, > + &client->dev); > + if (ret) > + return ret; > + > pm_runtime_set_autosuspend_delay(&client->dev, > MMA9551_AUTO_SUSPEND_DELAY_MS); > pm_runtime_use_autosuspend(&client->dev); > > - ret = iio_device_register(indio_dev); > - if (ret < 0) { > - dev_err(&client->dev, "unable to register iio device\n"); > - goto out_poweroff; > - } > - > - dev_dbg(&indio_dev->dev, "Registered device %s\n", name); > - return 0; > - > -out_poweroff: > - mma9551_set_device_state(client, false); > - return ret; > -} > - > -static int mma9553_remove(struct i2c_client *client) > -{ > - struct iio_dev *indio_dev = i2c_get_clientdata(client); > - struct mma9553_data *data = iio_priv(indio_dev); > - > - iio_device_unregister(indio_dev); > - > - pm_runtime_disable(&client->dev); > - pm_runtime_set_suspended(&client->dev); > - > - mutex_lock(&data->mutex); > - mma9551_set_device_state(data->client, false); > - mutex_unlock(&data->mutex); > - > - return 0; > + return devm_iio_device_register(&client->dev, indio_dev); > } > > static __maybe_unused int mma9553_runtime_suspend(struct device *dev) > @@ -1200,7 +1205,6 @@ static struct i2c_driver mma9553_driver = { > .pm = &mma9553_pm_ops, > }, > .probe = mma9553_probe, > - .remove = mma9553_remove, > .id_table = mma9553_id, > }; > > -- > 2.31.1 >
diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c index dc2a3316c1a3..14a9d5fedc06 100644 --- a/drivers/iio/accel/mma9553.c +++ b/drivers/iio/accel/mma9553.c @@ -375,6 +375,15 @@ static int mma9553_conf_gpio(struct mma9553_data *data) return 0; } +static void mma9553_disable_cb(void *_data) +{ + struct mma9553_data *data = _data; + + mutex_lock(&data->mutex); + mma9551_set_device_state(data->client, false); + mutex_unlock(&data->mutex); +} + static int mma9553_init(struct mma9553_data *data) { int ret; @@ -430,7 +439,11 @@ static int mma9553_init(struct mma9553_data *data) return ret; } - return mma9551_set_device_state(data->client, true); + ret = mma9551_set_device_state(data->client, true); + if (ret) + return ret; + + return devm_add_action_or_reset(&data->client->dev, mma9553_disable_cb, data); } static int mma9553_read_status_word(struct mma9553_data *data, u16 reg, @@ -1062,6 +1075,16 @@ static irqreturn_t mma9553_event_handler(int irq, void *private) return IRQ_HANDLED; } +static void mma9553_rpm_set_susp(void *d) +{ + pm_runtime_set_suspended(d); +} + +static void mma9553_rpm_disable(void *d) +{ + pm_runtime_disable(d); +} + static int mma9553_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -1105,48 +1128,30 @@ static int mma9553_probe(struct i2c_client *client, if (ret < 0) { dev_err(&client->dev, "request irq %d failed\n", client->irq); - goto out_poweroff; + return ret; } } ret = pm_runtime_set_active(&client->dev); if (ret < 0) - goto out_poweroff; + return ret; + + ret = devm_add_action_or_reset(&client->dev, mma9553_rpm_set_susp, + &client->dev); + if (ret) + return ret; pm_runtime_enable(&client->dev); + ret = devm_add_action_or_reset(&client->dev, mma9553_rpm_disable, + &client->dev); + if (ret) + return ret; + pm_runtime_set_autosuspend_delay(&client->dev, MMA9551_AUTO_SUSPEND_DELAY_MS); pm_runtime_use_autosuspend(&client->dev); - ret = iio_device_register(indio_dev); - if (ret < 0) { - dev_err(&client->dev, "unable to register iio device\n"); - goto out_poweroff; - } - - dev_dbg(&indio_dev->dev, "Registered device %s\n", name); - return 0; - -out_poweroff: - mma9551_set_device_state(client, false); - return ret; -} - -static int mma9553_remove(struct i2c_client *client) -{ - struct iio_dev *indio_dev = i2c_get_clientdata(client); - struct mma9553_data *data = iio_priv(indio_dev); - - iio_device_unregister(indio_dev); - - pm_runtime_disable(&client->dev); - pm_runtime_set_suspended(&client->dev); - - mutex_lock(&data->mutex); - mma9551_set_device_state(data->client, false); - mutex_unlock(&data->mutex); - - return 0; + return devm_iio_device_register(&client->dev, indio_dev); } static __maybe_unused int mma9553_runtime_suspend(struct device *dev) @@ -1200,7 +1205,6 @@ static struct i2c_driver mma9553_driver = { .pm = &mma9553_pm_ops, }, .probe = mma9553_probe, - .remove = mma9553_remove, .id_table = mma9553_id, };