* [PATCH v2 0/2] Improvements for MAX77620 GPIO driver
@ 2020-07-08 15:58 Dmitry Osipenko
2020-07-08 15:58 ` [PATCH v2 1/2] gpio: max77620: Initialize interrupts state Dmitry Osipenko
2020-07-08 15:58 ` [PATCH v2 2/2] gpio: max77620: Replace 8 with MAX77620_GPIO_NR Dmitry Osipenko
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2020-07-08 15:58 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Laxman Dewangan,
Bartosz Golaszewski, Linus Walleij, Andy Shevchenko
Cc: linux-tegra, linux-gpio, linux-kernel
Hello!
This series addresses a problem that I discovered on Nexus 7 device where
GPIO interrupts may be left enabled after bootloader and the driver isn't
prepared to this, it also makes a small improvement to the code.
Changelog:
v2: - Addressed review comment that were made by Andy Shevchenko to v1:
- Generic init_hw() callback is used now for resetting interrupts.
- These v1 patches are dropped:
gpio: max77620: Replace interrupt-enable array with bitmap
gpio: max77620: Don't handle disabled interrupts
gpio: max77620: Move variable declaration
Dmitry Osipenko (2):
gpio: max77620: Initialize interrupts state
gpio: max77620: Replace 8 with MAX77620_GPIO_NR
drivers/gpio/gpio-max77620.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
--
2.26.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/2] gpio: max77620: Initialize interrupts state 2020-07-08 15:58 [PATCH v2 0/2] Improvements for MAX77620 GPIO driver Dmitry Osipenko @ 2020-07-08 15:58 ` Dmitry Osipenko 2020-07-08 16:58 ` Andy Shevchenko 2020-07-08 15:58 ` [PATCH v2 2/2] gpio: max77620: Replace 8 with MAX77620_GPIO_NR Dmitry Osipenko 1 sibling, 1 reply; 6+ messages in thread From: Dmitry Osipenko @ 2020-07-08 15:58 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel I noticed on Nexus 7 that after rebooting from downstream kernel to upstream, the GPIO interrupt is triggering non-stop despite of interrupts being disabled for all of GPIOs. This happens because Nexus 7 uses a soft-reboot, meaning that bootloader should take care of resetting hardware, but bootloader doesn't do it well. In a result, GPIO interrupt may be left ON at a boot time. Let's mask all GPIO interrupts at the driver's initialization time in order to resolve the issue. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpio/gpio-max77620.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c index 313bd02dd893..970ad6397a56 100644 --- a/drivers/gpio/gpio-max77620.c +++ b/drivers/gpio/gpio-max77620.c @@ -260,6 +260,30 @@ static int max77620_gpio_set_config(struct gpio_chip *gc, unsigned int offset, return -ENOTSUPP; } +static int max77620_gpio_irq_init_hw(struct gpio_chip *gc) +{ + struct max77620_gpio *gpio = gpiochip_get_data(gc); + unsigned int i; + int err; + + /* + * GPIO interrupts may be left ON after bootloader, hence let's + * pre-initialize hardware to the expected state by disabling all + * the interrupts. + */ + for (i = 0; i < MAX77620_GPIO_NR; i++) { + err = regmap_update_bits(gpio->rmap, GPIO_REG_ADDR(i), + MAX77620_CNFG_GPIO_INT_MASK, 0); + if (err < 0) { + dev_err(gpio->dev, + "failed to disable interrupt: %d\n", err); + return err; + } + } + + return 0; +} + static int max77620_gpio_probe(struct platform_device *pdev) { struct max77620_chip *chip = dev_get_drvdata(pdev->dev.parent); @@ -288,6 +312,7 @@ static int max77620_gpio_probe(struct platform_device *pdev) mgpio->gpio_chip.ngpio = MAX77620_GPIO_NR; mgpio->gpio_chip.can_sleep = 1; mgpio->gpio_chip.base = -1; + mgpio->gpio_chip.irq.init_hw = max77620_gpio_irq_init_hw, #ifdef CONFIG_OF_GPIO mgpio->gpio_chip.of_node = pdev->dev.parent->of_node; #endif -- 2.26.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] gpio: max77620: Initialize interrupts state 2020-07-08 15:58 ` [PATCH v2 1/2] gpio: max77620: Initialize interrupts state Dmitry Osipenko @ 2020-07-08 16:58 ` Andy Shevchenko 2020-07-08 17:35 ` Dmitry Osipenko 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2020-07-08 16:58 UTC (permalink / raw) To: Dmitry Osipenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Wed, Jul 8, 2020 at 6:58 PM Dmitry Osipenko <digetx@gmail.com> wrote: > > I noticed on Nexus 7 that after rebooting from downstream kernel to > upstream, the GPIO interrupt is triggering non-stop despite of interrupts despite interrupts > being disabled for all of GPIOs. This happens because Nexus 7 uses a > soft-reboot, meaning that bootloader should take care of resetting > hardware, but bootloader doesn't do it well. In a result, GPIO interrupt but the bootloader As a result > may be left ON at a boot time. Let's mask all GPIO interrupts at the > driver's initialization time in order to resolve the issue. ... > mgpio->gpio_chip.ngpio = MAX77620_GPIO_NR; > mgpio->gpio_chip.can_sleep = 1; > mgpio->gpio_chip.base = -1; > + mgpio->gpio_chip.irq.init_hw = max77620_gpio_irq_init_hw, Now this seems a bit awkward. Perhaps you need first to switch to use GPIO IRQ chip structure? It's also in the TODO of the GPIO subsystem ;-) ... > #ifdef CONFIG_OF_GPIO > mgpio->gpio_chip.of_node = pdev->dev.parent->of_node; > #endif This seems to be done by GPIO library. Also point to improve: don't shadow error from platform_get_irq(). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] gpio: max77620: Initialize interrupts state 2020-07-08 16:58 ` Andy Shevchenko @ 2020-07-08 17:35 ` Dmitry Osipenko 0 siblings, 0 replies; 6+ messages in thread From: Dmitry Osipenko @ 2020-07-08 17:35 UTC (permalink / raw) To: Andy Shevchenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List 08.07.2020 19:58, Andy Shevchenko пишет: > On Wed, Jul 8, 2020 at 6:58 PM Dmitry Osipenko <digetx@gmail.com> wrote: >> >> I noticed on Nexus 7 that after rebooting from downstream kernel to >> upstream, the GPIO interrupt is triggering non-stop despite of interrupts > > despite interrupts > >> being disabled for all of GPIOs. This happens because Nexus 7 uses a >> soft-reboot, meaning that bootloader should take care of resetting >> hardware, but bootloader doesn't do it well. In a result, GPIO interrupt > > but the bootloader > > As a result Thanks! >> may be left ON at a boot time. Let's mask all GPIO interrupts at the >> driver's initialization time in order to resolve the issue. > > ... > >> mgpio->gpio_chip.ngpio = MAX77620_GPIO_NR; >> mgpio->gpio_chip.can_sleep = 1; >> mgpio->gpio_chip.base = -1; >> + mgpio->gpio_chip.irq.init_hw = max77620_gpio_irq_init_hw, > > Now this seems a bit awkward. Perhaps you need first to switch to use > GPIO IRQ chip structure? > It's also in the TODO of the GPIO subsystem ;-) Alright, I'll try to switch it to use the GPIO IRQ chip struct in v3! > ... > >> #ifdef CONFIG_OF_GPIO >> mgpio->gpio_chip.of_node = pdev->dev.parent->of_node; >> #endif > > This seems to be done by GPIO library. Indeed! > Also point to improve: don't shadow error from platform_get_irq(). > Good catch! ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] gpio: max77620: Replace 8 with MAX77620_GPIO_NR 2020-07-08 15:58 [PATCH v2 0/2] Improvements for MAX77620 GPIO driver Dmitry Osipenko 2020-07-08 15:58 ` [PATCH v2 1/2] gpio: max77620: Initialize interrupts state Dmitry Osipenko @ 2020-07-08 15:58 ` Dmitry Osipenko 2020-07-08 16:45 ` Andy Shevchenko 1 sibling, 1 reply; 6+ messages in thread From: Dmitry Osipenko @ 2020-07-08 15:58 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel The MAX77620_GPIO_NR enum value represents the total number of GPIOs, let's use it instead of a raw value in order to improve the code's readability a tad. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpio/gpio-max77620.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c index 970ad6397a56..08bd5b141437 100644 --- a/drivers/gpio/gpio-max77620.c +++ b/drivers/gpio/gpio-max77620.c @@ -19,8 +19,8 @@ struct max77620_gpio { struct regmap *rmap; struct device *dev; struct mutex buslock; /* irq_bus_lock */ - unsigned int irq_type[8]; - bool irq_enabled[8]; + unsigned int irq_type[MAX77620_GPIO_NR]; + bool irq_enabled[MAX77620_GPIO_NR]; }; static irqreturn_t max77620_gpio_irqhandler(int irq, void *data) @@ -38,7 +38,7 @@ static irqreturn_t max77620_gpio_irqhandler(int irq, void *data) pending = value; - for_each_set_bit(offset, &pending, 8) { + for_each_set_bit(offset, &pending, MAX77620_GPIO_NR) { unsigned int virq; virq = irq_find_mapping(gpio->gpio_chip.irq.domain, offset); -- 2.26.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] gpio: max77620: Replace 8 with MAX77620_GPIO_NR 2020-07-08 15:58 ` [PATCH v2 2/2] gpio: max77620: Replace 8 with MAX77620_GPIO_NR Dmitry Osipenko @ 2020-07-08 16:45 ` Andy Shevchenko 0 siblings, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2020-07-08 16:45 UTC (permalink / raw) To: Dmitry Osipenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Wed, Jul 8, 2020 at 6:58 PM Dmitry Osipenko <digetx@gmail.com> wrote: > > The MAX77620_GPIO_NR enum value represents the total number of GPIOs, > let's use it instead of a raw value in order to improve the code's > readability a tad. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/gpio/gpio-max77620.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c > index 970ad6397a56..08bd5b141437 100644 > --- a/drivers/gpio/gpio-max77620.c > +++ b/drivers/gpio/gpio-max77620.c > @@ -19,8 +19,8 @@ struct max77620_gpio { > struct regmap *rmap; > struct device *dev; > struct mutex buslock; /* irq_bus_lock */ > - unsigned int irq_type[8]; > - bool irq_enabled[8]; > + unsigned int irq_type[MAX77620_GPIO_NR]; > + bool irq_enabled[MAX77620_GPIO_NR]; > }; > > static irqreturn_t max77620_gpio_irqhandler(int irq, void *data) > @@ -38,7 +38,7 @@ static irqreturn_t max77620_gpio_irqhandler(int irq, void *data) > > pending = value; > > - for_each_set_bit(offset, &pending, 8) { > + for_each_set_bit(offset, &pending, MAX77620_GPIO_NR) { > unsigned int virq; > > virq = irq_find_mapping(gpio->gpio_chip.irq.domain, offset); > -- > 2.26.0 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-08 17:36 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-07-08 15:58 [PATCH v2 0/2] Improvements for MAX77620 GPIO driver Dmitry Osipenko 2020-07-08 15:58 ` [PATCH v2 1/2] gpio: max77620: Initialize interrupts state Dmitry Osipenko 2020-07-08 16:58 ` Andy Shevchenko 2020-07-08 17:35 ` Dmitry Osipenko 2020-07-08 15:58 ` [PATCH v2 2/2] gpio: max77620: Replace 8 with MAX77620_GPIO_NR Dmitry Osipenko 2020-07-08 16:45 ` Andy Shevchenko
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).