| Message ID | 20200723230344.1422750-4-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Some sx9310 iio driver updates | expand |
Hi, On Thu, Jul 23, 2020 at 4:03 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Enable the digital IO power supply (svdd) during probe so that the i2c > communication works properly on boards that aggressively power gate this > supply. > > Cc: Gwendal Grignou <gwendal@chromium.org> > Cc: Daniel Campello <campello@chromium.org> > Cc: Hartmut Knaack <knaack.h@gmx.de> > Cc: Lars-Peter Clausen <lars@metafoo.de> > Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net> > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > drivers/iio/proximity/sx9310.c | 49 ++++++++++++++++++++++++++++------ > 1 file changed, 41 insertions(+), 8 deletions(-) > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c > index 84c3c9ae80dc..d21c17a4d541 100644 > --- a/drivers/iio/proximity/sx9310.c > +++ b/drivers/iio/proximity/sx9310.c > @@ -19,6 +19,7 @@ > #include <linux/of.h> > #include <linux/pm.h> > #include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > #include <linux/slab.h> > > #include <linux/iio/buffer.h> > @@ -131,6 +132,7 @@ struct sx9310_data { > struct i2c_client *client; > struct iio_trigger *trig; > struct regmap *regmap; > + struct regulator *supply; Done need to store if you use devm. See below. > /* > * Last reading of the proximity status for each channel. > * We only send an event to user space when this changes. > @@ -914,21 +916,31 @@ static int sx9310_probe(struct i2c_client *client, > mutex_init(&data->mutex); > init_completion(&data->completion); > > + data->supply = devm_regulator_get(&client->dev, "svdd"); > + if (IS_ERR(data->supply)) > + return PTR_ERR(data->supply); > + > data->regmap = devm_regmap_init_i2c(client, &sx9310_regmap_config); > if (IS_ERR(data->regmap)) > return PTR_ERR(data->regmap); > > + ret = regulator_enable(data->supply); > + if (ret) > + return ret; > + /* Must wait for Tpor time after initial power up */ > + usleep_range(1000, 1100); ret = devm_add_action_or_reset(&client->dev, sx9310_regulator_disable, supply); if (ret) return ret; static void sx9310_regulator_disable(void *regulator) { regulator_disable(regulator); } Then drop all changes below this line. Seems like you could add support for the other regulator listed in the bindings, too? -Doug
Quoting Doug Anderson (2020-07-24 13:25:01) > On Thu, Jul 23, 2020 at 4:03 PM Stephen Boyd <swboyd@chromium.org> wrote: > > /* > > * Last reading of the proximity status for each channel. > > * We only send an event to user space when this changes. > > @@ -914,21 +916,31 @@ static int sx9310_probe(struct i2c_client *client, > > mutex_init(&data->mutex); > > init_completion(&data->completion); > > > > + data->supply = devm_regulator_get(&client->dev, "svdd"); > > + if (IS_ERR(data->supply)) > > + return PTR_ERR(data->supply); > > + > > data->regmap = devm_regmap_init_i2c(client, &sx9310_regmap_config); > > if (IS_ERR(data->regmap)) > > return PTR_ERR(data->regmap); > > > > + ret = regulator_enable(data->supply); > > + if (ret) > > + return ret; > > + /* Must wait for Tpor time after initial power up */ > > + usleep_range(1000, 1100); > > ret = devm_add_action_or_reset(&client->dev, sx9310_regulator_disable, supply); > if (ret) > return ret; > > static void sx9310_regulator_disable(void *regulator) { > regulator_disable(regulator); > } > > Then drop all changes below this line. Ok. I originally had code to turn off the supply during suspend/resume but I removed it because I wasn't sure that the device would be OK with the IO interface dropping after the device went to sleep. > > > Seems like you could add support for the other regulator listed in the > bindings, too? > Sure. I will use the bulk APIs.
diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c index 84c3c9ae80dc..d21c17a4d541 100644 --- a/drivers/iio/proximity/sx9310.c +++ b/drivers/iio/proximity/sx9310.c @@ -19,6 +19,7 @@ #include <linux/of.h> #include <linux/pm.h> #include <linux/regmap.h> +#include <linux/regulator/consumer.h> #include <linux/slab.h> #include <linux/iio/buffer.h> @@ -131,6 +132,7 @@ struct sx9310_data { struct i2c_client *client; struct iio_trigger *trig; struct regmap *regmap; + struct regulator *supply; /* * Last reading of the proximity status for each channel. * We only send an event to user space when this changes. @@ -914,21 +916,31 @@ static int sx9310_probe(struct i2c_client *client, mutex_init(&data->mutex); init_completion(&data->completion); + data->supply = devm_regulator_get(&client->dev, "svdd"); + if (IS_ERR(data->supply)) + return PTR_ERR(data->supply); + data->regmap = devm_regmap_init_i2c(client, &sx9310_regmap_config); if (IS_ERR(data->regmap)) return PTR_ERR(data->regmap); + ret = regulator_enable(data->supply); + if (ret) + return ret; + /* Must wait for Tpor time after initial power up */ + usleep_range(1000, 1100); + ret = regmap_read(data->regmap, SX9310_REG_WHOAMI, &data->whoami); if (ret < 0) { dev_err(&client->dev, "error in reading WHOAMI register: %d\n", ret); - return ret; + goto err; } ret = sx9310_set_indio_dev_name(&client->dev, indio_dev, id, data->whoami); if (ret < 0) - return ret; + goto err; ACPI_COMPANION_SET(&indio_dev->dev, ACPI_COMPANION(&client->dev)); indio_dev->dev.parent = &client->dev; @@ -940,7 +952,7 @@ static int sx9310_probe(struct i2c_client *client, ret = sx9310_init_device(indio_dev); if (ret < 0) - return ret; + goto err; if (client->irq) { ret = devm_request_threaded_irq(&client->dev, client->irq, @@ -949,13 +961,15 @@ static int sx9310_probe(struct i2c_client *client, IRQF_TRIGGER_LOW | IRQF_ONESHOT, "sx9310_event", indio_dev); if (ret < 0) - return ret; + goto err; data->trig = devm_iio_trigger_alloc(&client->dev, "%s-dev%d", indio_dev->name, indio_dev->id); - if (!data->trig) - return -ENOMEM; + if (!data->trig) { + ret = -ENOMEM; + goto err; + } data->trig->dev.parent = &client->dev; data->trig->ops = &sx9310_trigger_ops; @@ -963,7 +977,7 @@ static int sx9310_probe(struct i2c_client *client, ret = devm_iio_trigger_register(&client->dev, data->trig); if (ret) - return ret; + goto err; } ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, @@ -971,9 +985,27 @@ static int sx9310_probe(struct i2c_client *client, sx9310_trigger_handler, &sx9310_buffer_setup_ops); if (ret < 0) - return ret; + goto err; return devm_iio_device_register(&client->dev, indio_dev); +err: + regulator_disable(data->supply); + return ret; +} + +static int sx9310_remove(struct i2c_client *client) +{ + struct iio_dev *indio_dev; + struct sx9310_data *data; + + indio_dev = i2c_get_clientdata(client); + if (!indio_dev) + return -ENODEV; + + data = iio_priv(indio_dev); + regulator_disable(data->supply); + + return 0; } static int __maybe_unused sx9310_suspend(struct device *dev) @@ -1059,6 +1091,7 @@ static struct i2c_driver sx9310_driver = { .pm = &sx9310_pm_ops, }, .probe = sx9310_probe, + .remove = sx9310_remove, .id_table = sx9310_id, }; module_i2c_driver(sx9310_driver);
Enable the digital IO power supply (svdd) during probe so that the i2c communication works properly on boards that aggressively power gate this supply. Cc: Gwendal Grignou <gwendal@chromium.org> Cc: Daniel Campello <campello@chromium.org> Cc: Hartmut Knaack <knaack.h@gmx.de> Cc: Lars-Peter Clausen <lars@metafoo.de> Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net> Cc: Douglas Anderson <dianders@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/iio/proximity/sx9310.c | 49 ++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 8 deletions(-)