* [PATCH] mfd: stmfx: Improve error message triggered by regulator fault in .remove()
@ 2022-02-07 8:17 Uwe Kleine-König
2022-02-14 13:46 ` Lee Jones
0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2022-02-07 8:17 UTC (permalink / raw)
To: Lee Jones, Maxime Coquelin, Alexandre Torgue
Cc: linux-stm32, linux-arm-kernel, linux-kernel, kernel
Returning a non-zero value in an i2c remove callback results in the i2c
core emitting a very generic error message ("remove failed (-ESOMETHING),
will be ignored") and as the message indicates not further error handling
is done.
Instead emit a more specific error message and then return zero in
.remove().
The long-term goal is to make the i2c remove prototype return void, making
all implementations return 0 is preparatory work for this change.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/mfd/stmfx.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
index e095a3930142..16631c675f2f 100644
--- a/drivers/mfd/stmfx.c
+++ b/drivers/mfd/stmfx.c
@@ -392,17 +392,21 @@ static int stmfx_chip_init(struct i2c_client *client)
return ret;
}
-static int stmfx_chip_exit(struct i2c_client *client)
+static void stmfx_chip_exit(struct i2c_client *client)
{
struct stmfx *stmfx = i2c_get_clientdata(client);
regmap_write(stmfx->map, STMFX_REG_IRQ_SRC_EN, 0);
regmap_write(stmfx->map, STMFX_REG_SYS_CTRL, 0);
- if (stmfx->vdd)
- return regulator_disable(stmfx->vdd);
+ if (stmfx->vdd) {
+ int ret = regulator_disable(stmfx->vdd);
- return 0;
+ if (ret)
+ dev_err(&client->dev,
+ "Failed to disable vdd regulator: %pe\n",
+ ERR_PTR(ret));
+ }
}
static int stmfx_probe(struct i2c_client *client,
@@ -466,7 +470,9 @@ static int stmfx_remove(struct i2c_client *client)
{
stmfx_irq_exit(client);
- return stmfx_chip_exit(client);
+ stmfx_chip_exit(client);
+
+ return 0;
}
#ifdef CONFIG_PM_SLEEP
base-commit: dcb85f85fa6f142aae1fe86f399d4503d49f2b60
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mfd: stmfx: Improve error message triggered by regulator fault in .remove()
2022-02-07 8:17 [PATCH] mfd: stmfx: Improve error message triggered by regulator fault in .remove() Uwe Kleine-König
@ 2022-02-14 13:46 ` Lee Jones
2022-02-14 14:06 ` Uwe Kleine-König
0 siblings, 1 reply; 4+ messages in thread
From: Lee Jones @ 2022-02-14 13:46 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Maxime Coquelin, Alexandre Torgue, linux-stm32, linux-arm-kernel,
linux-kernel, kernel
On Mon, 07 Feb 2022, Uwe Kleine-König wrote:
> Returning a non-zero value in an i2c remove callback results in the i2c
> core emitting a very generic error message ("remove failed (-ESOMETHING),
> will be ignored") and as the message indicates not further error handling
> is done.
>
> Instead emit a more specific error message and then return zero in
> .remove().
>
> The long-term goal is to make the i2c remove prototype return void, making
> all implementations return 0 is preparatory work for this change.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> drivers/mfd/stmfx.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
> index e095a3930142..16631c675f2f 100644
> --- a/drivers/mfd/stmfx.c
> +++ b/drivers/mfd/stmfx.c
> @@ -392,17 +392,21 @@ static int stmfx_chip_init(struct i2c_client *client)
> return ret;
> }
>
> -static int stmfx_chip_exit(struct i2c_client *client)
> +static void stmfx_chip_exit(struct i2c_client *client)
> {
> struct stmfx *stmfx = i2c_get_clientdata(client);
>
> regmap_write(stmfx->map, STMFX_REG_IRQ_SRC_EN, 0);
> regmap_write(stmfx->map, STMFX_REG_SYS_CTRL, 0);
>
> - if (stmfx->vdd)
> - return regulator_disable(stmfx->vdd);
> + if (stmfx->vdd) {
> + int ret = regulator_disable(stmfx->vdd);
>
> - return 0;
> + if (ret)
Nit: Premise of the patch is fine, but please can you use the standard
function call, check the return value format please. Something about
this is triggering my OCD! :)
int ret;
ret = regulator_disable(stmfx->vdd);
if (ret)
do_thing();
> + dev_err(&client->dev,
> + "Failed to disable vdd regulator: %pe\n",
> + ERR_PTR(ret));
> + }
> }
>
> static int stmfx_probe(struct i2c_client *client,
> @@ -466,7 +470,9 @@ static int stmfx_remove(struct i2c_client *client)
> {
> stmfx_irq_exit(client);
>
> - return stmfx_chip_exit(client);
> + stmfx_chip_exit(client);
> +
> + return 0;
> }
>
> #ifdef CONFIG_PM_SLEEP
>
> base-commit: dcb85f85fa6f142aae1fe86f399d4503d49f2b60
--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mfd: stmfx: Improve error message triggered by regulator fault in .remove()
2022-02-14 13:46 ` Lee Jones
@ 2022-02-14 14:06 ` Uwe Kleine-König
2022-02-14 14:30 ` Lee Jones
0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2022-02-14 14:06 UTC (permalink / raw)
To: Lee Jones
Cc: Maxime Coquelin, linux-kernel, Alexandre Torgue, kernel,
linux-stm32, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 2332 bytes --]
On Mon, Feb 14, 2022 at 01:46:37PM +0000, Lee Jones wrote:
> On Mon, 07 Feb 2022, Uwe Kleine-König wrote:
>
> > Returning a non-zero value in an i2c remove callback results in the i2c
> > core emitting a very generic error message ("remove failed (-ESOMETHING),
> > will be ignored") and as the message indicates not further error handling
> > is done.
> >
> > Instead emit a more specific error message and then return zero in
> > .remove().
> >
> > The long-term goal is to make the i2c remove prototype return void, making
> > all implementations return 0 is preparatory work for this change.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > drivers/mfd/stmfx.c | 16 +++++++++++-----
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
> > index e095a3930142..16631c675f2f 100644
> > --- a/drivers/mfd/stmfx.c
> > +++ b/drivers/mfd/stmfx.c
> > @@ -392,17 +392,21 @@ static int stmfx_chip_init(struct i2c_client *client)
> > return ret;
> > }
> >
> > -static int stmfx_chip_exit(struct i2c_client *client)
> > +static void stmfx_chip_exit(struct i2c_client *client)
> > {
> > struct stmfx *stmfx = i2c_get_clientdata(client);
> >
> > regmap_write(stmfx->map, STMFX_REG_IRQ_SRC_EN, 0);
> > regmap_write(stmfx->map, STMFX_REG_SYS_CTRL, 0);
> >
> > - if (stmfx->vdd)
> > - return regulator_disable(stmfx->vdd);
> > + if (stmfx->vdd) {
> > + int ret = regulator_disable(stmfx->vdd);
> >
> > - return 0;
> > + if (ret)
>
> Nit: Premise of the patch is fine, but please can you use the standard
> function call, check the return value format please. Something about
> this is triggering my OCD! :)
>
> int ret;
>
> ret = regulator_disable(stmfx->vdd);
> if (ret)
> do_thing();
Not sure I understand you correctly. Do you want just:
regmap_write(stmfx->map, STMFX_REG_SYS_CTRL, 0);
if (stmfx->vdd) {
- int ret = regulator_disable(stmfx->vdd);
+ int ret;
+
+ ret = regulator_disable(stmfx->vdd);
if (ret)
...
squashed into the patch?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mfd: stmfx: Improve error message triggered by regulator fault in .remove()
2022-02-14 14:06 ` Uwe Kleine-König
@ 2022-02-14 14:30 ` Lee Jones
0 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2022-02-14 14:30 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Maxime Coquelin, linux-kernel, Alexandre Torgue, kernel,
linux-stm32, linux-arm-kernel
On Mon, 14 Feb 2022, Uwe Kleine-König wrote:
> On Mon, Feb 14, 2022 at 01:46:37PM +0000, Lee Jones wrote:
> > On Mon, 07 Feb 2022, Uwe Kleine-König wrote:
> >
> > > Returning a non-zero value in an i2c remove callback results in the i2c
> > > core emitting a very generic error message ("remove failed (-ESOMETHING),
> > > will be ignored") and as the message indicates not further error handling
> > > is done.
> > >
> > > Instead emit a more specific error message and then return zero in
> > > .remove().
> > >
> > > The long-term goal is to make the i2c remove prototype return void, making
> > > all implementations return 0 is preparatory work for this change.
> > >
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > drivers/mfd/stmfx.c | 16 +++++++++++-----
> > > 1 file changed, 11 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
> > > index e095a3930142..16631c675f2f 100644
> > > --- a/drivers/mfd/stmfx.c
> > > +++ b/drivers/mfd/stmfx.c
> > > @@ -392,17 +392,21 @@ static int stmfx_chip_init(struct i2c_client *client)
> > > return ret;
> > > }
> > >
> > > -static int stmfx_chip_exit(struct i2c_client *client)
> > > +static void stmfx_chip_exit(struct i2c_client *client)
> > > {
> > > struct stmfx *stmfx = i2c_get_clientdata(client);
> > >
> > > regmap_write(stmfx->map, STMFX_REG_IRQ_SRC_EN, 0);
> > > regmap_write(stmfx->map, STMFX_REG_SYS_CTRL, 0);
> > >
> > > - if (stmfx->vdd)
> > > - return regulator_disable(stmfx->vdd);
> > > + if (stmfx->vdd) {
> > > + int ret = regulator_disable(stmfx->vdd);
> > >
> > > - return 0;
> > > + if (ret)
> >
> > Nit: Premise of the patch is fine, but please can you use the standard
> > function call, check the return value format please. Something about
> > this is triggering my OCD! :)
> >
> > int ret;
> >
> > ret = regulator_disable(stmfx->vdd);
> > if (ret)
> > do_thing();
>
> Not sure I understand you correctly. Do you want just:
>
> regmap_write(stmfx->map, STMFX_REG_SYS_CTRL, 0);
>
> if (stmfx->vdd) {
> - int ret = regulator_disable(stmfx->vdd);
> + int ret;
> +
> + ret = regulator_disable(stmfx->vdd);
> if (ret)
> ...
>
> squashed into the patch?
Effectively, yes please.
The diff would look like:
> > > - if (stmfx->vdd)
> > > - return regulator_disable(stmfx->vdd);
> > > + if (stmfx->vdd) {
> > > + int ret;
> > > +
> > > + ret = regulator_disable(stmfx->vdd);
> > > -
> > > - return 0;
> > > + if (ret)
Thanks.
--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-14 14:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-07 8:17 [PATCH] mfd: stmfx: Improve error message triggered by regulator fault in .remove() Uwe Kleine-König
2022-02-14 13:46 ` Lee Jones
2022-02-14 14:06 ` Uwe Kleine-König
2022-02-14 14:30 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).