diff mbox series

[v1,v/1] gpio: mlxbf: enable GPIO interface and ACPI event for host-gpio[7]

Message ID 20230203184907.18786-1-davthompson@nvidia.com
State New
Headers show
Series [v1,v/1] gpio: mlxbf: enable GPIO interface and ACPI event for host-gpio[7] | expand

Commit Message

David Thompson Feb. 3, 2023, 6:49 p.m. UTC 
This commit adds ACPI handling for host-gpio[7] to trigger
the power-button event.

Signed-off-by: David Thompson <davthompson@nvidia.com>
Reviewed-by: Liming Sun <limings@nvidia.com>
---
 drivers/gpio/gpio-mlxbf.c | 65 +++++++++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 10 deletions(-)

Comments

Andy Shevchenko Feb. 9, 2023, 9:59 p.m. UTC | #1 
On Fri, Feb 03, 2023 at 01:49:07PM -0500, David Thompson wrote:
> This commit adds ACPI handling for host-gpio[7] to trigger
> the power-button event.

This has something to discuss...

...

>  /* Pad Electrical Controls. */
> -#define MLXBF_GPIO_PAD_CONTROL_FIRST_WORD 0x0700
> -#define MLXBF_GPIO_PAD_CONTROL_1_FIRST_WORD 0x0708
> -#define MLXBF_GPIO_PAD_CONTROL_2_FIRST_WORD 0x0710
> -#define MLXBF_GPIO_PAD_CONTROL_3_FIRST_WORD 0x0718
> +#define MLXBF_GPIO_PAD_CONTROL_FIRST_WORD	0x0700
> +#define MLXBF_GPIO_PAD_CONTROL_1_FIRST_WORD	0x0708
> +#define MLXBF_GPIO_PAD_CONTROL_2_FIRST_WORD	0x0710
> +#define MLXBF_GPIO_PAD_CONTROL_3_FIRST_WORD	0x0718

Unrelated change.

> +#define MLXBF_GPIO_PIN_DIR_I                    0x1040
> +#define MLXBF_GPIO_PIN_DIR_O                    0x1048
> +#define MLXBF_GPIO_PIN_STATE                    0x1000
> +#define MLXBF_GPIO_SCRATCHPAD                   0x20

Ditto. You already have it defined, why to change?

> -#define MLXBF_GPIO_PIN_DIR_I 0x1040
> -#define MLXBF_GPIO_PIN_DIR_O 0x1048
> -#define MLXBF_GPIO_PIN_STATE 0x1000
> -#define MLXBF_GPIO_SCRATCHPAD 0x20

...

> +	gs->hwirq = irq;
> +	gc->to_irq = mlxbf_gpio_to_irq;

This sounds incorrect. Seems like for _any_ interrupt you will give the same
result. Moreover, you should not use to_irq(). Try to model proper IRQ chip.

...

>  	platform_set_drvdata(pdev, gs);
> +	acpi_gpiochip_request_interrupts(gc);

This is done by GPIO library, no?

...

>  	dev_info(&pdev->dev, "registered Mellanox BlueField GPIO");
> +
>  	return 0;

Stray change.
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-mlxbf.c b/drivers/gpio/gpio-mlxbf.c
index 1fa9973f55b9..a8b6117c7593 100644
--- a/drivers/gpio/gpio-mlxbf.c
+++ b/drivers/gpio/gpio-mlxbf.c
@@ -1,6 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/acpi.h>
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/device.h>
 #include <linux/gpio/driver.h>
@@ -12,19 +13,30 @@ 
 #include <linux/resource.h>
 #include <linux/types.h>
 
+#include "gpiolib-acpi.h"
+
 /* Number of pins on BlueField */
 #define MLXBF_GPIO_NR 54
 
 /* Pad Electrical Controls. */
-#define MLXBF_GPIO_PAD_CONTROL_FIRST_WORD 0x0700
-#define MLXBF_GPIO_PAD_CONTROL_1_FIRST_WORD 0x0708
-#define MLXBF_GPIO_PAD_CONTROL_2_FIRST_WORD 0x0710
-#define MLXBF_GPIO_PAD_CONTROL_3_FIRST_WORD 0x0718
+#define MLXBF_GPIO_PAD_CONTROL_FIRST_WORD	0x0700
+#define MLXBF_GPIO_PAD_CONTROL_1_FIRST_WORD	0x0708
+#define MLXBF_GPIO_PAD_CONTROL_2_FIRST_WORD	0x0710
+#define MLXBF_GPIO_PAD_CONTROL_3_FIRST_WORD	0x0718
+
+#define MLXBF_GPIO_PIN_DIR_I                    0x1040
+#define MLXBF_GPIO_PIN_DIR_O                    0x1048
+#define MLXBF_GPIO_PIN_STATE                    0x1000
+#define MLXBF_GPIO_SCRATCHPAD                   0x20
+#define MLXBF_GPIO_INT_SETUP                    0x0040
+#define MLXBF_GPIO_INT_SETUP_GBL_ENA_MASK       0x1
+
+/* GPIO pin 7 is reserved for power good indication on BlueField. */
+#define MLXBF_GPIO_POWER_GOOD_PIN		7
 
-#define MLXBF_GPIO_PIN_DIR_I 0x1040
-#define MLXBF_GPIO_PIN_DIR_O 0x1048
-#define MLXBF_GPIO_PIN_STATE 0x1000
-#define MLXBF_GPIO_SCRATCHPAD 0x20
+/* GPIO pins enabled for interrupt */
+#define MLXBF_GPIO_INT_ENA_DEASSERT             0x3008
+#define MLXBF_GPIO_INT_DEASSERT_ENA_BITS	BIT(MLXBF_GPIO_POWER_GOOD_PIN)
 
 #ifdef CONFIG_PM
 struct mlxbf_gpio_context_save_regs {
@@ -42,17 +54,39 @@  struct mlxbf_gpio_state {
 	/* Memory Address */
 	void __iomem *base;
 
+	int hwirq;
+
 #ifdef CONFIG_PM
 	struct mlxbf_gpio_context_save_regs csave_regs;
 #endif
 };
 
+static int mlxbf_gpio_to_irq(struct gpio_chip *chip, u32 gpio)
+{
+	struct mlxbf_gpio_state *gs = gpiochip_get_data(chip);
+
+	return gs->hwirq;
+}
+
+static void mlxbf_gpio_cfg_irq(struct mlxbf_gpio_state *gs)
+{
+	u64 intr_cfg;
+
+	intr_cfg = readq(gs->base + MLXBF_GPIO_INT_SETUP);
+	intr_cfg |= MLXBF_GPIO_INT_SETUP_GBL_ENA_MASK;
+	writeq(intr_cfg, gs->base + MLXBF_GPIO_INT_SETUP);
+
+	intr_cfg = readq(gs->base + MLXBF_GPIO_INT_ENA_DEASSERT);
+	intr_cfg |= MLXBF_GPIO_INT_DEASSERT_ENA_BITS;
+	writeq(intr_cfg, gs->base + MLXBF_GPIO_INT_ENA_DEASSERT);
+}
+
 static int mlxbf_gpio_probe(struct platform_device *pdev)
 {
-	struct mlxbf_gpio_state *gs;
 	struct device *dev = &pdev->dev;
+	struct mlxbf_gpio_state *gs;
 	struct gpio_chip *gc;
-	int ret;
+	int ret, irq;
 
 	gs = devm_kzalloc(&pdev->dev, sizeof(*gs), GFP_KERNEL);
 	if (!gs)
@@ -63,6 +97,7 @@  static int mlxbf_gpio_probe(struct platform_device *pdev)
 		return PTR_ERR(gs->base);
 
 	gc = &gs->gc;
+
 	ret = bgpio_init(gc, dev, 8,
 			 gs->base + MLXBF_GPIO_PIN_STATE,
 			 NULL,
@@ -73,8 +108,16 @@  static int mlxbf_gpio_probe(struct platform_device *pdev)
 	if (ret)
 		return -ENODEV;
 
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
 	gc->owner = THIS_MODULE;
 	gc->ngpio = MLXBF_GPIO_NR;
+	gs->hwirq = irq;
+	gc->to_irq = mlxbf_gpio_to_irq;
+
+	mlxbf_gpio_cfg_irq(gs);
 
 	ret = devm_gpiochip_add_data(dev, &gs->gc, gs);
 	if (ret) {
@@ -83,7 +126,9 @@  static int mlxbf_gpio_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, gs);
+	acpi_gpiochip_request_interrupts(gc);
 	dev_info(&pdev->dev, "registered Mellanox BlueField GPIO");
+
 	return 0;
 }