Messages in this thread | | | From | Linus Walleij <> | Date | Thu, 23 Feb 2023 13:29:28 +0100 | Subject | Re: [PATCH v4 1/2] gpio: gpio-mlxbf3: Add gpio driver support |
| |
Hi Asamaa,
thanks for your patch!
getting a lot better all the time.
On Fri, Feb 17, 2023 at 10:27 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
> Add support for the BlueField-3 SoC GPIO driver. > This driver configures and handles GPIO interrupts. It also enables a user > to manipulate certain GPIO pins via libgpiod tools or other kernel drivers. > The usables pins are defined via the gpio-reserved-ranges property. > > Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
(...) > +config GPIO_MLXBF3 > + tristate "Mellanox BlueField 3 SoC GPIO" > + depends on (MELLANOX_PLATFORM && ARM64 && ACPI) || (64BIT && COMPILE_TEST) > + select GPIO_GENERIC
select GPIOLIB_IRQCHIP
since you use it.
(...) > +static void mlxbf3_gpio_irq_enable(struct irq_data *irqd) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); > + struct mlxbf3_gpio_context *gs = gpiochip_get_data(gc); > + int offset = irqd_to_hwirq(irqd); > + unsigned long flags; > + u32 val; > + > + raw_spin_lock_irqsave(&gs->gc.bgpio_lock, flags); > + writel(BIT(offset), gs->gpio_cause_io + MLXBF_GPIO_CAUSE_OR_CLRCAUSE); > + > + 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); > + raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
Here, at the end (*after* writing registers) call:
gpiochip_disable_irq(gc, offset);
> +static void mlxbf3_gpio_irq_disable(struct irq_data *irqd) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); > + struct mlxbf3_gpio_context *gs = gpiochip_get_data(gc); > + int offset = irqd_to_hwirq(irqd); > + unsigned long flags; > + u32 val; > +
Here, at the beginning (*before* writing registers) call:
gpiochip_disable_irq(gc, offset);
> + raw_spin_lock_irqsave(&gs->gc.bgpio_lock, flags); > + 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); > + raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags); > +}
(...)
> +static const struct irq_chip gpio_mlxbf3_irqchip = { > + .name = "MLNXBF33", > + .irq_set_type = mlxbf3_gpio_irq_set_type, > + .irq_enable = mlxbf3_gpio_irq_enable, > + .irq_disable = mlxbf3_gpio_irq_disable,
Like Andy said:
.flags = IRQCHIP_IMMUTABLE, GPIOCHIP_IRQ_RESOURCE_HELPERS,
Apart from this it looks all right.
Yours, Linus Walleij
| |