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

[Yixun Lan <dlan@gentoo.org>]

Hi Bartosz,

On 01:23 Mon 05 Jan     , Bartosz Golaszewski wrote:
> On Sat, 3 Jan 2026 22:33:39 +0100, Yixun Lan <dlan@gentoo.org> said:
> > 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 | 163 ++++++++++++++++++++++++++++------------
> >  1 file changed, 117 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
> > index eb66a15c002f..bca5c3dc13ca 100644
> > --- a/drivers/gpio/gpio-spacemit-k1.c
> > +++ b/drivers/gpio/gpio-spacemit-k1.c
> > @@ -15,29 +15,37 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/seq_file.h>
> >
> > -/* register offset */
> > -#define SPACEMIT_GPLR		0x00 /* port level - R */
> > -#define SPACEMIT_GPDR		0x0c /* port direction - R/W */
> > -#define SPACEMIT_GPSR		0x18 /* port set - W */
> > -#define SPACEMIT_GPCR		0x24 /* port clear - W */
> > -#define SPACEMIT_GRER		0x30 /* port rising edge R/W */
> > -#define SPACEMIT_GFER		0x3c /* port falling edge R/W */
> > -#define SPACEMIT_GEDR		0x48 /* edge detect status - R/W1C */
> > -#define SPACEMIT_GSDR		0x54 /* (set) direction - W */
> > -#define SPACEMIT_GCDR		0x60 /* (clear) direction - W */
> > -#define SPACEMIT_GSRER		0x6c /* (set) rising edge detect enable - W */
> > -#define SPACEMIT_GCRER		0x78 /* (clear) rising edge detect enable - W */
> > -#define SPACEMIT_GSFER		0x84 /* (set) falling edge detect enable - W */
> > -#define SPACEMIT_GCFER		0x90 /* (clear) falling edge detect enable - W */
> > -#define SPACEMIT_GAPMASK	0x9c /* interrupt mask , 0 disable, 1 enable - R/W */
> > -
> >  #define SPACEMIT_NR_BANKS		4
> >  #define SPACEMIT_NR_GPIOS_PER_BANK	32
> >
> >  #define to_spacemit_gpio_bank(x) container_of((x), struct spacemit_gpio_bank, gc)
> > +#define to_spacemit_gpio_regs(gb) ((gb)->sg->data->offsets)
> > +
> > +enum spacemit_gpio_registers {
> > +	SPACEMIT_GPLR = 0,	/* port level - R */
> 
> No need for the = 0 here.
ok, I will drop it

> > +static const unsigned int spacemit_gpio_k1_offsets[] = {
> > +	0x00,
> > +	0x0c,
> > +	0x18,
> > +	0x24,
> > +	0x30,
> > +	0x3c,
> > +	0x48,
> > +	0x54,
> > +	0x60,
> > +	0x6c,
> > +	0x78,
> > +	0x84,
> > +	0x90,
> > +	0x9c,
> > +	0xA8,
> > +};
> > +
> > +static const unsigned int spacemit_gpio_k3_offsets[] = {
> > +	0x0,
> > +	0x4,
> > +	0x8,
> > +	0xc,
> > +	0x10,
> > +	0x14,
> > +	0x18,
> > +	0x1c,
> > +	0x20,
> > +	0x24,
> > +	0x28,
> > +	0x2c,
> > +	0x30,
> > +	0x34,
> > +	0x38,
> > +};
> 
> I would very much prefer for you to use the
> 
>     [ENUM] = 0xVALUE
> 
> style of initialization here for better readability.
> 
ok, will do

> Otherwise looks good so LGTM on the next iteration.
> 
thanks

-- 
Yixun Lan (dlan)