| Message ID | 20220409034849.3717231-1-zheyuma97@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() | expand |
On Sat, 9 Apr 2022 11:48:48 +0800 Zheyu Ma <zheyuma97@gmail.com> 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 <zheyuma97@gmail.com> + 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); > } >
On Sun, Apr 10, 2022 at 5:46 PM Jonathan Cameron <jic23@kernel.org> wrote: > On Sat, 9 Apr 2022 11:48:48 +0800 > Zheyu Ma <zheyuma97@gmail.com> 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 <zheyuma97@gmail.com> > + 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 <linus.walleij@linaro.org> Yours, Linus Walleij
On Thu, 21 Apr 2022 at 01:14, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Sun, Apr 10, 2022 at 5:46 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sat, 9 Apr 2022 11:48:48 +0800 > > Zheyu Ma <zheyuma97@gmail.com> 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 <zheyuma97@gmail.com> > > + 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 <linus.walleij@linaro.org> > > Yours, > Linus Walleij Kind regards Uffe
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 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 <zheyuma97@gmail.com> --- drivers/iio/magnetometer/ak8974.c | 1 + 1 file changed, 1 insertion(+)