[PATCH 2/4] sunxi: gpio: Add per-bank drive and pull setters

[Heinrich Schuchardt]

On 10/21/21 06:52, Samuel Holland wrote:
> The GPIO and pinctrl drivers need these setters for pin configuration.
> Since they are DM drivers, they should not be using hardcoded base
> addresses. Factor out variants of the setter functions which take a
> pointer to the GPIO bank's MMIO registers.
> 
> Signed-off-by: Samuel Holland <samuel at sholland.org>
> ---
> 
>   arch/arm/include/asm/arch-sunxi/gpio.h |  2 ++
>   arch/arm/mach-sunxi/pinmux.c           | 20 ++++++++++++++++----
>   2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h
> index 2b72b2263b..106605adf5 100644
> --- a/arch/arm/include/asm/arch-sunxi/gpio.h
> +++ b/arch/arm/include/asm/arch-sunxi/gpio.h
> @@ -227,7 +227,9 @@ void sunxi_gpio_set_cfgpin(u32 pin, u32 val);
>   int sunxi_gpio_get_cfgbank(struct sunxi_gpio *pio, int bank_offset);
>   int sunxi_gpio_get_cfgpin(u32 pin);
>   void sunxi_gpio_set_drv(u32 pin, u32 val);

Please, add Sphinx style documentation for the new functions, preferably 
in the header file. Cf.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

> +void sunxi_gpio_set_drv_bank(struct sunxi_gpio *pio, u32 bank_offset, u32 val);
>   void sunxi_gpio_set_pull(u32 pin, u32 val);
> +void sunxi_gpio_set_pull_bank(struct sunxi_gpio *pio, int bank_offset, u32 val);
>   int sunxi_name_to_gpio(const char *name);
>   
>   #if !defined CONFIG_SPL_BUILD && defined CONFIG_AXP_GPIO
> diff --git a/arch/arm/mach-sunxi/pinmux.c b/arch/arm/mach-sunxi/pinmux.c
> index cf9d9daf7c..b2093b623a 100644
> --- a/arch/arm/mach-sunxi/pinmux.c
> +++ b/arch/arm/mach-sunxi/pinmux.c
> @@ -48,19 +48,31 @@ int sunxi_gpio_get_cfgpin(u32 pin)
>   void sunxi_gpio_set_drv(u32 pin, u32 val)
>   {
>   	u32 bank = GPIO_BANK(pin);
> -	u32 index = GPIO_DRV_INDEX(pin);
> -	u32 offset = GPIO_DRV_OFFSET(pin);
>   	struct sunxi_gpio *pio = BANK_TO_GPIO(bank);
>   
> +	sunxi_gpio_set_drv_bank(pio, pin, val);
> +}
> +
> +void sunxi_gpio_set_drv_bank(struct sunxi_gpio *pio, u32 bank_offset, u32 val)
> +{
> +	u32 index = GPIO_DRV_INDEX(bank_offset);
> +	u32 offset = GPIO_DRV_OFFSET(bank_offset);
> +
>   	clrsetbits_le32(&pio->drv[0] + index, 0x3 << offset, val << offset);
>   }
>   
>   void sunxi_gpio_set_pull(u32 pin, u32 val)
>   {
>   	u32 bank = GPIO_BANK(pin);
> -	u32 index = GPIO_PULL_INDEX(pin);
> -	u32 offset = GPIO_PULL_OFFSET(pin);
>   	struct sunxi_gpio *pio = BANK_TO_GPIO(bank);
>   
> +	sunxi_gpio_set_pull_bank(pio, pin, val);
> +}
> +
> +void sunxi_gpio_set_pull_bank(struct sunxi_gpio *pio, int bank_offset, u32 val)
> +{
> +	u32 index = GPIO_PULL_INDEX(bank_offset);
> +	u32 offset = GPIO_PULL_OFFSET(bank_offset);
> +
>   	clrsetbits_le32(&pio->pull[0] + index, 0x3 << offset, val << offset);

Please, simplify this:
%s/&pio->pull[0] + index/&pio->pull[index]/

Otherwise the change looks correct to me.

Best regards

Heinrich

>   }
>