Re: [PATCH v2 07/11] sunxi: Add support for UART1 on PG pins for the V3/S3

[Paul Kocialkowski <paulk@sys-base.io>]

[-- Attachment #1: Type: text/plain, Size: 3554 bytes --]

Hi,

Thansk for the review!

Le Wed 06 Aug 25, 01:21, Andre Przywara a écrit :
> On Tue,  5 Aug 2025 19:48:23 +0200
> Paul Kocialkowski <paulk@sys-base.io> wrote:
> 
> Hi,
> 
> > From: Paul Kocialkowski <contact@paulk.fr>
> > 
> > The V3/S3 has extra pins for UART1 on PG pins, which are not present
> > on the V3s. Add support for them and make them the default UART1 pins
> > on the V3. Note that PE pins are also available for UART1 but almost
> > never used since they also provide the CSI I2C lines.
> 
> So the patch itself looks alright, but what is this actually used for?
> This pinmux setting here is really just for the SPL to setup the debug
> UART. Is there any board where UART1 is the supposed debug UART?
> Because if not, we just don't need that here, and if yes, I'd love to
> see the respective defconfig file ;-)

This is the default on the V3 reference design, which uses UART1 on PG pins for
debug and I've seen it in use in all V3/S3 devices I could see, including:
- Olimex S3-OLinuXino
- SL631 action camera
- iT-X3 action camera

The pinecube is the odd one out using UART2 instead. For V3s devices it's a bit
messy and the reference design uses UART2 over PB pins, but the only device we
support (Lichee Pi Zero) uses UART0 on PB pins instead (muxed with TWI1).

So you're right, it's not currently useful for devices in the tree, but I
assumed it could be useful in the future. I also just noticed that there is
already a fallback for sun8i which will direct UART1 to PG pins, on the same
pins that I'm using. So this patch is definitely not necessary then!

I have some work in progress to add support for the devices above, but never
sat down to finish them. I'll definitely submit them eventually!

All the best,

Paul

> Cheers,
> Andre
> 
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  arch/arm/mach-sunxi/board.c | 4 ++++
> >  include/sunxi_gpio.h        | 1 +
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> > index a1a02b0fae50..d1b27b062d6d 100644
> > --- a/arch/arm/mach-sunxi/board.c
> > +++ b/arch/arm/mach-sunxi/board.c
> > @@ -169,6 +169,10 @@ static int gpio_init(void)
> >  	sunxi_gpio_set_cfgpin(SUNXI_GPG(3), SUN5I_GPG_UART1);
> >  	sunxi_gpio_set_cfgpin(SUNXI_GPG(4), SUN5I_GPG_UART1);
> >  	sunxi_gpio_set_pull(SUNXI_GPG(4), SUNXI_GPIO_PULL_UP);
> > +#elif CONFIG_CONS_INDEX == 2 && defined(CONFIG_MACH_SUN8I_V3)
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPG(6), SUN8I_V3_GPG_UART1);
> > +	sunxi_gpio_set_cfgpin(SUNXI_GPG(7), SUN8I_V3_GPG_UART1);
> > +	sunxi_gpio_set_pull(SUNXI_GPG(7), SUNXI_GPIO_PULL_UP);
> >  #elif CONFIG_CONS_INDEX == 3 && defined(CONFIG_MACH_SUN8I_H3)
> >  	sunxi_gpio_set_cfgpin(SUNXI_GPA(0), SUN8I_H3_GPA_UART2);
> >  	sunxi_gpio_set_cfgpin(SUNXI_GPA(1), SUN8I_H3_GPA_UART2);
> > diff --git a/include/sunxi_gpio.h b/include/sunxi_gpio.h
> > index e166b9758f44..7d142f65ea8a 100644
> > --- a/include/sunxi_gpio.h
> > +++ b/include/sunxi_gpio.h
> > @@ -141,6 +141,7 @@ enum sunxi_gpio_number {
> >  #define SUN8I_GPG_SDC1		2
> >  #define SUN8I_GPG_UART1		2
> >  #define SUN5I_GPG_UART1		4
> > +#define SUN8I_V3_GPG_UART1	2
> >  
> >  #define SUN6I_GPH_PWM		2
> >  #define SUN8I_GPH_PWM		2
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]