| Message ID | 20190213131409.26607-1-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | gpio: pcf857x: Simpify wake-up handling | expand |
Hi Geert, On 2019-02-13 14:14:09 +0100, Geert Uytterhoeven wrote: > Unlike gpio_rcar_irq_set_wake(), where this solution was based on, s/where/which/ ? > pcf857x_irq_set_wake() does not need to do anything else than calling > irq_set_irq_wake() for the upstream interrupt controller. Hence there > is no reason for making this call conditional, and no longer a need for > keeping a copy of the interrupt number that also serves as a flag. > > Just propagate irq_set_irq_wake() to the upstream interrupt controller, > using the original interrupt number. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> This change makes sens to me, Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > I can no longer reproduce the issue fixed by commit ffb8e44bd7617ede > ("gpio: pcf857x: Check for irq_set_irq_wake() failures") on current > kernels: if .irq_set_wake() fails, system suspend is aborted, and the > imbalance on resume never happens. > > Looking at old logs, the system used to continue suspending, leading to > the imbalance. Presumably I could run git bisect to find out when that > was changed, but that would require downgrading my cross-compiler (Linux > v4.1-rc3 didn't support building using gcc-7)... > --- > drivers/gpio/gpio-pcf857x.c | 15 +-------------- > 1 file changed, 1 insertion(+), 14 deletions(-) > > diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c > index 68a35b65925aca76..c9b650f617fa687c 100644 > --- a/drivers/gpio/gpio-pcf857x.c > +++ b/drivers/gpio/gpio-pcf857x.c > @@ -89,7 +89,6 @@ struct pcf857x { > struct mutex lock; /* protect 'out' */ > unsigned out; /* software latch */ > unsigned status; /* current status */ > - unsigned int irq_parent; > unsigned irq_enabled; /* enabled irqs */ > > int (*write)(struct i2c_client *client, unsigned data); > @@ -211,18 +210,7 @@ static int pcf857x_irq_set_wake(struct irq_data *data, unsigned int on) > { > struct pcf857x *gpio = irq_data_get_irq_chip_data(data); > > - int error = 0; > - > - if (gpio->irq_parent) { > - error = irq_set_irq_wake(gpio->irq_parent, on); > - if (error) { > - dev_dbg(&gpio->client->dev, > - "irq %u doesn't support irq_set_wake\n", > - gpio->irq_parent); > - gpio->irq_parent = 0; > - } > - } > - return error; > + return irq_set_irq_wake(gpio->client->irq, on); > } > > static void pcf857x_irq_enable(struct irq_data *data) > @@ -392,7 +380,6 @@ static int pcf857x_probe(struct i2c_client *client, > > gpiochip_set_nested_irqchip(&gpio->chip, &gpio->irqchip, > client->irq); > - gpio->irq_parent = client->irq; > } > > /* Let platform code set up the GPIOs and their users. > -- > 2.17.1 >
On Wed, Feb 13, 2019 at 2:14 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote: > Unlike gpio_rcar_irq_set_wake(), where this solution was based on, > pcf857x_irq_set_wake() does not need to do anything else than calling > irq_set_irq_wake() for the upstream interrupt controller. Hence there > is no reason for making this call conditional, and no longer a need for > keeping a copy of the interrupt number that also serves as a flag. > > Just propagate irq_set_irq_wake() to the upstream interrupt controller, > using the original interrupt number. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > I can no longer reproduce the issue fixed by commit ffb8e44bd7617ede > ("gpio: pcf857x: Check for irq_set_irq_wake() failures") on current > kernels: if .irq_set_wake() fails, system suspend is aborted, and the > imbalance on resume never happens. > > Looking at old logs, the system used to continue suspending, leading to > the imbalance. Presumably I could run git bisect to find out when that > was changed, but that would require downgrading my cross-compiler (Linux > v4.1-rc3 didn't support building using gcc-7)... Patch applied (used some fuzzing so check the result) and incorporated Niklas' review tag. Yours. Linus Walleij
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c index 68a35b65925aca76..c9b650f617fa687c 100644 --- a/drivers/gpio/gpio-pcf857x.c +++ b/drivers/gpio/gpio-pcf857x.c @@ -89,7 +89,6 @@ struct pcf857x { struct mutex lock; /* protect 'out' */ unsigned out; /* software latch */ unsigned status; /* current status */ - unsigned int irq_parent; unsigned irq_enabled; /* enabled irqs */ int (*write)(struct i2c_client *client, unsigned data); @@ -211,18 +210,7 @@ static int pcf857x_irq_set_wake(struct irq_data *data, unsigned int on) { struct pcf857x *gpio = irq_data_get_irq_chip_data(data); - int error = 0; - - if (gpio->irq_parent) { - error = irq_set_irq_wake(gpio->irq_parent, on); - if (error) { - dev_dbg(&gpio->client->dev, - "irq %u doesn't support irq_set_wake\n", - gpio->irq_parent); - gpio->irq_parent = 0; - } - } - return error; + return irq_set_irq_wake(gpio->client->irq, on); } static void pcf857x_irq_enable(struct irq_data *data) @@ -392,7 +380,6 @@ static int pcf857x_probe(struct i2c_client *client, gpiochip_set_nested_irqchip(&gpio->chip, &gpio->irqchip, client->irq); - gpio->irq_parent = client->irq; } /* Let platform code set up the GPIOs and their users.
Unlike gpio_rcar_irq_set_wake(), where this solution was based on, pcf857x_irq_set_wake() does not need to do anything else than calling irq_set_irq_wake() for the upstream interrupt controller. Hence there is no reason for making this call conditional, and no longer a need for keeping a copy of the interrupt number that also serves as a flag. Just propagate irq_set_irq_wake() to the upstream interrupt controller, using the original interrupt number. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- I can no longer reproduce the issue fixed by commit ffb8e44bd7617ede ("gpio: pcf857x: Check for irq_set_irq_wake() failures") on current kernels: if .irq_set_wake() fails, system suspend is aborted, and the imbalance on resume never happens. Looking at old logs, the system used to continue suspending, leading to the imbalance. Presumably I could run git bisect to find out when that was changed, but that would require downgrading my cross-compiler (Linux v4.1-rc3 didn't support building using gcc-7)... --- drivers/gpio/gpio-pcf857x.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-)