Re: [PATCH 2/2] gpio: spacemit: Add GPIO support for K3 SoC

[Yixun Lan <dlan@gentoo.org>]

Hi Bartosz,

On 08:38 Fri 02 Jan     , Bartosz Golaszewski wrote:
> On Fri, 2 Jan 2026 13:20:45 +0100, Yixun Lan <dlan@gentoo.org> said:
> > Hi bart,
> >
> > On 19:36 Fri 02 Jan     , Yixun Lan wrote:
> >> Hi Bart,
> >>
> >> On 12:10 Fri 02 Jan     , Bartosz Golaszewski wrote:
> >> > On Mon, Dec 29, 2025 at 1:47 PM Yixun Lan <dlan@gentoo.org> wrote:
> >> > >
> >> > > SpacemiT K3 SoC has changed gpio register layout while comparing
> >> > > with previous generation, the register offset and bank offset
> >> > > need to be adjusted, introduce a compatible data to extend the
> >> > > driver to support this.
> >> > >
> >> > > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> >> > > ---
> >> > >  drivers/gpio/gpio-spacemit-k1.c | 150 ++++++++++++++++++++++++++++------------
> >> > >  1 file changed, 106 insertions(+), 44 deletions(-)
> >> > >
> >> > > diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
> >> > > index eb66a15c002f..02cc5c11b617 100644
> >> > > --- a/drivers/gpio/gpio-spacemit-k1.c
> >> > > +++ b/drivers/gpio/gpio-spacemit-k1.c
> >> > > @@ -15,28 +15,19 @@
> >> > >  #include <linux/platform_device.h>
> >> > >  #include <linux/seq_file.h>
> >> > >
> > [snip]...
> >> > >  static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
> >> > >  {
> >> > >         return (u32)(gb - gb->sg->sgb);
> >> > > @@ -60,13 +70,14 @@ static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
> >> > >  static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
> >> > >  {
> >> > >         struct spacemit_gpio_bank *gb = dev_id;
> >> > > +       struct spacemit_gpio *sg = gb->sg;
> >> > >         unsigned long pending;
> >> > >         u32 n, gedr;
> >> > >
> >> > > -       gedr = readl(gb->base + SPACEMIT_GEDR);
> >> > > +       gedr = readl(gb->base + to_spacemit_gpio_regs(sg)->gedr);
> >> >
> >> > Since you're already touching all these register accesses - can you
> >> > maybe provide dedicated wrapper functions around readl()/writel() and
> >> > avoid any file-wide changes in the future if anything requires further
> >> > modification?
> >> >
> >> can you elaborate a bit further on this?
> >> I don't get how a wrapper helper could help to avoid file-wide changes..
> >>
> > here is my attempt to solve this, define a macro to register address:
> >
> > #define to_spacemit_gpio_regs(gb) ((gb)->sg->data->reg_offsets)
> >
> > #define SPACEMIT_GEDR(gb)      ((gb)->base + to_spacemit_gpio_regs(gb)->gedr)
> >
> > 	gedr = readl(SPACEMIT_GEDR(gb));
> >
> > please let me know if this follow your suggestion or not
> >
> > --
> > Yixun Lan (dlan)
> >
> 
> I was thinking more of something like this:
> 
> enum spacemit_gpio_registers {
> 	SPACEMIT_GPLR,
> 	SPACEMIT_GPDR,
> 	...
> };
> 
> static const unsigned int spacemit_gpio_k1_offsets = {
> 	[SPACEMIT_GPLR] = 0x00,
> 	[SPACEMIT_GPDR] = 0x0c,
> 	...
> };
> 
> static const unsigned int spacemit_gpio_k3_offsets = ...
> 
> struct spacemit_gpio_data {
> 	const unsigned int *offsets;
> 	u32 bank_offsets[4];
> };
> 
> static void spacemit_gpio_write(struct spacemit_gpio_bank *gb,
> 				enum spacemit_gpio_registers reg, u32 val)
> {
> 	writel(val, gb->base + gb->data->offsets[reg]);
> }
> 
Ok, I will implement this, thanks for the detail prototype

-- 
Yixun Lan (dlan)