Re: [PATCH 1/5] pinctrl: sunxi: Fix a100 emac pin function name

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

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

Hi Andre,

Le Sat 05 Jul 25, 15:38, Andre Przywara a écrit :
> On Fri, 4 Jul 2025 23:35:35 +0100
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
> Hi,
> 
> > On Thu, 26 Jun 2025 10:09:19 +0200
> > Paul Kocialkowski <paulk@sys-base.io> wrote:
> > 
> > Hi Paul,
> > 
> > > The Allwinner A100/A133 only has a single emac instance, which is
> > > referred to as "emac" everywhere. Fix the pin names to drop the
> > > trailing "0" that has no reason to be.  
> > 
> > Sorry, but this is wrong. There *is* a second EMAC on the A133 die: it's
> > indeed not mentioned in the manual, but you can probe its MMIO
> > registers (@0x5030000), and there is a second syscon register
> > (@0x03000034). It's mentioned in several BSP code places ([1]).
> > It seem like no suitable pins are connected on the A133
> > package, but that should not affect the A100 .dtsi (we use a similar
> > approach for the H616 and A523).
> > 
> > So I think we should keep the emac0 name.
> 
> just thinking that it's even worse: this changes the DT visible pinctrl
> function name, so it's a DT ABI change. With the "emac0" function name,
> Ethernet would work with stable kernels already (as everything is
> compatible, it's just about DT changes). But with this change, pinctrl
> drivers in older kernels would not match.

Given that the port is still very early and experimental and has very few users
and no field deployment so I don't really think it would have annoyed anybody in
practice. But yes in principle you are right, while the header renames keep the
same value, the string names are used to match the device-tree definitions and
this constitues ABI that needs to remain stable.

> So I would very much like to see this patch moved out. Is it just in
> LinusW's tree so far? I don't see it in -next yet.

I don't think the patches were accepted for over a week so we can probably
still act. I will send reverts, unless maintainers want to manually remove
these commits?

Cheers,

Paul

> Cheers,
> Andre.
> 
> > [1]
> > https://github.com/qiaoweibiao/T507_Kernel/blob/main/arch/arm64/boot/dts/sunxi/sun50iw10p1.dtsi
> > 
> > 
> > > 
> > > Fixes: 473436e7647d ("pinctrl: sunxi: add support for the Allwinner A100 pin controller")
> > > Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> > > ---
> > >  drivers/pinctrl/sunxi/pinctrl-sun50i-a100.c | 32 ++++++++++-----------
> > >  1 file changed, 16 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/pinctrl/sunxi/pinctrl-sun50i-a100.c b/drivers/pinctrl/sunxi/pinctrl-sun50i-a100.c
> > > index b97de80ae2f3..95b764ee1c0d 100644
> > > --- a/drivers/pinctrl/sunxi/pinctrl-sun50i-a100.c
> > > +++ b/drivers/pinctrl/sunxi/pinctrl-sun50i-a100.c
> > > @@ -546,33 +546,33 @@ static const struct sunxi_desc_pin a100_pins[] = {
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x2, "i2c0"),		/* SCK */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* RXD1 */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* RXD1 */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 0)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 1),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x2, "i2c0"),		/* SDA */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* RXD0 */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* RXD0 */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 1)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 2),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x2, "i2c1"),		/* SCK */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* RXCTL */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* RXCTL */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 2)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 3),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x2, "i2c1"),		/* SDA */
> > >  		  SUNXI_FUNCTION(0x3, "cir0"),		/* OUT */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* CLKIN */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* CLKIN */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 3)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 4),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x2, "uart3"),		/* TX */
> > >  		  SUNXI_FUNCTION(0x3, "spi1"),		/* CS */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* TXD1 */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* TXD1 */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 4)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 5),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > > @@ -580,14 +580,14 @@ static const struct sunxi_desc_pin a100_pins[] = {
> > >  		  SUNXI_FUNCTION(0x2, "uart3"),		/* RX */
> > >  		  SUNXI_FUNCTION(0x3, "spi1"),		/* CLK */
> > >  		  SUNXI_FUNCTION(0x4, "ledc"),
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* TXD0 */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* TXD0 */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 5)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 6),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x2, "uart3"),		/* RTS */
> > >  		  SUNXI_FUNCTION(0x3, "spi1"),		/* MOSI */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* TXCK */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* TXCK */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 6)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 7),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > > @@ -595,7 +595,7 @@ static const struct sunxi_desc_pin a100_pins[] = {
> > >  		  SUNXI_FUNCTION(0x2, "uart3"),		/* CTS */
> > >  		  SUNXI_FUNCTION(0x3, "spi1"),		/* MISO */
> > >  		  SUNXI_FUNCTION(0x4, "spdif"),		/* OUT */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* TXCTL */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* TXCTL */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 7)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 8),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > > @@ -611,7 +611,7 @@ static const struct sunxi_desc_pin a100_pins[] = {
> > >  		  SUNXI_FUNCTION(0x2, "dmic"),		/* DATA0 */
> > >  		  SUNXI_FUNCTION(0x3, "spi2"),		/* CLK */
> > >  		  SUNXI_FUNCTION(0x4, "i2s2"),		/* BCLK */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* MDC */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* MDC */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 9)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 10),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > > @@ -619,7 +619,7 @@ static const struct sunxi_desc_pin a100_pins[] = {
> > >  		  SUNXI_FUNCTION(0x2, "dmic"),		/* DATA1 */
> > >  		  SUNXI_FUNCTION(0x3, "spi2"),		/* MOSI */
> > >  		  SUNXI_FUNCTION(0x4, "i2s2"),		/* LRCK */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* MDIO */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* MDIO */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 10)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 11),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > > @@ -642,33 +642,33 @@ static const struct sunxi_desc_pin a100_pins[] = {
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x3, "i2c3"),		/* SCK */
> > >  		  SUNXI_FUNCTION(0x4, "i2s3"),		/* MCLK */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* EPHY */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* EPHY */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 13)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 14),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x4, "i2s3"),		/* BCLK */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* RXD3 */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* RXD3 */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 14)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 15),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x4, "i2s3"),		/* LRCK */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* RXD2 */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* RXD2 */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 15)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 16),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x3, "i2s3_dout0"),	/* DOUT0 */
> > >  		  SUNXI_FUNCTION(0x4, "i2s3_din1"),	/* DIN1 */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* RXCK */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* RXCK */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 16)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 17),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > >  		  SUNXI_FUNCTION(0x1, "gpio_out"),
> > >  		  SUNXI_FUNCTION(0x3, "i2s3_dout1"),	/* DOUT1 */
> > >  		  SUNXI_FUNCTION(0x4, "i2s3_din0"),	/* DIN0 */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* TXD3 */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* TXD3 */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 17)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 18),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),
> > > @@ -676,7 +676,7 @@ static const struct sunxi_desc_pin a100_pins[] = {
> > >  		  SUNXI_FUNCTION(0x2, "cir0"),		/* OUT */
> > >  		  SUNXI_FUNCTION(0x3, "i2s3_dout2"),	/* DOUT2 */
> > >  		  SUNXI_FUNCTION(0x4, "i2s3_din2"),	/* DIN2 */
> > > -		  SUNXI_FUNCTION(0x5, "emac0"),		/* TXD2 */
> > > +		  SUNXI_FUNCTION(0x5, "emac"),		/* TXD2 */
> > >  		  SUNXI_FUNCTION_IRQ_BANK(0x6, 6, 18)),
> > >  	SUNXI_PIN(SUNXI_PINCTRL_PIN(H, 19),
> > >  		  SUNXI_FUNCTION(0x0, "gpio_in"),  
> > 
> > 
> 

-- 
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 --]