* [PATCH v3 0/6] Improvements for MAX77620 GPIO driver @ 2020-07-08 20:23 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA 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 improvements to the code, fixes the non-released interrupt bug and converts driver to use irqchip template. Changelog: v3: - Addressed review comment that were made by Andy Shevchenko to v2: - Improved the commit message of the "Initialize hardware state of interrupts" patch. - Added these new patches: gpio: max77620: Don't set of_node gpio: max77620: Don't shadow error code of platform_get_irq() gpio: max77620: Use irqchip template - Added "Fix missing release of interrupt" patch. 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 (6): gpio: max77620: Replace 8 with MAX77620_GPIO_NR gpio: max77620: Fix missing release of interrupt gpio: max77620: Don't set of_node gpio: max77620: Don't shadow error code of platform_get_irq() gpio: max77620: Use irqchip template gpio: max77620: Initialize hardware state of interrupts drivers/gpio/gpio-max77620.c | 65 ++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 21 deletions(-) -- 2.26.0 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 0/6] Improvements for MAX77620 GPIO driver @ 2020-07-08 20:23 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 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 improvements to the code, fixes the non-released interrupt bug and converts driver to use irqchip template. Changelog: v3: - Addressed review comment that were made by Andy Shevchenko to v2: - Improved the commit message of the "Initialize hardware state of interrupts" patch. - Added these new patches: gpio: max77620: Don't set of_node gpio: max77620: Don't shadow error code of platform_get_irq() gpio: max77620: Use irqchip template - Added "Fix missing release of interrupt" patch. 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 (6): gpio: max77620: Replace 8 with MAX77620_GPIO_NR gpio: max77620: Fix missing release of interrupt gpio: max77620: Don't set of_node gpio: max77620: Don't shadow error code of platform_get_irq() gpio: max77620: Use irqchip template gpio: max77620: Initialize hardware state of interrupts drivers/gpio/gpio-max77620.c | 65 ++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 21 deletions(-) -- 2.26.0 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 1/6] gpio: max77620: Replace 8 with MAX77620_GPIO_NR 2020-07-08 20:23 ` Dmitry Osipenko (?) @ 2020-07-08 20:23 ` Dmitry Osipenko 2020-07-09 14:54 ` Laxman Dewangan -1 siblings, 1 reply; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 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. 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 313bd02dd893..4c0c9ec2587d 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] 32+ messages in thread
* Re: [PATCH v3 1/6] gpio: max77620: Replace 8 with MAX77620_GPIO_NR 2020-07-08 20:23 ` [PATCH v3 1/6] gpio: max77620: Replace 8 with MAX77620_GPIO_NR Dmitry Osipenko @ 2020-07-09 14:54 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 14:54 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 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> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 1/6] gpio: max77620: Replace 8 with MAX77620_GPIO_NR @ 2020-07-09 14:54 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 14:54 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 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> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 2/6] gpio: max77620: Fix missing release of interrupt 2020-07-08 20:23 ` Dmitry Osipenko (?) (?) @ 2020-07-08 20:23 ` Dmitry Osipenko [not found] ` <20200708202355.28507-3-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> -1 siblings, 1 reply; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel The requested interrupt is never released by the driver. Fix this by using the resource-managed variant of request_threaded_irq(). Fixes: ab3dd9cc24d4 ("gpio: max77620: Fix interrupt handling") Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpio/gpio-max77620.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c index 4c0c9ec2587d..7f7e8d4bf0d3 100644 --- a/drivers/gpio/gpio-max77620.c +++ b/drivers/gpio/gpio-max77620.c @@ -305,8 +305,9 @@ static int max77620_gpio_probe(struct platform_device *pdev) gpiochip_irqchip_add_nested(&mgpio->gpio_chip, &max77620_gpio_irqchip, 0, handle_edge_irq, IRQ_TYPE_NONE); - ret = request_threaded_irq(gpio_irq, NULL, max77620_gpio_irqhandler, - IRQF_ONESHOT, "max77620-gpio", mgpio); + ret = devm_request_threaded_irq(&pdev->dev, gpio_irq, NULL, + max77620_gpio_irqhandler, IRQF_ONESHOT, + "max77620-gpio", mgpio); if (ret < 0) { dev_err(&pdev->dev, "failed to request IRQ: %d\n", ret); return ret; -- 2.26.0 ^ permalink raw reply related [flat|nested] 32+ messages in thread
[parent not found: <20200708202355.28507-3-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 2/6] gpio: max77620: Fix missing release of interrupt 2020-07-08 20:23 ` [PATCH v3 2/6] gpio: max77620: Fix missing release of interrupt Dmitry Osipenko @ 2020-07-09 14:57 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 14:57 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > The requested interrupt is never released by the driver. Fix this by > using the resource-managed variant of request_threaded_irq(). > > Fixes: ab3dd9cc24d4 ("gpio: max77620: Fix interrupt handling") > Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Looks good to me. Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/6] gpio: max77620: Fix missing release of interrupt @ 2020-07-09 14:57 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 14:57 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > The requested interrupt is never released by the driver. Fix this by > using the resource-managed variant of request_threaded_irq(). > > Fixes: ab3dd9cc24d4 ("gpio: max77620: Fix interrupt handling") > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> Looks good to me. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <80f4d1ff-8096-9060-3cf0-a59448866c40-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v3 2/6] gpio: max77620: Fix missing release of interrupt 2020-07-09 14:57 ` Laxman Dewangan @ 2020-07-09 16:36 ` Dmitry Osipenko -1 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-09 16:36 UTC (permalink / raw) To: Laxman Dewangan, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA 09.07.2020 17:57, Laxman Dewangan пишет: > > > On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> The requested interrupt is never released by the driver. Fix this by >> using the resource-managed variant of request_threaded_irq(). >> >> Fixes: ab3dd9cc24d4 ("gpio: max77620: Fix interrupt handling") >> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > > Looks good to me. > Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> Hello, Laxman! Thank you very much for taking a look at this series! I missed to add the stable tag to this patch, so will prepare a v4 with the corrected tag. I'll also add yours acks and Andy's r-bs to the patches. Thanks! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 2/6] gpio: max77620: Fix missing release of interrupt @ 2020-07-09 16:36 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-09 16:36 UTC (permalink / raw) To: Laxman Dewangan, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel 09.07.2020 17:57, Laxman Dewangan пишет: > > > On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> The requested interrupt is never released by the driver. Fix this by >> using the resource-managed variant of request_threaded_irq(). >> >> Fixes: ab3dd9cc24d4 ("gpio: max77620: Fix interrupt handling") >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > > > Looks good to me. > Acked-by: Laxman Dewangan <ldewangan@nvidia.com> Hello, Laxman! Thank you very much for taking a look at this series! I missed to add the stable tag to this patch, so will prepare a v4 with the corrected tag. I'll also add yours acks and Andy's r-bs to the patches. Thanks! ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <20200708202355.28507-1-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCH v3 3/6] gpio: max77620: Don't set of_node 2020-07-08 20:23 ` Dmitry Osipenko @ 2020-07-08 20:23 ` Dmitry Osipenko -1 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA The gpiochip_add_data() takes care of setting the of_node to the parent's device of_node, hence there is no need to do it manually in the driver's code. This patch corrects the parent's device pointer and removes the unnecessary setting of the of_node. Suggested-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/gpio/gpio-max77620.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c index 7f7e8d4bf0d3..39d431da2dbc 100644 --- a/drivers/gpio/gpio-max77620.c +++ b/drivers/gpio/gpio-max77620.c @@ -279,7 +279,7 @@ static int max77620_gpio_probe(struct platform_device *pdev) mgpio->dev = &pdev->dev; mgpio->gpio_chip.label = pdev->name; - mgpio->gpio_chip.parent = &pdev->dev; + mgpio->gpio_chip.parent = pdev->dev.parent; mgpio->gpio_chip.direction_input = max77620_gpio_dir_input; mgpio->gpio_chip.get = max77620_gpio_get; mgpio->gpio_chip.direction_output = max77620_gpio_dir_output; @@ -288,9 +288,6 @@ 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; -#ifdef CONFIG_OF_GPIO - mgpio->gpio_chip.of_node = pdev->dev.parent->of_node; -#endif platform_set_drvdata(pdev, mgpio); -- 2.26.0 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 3/6] gpio: max77620: Don't set of_node @ 2020-07-08 20:23 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel The gpiochip_add_data() takes care of setting the of_node to the parent's device of_node, hence there is no need to do it manually in the driver's code. This patch corrects the parent's device pointer and removes the unnecessary setting of the of_node. Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpio/gpio-max77620.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c index 7f7e8d4bf0d3..39d431da2dbc 100644 --- a/drivers/gpio/gpio-max77620.c +++ b/drivers/gpio/gpio-max77620.c @@ -279,7 +279,7 @@ static int max77620_gpio_probe(struct platform_device *pdev) mgpio->dev = &pdev->dev; mgpio->gpio_chip.label = pdev->name; - mgpio->gpio_chip.parent = &pdev->dev; + mgpio->gpio_chip.parent = pdev->dev.parent; mgpio->gpio_chip.direction_input = max77620_gpio_dir_input; mgpio->gpio_chip.get = max77620_gpio_get; mgpio->gpio_chip.direction_output = max77620_gpio_dir_output; @@ -288,9 +288,6 @@ 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; -#ifdef CONFIG_OF_GPIO - mgpio->gpio_chip.of_node = pdev->dev.parent->of_node; -#endif platform_set_drvdata(pdev, mgpio); -- 2.26.0 ^ permalink raw reply related [flat|nested] 32+ messages in thread
[parent not found: <CAHp75VejftNuSqdYvd1YE1SdRON6=mQ_iD2dEr4K9D8YGgeRBQ@mail.gmail.com>]
[parent not found: <CAHp75VejftNuSqdYvd1YE1SdRON6=mQ_iD2dEr4K9D8YGgeRBQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v3 3/6] gpio: max77620: Don't set of_node [not found] ` <CAHp75VejftNuSqdYvd1YE1SdRON6=mQ_iD2dEr4K9D8YGgeRBQ@mail.gmail.com> @ 2020-07-08 21:44 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 21:44 UTC (permalink / raw) To: Andy Shevchenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA 08.07.2020 23:57, Andy Shevchenko пишет: > > > On Wednesday, July 8, 2020, Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org > <mailto:digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote: > > The gpiochip_add_data() takes care of setting the of_node to the > parent's > device of_node, hence there is no need to do it manually in the driver's > code. This patch corrects the parent's device pointer and removes the > unnecessary setting of the of_node. > > > I gave a second look and I think my suggestion is wrong. Here is an > interesting propagation of the parent device node to its grand son, > leaving son’s one untouched. Original code has intentions to do that way. The [1] says that gpio_chip.parent should point at the "device providing the GPIOs". That's the pdev->dev.parent in the case of this driver. MAX77620 is an MFD PMIC device that has virtual sub-devices like GPIO controller, PINCTRL and RTC. The MFD is the parent device that provides the GPIOs [2]. [1] https://elixir.bootlin.com/linux/v5.8-rc3/source/include/linux/gpio/driver.h#L276 [2] https://elixir.bootlin.com/linux/v5.8-rc3/source/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi#L48 I think the old code was wrong and this patch is correct, please correct me if I'm missing something. > Suggested-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org > <mailto:andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> > Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org > <mailto:digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> > --- > drivers/gpio/gpio-max77620.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c > index 7f7e8d4bf0d3..39d431da2dbc 100644 > --- a/drivers/gpio/gpio-max77620.c > +++ b/drivers/gpio/gpio-max77620.c > @@ -279,7 +279,7 @@ static int max77620_gpio_probe(struct > platform_device *pdev) > mgpio->dev = &pdev->dev; > > mgpio->gpio_chip.label = pdev->name; > - mgpio->gpio_chip.parent = &pdev->dev; > + mgpio->gpio_chip.parent = pdev->dev.parent; > mgpio->gpio_chip.direction_input = max77620_gpio_dir_input; > mgpio->gpio_chip.get = max77620_gpio_get; > mgpio->gpio_chip.direction_output = max77620_gpio_dir_output; > @@ -288,9 +288,6 @@ 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; > -#ifdef CONFIG_OF_GPIO > - mgpio->gpio_chip.of_node = pdev->dev.parent->of_node; > -#endif > > platform_set_drvdata(pdev, mgpio); > > -- > 2.26.0 > > > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/6] gpio: max77620: Don't set of_node @ 2020-07-08 21:44 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 21:44 UTC (permalink / raw) To: Andy Shevchenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra, linux-gpio, linux-kernel 08.07.2020 23:57, Andy Shevchenko пишет: > > > On Wednesday, July 8, 2020, Dmitry Osipenko <digetx@gmail.com > <mailto:digetx@gmail.com>> wrote: > > The gpiochip_add_data() takes care of setting the of_node to the > parent's > device of_node, hence there is no need to do it manually in the driver's > code. This patch corrects the parent's device pointer and removes the > unnecessary setting of the of_node. > > > I gave a second look and I think my suggestion is wrong. Here is an > interesting propagation of the parent device node to its grand son, > leaving son’s one untouched. Original code has intentions to do that way. The [1] says that gpio_chip.parent should point at the "device providing the GPIOs". That's the pdev->dev.parent in the case of this driver. MAX77620 is an MFD PMIC device that has virtual sub-devices like GPIO controller, PINCTRL and RTC. The MFD is the parent device that provides the GPIOs [2]. [1] https://elixir.bootlin.com/linux/v5.8-rc3/source/include/linux/gpio/driver.h#L276 [2] https://elixir.bootlin.com/linux/v5.8-rc3/source/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi#L48 I think the old code was wrong and this patch is correct, please correct me if I'm missing something. > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com > <mailto:andy.shevchenko@gmail.com>> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com > <mailto:digetx@gmail.com>> > --- > drivers/gpio/gpio-max77620.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c > index 7f7e8d4bf0d3..39d431da2dbc 100644 > --- a/drivers/gpio/gpio-max77620.c > +++ b/drivers/gpio/gpio-max77620.c > @@ -279,7 +279,7 @@ static int max77620_gpio_probe(struct > platform_device *pdev) > mgpio->dev = &pdev->dev; > > mgpio->gpio_chip.label = pdev->name; > - mgpio->gpio_chip.parent = &pdev->dev; > + mgpio->gpio_chip.parent = pdev->dev.parent; > mgpio->gpio_chip.direction_input = max77620_gpio_dir_input; > mgpio->gpio_chip.get = max77620_gpio_get; > mgpio->gpio_chip.direction_output = max77620_gpio_dir_output; > @@ -288,9 +288,6 @@ 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; > -#ifdef CONFIG_OF_GPIO > - mgpio->gpio_chip.of_node = pdev->dev.parent->of_node; > -#endif > > platform_set_drvdata(pdev, mgpio); > > -- > 2.26.0 > > > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <675c4691-d372-4fe1-d515-c86fdba2f588-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 3/6] gpio: max77620: Don't set of_node 2020-07-08 21:44 ` Dmitry Osipenko @ 2020-07-09 9:07 ` Andy Shevchenko -1 siblings, 0 replies; 32+ messages in thread From: Andy Shevchenko @ 2020-07-09 9:07 UTC (permalink / raw) To: Dmitry Osipenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thu, Jul 9, 2020 at 12:44 AM Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > 08.07.2020 23:57, Andy Shevchenko пишет: > > On Wednesday, July 8, 2020, Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org > > <mailto:digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote: ... > > I gave a second look and I think my suggestion is wrong. Here is an > > interesting propagation of the parent device node to its grand son, > > leaving son’s one untouched. Original code has intentions to do that way. > > The [1] says that gpio_chip.parent should point at the "device providing > the GPIOs". Yes, physical device I believe. > That's the pdev->dev.parent in the case of this driver. > MAX77620 is an MFD PMIC device that has virtual sub-devices like GPIO > controller, PINCTRL and RTC. The MFD is the parent device that provides > the GPIOs [2]. > > [1] > https://elixir.bootlin.com/linux/v5.8-rc3/source/include/linux/gpio/driver.h#L276 > > [2] > https://elixir.bootlin.com/linux/v5.8-rc3/source/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi#L48 > > I think the old code was wrong and this patch is correct, please correct > me if I'm missing something. Hmm... I have checked through GPIO drivers I have knowledge of / care about and PMIC ones do like you suggested in this patch, the rest (which are instantiated from MFD) take a virtual platform device. Looking at DT excerpt I think you're rather right than wrong, so I leave it to you and maintainers. Thanks! -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/6] gpio: max77620: Don't set of_node @ 2020-07-09 9:07 ` Andy Shevchenko 0 siblings, 0 replies; 32+ messages in thread From: Andy Shevchenko @ 2020-07-09 9:07 UTC (permalink / raw) To: Dmitry Osipenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra, linux-gpio, linux-kernel On Thu, Jul 9, 2020 at 12:44 AM Dmitry Osipenko <digetx@gmail.com> wrote: > 08.07.2020 23:57, Andy Shevchenko пишет: > > On Wednesday, July 8, 2020, Dmitry Osipenko <digetx@gmail.com > > <mailto:digetx@gmail.com>> wrote: ... > > I gave a second look and I think my suggestion is wrong. Here is an > > interesting propagation of the parent device node to its grand son, > > leaving son’s one untouched. Original code has intentions to do that way. > > The [1] says that gpio_chip.parent should point at the "device providing > the GPIOs". Yes, physical device I believe. > That's the pdev->dev.parent in the case of this driver. > MAX77620 is an MFD PMIC device that has virtual sub-devices like GPIO > controller, PINCTRL and RTC. The MFD is the parent device that provides > the GPIOs [2]. > > [1] > https://elixir.bootlin.com/linux/v5.8-rc3/source/include/linux/gpio/driver.h#L276 > > [2] > https://elixir.bootlin.com/linux/v5.8-rc3/source/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi#L48 > > I think the old code was wrong and this patch is correct, please correct > me if I'm missing something. Hmm... I have checked through GPIO drivers I have knowledge of / care about and PMIC ones do like you suggested in this patch, the rest (which are instantiated from MFD) take a virtual platform device. Looking at DT excerpt I think you're rather right than wrong, so I leave it to you and maintainers. Thanks! -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <CAHp75Vd89QpwaGvkpzG+pxnLd8S2guPCARLW5xPwhxXL8ZRfFw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v3 3/6] gpio: max77620: Don't set of_node 2020-07-09 9:07 ` Andy Shevchenko @ 2020-07-09 10:38 ` Dmitry Osipenko -1 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-09 10:38 UTC (permalink / raw) To: Andy Shevchenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA 09.07.2020 12:07, Andy Shevchenko пишет: > On Thu, Jul 9, 2020 at 12:44 AM Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> 08.07.2020 23:57, Andy Shevchenko пишет: >>> On Wednesday, July 8, 2020, Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org >>> <mailto:digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote: > > ... > >>> I gave a second look and I think my suggestion is wrong. Here is an >>> interesting propagation of the parent device node to its grand son, >>> leaving son’s one untouched. Original code has intentions to do that way. >> >> The [1] says that gpio_chip.parent should point at the "device providing >> the GPIOs". > > Yes, physical device I believe. > >> That's the pdev->dev.parent in the case of this driver. >> MAX77620 is an MFD PMIC device that has virtual sub-devices like GPIO >> controller, PINCTRL and RTC. The MFD is the parent device that provides >> the GPIOs [2]. >> >> [1] >> https://elixir.bootlin.com/linux/v5.8-rc3/source/include/linux/gpio/driver.h#L276 >> >> [2] >> https://elixir.bootlin.com/linux/v5.8-rc3/source/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi#L48 >> >> I think the old code was wrong and this patch is correct, please correct >> me if I'm missing something. > > Hmm... I have checked through GPIO drivers I have knowledge of / care > about and PMIC ones do like you suggested in this patch, the rest > (which are instantiated from MFD) take a virtual platform device. > > Looking at DT excerpt I think you're rather right than wrong, so I > leave it to you and maintainers. > Thanks! Okay, waiting for the maintainers then :) Thank you very much for the review! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/6] gpio: max77620: Don't set of_node @ 2020-07-09 10:38 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-09 10:38 UTC (permalink / raw) To: Andy Shevchenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra, linux-gpio, linux-kernel 09.07.2020 12:07, Andy Shevchenko пишет: > On Thu, Jul 9, 2020 at 12:44 AM Dmitry Osipenko <digetx@gmail.com> wrote: >> 08.07.2020 23:57, Andy Shevchenko пишет: >>> On Wednesday, July 8, 2020, Dmitry Osipenko <digetx@gmail.com >>> <mailto:digetx@gmail.com>> wrote: > > ... > >>> I gave a second look and I think my suggestion is wrong. Here is an >>> interesting propagation of the parent device node to its grand son, >>> leaving son’s one untouched. Original code has intentions to do that way. >> >> The [1] says that gpio_chip.parent should point at the "device providing >> the GPIOs". > > Yes, physical device I believe. > >> That's the pdev->dev.parent in the case of this driver. >> MAX77620 is an MFD PMIC device that has virtual sub-devices like GPIO >> controller, PINCTRL and RTC. The MFD is the parent device that provides >> the GPIOs [2]. >> >> [1] >> https://elixir.bootlin.com/linux/v5.8-rc3/source/include/linux/gpio/driver.h#L276 >> >> [2] >> https://elixir.bootlin.com/linux/v5.8-rc3/source/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi#L48 >> >> I think the old code was wrong and this patch is correct, please correct >> me if I'm missing something. > > Hmm... I have checked through GPIO drivers I have knowledge of / care > about and PMIC ones do like you suggested in this patch, the rest > (which are instantiated from MFD) take a virtual platform device. > > Looking at DT excerpt I think you're rather right than wrong, so I > leave it to you and maintainers. > Thanks! Okay, waiting for the maintainers then :) Thank you very much for the review! ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <20200708202355.28507-4-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 3/6] gpio: max77620: Don't set of_node 2020-07-08 20:23 ` Dmitry Osipenko @ 2020-07-09 14:58 ` Laxman Dewangan -1 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 14:58 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > The gpiochip_add_data() takes care of setting the of_node to the parent's > device of_node, hence there is no need to do it manually in the driver's > code. This patch corrects the parent's device pointer and removes the > unnecessary setting of the of_node. > > Suggested-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/6] gpio: max77620: Don't set of_node @ 2020-07-09 14:58 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 14:58 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > The gpiochip_add_data() takes care of setting the of_node to the parent's > device of_node, hence there is no need to do it manually in the driver's > code. This patch corrects the parent's device pointer and removes the > unnecessary setting of the of_node. > > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 6/6] gpio: max77620: Initialize hardware state of interrupts 2020-07-08 20:23 ` Dmitry Osipenko @ 2020-07-08 20:23 ` Dmitry Osipenko -1 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA I noticed on Nexus 7 that after rebooting from downstream kernel to upstream, the GPIO interrupt is triggering non-stop 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 the bootloader doesn't do it well. As 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- 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 6c516aa7732d..e090979659eb 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); @@ -295,6 +319,7 @@ static int max77620_gpio_probe(struct platform_device *pdev) mgpio->gpio_chip.irq.chip = &max77620_gpio_irqchip; mgpio->gpio_chip.irq.default_type = IRQ_TYPE_NONE; mgpio->gpio_chip.irq.handler = handle_edge_irq; + mgpio->gpio_chip.irq.init_hw = max77620_gpio_irq_init_hw, mgpio->gpio_chip.irq.threaded = true; platform_set_drvdata(pdev, mgpio); -- 2.26.0 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 6/6] gpio: max77620: Initialize hardware state of interrupts @ 2020-07-08 20:23 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 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 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 the bootloader doesn't do it well. As 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 6c516aa7732d..e090979659eb 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); @@ -295,6 +319,7 @@ static int max77620_gpio_probe(struct platform_device *pdev) mgpio->gpio_chip.irq.chip = &max77620_gpio_irqchip; mgpio->gpio_chip.irq.default_type = IRQ_TYPE_NONE; mgpio->gpio_chip.irq.handler = handle_edge_irq; + mgpio->gpio_chip.irq.init_hw = max77620_gpio_irq_init_hw, mgpio->gpio_chip.irq.threaded = true; platform_set_drvdata(pdev, mgpio); -- 2.26.0 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v3 6/6] gpio: max77620: Initialize hardware state of interrupts 2020-07-08 20:23 ` Dmitry Osipenko @ 2020-07-09 15:03 ` Laxman Dewangan -1 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 15:03 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > I noticed on Nexus 7 that after rebooting from downstream kernel to > upstream, the GPIO interrupt is triggering non-stop 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 the bootloader doesn't do it well. As 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> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 6/6] gpio: max77620: Initialize hardware state of interrupts @ 2020-07-09 15:03 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 15:03 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > I noticed on Nexus 7 that after rebooting from downstream kernel to > upstream, the GPIO interrupt is triggering non-stop 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 the bootloader doesn't do it well. As 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> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 4/6] gpio: max77620: Don't shadow error code of platform_get_irq() 2020-07-08 20:23 ` Dmitry Osipenko ` (3 preceding siblings ...) (?) @ 2020-07-08 20:23 ` Dmitry Osipenko [not found] ` <20200708202355.28507-5-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> -1 siblings, 1 reply; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel The platform_get_irq() returns a positive interrupt number on success and negative error code on failure (zero shouldn't ever happen in practice, it would produce a noisy warning). Hence let's return the error code directly instead of overriding it with -ENODEV. Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpio/gpio-max77620.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c index 39d431da2dbc..9121d2507f60 100644 --- a/drivers/gpio/gpio-max77620.c +++ b/drivers/gpio/gpio-max77620.c @@ -264,12 +264,14 @@ static int max77620_gpio_probe(struct platform_device *pdev) { struct max77620_chip *chip = dev_get_drvdata(pdev->dev.parent); struct max77620_gpio *mgpio; - int gpio_irq; + unsigned int gpio_irq; int ret; - gpio_irq = platform_get_irq(pdev, 0); - if (gpio_irq <= 0) - return -ENODEV; + ret = platform_get_irq(pdev, 0); + if (ret < 0) + return ret; + + gpio_irq = ret; mgpio = devm_kzalloc(&pdev->dev, sizeof(*mgpio), GFP_KERNEL); if (!mgpio) -- 2.26.0 ^ permalink raw reply related [flat|nested] 32+ messages in thread
[parent not found: <20200708202355.28507-5-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 4/6] gpio: max77620: Don't shadow error code of platform_get_irq() 2020-07-08 20:23 ` [PATCH v3 4/6] gpio: max77620: Don't shadow error code of platform_get_irq() Dmitry Osipenko @ 2020-07-09 15:01 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 15:01 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > The platform_get_irq() returns a positive interrupt number on success and > negative error code on failure (zero shouldn't ever happen in practice, it > would produce a noisy warning). Hence let's return the error code directly > instead of overriding it with -ENODEV. > > Suggested-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 4/6] gpio: max77620: Don't shadow error code of platform_get_irq() @ 2020-07-09 15:01 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 15:01 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > The platform_get_irq() returns a positive interrupt number on success and > negative error code on failure (zero shouldn't ever happen in practice, it > would produce a noisy warning). Hence let's return the error code directly > instead of overriding it with -ENODEV. > > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 5/6] gpio: max77620: Use irqchip template 2020-07-08 20:23 ` Dmitry Osipenko ` (4 preceding siblings ...) (?) @ 2020-07-08 20:23 ` Dmitry Osipenko [not found] ` <20200708202355.28507-6-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> -1 siblings, 1 reply; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 20:23 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel This change addresses one of the GPIO-core TODOs for the MAX77620 driver which requires modern drivers to use the irqchip template. Instead of using the GPIO's irqchip-helpers for creating the IRQ domain, the gpio_irq_chip structure is now filled by the driver itself and then gpiochip_add_data() takes care of instantiating the IRQ domain for us. Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpio/gpio-max77620.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c index 9121d2507f60..6c516aa7732d 100644 --- a/drivers/gpio/gpio-max77620.c +++ b/drivers/gpio/gpio-max77620.c @@ -277,6 +277,7 @@ static int max77620_gpio_probe(struct platform_device *pdev) if (!mgpio) return -ENOMEM; + mutex_init(&mgpio->buslock); mgpio->rmap = chip->rmap; mgpio->dev = &pdev->dev; @@ -291,6 +292,11 @@ static int max77620_gpio_probe(struct platform_device *pdev) mgpio->gpio_chip.can_sleep = 1; mgpio->gpio_chip.base = -1; + mgpio->gpio_chip.irq.chip = &max77620_gpio_irqchip; + mgpio->gpio_chip.irq.default_type = IRQ_TYPE_NONE; + mgpio->gpio_chip.irq.handler = handle_edge_irq; + mgpio->gpio_chip.irq.threaded = true; + platform_set_drvdata(pdev, mgpio); ret = devm_gpiochip_add_data(&pdev->dev, &mgpio->gpio_chip, mgpio); @@ -299,11 +305,6 @@ static int max77620_gpio_probe(struct platform_device *pdev) return ret; } - mutex_init(&mgpio->buslock); - - gpiochip_irqchip_add_nested(&mgpio->gpio_chip, &max77620_gpio_irqchip, - 0, handle_edge_irq, IRQ_TYPE_NONE); - ret = devm_request_threaded_irq(&pdev->dev, gpio_irq, NULL, max77620_gpio_irqhandler, IRQF_ONESHOT, "max77620-gpio", mgpio); @@ -312,9 +313,6 @@ static int max77620_gpio_probe(struct platform_device *pdev) return ret; } - gpiochip_set_nested_irqchip(&mgpio->gpio_chip, &max77620_gpio_irqchip, - gpio_irq); - return 0; } -- 2.26.0 ^ permalink raw reply related [flat|nested] 32+ messages in thread
[parent not found: <20200708202355.28507-6-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 5/6] gpio: max77620: Use irqchip template 2020-07-08 20:23 ` [PATCH v3 5/6] gpio: max77620: Use irqchip template Dmitry Osipenko @ 2020-07-09 15:03 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 15:03 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > This change addresses one of the GPIO-core TODOs for the MAX77620 driver > which requires modern drivers to use the irqchip template. Instead of > using the GPIO's irqchip-helpers for creating the IRQ domain, the > gpio_irq_chip structure is now filled by the driver itself and then > gpiochip_add_data() takes care of instantiating the IRQ domain for us. > > Suggested-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 5/6] gpio: max77620: Use irqchip template @ 2020-07-09 15:03 ` Laxman Dewangan 0 siblings, 0 replies; 32+ messages in thread From: Laxman Dewangan @ 2020-07-09 15:03 UTC (permalink / raw) To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko Cc: linux-tegra, linux-gpio, linux-kernel On Thursday 09 July 2020 01:53 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > This change addresses one of the GPIO-core TODOs for the MAX77620 driver > which requires modern drivers to use the irqchip template. Instead of > using the GPIO's irqchip-helpers for creating the IRQ domain, the > gpio_irq_chip structure is now filled by the driver itself and then > gpiochip_add_data() takes care of instantiating the IRQ domain for us. > > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > Looks good to me. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <CAHp75VcNao84UajMYDJRH2gX7t8n=uo_Th8VVgcxJj3YkiA+pA@mail.gmail.com>]
[parent not found: <CAHp75VcNao84UajMYDJRH2gX7t8n=uo_Th8VVgcxJj3YkiA+pA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v1 0/5] Improvements for MAX77620 GPIO driver [not found] ` <CAHp75VcNao84UajMYDJRH2gX7t8n=uo_Th8VVgcxJj3YkiA+pA@mail.gmail.com> @ 2020-07-08 21:49 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 21:49 UTC (permalink / raw) To: Andy Shevchenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA 09.07.2020 00:00, Andy Shevchenko пишет: > > > On Wednesday, July 8, 2020, Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org > <mailto:digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote: > > 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 improvements to the code, > fixes the > non-released interrupt bug and converts driver to use irqchip template. > > > Thanks! I commented on one patch, the rest looks good and you may add my > Rb tag. Thank you! :) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v1 0/5] Improvements for MAX77620 GPIO driver @ 2020-07-08 21:49 ` Dmitry Osipenko 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Osipenko @ 2020-07-08 21:49 UTC (permalink / raw) To: Andy Shevchenko Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Bartosz Golaszewski, Linus Walleij, linux-tegra, linux-gpio, linux-kernel 09.07.2020 00:00, Andy Shevchenko пишет: > > > On Wednesday, July 8, 2020, Dmitry Osipenko <digetx@gmail.com > <mailto:digetx@gmail.com>> wrote: > > 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 improvements to the code, > fixes the > non-released interrupt bug and converts driver to use irqchip template. > > > Thanks! I commented on one patch, the rest looks good and you may add my > Rb tag. Thank you! :) ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2020-07-09 16:36 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-08 20:23 [PATCH v3 0/6] Improvements for MAX77620 GPIO driver Dmitry Osipenko 2020-07-08 20:23 ` Dmitry Osipenko 2020-07-08 20:23 ` [PATCH v3 1/6] gpio: max77620: Replace 8 with MAX77620_GPIO_NR Dmitry Osipenko 2020-07-09 14:54 ` Laxman Dewangan 2020-07-09 14:54 ` Laxman Dewangan 2020-07-08 20:23 ` [PATCH v3 2/6] gpio: max77620: Fix missing release of interrupt Dmitry Osipenko [not found] ` <20200708202355.28507-3-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2020-07-09 14:57 ` Laxman Dewangan 2020-07-09 14:57 ` Laxman Dewangan [not found] ` <80f4d1ff-8096-9060-3cf0-a59448866c40-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2020-07-09 16:36 ` Dmitry Osipenko 2020-07-09 16:36 ` Dmitry Osipenko [not found] ` <20200708202355.28507-1-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2020-07-08 20:23 ` [PATCH v3 3/6] gpio: max77620: Don't set of_node Dmitry Osipenko 2020-07-08 20:23 ` Dmitry Osipenko [not found] ` <CAHp75VejftNuSqdYvd1YE1SdRON6=mQ_iD2dEr4K9D8YGgeRBQ@mail.gmail.com> [not found] ` <CAHp75VejftNuSqdYvd1YE1SdRON6=mQ_iD2dEr4K9D8YGgeRBQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2020-07-08 21:44 ` Dmitry Osipenko 2020-07-08 21:44 ` Dmitry Osipenko [not found] ` <675c4691-d372-4fe1-d515-c86fdba2f588-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2020-07-09 9:07 ` Andy Shevchenko 2020-07-09 9:07 ` Andy Shevchenko [not found] ` <CAHp75Vd89QpwaGvkpzG+pxnLd8S2guPCARLW5xPwhxXL8ZRfFw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2020-07-09 10:38 ` Dmitry Osipenko 2020-07-09 10:38 ` Dmitry Osipenko [not found] ` <20200708202355.28507-4-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2020-07-09 14:58 ` Laxman Dewangan 2020-07-09 14:58 ` Laxman Dewangan 2020-07-08 20:23 ` [PATCH v3 6/6] gpio: max77620: Initialize hardware state of interrupts Dmitry Osipenko 2020-07-08 20:23 ` Dmitry Osipenko 2020-07-09 15:03 ` Laxman Dewangan 2020-07-09 15:03 ` Laxman Dewangan 2020-07-08 20:23 ` [PATCH v3 4/6] gpio: max77620: Don't shadow error code of platform_get_irq() Dmitry Osipenko [not found] ` <20200708202355.28507-5-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2020-07-09 15:01 ` Laxman Dewangan 2020-07-09 15:01 ` Laxman Dewangan 2020-07-08 20:23 ` [PATCH v3 5/6] gpio: max77620: Use irqchip template Dmitry Osipenko [not found] ` <20200708202355.28507-6-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2020-07-09 15:03 ` Laxman Dewangan 2020-07-09 15:03 ` Laxman Dewangan [not found] ` <CAHp75VcNao84UajMYDJRH2gX7t8n=uo_Th8VVgcxJj3YkiA+pA@mail.gmail.com> [not found] ` <CAHp75VcNao84UajMYDJRH2gX7t8n=uo_Th8VVgcxJj3YkiA+pA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2020-07-08 21:49 ` [PATCH v1 0/5] Improvements for MAX77620 GPIO driver Dmitry Osipenko 2020-07-08 21:49 ` Dmitry Osipenko
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.