Message ID | 20200724213329.899216-6-swboyd@chromium.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Some sx9310 iio driver updates | expand |
Hi, On Fri, Jul 24, 2020 at 2:33 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Enable the main power supply (vdd) and digital IO power supply (svdd) > during probe so that the i2c communication and device works properly on > boards that aggressively power gate these supplies. > > 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 | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c > index 1e1f6bba50f6..ad6ed100c7a6 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> > @@ -899,12 +900,21 @@ static int sx9310_set_indio_dev_name(struct device *dev, > return 0; > } > > +static void sx9310_regulator_disable(void *supplies) > +{ > + regulator_bulk_disable(2, supplies); > +} > + > static int sx9310_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > int ret; > struct iio_dev *indio_dev; > struct sx9310_data *data; > + struct regulator_bulk_data supplies[] = { > + { .supply = "vdd" }, > + { .supply = "svdd" }, > + }; > > indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > if (indio_dev == NULL) > @@ -919,6 +929,23 @@ static int sx9310_probe(struct i2c_client *client, > if (IS_ERR(data->regmap)) > return PTR_ERR(data->regmap); > > + ret = devm_regulator_bulk_get(&client->dev, ARRAY_SIZE(supplies), supplies); > + if (ret) > + return ret; > + > + ret = regulator_bulk_enable(ARRAY_SIZE(supplies), supplies); > + if (ret) > + return ret; > + /* Must wait for Tpor time after initial power up */ > + usleep_range(1000, 1100); > + > + /* Update sx9310_regulator_disable() size if this bug is hit */ > + BUILD_BUG_ON(ARRAY_SIZE(supplies) != 2); > + ret = devm_add_action_or_reset(&client->dev, sx9310_regulator_disable, > + supplies); ...but, but... Aren't you storing a pointer to stack memory? How does that work? I think you either need to store the "struct regulator_bulk_data" in your private data or just make two normal regulator calls. -Doug
Quoting Doug Anderson (2020-07-24 15:02:23) > On Fri, Jul 24, 2020 at 2:33 PM Stephen Boyd <swboyd@chromium.org> wrote: > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c > > index 1e1f6bba50f6..ad6ed100c7a6 100644 > > --- a/drivers/iio/proximity/sx9310.c > > +++ b/drivers/iio/proximity/sx9310.c > > @@ -919,6 +929,23 @@ static int sx9310_probe(struct i2c_client *client, > > if (IS_ERR(data->regmap)) > > return PTR_ERR(data->regmap); > > > > + ret = devm_regulator_bulk_get(&client->dev, ARRAY_SIZE(supplies), supplies); > > + if (ret) > > + return ret; > > + > > + ret = regulator_bulk_enable(ARRAY_SIZE(supplies), supplies); > > + if (ret) > > + return ret; > > + /* Must wait for Tpor time after initial power up */ > > + usleep_range(1000, 1100); > > + > > + /* Update sx9310_regulator_disable() size if this bug is hit */ > > + BUILD_BUG_ON(ARRAY_SIZE(supplies) != 2); > > + ret = devm_add_action_or_reset(&client->dev, sx9310_regulator_disable, > > + supplies); > > ...but, but... Aren't you storing a pointer to stack memory? How > does that work? I think you either need to store the "struct > regulator_bulk_data" in your private data or just make two normal > regulator calls. > Doh, no coffee today.
diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c index 1e1f6bba50f6..ad6ed100c7a6 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> @@ -899,12 +900,21 @@ static int sx9310_set_indio_dev_name(struct device *dev, return 0; } +static void sx9310_regulator_disable(void *supplies) +{ + regulator_bulk_disable(2, supplies); +} + static int sx9310_probe(struct i2c_client *client, const struct i2c_device_id *id) { int ret; struct iio_dev *indio_dev; struct sx9310_data *data; + struct regulator_bulk_data supplies[] = { + { .supply = "vdd" }, + { .supply = "svdd" }, + }; indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); if (indio_dev == NULL) @@ -919,6 +929,23 @@ static int sx9310_probe(struct i2c_client *client, if (IS_ERR(data->regmap)) return PTR_ERR(data->regmap); + ret = devm_regulator_bulk_get(&client->dev, ARRAY_SIZE(supplies), supplies); + if (ret) + return ret; + + ret = regulator_bulk_enable(ARRAY_SIZE(supplies), supplies); + if (ret) + return ret; + /* Must wait for Tpor time after initial power up */ + usleep_range(1000, 1100); + + /* Update sx9310_regulator_disable() size if this bug is hit */ + BUILD_BUG_ON(ARRAY_SIZE(supplies) != 2); + ret = devm_add_action_or_reset(&client->dev, sx9310_regulator_disable, + supplies); + if (ret) + return ret; + ret = regmap_read(data->regmap, SX9310_REG_WHOAMI, &data->whoami); if (ret < 0) { dev_err(&client->dev, "error in reading WHOAMI register: %d\n",
Enable the main power supply (vdd) and digital IO power supply (svdd) during probe so that the i2c communication and device works properly on boards that aggressively power gate these supplies. 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 | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)