| Message ID | 20240611171509.22151-1-asmaa@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/1] gpio: mlxbf3: Support shutdown() function | expand |
On Tue, Jun 11, 2024 at 8:15 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote: > > During Linux graceful reboot, the GPIO interrupts are not disabled. > Since the drivers are not removed during graceful reboot, > the logic to call mlxbf3_gpio_irq_disable() is not triggered. > Interrupts that remain enabled can cause issues on subsequent boots. > > For example, the mlxbf-gige driver contains PHY logic to bring up the link. > If the gpio-mlxbf3 driver loads first, the mlxbf-gige driver > will use a GPIO interrupt to bring up the link. > Otherwise, it will use polling. > The next time Linux boots and loads the drivers in this order, we encounter the issue: > - mlxbf-gige loads first and uses polling while the GPIO10 > interrupt is still enabled from the previous boot. So if > the interrupt triggers, there is nothing to clear it. > - gpio-mlxbf3 loads. > - i2c-mlxbf loads. The interrupt doesn't trigger for I2C > because it is shared with the GPIO interrupt line which > was not cleared. > > The solution is to add a shutdown function to the GPIO driver to clear and disable > all interrupts. Also clear the interrupt after disabling it in mlxbf3_gpio_irq_disable(). LGTM, FWIW, Reviewed-by: Andy Shevchenko <andy@kernel.org>
On Tue, Jun 11, 2024 at 7:15 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote: > During Linux graceful reboot, the GPIO interrupts are not disabled. > Since the drivers are not removed during graceful reboot, > the logic to call mlxbf3_gpio_irq_disable() is not triggered. > Interrupts that remain enabled can cause issues on subsequent boots. > > For example, the mlxbf-gige driver contains PHY logic to bring up the link. > If the gpio-mlxbf3 driver loads first, the mlxbf-gige driver > will use a GPIO interrupt to bring up the link. > Otherwise, it will use polling. > The next time Linux boots and loads the drivers in this order, we encounter the issue: > - mlxbf-gige loads first and uses polling while the GPIO10 > interrupt is still enabled from the previous boot. So if > the interrupt triggers, there is nothing to clear it. > - gpio-mlxbf3 loads. > - i2c-mlxbf loads. The interrupt doesn't trigger for I2C > because it is shared with the GPIO interrupt line which > was not cleared. > > The solution is to add a shutdown function to the GPIO driver to clear and disable > all interrupts. Also clear the interrupt after disabling it in mlxbf3_gpio_irq_disable(). > > Fixes: 38a700efc510 ("gpio: mlxbf3: Add gpio driver support") > Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> > Reviewed-by: David Thompson <davthompson@nvidia.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
Thank you Andy and Linus for approving my patch! Hi Bart, could you please apply this patch? Thank you, Asmaa > -----Original Message----- > From: Linus Walleij <linus.walleij@linaro.org> > Sent: Monday, June 17, 2024 5:05 AM > To: Asmaa Mnebhi <asmaa@nvidia.com> > Cc: andy.shevchenko@gmail.com; bgolaszewski@baylibre.com; linux- > gpio@vger.kernel.org; David Thompson <davthompson@nvidia.com> > Subject: Re: [PATCH v2 1/1] gpio: mlxbf3: Support shutdown() function > Importance: High > > On Tue, Jun 11, 2024 at 7:15 PM Asmaa Mnebhi <asmaa@nvidia.com> > wrote: > > > During Linux graceful reboot, the GPIO interrupts are not disabled. > > Since the drivers are not removed during graceful reboot, the logic to > > call mlxbf3_gpio_irq_disable() is not triggered. > > Interrupts that remain enabled can cause issues on subsequent boots. > > > > For example, the mlxbf-gige driver contains PHY logic to bring up the link. > > If the gpio-mlxbf3 driver loads first, the mlxbf-gige driver will use > > a GPIO interrupt to bring up the link. > > Otherwise, it will use polling. > > The next time Linux boots and loads the drivers in this order, we > encounter the issue: > > - mlxbf-gige loads first and uses polling while the GPIO10 > > interrupt is still enabled from the previous boot. So if > > the interrupt triggers, there is nothing to clear it. > > - gpio-mlxbf3 loads. > > - i2c-mlxbf loads. The interrupt doesn't trigger for I2C > > because it is shared with the GPIO interrupt line which > > was not cleared. > > > > The solution is to add a shutdown function to the GPIO driver to clear > > and disable all interrupts. Also clear the interrupt after disabling it in > mlxbf3_gpio_irq_disable(). > > > > Fixes: 38a700efc510 ("gpio: mlxbf3: Add gpio driver support") > > Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> > > Reviewed-by: David Thompson <davthompson@nvidia.com> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Yours, > Linus Walleij
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> On Tue, 11 Jun 2024 13:15:09 -0400, Asmaa Mnebhi wrote: > During Linux graceful reboot, the GPIO interrupts are not disabled. > Since the drivers are not removed during graceful reboot, > the logic to call mlxbf3_gpio_irq_disable() is not triggered. > Interrupts that remain enabled can cause issues on subsequent boots. > > For example, the mlxbf-gige driver contains PHY logic to bring up the link. > If the gpio-mlxbf3 driver loads first, the mlxbf-gige driver > will use a GPIO interrupt to bring up the link. > Otherwise, it will use polling. > The next time Linux boots and loads the drivers in this order, we encounter the issue: > - mlxbf-gige loads first and uses polling while the GPIO10 > interrupt is still enabled from the previous boot. So if > the interrupt triggers, there is nothing to clear it. > - gpio-mlxbf3 loads. > - i2c-mlxbf loads. The interrupt doesn't trigger for I2C > because it is shared with the GPIO interrupt line which > was not cleared. > > [...] Applied, thanks! [1/1] gpio: mlxbf3: Support shutdown() function commit: aad41832326723627ad8ac9ee8a543b6dca4454d Best regards,
On Fri, Aug 9, 2024 at 10:25 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote: > > Thank you Andy and Linus for approving my patch! > > Hi Bart, could you please apply this patch? > > Thank you, > Asmaa > Sorry, it got lost in my inbox. Bart
Thank you very much! > -----Original Message----- > From: Bartosz Golaszewski <brgl@bgdev.pl> > Sent: Saturday, August 10, 2024 3:36 PM > To: andy.shevchenko@gmail.com; linus.walleij@linaro.org; linux- > gpio@vger.kernel.org; Bartosz Golaszewski <brgl@bgdev.pl>; Asmaa Mnebhi > <asmaa@nvidia.com> > Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>; David Thompson > <davthompson@nvidia.com> > Subject: Re: [PATCH v2 1/1] gpio: mlxbf3: Support shutdown() function > Importance: High > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > On Tue, 11 Jun 2024 13:15:09 -0400, Asmaa Mnebhi wrote: > > During Linux graceful reboot, the GPIO interrupts are not disabled. > > Since the drivers are not removed during graceful reboot, the logic to > > call mlxbf3_gpio_irq_disable() is not triggered. > > Interrupts that remain enabled can cause issues on subsequent boots. > > > > For example, the mlxbf-gige driver contains PHY logic to bring up the link. > > If the gpio-mlxbf3 driver loads first, the mlxbf-gige driver will use > > a GPIO interrupt to bring up the link. > > Otherwise, it will use polling. > > The next time Linux boots and loads the drivers in this order, we > encounter the issue: > > - mlxbf-gige loads first and uses polling while the GPIO10 > > interrupt is still enabled from the previous boot. So if > > the interrupt triggers, there is nothing to clear it. > > - gpio-mlxbf3 loads. > > - i2c-mlxbf loads. The interrupt doesn't trigger for I2C > > because it is shared with the GPIO interrupt line which > > was not cleared. > > > > [...] > > Applied, thanks! > > [1/1] gpio: mlxbf3: Support shutdown() function > commit: aad41832326723627ad8ac9ee8a543b6dca4454d > > Best regards, > -- > Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
diff --git a/drivers/gpio/gpio-mlxbf3.c b/drivers/gpio/gpio-mlxbf3.c index d5906d419b0a..10ea71273c89 100644 --- a/drivers/gpio/gpio-mlxbf3.c +++ b/drivers/gpio/gpio-mlxbf3.c @@ -39,6 +39,8 @@ #define MLXBF_GPIO_CAUSE_OR_EVTEN0 0x14 #define MLXBF_GPIO_CAUSE_OR_CLRCAUSE 0x18 +#define MLXBF_GPIO_CLR_ALL_INTS GENMASK(31, 0) + struct mlxbf3_gpio_context { struct gpio_chip gc; @@ -82,6 +84,8 @@ static void mlxbf3_gpio_irq_disable(struct irq_data *irqd) val = readl(gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0); val &= ~BIT(offset); writel(val, gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0); + + writel(BIT(offset), gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_CLRCAUSE); raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags); gpiochip_disable_irq(gc, offset); @@ -253,6 +257,15 @@ static int mlxbf3_gpio_probe(struct platform_device *pdev) return 0; } +static void mlxbf3_gpio_shutdown(struct platform_device *pdev) +{ + struct mlxbf3_gpio_context *gs = platform_get_drvdata(pdev); + + /* Disable and clear all interrupts */ + writel(0, gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_EVTEN0); + writel(MLXBF_GPIO_CLR_ALL_INTS, gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_CLRCAUSE); +} + static const struct acpi_device_id mlxbf3_gpio_acpi_match[] = { { "MLNXBF33", 0 }, {} @@ -265,6 +278,7 @@ static struct platform_driver mlxbf3_gpio_driver = { .acpi_match_table = mlxbf3_gpio_acpi_match, }, .probe = mlxbf3_gpio_probe, + .shutdown = mlxbf3_gpio_shutdown, }; module_platform_driver(mlxbf3_gpio_driver);
During Linux graceful reboot, the GPIO interrupts are not disabled. Since the drivers are not removed during graceful reboot, the logic to call mlxbf3_gpio_irq_disable() is not triggered. Interrupts that remain enabled can cause issues on subsequent boots. For example, the mlxbf-gige driver contains PHY logic to bring up the link. If the gpio-mlxbf3 driver loads first, the mlxbf-gige driver will use a GPIO interrupt to bring up the link. Otherwise, it will use polling. The next time Linux boots and loads the drivers in this order, we encounter the issue: - mlxbf-gige loads first and uses polling while the GPIO10 interrupt is still enabled from the previous boot. So if the interrupt triggers, there is nothing to clear it. - gpio-mlxbf3 loads. - i2c-mlxbf loads. The interrupt doesn't trigger for I2C because it is shared with the GPIO interrupt line which was not cleared. The solution is to add a shutdown function to the GPIO driver to clear and disable all interrupts. Also clear the interrupt after disabling it in mlxbf3_gpio_irq_disable(). Fixes: 38a700efc510 ("gpio: mlxbf3: Add gpio driver support") Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> Reviewed-by: David Thompson <davthompson@nvidia.com> --- v1->v2: - Change 'gpio' to 'GPIO' - Use GENMASK() - Use platform_get_drvdata() drivers/gpio/gpio-mlxbf3.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)