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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMRc=Meq8T_HJwDvf3wm5W2+ZFuHezGjqQx6sj-6HNJhThKjhg@mail.gmail.com>
Date: Fri, 2 Jan 2026 08:38:12 -0500
From: Bartosz Golaszewski <brgl@...nel.org>
To: Yixun Lan <dlan@...too.org>
Cc: Bartosz Golaszewski <brgl@...nel.org>, Linus Walleij <linusw@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, linux-gpio@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	spacemit@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] gpio: spacemit: Add GPIO support for K3 SoC

On Fri, 2 Jan 2026 13:20:45 +0100, Yixun Lan <dlan@...too.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@...too.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@...too.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]);
}

Bart