* [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() @ 2022-04-09 3:48 Zheyu Ma 2022-04-09 3:48 ` [PATCH] iio: magnetometer: ak8975: Fix the error handling in ak8975_power_on() Zheyu Ma 2022-04-10 15:54 ` [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() Jonathan Cameron 0 siblings, 2 replies; 7+ messages in thread From: Zheyu Ma @ 2022-04-09 3:48 UTC (permalink / raw) To: jic23, lars; +Cc: linux-iio, linux-kernel, Zheyu Ma 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(+) 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] iio: magnetometer: ak8975: Fix the error handling in ak8975_power_on() 2022-04-09 3:48 [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() Zheyu Ma @ 2022-04-09 3:48 ` Zheyu Ma 2022-04-10 16:01 ` Jonathan Cameron 2022-04-10 15:54 ` [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() Jonathan Cameron 1 sibling, 1 reply; 7+ messages in thread From: Zheyu Ma @ 2022-04-09 3:48 UTC (permalink / raw) To: jic23, lars; +Cc: linux-iio, linux-kernel, Zheyu Ma 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 <zheyuma97@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: magnetometer: ak8975: Fix the error handling in ak8975_power_on() 2022-04-09 3:48 ` [PATCH] iio: magnetometer: ak8975: Fix the error handling in ak8975_power_on() Zheyu Ma @ 2022-04-10 16:01 ` Jonathan Cameron 2022-04-11 5:57 ` Zheyu Ma 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2022-04-10 16:01 UTC (permalink / raw) To: Zheyu Ma; +Cc: lars, linux-iio, linux-kernel, linus.walleij On Sat, 9 Apr 2022 11:48:49 +0800 Zheyu Ma <zheyuma97@gmail.com> wrote: > 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 <zheyuma97@gmail.com> This driver doesn't really have a maintainer any more, though Linus W actually last touched this bit of code. So +CC. However, it's pretty obviously correct so applied to the fixes-togreg branch of iio.git. For future reference, please don't send one fix as reply to a fix on a different driver. Makes a mess of thread handling and tracking in patchwork etc. Ideally also please provide a fixes tag. For this one I have applied with out it because it was a long time back and looks like the bug predates a bunch of refactoring of this code. Applied to the fixes-togreg branch of iio.git and marked for stable. Jonathan > --- > 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; > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: magnetometer: ak8975: Fix the error handling in ak8975_power_on() 2022-04-10 16:01 ` Jonathan Cameron @ 2022-04-11 5:57 ` Zheyu Ma 0 siblings, 0 replies; 7+ messages in thread From: Zheyu Ma @ 2022-04-11 5:57 UTC (permalink / raw) To: Jonathan Cameron Cc: lars, linux-iio, Linux Kernel Mailing List, linus.walleij On Sun, Apr 10, 2022 at 11:53 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sat, 9 Apr 2022 11:48:49 +0800 > Zheyu Ma <zheyuma97@gmail.com> wrote: > > > 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 <zheyuma97@gmail.com> > This driver doesn't really have a maintainer any more, though Linus W actually > last touched this bit of code. So +CC. > > However, it's pretty obviously correct so applied to the fixes-togreg branch of iio.git. > > For future reference, please don't send one fix as reply to a fix on a different driver. > Makes a mess of thread handling and tracking in patchwork etc. Sorry for the confusion, I will take care of it next time. > Ideally also please provide a fixes tag. For this one I have applied with out it > because it was a long time back and looks like the bug predates a bunch of refactoring > of this code. Thanks for your reminder, I will add the fixes tag in the next submission. Regards, Zheyu Ma ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() 2022-04-09 3:48 [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() Zheyu Ma 2022-04-09 3:48 ` [PATCH] iio: magnetometer: ak8975: Fix the error handling in ak8975_power_on() Zheyu Ma @ 2022-04-10 15:54 ` Jonathan Cameron 2022-04-20 23:14 ` Linus Walleij 1 sibling, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2022-04-10 15:54 UTC (permalink / raw) To: Zheyu Ma; +Cc: lars, linux-iio, linux-kernel, Linus Walleij 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); > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() 2022-04-10 15:54 ` [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() Jonathan Cameron @ 2022-04-20 23:14 ` Linus Walleij 2022-04-21 14:07 ` Ulf Hansson 0 siblings, 1 reply; 7+ messages in thread From: Linus Walleij @ 2022-04-20 23:14 UTC (permalink / raw) To: Jonathan Cameron; +Cc: Zheyu Ma, lars, linux-iio, linux-kernel, Ulf Hansson 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() 2022-04-20 23:14 ` Linus Walleij @ 2022-04-21 14:07 ` Ulf Hansson 0 siblings, 0 replies; 7+ messages in thread From: Ulf Hansson @ 2022-04-21 14:07 UTC (permalink / raw) To: Linus Walleij; +Cc: Jonathan Cameron, Zheyu Ma, lars, linux-iio, linux-kernel 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-04-21 14:08 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-09 3:48 [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() Zheyu Ma 2022-04-09 3:48 ` [PATCH] iio: magnetometer: ak8975: Fix the error handling in ak8975_power_on() Zheyu Ma 2022-04-10 16:01 ` Jonathan Cameron 2022-04-11 5:57 ` Zheyu Ma 2022-04-10 15:54 ` [PATCH] iio: magnetometer: ak8974: Fix the error handling of ak8974_probe() Jonathan Cameron 2022-04-20 23:14 ` Linus Walleij 2022-04-21 14:07 ` Ulf Hansson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.