Hi Asmaa,
thanks for your patch!
On Wed, Feb 8, 2023 at 7:57 PM Asmaa Mnebhi <asmaa@xxxxxxxxxx> wrote:
> This patch adds support for the BlueField-3 SoC GPIO driver
> which allows:
> - setting certain GPIOs as interrupts from other dependent drivers
> - ability to manipulate certain GPIO pins via libgpiod tools for instance
>
> BlueField-3 has 56 GPIOs but the user is only allowed to change some
> of them into GPIO mode. Use valid_mask to make it impossible to alter
> the rest of the GPIOs.
>
> Signed-off-by: Asmaa Mnebhi <asmaa@xxxxxxxxxx>
(...)
> +struct mlxbf3_gpio_context {
> + struct gpio_chip gc;
> + struct irq_chip irq_chip;
> +
> + /* YU GPIO block address */
> + void __iomem *gpio_io;
> +
> + /* YU GPIO cause block address */
> + void __iomem *gpio_cause_io;
I especially like that you use the word "cause" (as in cause-and-effect)
over the imprecise and overused word "reason" (brrrr)
> +static int mlxbf3_gpio_direction_input(struct gpio_chip *chip,
> + unsigned int offset)
> +{
> + struct mlxbf3_gpio_context *gs = gpiochip_get_data(chip);
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
> +
> + writel(BIT(offset), gs->gpio_io + MLXBF_GPIO_FW_OUTPUT_ENABLE_CLEAR);
> +
> + raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
> +
> + return 0;
> +}
> +
> +static int mlxbf3_gpio_direction_output(struct gpio_chip *chip,
> + unsigned int offset,
> + int value)
> +{
> + struct mlxbf3_gpio_context *gs = gpiochip_get_data(chip);
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&gs->gc.bgpio_lock, flags);
> +
> + writel(BIT(offset), gs->gpio_io + MLXBF_GPIO_FW_OUTPUT_ENABLE_SET);
> +
> + if (value)
> + writel(BIT(offset), gs->gpio_io + MLXBF_GPIO_FW_DATA_OUT_SET);
> + else
> + writel(BIT(offset), gs->gpio_io + MLXBF_GPIO_FW_DATA_OUT_CLEAR);
> +
> + raw_spin_unlock_irqrestore(&gs->gc.bgpio_lock, flags);
> +
> + return 0;
> +}
GPIO_GENERIC / bgpio should be able to handle these too.
> + ret = bgpio_init(gc, dev, 4,
> + gs->gpio_io + MLXBF_GPIO_READ_DATA_IN,
> + gs->gpio_io + MLXBF_GPIO_FW_DATA_OUT_SET,
> + gs->gpio_io + MLXBF_GPIO_FW_DATA_OUT_CLEAR,
> + NULL, NULL, 0);
Instead of NULL, NULL, 0
use
gs->gpio_io + MLXBF_GPIO_FW_OUTPUT_ENABLE_SET,
gs->gpio_io + MLXBF_GPIO_FW_OUTPUT_ENABLE_CLEAR,
0);
The generic library will make sure to also set the output value when
changing direction to output.
Yours,
Linus Walleij
