RE: Re: [PATCH v2 07/10] pinctrl: ambarella: add Ambarella pin controller
From: Long Zhao
Date: Mon Aug 10 2026 - 00:02:47 EST
Hi Linus,
Thanks for the review. I will rework this to reuse the PL061 driver with
per-variant register offsets, and evaluate modeling each GPIO bank as a
separate DT entity. The duplicated GPIO implementation will be removed.
Best regards,
Long
Ambarella Shanghai Co., Ltd
-----Original Message-----
From: Linus Walleij <linusw@xxxxxxxxxx>
Sent: Saturday, August 8, 2026 01:45
To: Long Zhao <longzhao@xxxxxxxxxxxxx>; Bartosz Golaszewski <brgl@xxxxxxxxxx>
Cc: Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Michael Turquette <mturquette@xxxxxxxxxxxx>; Stephen Boyd <sboyd@xxxxxxxxxx>; Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Jiri Slaby <jirislaby@xxxxxxxxxx>; Catalin Marinas <catalin.marinas@xxxxxxx>; Will Deacon <will@xxxxxxxxxx>; Arnd Bergmann <arnd@xxxxxxxx>; Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx; linux-gpio@xxxxxxxxxxxxxxx; linux-serial@xxxxxxxxxxxxxxx; soc@xxxxxxxxxxxxxxx
Subject: [EXT] Re: [PATCH v2 07/10] pinctrl: ambarella: add Ambarella pin controller
Hi Long,
thanks for your patch!
On Thu, Aug 6, 2026 at 11:34 AM Long Zhao via B4 Relay <devnull+longzhao.ambarella.com@xxxxxxxxxx> wrote:
> +#define GPIO_DATA_OFFSET 0x00
> +#define GPIO_DIR_OFFSET 0x04
> +#define GPIO_IS_OFFSET 0x08
> +#define GPIO_IBE_OFFSET 0x0c
> +#define GPIO_IEV_OFFSET 0x10
> +#define GPIO_IE_OFFSET 0x14
> +#define GPIO_AFSEL_OFFSET 0x18
> +#define GPIO_RIS_OFFSET 0x1c
> +#define GPIO_MIS_OFFSET 0x20
> +#define GPIO_IC_OFFSET 0x24
> +#define GPIO_MASK_OFFSET 0x28
> +#define GPIO_ENABLE_OFFSET 0x2c
As everyone can see from a mile away this is a modified ARM PL061 primecell, see drivers/gpio/gpio-pl061.c.
I'm not very interested in having two drivers for pretty much the same hardware in the kernel. Odds are that there will be new quirks and fixed to one of the drivers that the other one doesn't get :/ Also the PL061 driver is pretty complex and specialized as GPIO drivers go already. I certainly don't want to have a second copy of that.
We need to find a way to re-use the PL061 driver with this pin controller.
1. Modify the existing PL061 driver to use a dynamic register
layout, i.e. make the different register locations part of some
per-variant data:
struct vendor_data {
u32 data_offset;
u32 dir_offset;
...
};
Add:
struct pl061 {
struct vendor_data *variant;
...
};
Add vendor data to the PL061 variant:
static struct vendor_data vendor_arm = {...};
static struct vendor_data vendor_ambarella = {...};
static const struct amba_id pl061_ids[] = {
{
.id = 0x00041061,
.mask = 0x000fffff,
. data = &vendor_arm;
},
{
.id = /* make something up, see include/linux/amba/bus.h */
.mask = ....
.data = &vendor_ambarella;
},
Essentially follow the pattern from drivers/spi/spi-pl022.c.
2. Contemplate if the GPIO should be a separate entity in the device tree or not. If not, Linux needs to spawn the new AMBA device with this pin control driver using e.g.
amba_device_alloc() and amba_device_register() etc, see drivers/of/platform.c
3. select GPIO_PL061 in Kconfig for this pin controller...
This is not a simple solution but it is the right one.
Yours,
Linus Walleij
######################################################################
This EXTERNAL email has been scanned by Proofpoint Email Protect service.
**********************************************************************
This email and attachments contain Ambarella Proprietary and/or Confidential Information and is intended solely for the use of the individual(s) to whom it is addressed. Any unauthorized review, use, disclosure, distribute, copy, or print is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message. Thank you.