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

[Andre Przywara]

On Fri, 5 Nov 2021 15:12:16 +0100
Heinrich Schuchardt <heinrich.schuchardt at canonical.com> wrote:

> 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

I don't think this is necessary or useful, really. Those function are
not really an advertised interface, more something we use internally,
mostly for our SPL.

> > +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]/

Fixing this up locally.

Cheers,
Andre

> 
> Otherwise the change looks correct to me.
> 
> Best regards
> 
> Heinrich
> 
> >   }
> >   
>