* [PATCH] driver: pca953x: avoid error message when resuming
@ 2022-03-07 14:19 Alifer Moraes
2022-03-07 14:33 ` Bartosz Golaszewski
2022-03-07 16:22 ` Andy Shevchenko
0 siblings, 2 replies; 6+ messages in thread
From: Alifer Moraes @ 2022-03-07 14:19 UTC (permalink / raw)
To: linux-kernel
Cc: brgl, eran.m, linus.walleij, linux-gpio, pierluigi.p,
FrancescoFerraro, Alifer Moraes
From: FrancescoFerraro <francesco.f@variscite.com>
Avoids the error messages "pca953x 1-0020: failed reading register"
when resuming from suspend using gpio-key attached to pca9534.
Signed-off-by: Francesco Ferraro <francesco.f@variscite.com>
Signed-off-by: Alifer Moraes <alifer.m@variscite.com>
---
drivers/gpio/gpio-pca953x.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d2fe76f3f34f..4f35b75dcbb1 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -211,6 +211,7 @@ struct pca953x_chip {
struct regulator *regulator;
const struct pca953x_reg_config *regs;
+ int is_in_suspend;
};
static int pca953x_bank_shift(struct pca953x_chip *chip)
@@ -412,7 +413,8 @@ static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long *
ret = regmap_bulk_read(chip->regmap, regaddr, value, NBANK(chip));
if (ret < 0) {
- dev_err(&chip->client->dev, "failed reading register\n");
+ if (!chip->is_in_suspend)
+ dev_err(&chip->client->dev, "failed reading register\n");
return ret;
}
@@ -954,6 +956,7 @@ static int pca953x_probe(struct i2c_client *client,
chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
if (chip == NULL)
return -ENOMEM;
+ chip->is_in_suspend = 0;
pdata = dev_get_platdata(&client->dev);
if (pdata) {
@@ -1161,6 +1164,8 @@ static int pca953x_suspend(struct device *dev)
else
regulator_disable(chip->regulator);
+ chip->is_in_suspend = 1;
+
return 0;
}
@@ -1189,6 +1194,8 @@ static int pca953x_resume(struct device *dev)
return ret;
}
+ chip->is_in_suspend = 0;
+
return 0;
}
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] driver: pca953x: avoid error message when resuming
2022-03-07 14:19 [PATCH] driver: pca953x: avoid error message when resuming Alifer Moraes
@ 2022-03-07 14:33 ` Bartosz Golaszewski
2022-03-07 16:22 ` Andy Shevchenko
1 sibling, 0 replies; 6+ messages in thread
From: Bartosz Golaszewski @ 2022-03-07 14:33 UTC (permalink / raw)
To: Alifer Moraes
Cc: Linux Kernel Mailing List, eran.m, Linus Walleij,
open list:GPIO SUBSYSTEM, pierluigi.p, FrancescoFerraro
On Mon, Mar 7, 2022 at 3:20 PM Alifer Moraes <alifer.m@variscite.com> wrote:
>
> From: FrancescoFerraro <francesco.f@variscite.com>
>
> Avoids the error messages "pca953x 1-0020: failed reading register"
> when resuming from suspend using gpio-key attached to pca9534.
>
The commit message should read: "gpio: pca953x: ..."
> Signed-off-by: Francesco Ferraro <francesco.f@variscite.com>
> Signed-off-by: Alifer Moraes <alifer.m@variscite.com>
> ---
> drivers/gpio/gpio-pca953x.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index d2fe76f3f34f..4f35b75dcbb1 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -211,6 +211,7 @@ struct pca953x_chip {
> struct regulator *regulator;
>
> const struct pca953x_reg_config *regs;
> + int is_in_suspend;
Something like this is not needed because we already have
pm_runtime_status_suspended().
> };
>
> static int pca953x_bank_shift(struct pca953x_chip *chip)
> @@ -412,7 +413,8 @@ static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long *
>
> ret = regmap_bulk_read(chip->regmap, regaddr, value, NBANK(chip));
> if (ret < 0) {
> - dev_err(&chip->client->dev, "failed reading register\n");
> + if (!chip->is_in_suspend)
> + dev_err(&chip->client->dev, "failed reading register\n");
> return ret;
> }
>
> @@ -954,6 +956,7 @@ static int pca953x_probe(struct i2c_client *client,
> chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> if (chip == NULL)
> return -ENOMEM;
> + chip->is_in_suspend = 0;
>
> pdata = dev_get_platdata(&client->dev);
> if (pdata) {
> @@ -1161,6 +1164,8 @@ static int pca953x_suspend(struct device *dev)
> else
> regulator_disable(chip->regulator);
>
> + chip->is_in_suspend = 1;
> +
> return 0;
> }
>
> @@ -1189,6 +1194,8 @@ static int pca953x_resume(struct device *dev)
> return ret;
> }
>
> + chip->is_in_suspend = 0;
> +
> return 0;
> }
> #endif
> --
> 2.25.1
>
Can you elaborate more on the circumstances in which you're seeing this?
Bart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver: pca953x: avoid error message when resuming
2022-03-07 14:19 [PATCH] driver: pca953x: avoid error message when resuming Alifer Moraes
2022-03-07 14:33 ` Bartosz Golaszewski
@ 2022-03-07 16:22 ` Andy Shevchenko
2022-06-20 14:18 ` Pierluigi Passaro
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-03-07 16:22 UTC (permalink / raw)
To: Alifer Moraes
Cc: Linux Kernel Mailing List, Bartosz Golaszewski, eran.m,
Linus Walleij, open list:GPIO SUBSYSTEM, pierluigi.p,
FrancescoFerraro
On Mon, Mar 7, 2022 at 5:04 PM Alifer Moraes <alifer.m@variscite.com> wrote:
>
> From: FrancescoFerraro <francesco.f@variscite.com>
>
> Avoids the error messages "pca953x 1-0020: failed reading register"
> when resuming from suspend using gpio-key attached to pca9534.
Thanks for your report and fix. My comments below.
First of all, how many of them do you get and why is it a problem?
...
> const struct pca953x_reg_config *regs;
> + int is_in_suspend;
Usually we call it is_suspended or so, check existing code by `git
grep ...`. And it can be boolean.
...
> ret = regmap_bulk_read(chip->regmap, regaddr, value, NBANK(chip));
> if (ret < 0) {
> - dev_err(&chip->client->dev, "failed reading register\n");
> + if (!chip->is_in_suspend)
> + dev_err(&chip->client->dev, "failed reading register\n");
Hmm... Maybe we can simply move it to debug level?
> return ret;
> }
...
> chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> if (chip == NULL)
> return -ENOMEM;
> + chip->is_in_suspend = 0;
Redundant change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver: pca953x: avoid error message when resuming
2022-03-07 16:22 ` Andy Shevchenko
@ 2022-06-20 14:18 ` Pierluigi Passaro
2022-06-20 16:02 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Pierluigi Passaro @ 2022-06-20 14:18 UTC (permalink / raw)
To: Andy Shevchenko, Alifer Willians de Moraes
Cc: Linux Kernel Mailing List, Bartosz Golaszewski, Eran Matityahu,
Linus Walleij, open list:GPIO SUBSYSTEM, Francesco Ferraro
Hi All,
> > From: FrancescoFerraro <francesco.f@variscite.com>
> >
> > Avoids the error messages "pca953x 1-0020: failed reading register"
> > when resuming from suspend using gpio-key attached to pca9534.
> Thanks for your report and fix. My comments below.
> First of all, how many of them do you get and why is it a problem?
> ...
The number of occurrences depends on the time required to I2C bus to fully wake-up.
It's not a real problem, but the message may lead to think about a real I2C problem.
> > const struct pca953x_reg_config *regs;
> > + int is_in_suspend;
> Usually we call it is_suspended or so, check existing code by `git
> grep ...`. And it can be boolean.
> ...
Do you mean something like in drivers/gpio/gpio-omap.c ?
> > ret = regmap_bulk_read(chip->regmap, regaddr, value, NBANK(chip));
> > if (ret < 0) {
> > - dev_err(&chip->client->dev, "failed reading register\n");
> > + if (!chip->is_in_suspend)
> > + dev_err(&chip->client->dev, "failed reading register\n");
> Hmm... Maybe we can simply move it to debug level?
On the other side, this can be a serious problem, so I'm not sure the level should be changed.
> > return ret;
> > }
> ...
> > chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> > if (chip == NULL)
> > return -ENOMEM;
> > + chip->is_in_suspend = 0;
> Redundant change.
> --
> With Best Regards,
> Andy Shevchenko
Thanks
Best Regards
Pier
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver: pca953x: avoid error message when resuming
2022-06-20 14:18 ` Pierluigi Passaro
@ 2022-06-20 16:02 ` Andy Shevchenko
2022-06-28 12:35 ` Linus Walleij
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-06-20 16:02 UTC (permalink / raw)
To: Pierluigi Passaro, Wolfram Sang
Cc: Alifer Willians de Moraes, Linux Kernel Mailing List,
Bartosz Golaszewski, Eran Matityahu, Linus Walleij,
open list:GPIO SUBSYSTEM, Francesco Ferraro
On Mon, Jun 20, 2022 at 4:18 PM Pierluigi Passaro
<pierluigi.p@variscite.com> wrote:
...
> > > Avoids the error messages "pca953x 1-0020: failed reading register"
> > > when resuming from suspend using gpio-key attached to pca9534.
> > Thanks for your report and fix. My comments below.
> > First of all, how many of them do you get and why is it a problem?
>
> The number of occurrences depends on the time required to I2C bus to fully wake-up.
> It's not a real problem, but the message may lead to think about a real I2C problem.
Wolfram, do we have any mechanisms that guarantees that I2C traffic is
not going on a semi-woken up host controller?
Writing this, I'm in doubt this patch is a fix we want. Wouldn't it
just hide the real issue with some resume ordering?
...
> > > + int is_in_suspend;
> > Usually we call it is_suspended or so, check existing code by `git
> > grep ...`. And it can be boolean.
>
> Do you mean soomething like in drivers/gpio/gpio-omap.c ?
I believe almost any from the list `git grep -nl -w is_suspended` will suffice.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver: pca953x: avoid error message when resuming
2022-06-20 16:02 ` Andy Shevchenko
@ 2022-06-28 12:35 ` Linus Walleij
0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2022-06-28 12:35 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Pierluigi Passaro, Wolfram Sang, Alifer Willians de Moraes,
Linux Kernel Mailing List, Bartosz Golaszewski, Eran Matityahu,
open list:GPIO SUBSYSTEM, Francesco Ferraro
On Mon, Jun 20, 2022 at 6:03 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Jun 20, 2022 at 4:18 PM Pierluigi Passaro
> <pierluigi.p@variscite.com> wrote:
> > > > Avoids the error messages "pca953x 1-0020: failed reading register"
> > > > when resuming from suspend using gpio-key attached to pca9534.
> > > Thanks for your report and fix. My comments below.
> > > First of all, how many of them do you get and why is it a problem?
> >
> > The number of occurrences depends on the time required to I2C bus to fully wake-up.
> > It's not a real problem, but the message may lead to think about a real I2C problem.
>
> Wolfram, do we have any mechanisms that guarantees that I2C traffic is
> not going on a semi-woken up host controller?
>
> Writing this, I'm in doubt this patch is a fix we want. Wouldn't it
> just hide the real issue with some resume ordering?
That sounds like a bug in the pm routines in the I2C driver. Why is it
returning from [runtime_]resume() if it is not properly resumed?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-28 12:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-07 14:19 [PATCH] driver: pca953x: avoid error message when resuming Alifer Moraes
2022-03-07 14:33 ` Bartosz Golaszewski
2022-03-07 16:22 ` Andy Shevchenko
2022-06-20 14:18 ` Pierluigi Passaro
2022-06-20 16:02 ` Andy Shevchenko
2022-06-28 12:35 ` Linus Walleij
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.