| Message ID | 1429637257-11055-1-git-send-email-semen.protsenko@globallogic.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Apr 21, 2015 at 7:27 PM, Semen Protsenko <semen.protsenko@globallogic.com> wrote: > Set .irq_set_wake callback to prevent possible issues on wake-up. > > This patch was inspired by this commit: > b80eef95beb04760629822fa130aeed54cdfafca > > Signed-off-by: Semen Protsenko <semen.protsenko@globallogic.com> Are these real, observed issues? Patch applied, but it seems we need a general approach to cover a few GPIO drivers with this kind of thing. Is this how we should always do it? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 4, 2015 at 4:32 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > Are these real, observed issues? The issue was observed for PCF857x expander (not by me), and it's described in this commit: b80eef95beb04760629822fa130aeed54cdfafca . It seems to me that the issue is real. But we need some really particular hardware configuration and particular use-case to reproduce it. As I understand from commit message (for mentioned commit), this issue was catch on system with "arch/arm/boot/dts/sh73a0-kzm9g-reference.dts" device tree file. As you can see from that file, there is a keyboard ("gpio-keys") connected to PCF8575 expander. In order to wake system up from keyboard event (key pressed), parent interrupt controller ("interrupt-parent" field in "pcf8575" node) should has the same wake-up settings as PCF8575 has. So this issue may affect other expanders (like MAX732X) as well. I haven't tested if it's true (only validated that everything works fine with this patch). I'm now in the middle of building my own PCB with MAX7325 chip for testing such things (since I was relocated from project where I had board with MAX732X). > Patch applied, but it seems we need a general approach to > cover a few GPIO drivers with this kind of thing. > > Is this how we should always do it? I think so (well, at least it seems to be correct for GPIO expanders). But I'd verify each particular driver to be completely sure that it's really needed and it wouldn't create some new issues. MAX732X chip seems to be very similar to PCF857X one, so in this particular case I'm sure that this patch is a good thing to have. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 4, 2015 at 8:23 PM, Sam Protsenko <semen.protsenko@globallogic.com> wrote: > On Mon, May 4, 2015 at 4:32 PM, Linus Walleij <linus.walleij@linaro.org> wrote: >> Patch applied, but it seems we need a general approach to >> cover a few GPIO drivers with this kind of thing. >> >> Is this how we should always do it? > > I think so (well, at least it seems to be correct for GPIO expanders). But I'd > verify each particular driver to be completely sure that it's really needed and > it wouldn't create some new issues. MAX732X chip seems to be very similar to > PCF857X one, so in this particular case I'm sure that this patch is a good thing > to have. OK yeah, maybe we could provide a list of "usual suspects", what do you say about "my" expanders: drivers/gpio/gpio-stmpe.c drivers/gpio/gpio-tc3589x.c I can surely patch and test these. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 12, 2015 at 10:34 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > OK yeah, maybe we could provide a list of "usual suspects", what do you > say about "my" expanders: > > drivers/gpio/gpio-stmpe.c > drivers/gpio/gpio-tc3589x.c > > I can surely patch and test these. This issue seems to be pretty common for all GPIO chips that have IRQ chip capability. So your expanders should be patched too. But still, the best course here is to actually reproduce the issue first, and only then proceed to patching. So if you have boards with those expanders -- it should be easy to come up with some use-case that reproduces the issue mentioned in the original commit. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-max732x.c b/drivers/gpio/gpio-max732x.c index 0fa4543..1885e5c 100644 --- a/drivers/gpio/gpio-max732x.c +++ b/drivers/gpio/gpio-max732x.c @@ -429,6 +429,14 @@ static int max732x_irq_set_type(struct irq_data *d, unsigned int type) return 0; } +static int max732x_irq_set_wake(struct irq_data *data, unsigned int on) +{ + struct max732x_chip *chip = irq_data_get_irq_chip_data(data); + + irq_set_irq_wake(chip->client->irq, on); + return 0; +} + static struct irq_chip max732x_irq_chip = { .name = "max732x", .irq_mask = max732x_irq_mask, @@ -436,6 +444,7 @@ static struct irq_chip max732x_irq_chip = { .irq_bus_lock = max732x_irq_bus_lock, .irq_bus_sync_unlock = max732x_irq_bus_sync_unlock, .irq_set_type = max732x_irq_set_type, + .irq_set_wake = max732x_irq_set_wake, }; static uint8_t max732x_irq_pending(struct max732x_chip *chip)
Set .irq_set_wake callback to prevent possible issues on wake-up. This patch was inspired by this commit: b80eef95beb04760629822fa130aeed54cdfafca Signed-off-by: Semen Protsenko <semen.protsenko@globallogic.com> --- drivers/gpio/gpio-max732x.c | 9 +++++++++ 1 file changed, 9 insertions(+)