[PATCH 2/3] gpio: at91: Implement ops set_flags

[Eugen Hristev]

Hello,

On 10/18/24 23:27, Zixun LI wrote:
> Support GPIO configuration with following flags:
> - in, out, out_active
> - open_drain, pull_up
> 
> Signed-off-by: Zixun LI <admin at hifiphile.com>
> ---
>  drivers/gpio/at91_gpio.c | 41 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c
> index 64a6e8a4a5..7828f9b447 100644
> --- a/drivers/gpio/at91_gpio.c
> +++ b/drivers/gpio/at91_gpio.c
> @@ -229,6 +229,17 @@ static bool at91_get_port_pio(struct at91_port *at91_port, int offset)
>  	val = readl(&at91_port->psr);
>  	return val & mask;
>  }
> +
> +static void at91_set_port_multi_drive(struct at91_port *at91_port, int offset, int is_on)
> +{
> +	u32 mask;
> +
> +	mask = 1 << offset;
> +	if (is_on)
> +		writel(mask, &at91_port->mder);
> +	else
> +		writel(mask, &at91_port->mddr);
> +}
>  #endif
>  
>  static void at91_set_port_input(struct at91_port *at91_port, int offset,
> @@ -568,6 +579,35 @@ static int at91_gpio_get_function(struct udevice *dev, unsigned offset)
>  		return GPIOF_INPUT;
>  }
>  
> +static int at91_gpio_set_flags(struct udevice *dev, unsigned int offset,
> +				ulong flags)
> +{
> +	struct at91_port_priv *port = dev_get_priv(dev);
> +	ulong supported_mask;
> +
> +	supported_mask = GPIOD_OPEN_DRAIN | GPIOD_MASK_DIR | GPIOD_PULL_UP;
> +	if (flags & ~supported_mask)
> +		return -EINVAL;

Does this change the current behavior? there is no set_flag ops
implemented, previously it would use a default that would just return
success regardless of the given flags parameters ?
Btw maybe ENOTSUPP ?

> +
> +	if (flags & GPIOD_IS_OUT) {
> +		bool value = flags & GPIOD_IS_OUT_ACTIVE;

Can you please declare this value at the start of the function, or
inline it below when it's being used

> +
> +		if (flags & GPIOD_OPEN_DRAIN)
> +			at91_set_port_multi_drive(port->regs, offset, true);
> +		else
> +			at91_set_port_multi_drive(port->regs, offset, false);
> +
> +		at91_set_port_output(port->regs, offset, value);
> +
> +	} else if (flags & GPIOD_IS_IN) {
> +		at91_set_port_input(port->regs, offset, false);
> +	}
> +	if (flags & GPIOD_PULL_UP)
> +		at91_set_port_pullup(port->regs, offset, true);
> +
> +	return 0;
> +}
> +
>  static const char *at91_get_bank_name(uint32_t base_addr)
>  {
>  	switch (base_addr) {
> @@ -596,6 +636,7 @@ static const struct dm_gpio_ops gpio_at91_ops = {
>  	.get_value		= at91_gpio_get_value,
>  	.set_value		= at91_gpio_set_value,
>  	.get_function		= at91_gpio_get_function,
> +	.set_flags		= at91_gpio_set_flags,
>  };
>  
>  static int at91_gpio_probe(struct udevice *dev)