[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACRpkdYgGjqL85CcNw=38_XrzsWPBX8GwhL9t3m_+fSnEjHrdw@mail.gmail.com>
Date: Thu, 23 Feb 2023 13:29:28 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: Asmaa Mnebhi <asmaa@...dia.com>
Cc: andy.shevchenko@...il.com, bgolaszewski@...libre.com,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org
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@...dia.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@...dia.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
Powered by blists - more mailing lists