On Sat, 15 Feb 2020 20:42:59 +0100 David Heidelberg <david@xxxxxxx> wrote: > Thank you for fixes, it does look good. > Only thing is fixes which should go into commit "iio: light: al3320a > implement suspend support"[1] when into "iio: light: al3320a implement > devm_add_action_or_reset" [2]. > > + return i2c_smbus_write_byte_data(client, AL3320A_REG_CONFIG, val); > > and > > +static SIMPLE_DEV_PM_OPS(al3320a_pm_ops, al3320a_suspend, > al3320a_resume); > > > Thank you very much for merging and next time > I'll spend more time with review and doing final build test. Gah. I messed that up. Anyhow, now fixed and pushed out with changes in the right patch. Thanks, Jonathan > > David > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=d74856400b2dcb2e0eab2132c779408809566431 > [2] > https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=4b1b42bcd993258628fabc5e35f20a202b1e9f7b > > > On Sat, Feb 15, 2020 at 18:38, Jonathan Cameron <jic23@xxxxxxxxxx> > wrote: > > On Tue, 11 Feb 2020 20:12:00 +0100 > > David Heidelberg <david@xxxxxxx> wrote: > > > >> Use devm_add_action_or_reset to automatically disable the device > >> and allow you to get rid of the remove function entirely. > >> > >> Signed-off-by: David Heidelberg <david@xxxxxxx> > > > > This doesn't build as is. I've fixed up but please take a close > > look at the result. > > > >> --- > >> drivers/iio/light/al3320a.c | 20 ++++++++++++++------ > >> 1 file changed, 14 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/iio/light/al3320a.c > >> b/drivers/iio/light/al3320a.c > >> index affa4c6c199a..49e73e24fff6 100644 > >> --- a/drivers/iio/light/al3320a.c > >> +++ b/drivers/iio/light/al3320a.c > >> @@ -87,6 +87,13 @@ static int al3320a_set_pwr(struct i2c_client > >> *client, bool pwr) > >> return i2c_smbus_write_byte_data(data->client, > >> AL3320A_REG_CONFIG, val); > >> } > >> > >> +static void al3320a_set_pwr_off(void *_data) > >> +{ > >> + struct al3320a_data *data = _data; > >> + > >> + al3320a_set_pwr(data->client, false); > >> +} > >> + > >> static int al3320a_init(struct al3320a_data *data) > >> { > >> int ret; > >> @@ -206,12 +213,14 @@ static int al3320a_probe(struct i2c_client > >> *client, > >> dev_err(&client->dev, "al3320a chip init failed\n"); > >> return ret; > >> } > >> - return devm_iio_device_register(&client->dev, indio_dev); > >> -} > >> > >> -static int al3320a_remove(struct i2c_client *client) > >> -{ > >> - return al3320a_set_pwr(client, false); > >> + ret = devm_add_action_or_reset(&client->dev, > >> + al3320a_set_pwr_off, > >> + data); > >> + if (ret < 0) > >> + return ret; > >> + > >> + return devm_iio_device_register(&client->dev, indio_dev); > >> } > >> > >> static int __maybe_unused al3320a_suspend(struct device *dev) > >> @@ -238,7 +247,6 @@ static struct i2c_driver al3320a_driver = { > >> .pm = &al3320a_pm_ops, > >> }, > >> .probe = al3320a_probe, > >> - .remove = al3320a_remove, > >> .id_table = al3320a_id, > >> }; > >> > > > >
