diff mbox series

[v4,1/2] gpio: mlxbf2: Introduce IRQ support

Message ID 20211008144920.10975-2-asmaa@nvidia.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series gpio: mlxbf2: Introduce proper interrupt handling | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Asmaa Mnebhi Oct. 8, 2021, 2:49 p.m. UTC 
Introduce standard IRQ handling in the gpio-mlxbf2.c
driver.

Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 drivers/gpio/gpio-mlxbf2.c | 147 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 145 insertions(+), 2 deletions(-)

Comments

Bartosz Golaszewski Oct. 15, 2021, 7:01 a.m. UTC | #1 
On Fri, Oct 8, 2021 at 4:50 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> Introduce standard IRQ handling in the gpio-mlxbf2.c
> driver.
>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> ---
>  drivers/gpio/gpio-mlxbf2.c | 147 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 145 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mlxbf2.c b/drivers/gpio/gpio-mlxbf2.c
> index 177d03ef4529..3d89912a05b8 100644
> --- a/drivers/gpio/gpio-mlxbf2.c
> +++ b/drivers/gpio/gpio-mlxbf2.c
> @@ -1,9 +1,14 @@
>  // SPDX-License-Identifier: GPL-2.0
>
> +/*
> + * Copyright (C) 2020-2021 NVIDIA CORPORATION & AFFILIATES
> + */
> +
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/device.h>
>  #include <linux/gpio/driver.h>
> +#include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/ioport.h>
>  #include <linux/kernel.h>
> @@ -43,9 +48,14 @@
>  #define YU_GPIO_MODE0                  0x0c
>  #define YU_GPIO_DATASET                        0x14
>  #define YU_GPIO_DATACLEAR              0x18
> +#define YU_GPIO_CAUSE_RISE_EN          0x44
> +#define YU_GPIO_CAUSE_FALL_EN          0x48
>  #define YU_GPIO_MODE1_CLEAR            0x50
>  #define YU_GPIO_MODE0_SET              0x54
>  #define YU_GPIO_MODE0_CLEAR            0x58
> +#define YU_GPIO_CAUSE_OR_CAUSE_EVTEN0  0x80
> +#define YU_GPIO_CAUSE_OR_EVTEN0                0x94
> +#define YU_GPIO_CAUSE_OR_CLRCAUSE      0x98
>
>  struct mlxbf2_gpio_context_save_regs {
>         u32 gpio_mode0;
> @@ -55,6 +65,7 @@ struct mlxbf2_gpio_context_save_regs {
>  /* BlueField-2 gpio block context structure. */
>  struct mlxbf2_gpio_context {
>         struct gpio_chip gc;
> +       struct irq_chip irq_chip;
>
>         /* YU GPIO blocks address */
>         void __iomem *gpio_io;
> @@ -218,15 +229,114 @@ static int mlxbf2_gpio_direction_output(struct gpio_chip *chip,
>         return ret;
>  }
>
> +static void mlxbf2_gpio_irq_enable(struct irq_data *irqd)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +       struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc);
> +       int offset = irqd_to_hwirq(irqd);
> +       unsigned long flags;
> +       u32 val;
> +
> +       spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
> +       val = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_CLRCAUSE);
> +       val |= BIT(offset);
> +       writel(val, gs->gpio_io + YU_GPIO_CAUSE_OR_CLRCAUSE);
> +
> +       val = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0);
> +       val |= BIT(offset);
> +       writel(val, gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0);
> +       spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
> +}
> +
> +static void mlxbf2_gpio_irq_disable(struct irq_data *irqd)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +       struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc);
> +       int offset = irqd_to_hwirq(irqd);
> +       unsigned long flags;
> +       u32 val;
> +
> +       spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
> +       val = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0);
> +       val &= ~BIT(offset);
> +       writel(val, gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0);
> +       spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
> +}
> +
> +static irqreturn_t mlxbf2_gpio_irq_handler(int irq, void *ptr)
> +{
> +       struct mlxbf2_gpio_context *gs = ptr;
> +       struct gpio_chip *gc = &gs->gc;
> +       unsigned long pending;
> +       u32 level;
> +
> +       pending = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_CAUSE_EVTEN0);
> +       writel(pending, gs->gpio_io + YU_GPIO_CAUSE_OR_CLRCAUSE);
> +
> +       for_each_set_bit(level, &pending, gc->ngpio) {
> +               int gpio_irq = irq_find_mapping(gc->irq.domain, level);
> +               generic_handle_irq(gpio_irq);
> +       }
> +
> +       return IRQ_RETVAL(pending);
> +}
> +
> +static int
> +mlxbf2_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +       struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc);
> +       int offset = irqd_to_hwirq(irqd);
> +       unsigned long flags;
> +       bool fall = false;
> +       bool rise = false;
> +       u32 val;
> +
> +       switch (type & IRQ_TYPE_SENSE_MASK) {
> +       case IRQ_TYPE_EDGE_BOTH:
> +               fall = true;
> +               rise = true;
> +               break;
> +       case IRQ_TYPE_EDGE_RISING:
> +               rise = true;
> +               break;
> +       case IRQ_TYPE_EDGE_FALLING:
> +               fall = true;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
> +       if (fall) {
> +               val = readl(gs->gpio_io + YU_GPIO_CAUSE_FALL_EN);
> +               val |= BIT(offset);
> +               writel(val, gs->gpio_io + YU_GPIO_CAUSE_FALL_EN);
> +       }
> +
> +       if (rise) {
> +               val = readl(gs->gpio_io + YU_GPIO_CAUSE_RISE_EN);
> +               val |= BIT(offset);
> +               writel(val, gs->gpio_io + YU_GPIO_CAUSE_RISE_EN);
> +       }
> +       spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
> +
> +       return 0;
> +}
> +
>  /* BlueField-2 GPIO driver initialization routine. */
>  static int
>  mlxbf2_gpio_probe(struct platform_device *pdev)
>  {
>         struct mlxbf2_gpio_context *gs;
>         struct device *dev = &pdev->dev;
> +       struct gpio_irq_chip *girq;
>         struct gpio_chip *gc;
>         unsigned int npins;
> -       int ret;
> +       const char *name;
> +       int ret, irq;
> +
> +       name = dev_name(dev);
>
>         gs = devm_kzalloc(dev, sizeof(*gs), GFP_KERNEL);
>         if (!gs)
> @@ -256,11 +366,44 @@ mlxbf2_gpio_probe(struct platform_device *pdev)
>                         NULL,
>                         0);
>
> +       if (ret) {
> +               dev_err(dev, "bgpio_init failed\n");
> +               return ret;
> +       }

This is a correct fix but it should be sent as a fix aimed for stable
in a separate branch, as we want that to be backported.

Other than that it looks good to me, which tree do you want it to go through?

Bart

> +
>         gc->direction_input = mlxbf2_gpio_direction_input;
>         gc->direction_output = mlxbf2_gpio_direction_output;
>         gc->ngpio = npins;
>         gc->owner = THIS_MODULE;
>
> +       irq = platform_get_irq(pdev, 0);
> +       if (irq >= 0) {
> +               gs->irq_chip.name = name;
> +               gs->irq_chip.irq_set_type = mlxbf2_gpio_irq_set_type;
> +               gs->irq_chip.irq_enable = mlxbf2_gpio_irq_enable;
> +               gs->irq_chip.irq_disable = mlxbf2_gpio_irq_disable;
> +
> +               girq = &gs->gc.irq;
> +               girq->chip = &gs->irq_chip;
> +               girq->handler = handle_simple_irq;
> +               girq->default_type = IRQ_TYPE_NONE;
> +               /* This will let us handle the parent IRQ in the driver */
> +               girq->num_parents = 0;
> +               girq->parents = NULL;
> +               girq->parent_handler = NULL;
> +
> +               /*
> +                * Directly request the irq here instead of passing
> +                * a flow-handler because the irq is shared.
> +                */
> +               ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler,
> +                                      IRQF_SHARED, name, gs);
> +               if (ret) {
> +                       dev_err(dev, "failed to request IRQ");
> +                       return ret;
> +               }
> +       }
> +
>         platform_set_drvdata(pdev, gs);
>
>         ret = devm_gpiochip_add_data(dev, &gs->gc, gs);
> @@ -315,5 +458,5 @@ static struct platform_driver mlxbf2_gpio_driver = {
>  module_platform_driver(mlxbf2_gpio_driver);
>
>  MODULE_DESCRIPTION("Mellanox BlueField-2 GPIO Driver");
> -MODULE_AUTHOR("Mellanox Technologies");
> +MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>");
>  MODULE_LICENSE("GPL v2");
> --
> 2.30.1
>
Asmaa Mnebhi Oct. 15, 2021, 3:44 p.m. UTC | #2 
>  /* BlueField-2 GPIO driver initialization routine. */  static int  
> mlxbf2_gpio_probe(struct platform_device *pdev)  {
>         struct mlxbf2_gpio_context *gs;
>         struct device *dev = &pdev->dev;
> +       struct gpio_irq_chip *girq;
>         struct gpio_chip *gc;
>         unsigned int npins;
> -       int ret;
> +       const char *name;
> +       int ret, irq;
> +
> +       name = dev_name(dev);
>
>         gs = devm_kzalloc(dev, sizeof(*gs), GFP_KERNEL);
>         if (!gs)
> @@ -256,11 +366,44 @@ mlxbf2_gpio_probe(struct platform_device *pdev)
>                         NULL,
>                         0);
>
> +       if (ret) {
> +               dev_err(dev, "bgpio_init failed\n");
> +               return ret;
> +       }

> This is a correct fix but it should be sent as a fix aimed for stable in a separate branch, as we want that to be backported.
Ok! So I will send v5 patch, leaving these 4 lines out, then will add this small fix to a separate patch aimed to the stable branch.

> Other than that it looks good to me, which tree do you want it to go through?

Since the mlxbf-gige driver is dependent on this commit, I would like to push both this gpio
commit and
"[PATCH v4 2/2] net: mellanox: mlxbf_gige: Replace non-standard interrupt handling"
to the same tree/branch.

I apologize I am not very familiar with the push process when 2 dependent commits
target different drivers. So any input is greatly appreciated!
Since the mlxbf_gige driver is targeting the net master branch, would it be possible to push
The Gpio change as well to net? 
If this is not possible, maybe we can push the commit to linux-gpio master and
The mlxbf_gige to the net branch. But we will need to sync the timing for when both
requests go to the mainline.

David Miller, Andrew Lunn, I know you have acked/reviewed v1 and v2 patches for
the mlxbf_gige driver. I just want to make sure you are still ok with this implementation.
I mentioned in a previous email that our management/product manager approved using 
PHY interrupt instead of polling.

Thank you.
Asmaa
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-mlxbf2.c b/drivers/gpio/gpio-mlxbf2.c
index 177d03ef4529..3d89912a05b8 100644
--- a/drivers/gpio/gpio-mlxbf2.c
+++ b/drivers/gpio/gpio-mlxbf2.c
@@ -1,9 +1,14 @@ 
 // SPDX-License-Identifier: GPL-2.0
 
+/*
+ * Copyright (C) 2020-2021 NVIDIA CORPORATION & AFFILIATES
+ */
+
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/device.h>
 #include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/kernel.h>
@@ -43,9 +48,14 @@ 
 #define YU_GPIO_MODE0			0x0c
 #define YU_GPIO_DATASET			0x14
 #define YU_GPIO_DATACLEAR		0x18
+#define YU_GPIO_CAUSE_RISE_EN		0x44
+#define YU_GPIO_CAUSE_FALL_EN		0x48
 #define YU_GPIO_MODE1_CLEAR		0x50
 #define YU_GPIO_MODE0_SET		0x54
 #define YU_GPIO_MODE0_CLEAR		0x58
+#define YU_GPIO_CAUSE_OR_CAUSE_EVTEN0	0x80
+#define YU_GPIO_CAUSE_OR_EVTEN0		0x94
+#define YU_GPIO_CAUSE_OR_CLRCAUSE	0x98
 
 struct mlxbf2_gpio_context_save_regs {
 	u32 gpio_mode0;
@@ -55,6 +65,7 @@  struct mlxbf2_gpio_context_save_regs {
 /* BlueField-2 gpio block context structure. */
 struct mlxbf2_gpio_context {
 	struct gpio_chip gc;
+	struct irq_chip irq_chip;
 
 	/* YU GPIO blocks address */
 	void __iomem *gpio_io;
@@ -218,15 +229,114 @@  static int mlxbf2_gpio_direction_output(struct gpio_chip *chip,
 	return ret;
 }
 
+static void mlxbf2_gpio_irq_enable(struct irq_data *irqd)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc);
+	int offset = irqd_to_hwirq(irqd);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
+	val = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_CLRCAUSE);
+	val |= BIT(offset);
+	writel(val, gs->gpio_io + YU_GPIO_CAUSE_OR_CLRCAUSE);
+
+	val = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0);
+	val |= BIT(offset);
+	writel(val, gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0);
+	spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
+}
+
+static void mlxbf2_gpio_irq_disable(struct irq_data *irqd)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc);
+	int offset = irqd_to_hwirq(irqd);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
+	val = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0);
+	val &= ~BIT(offset);
+	writel(val, gs->gpio_io + YU_GPIO_CAUSE_OR_EVTEN0);
+	spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
+}
+
+static irqreturn_t mlxbf2_gpio_irq_handler(int irq, void *ptr)
+{
+	struct mlxbf2_gpio_context *gs = ptr;
+	struct gpio_chip *gc = &gs->gc;
+	unsigned long pending;
+	u32 level;
+
+	pending = readl(gs->gpio_io + YU_GPIO_CAUSE_OR_CAUSE_EVTEN0);
+	writel(pending, gs->gpio_io + YU_GPIO_CAUSE_OR_CLRCAUSE);
+
+	for_each_set_bit(level, &pending, gc->ngpio) {
+		int gpio_irq = irq_find_mapping(gc->irq.domain, level);
+		generic_handle_irq(gpio_irq);
+	}
+
+	return IRQ_RETVAL(pending);
+}
+
+static int
+mlxbf2_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
+	struct mlxbf2_gpio_context *gs = gpiochip_get_data(gc);
+	int offset = irqd_to_hwirq(irqd);
+	unsigned long flags;
+	bool fall = false;
+	bool rise = false;
+	u32 val;
+
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_EDGE_BOTH:
+		fall = true;
+		rise = true;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		rise = true;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		fall = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
+	if (fall) {
+		val = readl(gs->gpio_io + YU_GPIO_CAUSE_FALL_EN);
+		val |= BIT(offset);
+		writel(val, gs->gpio_io + YU_GPIO_CAUSE_FALL_EN);
+	}
+
+	if (rise) {
+		val = readl(gs->gpio_io + YU_GPIO_CAUSE_RISE_EN);
+		val |= BIT(offset);
+		writel(val, gs->gpio_io + YU_GPIO_CAUSE_RISE_EN);
+	}
+	spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
+
+	return 0;
+}
+
 /* BlueField-2 GPIO driver initialization routine. */
 static int
 mlxbf2_gpio_probe(struct platform_device *pdev)
 {
 	struct mlxbf2_gpio_context *gs;
 	struct device *dev = &pdev->dev;
+	struct gpio_irq_chip *girq;
 	struct gpio_chip *gc;
 	unsigned int npins;
-	int ret;
+	const char *name;
+	int ret, irq;
+
+	name = dev_name(dev);
 
 	gs = devm_kzalloc(dev, sizeof(*gs), GFP_KERNEL);
 	if (!gs)
@@ -256,11 +366,44 @@  mlxbf2_gpio_probe(struct platform_device *pdev)
 			NULL,
 			0);
 
+	if (ret) {
+		dev_err(dev, "bgpio_init failed\n");
+		return ret;
+	}
+
 	gc->direction_input = mlxbf2_gpio_direction_input;
 	gc->direction_output = mlxbf2_gpio_direction_output;
 	gc->ngpio = npins;
 	gc->owner = THIS_MODULE;
 
+	irq = platform_get_irq(pdev, 0);
+	if (irq >= 0) {
+		gs->irq_chip.name = name;
+		gs->irq_chip.irq_set_type = mlxbf2_gpio_irq_set_type;
+		gs->irq_chip.irq_enable = mlxbf2_gpio_irq_enable;
+		gs->irq_chip.irq_disable = mlxbf2_gpio_irq_disable;
+
+		girq = &gs->gc.irq;
+		girq->chip = &gs->irq_chip;
+		girq->handler = handle_simple_irq;
+		girq->default_type = IRQ_TYPE_NONE;
+		/* This will let us handle the parent IRQ in the driver */
+		girq->num_parents = 0;
+		girq->parents = NULL;
+		girq->parent_handler = NULL;
+
+		/*
+		 * Directly request the irq here instead of passing
+		 * a flow-handler because the irq is shared.
+		 */
+		ret = devm_request_irq(dev, irq, mlxbf2_gpio_irq_handler,
+				       IRQF_SHARED, name, gs);
+		if (ret) {
+			dev_err(dev, "failed to request IRQ");
+			return ret;
+		}
+	}
+
 	platform_set_drvdata(pdev, gs);
 
 	ret = devm_gpiochip_add_data(dev, &gs->gc, gs);
@@ -315,5 +458,5 @@  static struct platform_driver mlxbf2_gpio_driver = {
 module_platform_driver(mlxbf2_gpio_driver);
 
 MODULE_DESCRIPTION("Mellanox BlueField-2 GPIO Driver");
-MODULE_AUTHOR("Mellanox Technologies");
+MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>");
 MODULE_LICENSE("GPL v2");