When the driver fail at devm_regmap_init_i2c(), we will get the
following splat:
[ 106.797388] WARNING: CPU: 4 PID: 413 at drivers/regulator/core.c:2257 _regulator_put+0x3ec/0x4e0
[ 106.802183] RIP: 0010:_regulator_put+0x3ec/0x4e0
[ 106.811237] Call Trace:
[ 106.811515] <TASK>
[ 106.811695] regulator_bulk_free+0x82/0xe0
[ 106.812032] devres_release_group+0x319/0x3d0
[ 106.812425] i2c_device_probe+0x766/0x940
Fix this by disabling the regulators at the error path.
Signed-off-by: Zheyu Ma <[email protected]>
---
drivers/iio/magnetometer/ak8974.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
index e54feacfb980..84bbf7ccc887 100644
--- a/drivers/iio/magnetometer/ak8974.c
+++ b/drivers/iio/magnetometer/ak8974.c
@@ -862,6 +862,7 @@ static int ak8974_probe(struct i2c_client *i2c,
dev_err(&i2c->dev, "failed to allocate register map\n");
pm_runtime_put_noidle(&i2c->dev);
pm_runtime_disable(&i2c->dev);
+ regulator_bulk_disable(ARRAY_SIZE(ak8974->regs), ak8974->regs);
return PTR_ERR(ak8974->map);
}
--
2.25.1
On Sat, 9 Apr 2022 11:48:48 +0800
Zheyu Ma <[email protected]> wrote:
> When the driver fail at devm_regmap_init_i2c(), we will get the
> following splat:
>
> [ 106.797388] WARNING: CPU: 4 PID: 413 at drivers/regulator/core.c:2257 _regulator_put+0x3ec/0x4e0
> [ 106.802183] RIP: 0010:_regulator_put+0x3ec/0x4e0
> [ 106.811237] Call Trace:
> [ 106.811515] <TASK>
> [ 106.811695] regulator_bulk_free+0x82/0xe0
> [ 106.812032] devres_release_group+0x319/0x3d0
> [ 106.812425] i2c_device_probe+0x766/0x940
>
> Fix this by disabling the regulators at the error path.
>
> Signed-off-by: Zheyu Ma <[email protected]>
+ CC Linus W as it's his driver.
Fix looks correct to me, though the handling of runtime pm in here is
probably more complex than it needs to be (and hence this odd error
handling for this one place in the probe).
Jonathan
> ---
> drivers/iio/magnetometer/ak8974.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> index e54feacfb980..84bbf7ccc887 100644
> --- a/drivers/iio/magnetometer/ak8974.c
> +++ b/drivers/iio/magnetometer/ak8974.c
> @@ -862,6 +862,7 @@ static int ak8974_probe(struct i2c_client *i2c,
> dev_err(&i2c->dev, "failed to allocate register map\n");
> pm_runtime_put_noidle(&i2c->dev);
> pm_runtime_disable(&i2c->dev);
> + regulator_bulk_disable(ARRAY_SIZE(ak8974->regs), ak8974->regs);
> return PTR_ERR(ak8974->map);
> }
>
When the driver fails to enable the regulator 'vid', we will get the
following splat:
[ 79.955610] WARNING: CPU: 5 PID: 441 at drivers/regulator/core.c:2257 _regulator_put+0x3ec/0x4e0
[ 79.959641] RIP: 0010:_regulator_put+0x3ec/0x4e0
[ 79.967570] Call Trace:
[ 79.967773] <TASK>
[ 79.967951] regulator_put+0x1f/0x30
[ 79.968254] devres_release_group+0x319/0x3d0
[ 79.968608] i2c_device_probe+0x766/0x940
Fix this by disabling the 'vdd' regulator when failing to enable 'vid'
regulator.
Signed-off-by: Zheyu Ma <[email protected]>
---
drivers/iio/magnetometer/ak8975.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 088f748b683e..2432e697150c 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -416,6 +416,7 @@ static int ak8975_power_on(const struct ak8975_data *data)
if (ret) {
dev_warn(&data->client->dev,
"Failed to enable specified Vid supply\n");
+ regulator_disable(data->vdd);
return ret;
}
--
2.25.1
On Sun, Apr 10, 2022 at 5:46 PM Jonathan Cameron <[email protected]> wrote:
> On Sat, 9 Apr 2022 11:48:48 +0800
> Zheyu Ma <[email protected]> wrote:
>
> > When the driver fail at devm_regmap_init_i2c(), we will get the
> > following splat:
> >
> > [ 106.797388] WARNING: CPU: 4 PID: 413 at drivers/regulator/core.c:2257 _regulator_put+0x3ec/0x4e0
> > [ 106.802183] RIP: 0010:_regulator_put+0x3ec/0x4e0
> > [ 106.811237] Call Trace:
> > [ 106.811515] <TASK>
> > [ 106.811695] regulator_bulk_free+0x82/0xe0
> > [ 106.812032] devres_release_group+0x319/0x3d0
> > [ 106.812425] i2c_device_probe+0x766/0x940
> >
> > Fix this by disabling the regulators at the error path.
> >
> > Signed-off-by: Zheyu Ma <[email protected]>
> + CC Linus W as it's his driver.
>
> Fix looks correct to me, though the handling of runtime pm in here is
> probably more complex than it needs to be (and hence this odd error
> handling for this one place in the probe).
At the time I discussed how to do runtime pm with Ulf Hansson a lot
and I think it was the state of the art at that time. It might have
changed since.
> > ---
> > drivers/iio/magnetometer/ak8974.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> > index e54feacfb980..84bbf7ccc887 100644
> > --- a/drivers/iio/magnetometer/ak8974.c
> > +++ b/drivers/iio/magnetometer/ak8974.c
> > @@ -862,6 +862,7 @@ static int ak8974_probe(struct i2c_client *i2c,
> > dev_err(&i2c->dev, "failed to allocate register map\n");
> > pm_runtime_put_noidle(&i2c->dev);
> > pm_runtime_disable(&i2c->dev);
> > + regulator_bulk_disable(ARRAY_SIZE(ak8974->regs), ak8974->regs);
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Thu, 21 Apr 2022 at 01:14, Linus Walleij <[email protected]> wrote:
>
> On Sun, Apr 10, 2022 at 5:46 PM Jonathan Cameron <[email protected]> wrote:
> > On Sat, 9 Apr 2022 11:48:48 +0800
> > Zheyu Ma <[email protected]> wrote:
> >
> > > When the driver fail at devm_regmap_init_i2c(), we will get the
> > > following splat:
> > >
> > > [ 106.797388] WARNING: CPU: 4 PID: 413 at drivers/regulator/core.c:2257 _regulator_put+0x3ec/0x4e0
> > > [ 106.802183] RIP: 0010:_regulator_put+0x3ec/0x4e0
> > > [ 106.811237] Call Trace:
> > > [ 106.811515] <TASK>
> > > [ 106.811695] regulator_bulk_free+0x82/0xe0
> > > [ 106.812032] devres_release_group+0x319/0x3d0
> > > [ 106.812425] i2c_device_probe+0x766/0x940
> > >
> > > Fix this by disabling the regulators at the error path.
> > >
> > > Signed-off-by: Zheyu Ma <[email protected]>
> > + CC Linus W as it's his driver.
> >
> > Fix looks correct to me, though the handling of runtime pm in here is
> > probably more complex than it needs to be (and hence this odd error
> > handling for this one place in the probe).
>
> At the time I discussed how to do runtime pm with Ulf Hansson a lot
> and I think it was the state of the art at that time. It might have
> changed since.
It still looks good to me. One could switch the order of the calls to
pm_runtime_disable() and pm_runtime_put_noidle(), but it's not a big
thing.
Whether it looks complicated or not, that's a different story. :-)
Note that some drivers are always being built with CONFIG_PM being
set, which allows some simplifications. Although, I don't think that's
the case here, right?
>
> > > ---
> > > drivers/iio/magnetometer/ak8974.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c
> > > index e54feacfb980..84bbf7ccc887 100644
> > > --- a/drivers/iio/magnetometer/ak8974.c
> > > +++ b/drivers/iio/magnetometer/ak8974.c
> > > @@ -862,6 +862,7 @@ static int ak8974_probe(struct i2c_client *i2c,
> > > dev_err(&i2c->dev, "failed to allocate register map\n");
> > > pm_runtime_put_noidle(&i2c->dev);
> > > pm_runtime_disable(&i2c->dev);
> > > + regulator_bulk_disable(ARRAY_SIZE(ak8974->regs), ak8974->regs);
>
> Reviewed-by: Linus Walleij <[email protected]>
>
> Yours,
> Linus Walleij
Kind regards
Uffe