Re: [PATCH V2 7/9] gpio/rockchip: rk_gpio support v2 gpio controller

Simon Glass
2023-03-01T15:06:55Z

Hi,

On Wed, 1 Mar 2023 at 01:25, Eugen Hristev <eugen.hristev@collabora.com> wrote:
>
> On 2/14/23 00:27, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Add support for the newer GPIO controller used by the rk356x series,
> > as well as the pinctrl device for the rk356x series. The GPIOv2
> > controller has a write enable bit for some registers which differs
> > from the older versions of the GPIO controller.
> >
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>
> Hi Chris,
>
> In the file below you have added
>
> > + * Jianqun Xu, Software Engineering, <jay.xu@rock-chips.com>.
>
> as copyright owner, maybe add him as co-author of this patch ? Or what
> was his contribution ?
>
>
> > ---
> > arch/arm/include/asm/arch-rockchip/gpio.h | 38 ++
> > drivers/gpio/rk_gpio.c | 49 +-
> > drivers/pinctrl/rockchip/Makefile | 1 +
> > drivers/pinctrl/rockchip/pinctrl-rk3568.c | 453 ++++++++++++++++++
> > .../pinctrl/rockchip/pinctrl-rockchip-core.c | 12 +-
> > 5 files changed, 540 insertions(+), 13 deletions(-)
> > create mode 100644 drivers/pinctrl/rockchip/pinctrl-rk3568.c
> >
> > diff --git a/arch/arm/include/asm/arch-rockchip/gpio.h b/arch/arm/include/asm/arch-rockchip/gpio.h
> > index 1aaec5faec..15f5de321b 100644
> > --- a/arch/arm/include/asm/arch-rockchip/gpio.h
> > +++ b/arch/arm/include/asm/arch-rockchip/gpio.h
> > @@ -6,6 +6,7 @@
> > #ifndef _ASM_ARCH_GPIO_H
> > #define _ASM_ARCH_GPIO_H
> >
> > +#if !defined(CONFIG_ROCKCHIP_RK3568)
>
> Can't we figure out from the compatible which struct layout to use ?
> Using conditionally compile code makes things difficult to read after
> some time.

Yes, we must not add arch-specific CONFIG things to drivers. Handle it
at runtime instead, i.e. the driver should handle both.

Regards,
SImon