gpio: mlxbf3: Support shutdown() function

PATCH Applied Asmaa Mnebhi 4 participants 7 messages Jun 11, 2024 – Aug 11, 2024 GPIO

Share & tools

Asmaa Mnebhi

author

2024-06-11 17:15 UTC #0 lore raw

PATCH v2 1/1 gpio: mlxbf3: Support shutdown() function

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 <[email protected]>

Reviewed-by David Thompson <[email protected]>

---
v1->v2:
- Change 'gpio' to 'GPIO'
- Use GENMASK()
- Use platform_get_drvdata()

 drivers/gpio/gpio-mlxbf3.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

drivers/gpio/gpio-mlxbf3.c +14 -0

index d5906d419b0a..10ea71273c89 100644
@@ -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);

Signature · 1 line

2.30.1

Andy Shevchenko

Reviewed-by on v2 1/1

2024-06-11 17:29 UTC #1 lore raw

Quoted 19 lines · Asmaa Mnebhi

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.

Quoted 2 lines

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 <[email protected]>

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Linus Walleij

Reviewed-by on v2 1/1

2024-06-17 09:05 UTC #2 lore raw

In reply to Asmaa Mnebhi

Quoted 18 lines · Asmaa Mnebhi

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.

Quoted 6 lines

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 <[email protected]>
Reviewed-by David Thompson <[email protected]>

Reviewed-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

Asmaa Mnebhi

author

2024-08-09 20:25 UTC #3 lore raw

Thank you Andy and Linus for approving my patch!

Hi Bart, could you please apply this patch?

Thank you,
Asmaa

Quoted 43 lines

-----Original Message-----
From: Linus Walleij <[email protected]>
Sent: Monday, June 17, 2024 5:05 AM
To: Asmaa Mnebhi <[email protected]>
Cc: [email protected]; [email protected]; linux- [email protected]; David Thompson <[email protected]>
Subject: Re: [PATCH v2 1/1] gpio: mlxbf3: Support shutdown() function
Importance: High
Quoted 10 lines · Asmaa Mnebhi
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:
Quoted 10 lines
- 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().
Quoted 4 lines
Fixes 38a700efc510 ("gpio: mlxbf3: Add gpio driver support")
Signed-off-by Asmaa Mnebhi <[email protected]>
Reviewed-by David Thompson <[email protected]>
Reviewed-by Linus Walleij <[email protected]>
Yours,
Linus Walleij

Bartosz Golaszewski

2024-08-10 19:36 UTC #4 lore raw

Quoted 8 lines · Asmaa Mnebhi

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

Bartosz Golaszewski

Applied

2024-08-10 19:35 UTC #5 lore raw

In reply to Asmaa Mnebhi

From: Bartosz Golaszewski <[email protected]>

Quoted 18 lines · Asmaa Mnebhi

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.

Quoted 1 line

[...]

Applied, thanks!

[1/1] gpio: mlxbf3: Support shutdown() function

      commit: aad41832326723627ad8ac9ee8a543b6dca4454d

Best regards,

Signature · 1 line

Bartosz Golaszewski <[email protected]>

Asmaa Mnebhi

author

2024-08-11 22:04 UTC #6 lore raw

Thank you very much!

Quoted 44 lines

-----Original Message-----
From: Bartosz Golaszewski <[email protected]>
Sent: Saturday, August 10, 2024 3:36 PM
To: [email protected]; [email protected]; linux- [email protected]; Bartosz Golaszewski <[email protected]>; Asmaa Mnebhi <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>; David Thompson <[email protected]>
Subject: Re: [PATCH v2 1/1] gpio: mlxbf3: Support shutdown() function
Importance: High
From: Bartosz Golaszewski <[email protected]>
Quoted 10 lines · Asmaa Mnebhi
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:
Quoted 9 lines
- 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 <[email protected]>