diff mbox series

[2/5] sunxi: spl: spi: Clean up SPI0 pinmux setting

Message ID 20260511213713.15943-3-andre.przywara@arm.com (mailing list archive)
State New
Headers
Series sunxi: A523: Add SPI support |

Commit Message

Andre Przywara May 11, 2026, 9:37 p.m. UTC 
The function to set the pinmux for the Port C SPI0 pins was looking more
like a logic puzzle from a magazine than something that readers could
understand and extend.

Replace the convoluted pinmux setup, grouped by pin, with a simple array
of the four pins involved, and just initialise this array at build time,
based on the selected SoC.

This makes it easy to see which pins are needed, and even easier to extend.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/mach-sunxi/spl_spi_sunxi.c | 43 ++++++++++++-----------------
 1 file changed, 18 insertions(+), 25 deletions(-)

Comments

Jernej Škrabec May 12, 2026, 5:37 p.m. UTC | #1 
Dne ponedeljek, 11. maj 2026 ob 23:37:10 Srednjeevropski poletni čas je Andre Przywara napisal(a):
> The function to set the pinmux for the Port C SPI0 pins was looking more
> like a logic puzzle from a magazine than something that readers could
> understand and extend.
> 
> Replace the convoluted pinmux setup, grouped by pin, with a simple array
> of the four pins involved, and just initialise this array at build time,
> based on the selected SoC.
> 
> This makes it easy to see which pins are needed, and even easier to extend.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/mach-sunxi/spl_spi_sunxi.c | 43 ++++++++++++-----------------
>  1 file changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> index 5f72e809952..905a7db2a77 100644
> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> @@ -105,35 +105,28 @@
>  
>  /*
>   * Allwinner A10/A20 SoCs were using pins PC0,PC1,PC2,PC23 for booting
> - * from SPI Flash, everything else is using pins PC0,PC1,PC2,PC3.
> - * The H6 uses PC0, PC2, PC3, PC5, the H616 PC0, PC2, PC3, PC4.
> + * from SPI Flash, later SoCs are using pins PC0,PC1,PC2,PC3.
> + * Newer SoCs are all over the place.
>   */
>  static void spi0_pinmux_setup(unsigned int pin_function)
>  {
> -	/* All chips use PC2. And all chips use PC0, except R528/T113 */
> -	if (!IS_ENABLED(CONFIG_MACH_SUN8I_R528))
> -		sunxi_gpio_set_cfgpin(SUNXI_GPC(0), pin_function);
> -
> -	sunxi_gpio_set_cfgpin(SUNXI_GPC(2), pin_function);
> +	const u16 spi0_pc_pins[4] = {
> +#if IS_ENABLED(CONFIG_MACH_SUN8I_R528)
> +		SUNXI_GPC(2), SUNXI_GPC(3), SUNXI_GPC(4), SUNXI_GPC(5)
> +#elif IS_ENABLED(CONFIG_MACH_SUN50I_H616)
> +		SUNXI_GPC(0), SUNXI_GPC(2), SUNXI_GPC(3), SUNXI_GPC(4)
> +#elif IS_ENABLED(CONFIG_MACH_SUN50I_H6)
> +		SUNXI_GPC(0), SUNXI_GPC(2), SUNXI_GPC(3), SUNXI_GPC(5)
> +#elif IS_ENABLED(CONFIG_MACH_SUN4I) || IS_ENABLED(CONFIG_MACH_SUN5I) || \

Are you sure about CONFIG_MACH_SUN5I? It seems to me that it should fall to PC3 pin.
Also A13 datasheet confirms it.

Best regards,
Jernej

> +      IS_ENABLED(CONFIG_MACH_SUN7I) || IS_ENABLED(CONFIG_MACH_SUN8I_R40)
> +		SUNXI_GPC(0), SUNXI_GPC(1), SUNXI_GPC(2), SUNXI_GPC(23)
> +#else
> +		SUNXI_GPC(0), SUNXI_GPC(1), SUNXI_GPC(2), SUNXI_GPC(3)
> +#endif
> +	};
>  
> -	/* All chips except H6/H616/R528/T113 use PC1. */
> -	if (!IS_ENABLED(CONFIG_SUN50I_GEN_H6) &&
> -	    !IS_ENABLED(CONFIG_MACH_SUN8I_R528))
> -		sunxi_gpio_set_cfgpin(SUNXI_GPC(1), pin_function);
> -
> -	if (IS_ENABLED(CONFIG_MACH_SUN50I_H6) ||
> -	    IS_ENABLED(CONFIG_MACH_SUN8I_R528))
> -		sunxi_gpio_set_cfgpin(SUNXI_GPC(5), pin_function);
> -	if (IS_ENABLED(CONFIG_MACH_SUN50I_H616) ||
> -	    IS_ENABLED(CONFIG_MACH_SUN8I_R528))
> -		sunxi_gpio_set_cfgpin(SUNXI_GPC(4), pin_function);
> -
> -	/* Older generations use PC23 for CS, newer ones use PC3. */
> -	if (IS_ENABLED(CONFIG_MACH_SUN4I) || IS_ENABLED(CONFIG_MACH_SUN7I) ||
> -	    IS_ENABLED(CONFIG_MACH_SUN8I_R40))
> -		sunxi_gpio_set_cfgpin(SUNXI_GPC(23), pin_function);
> -	else
> -		sunxi_gpio_set_cfgpin(SUNXI_GPC(3), pin_function);
> +	for (int i = 0; i < 4; i++)
> +		sunxi_gpio_set_cfgpin(spi0_pc_pins[i], pin_function);
>  }
>  
>  static bool is_sun6i_gen_spi(void)
>
Andre Przywara May 15, 2026, 10:03 p.m. UTC | #2 
On Tue, 12 May 2026 19:37:06 +0200
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

> Dne ponedeljek, 11. maj 2026 ob 23:37:10 Srednjeevropski poletni čas je Andre Przywara napisal(a):
> > The function to set the pinmux for the Port C SPI0 pins was looking more
> > like a logic puzzle from a magazine than something that readers could
> > understand and extend.
> > 
> > Replace the convoluted pinmux setup, grouped by pin, with a simple array
> > of the four pins involved, and just initialise this array at build time,
> > based on the selected SoC.
> > 
> > This makes it easy to see which pins are needed, and even easier to extend.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  arch/arm/mach-sunxi/spl_spi_sunxi.c | 43 ++++++++++++-----------------
> >  1 file changed, 18 insertions(+), 25 deletions(-)
> > 
> > diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> > index 5f72e809952..905a7db2a77 100644
> > --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> > +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> > @@ -105,35 +105,28 @@
> >  
> >  /*
> >   * Allwinner A10/A20 SoCs were using pins PC0,PC1,PC2,PC23 for booting
> > - * from SPI Flash, everything else is using pins PC0,PC1,PC2,PC3.
> > - * The H6 uses PC0, PC2, PC3, PC5, the H616 PC0, PC2, PC3, PC4.
> > + * from SPI Flash, later SoCs are using pins PC0,PC1,PC2,PC3.
> > + * Newer SoCs are all over the place.
> >   */
> >  static void spi0_pinmux_setup(unsigned int pin_function)
> >  {
> > -	/* All chips use PC2. And all chips use PC0, except R528/T113 */
> > -	if (!IS_ENABLED(CONFIG_MACH_SUN8I_R528))
> > -		sunxi_gpio_set_cfgpin(SUNXI_GPC(0), pin_function);
> > -
> > -	sunxi_gpio_set_cfgpin(SUNXI_GPC(2), pin_function);
> > +	const u16 spi0_pc_pins[4] = {
> > +#if IS_ENABLED(CONFIG_MACH_SUN8I_R528)
> > +		SUNXI_GPC(2), SUNXI_GPC(3), SUNXI_GPC(4), SUNXI_GPC(5)
> > +#elif IS_ENABLED(CONFIG_MACH_SUN50I_H616)
> > +		SUNXI_GPC(0), SUNXI_GPC(2), SUNXI_GPC(3), SUNXI_GPC(4)
> > +#elif IS_ENABLED(CONFIG_MACH_SUN50I_H6)
> > +		SUNXI_GPC(0), SUNXI_GPC(2), SUNXI_GPC(3), SUNXI_GPC(5)
> > +#elif IS_ENABLED(CONFIG_MACH_SUN4I) || IS_ENABLED(CONFIG_MACH_SUN5I) || \  
> 
> Are you sure about CONFIG_MACH_SUN5I? It seems to me that it should fall to PC3 pin.
> Also A13 datasheet confirms it.

Yes, you are right, good catch, will fix that!

Thanks for having a look!

Cheers,
Andre

> 
> Best regards,
> Jernej
> 
> > +      IS_ENABLED(CONFIG_MACH_SUN7I) || IS_ENABLED(CONFIG_MACH_SUN8I_R40)
> > +		SUNXI_GPC(0), SUNXI_GPC(1), SUNXI_GPC(2), SUNXI_GPC(23)
> > +#else
> > +		SUNXI_GPC(0), SUNXI_GPC(1), SUNXI_GPC(2), SUNXI_GPC(3)
> > +#endif
> > +	};
> >  
> > -	/* All chips except H6/H616/R528/T113 use PC1. */
> > -	if (!IS_ENABLED(CONFIG_SUN50I_GEN_H6) &&
> > -	    !IS_ENABLED(CONFIG_MACH_SUN8I_R528))
> > -		sunxi_gpio_set_cfgpin(SUNXI_GPC(1), pin_function);
> > -
> > -	if (IS_ENABLED(CONFIG_MACH_SUN50I_H6) ||
> > -	    IS_ENABLED(CONFIG_MACH_SUN8I_R528))
> > -		sunxi_gpio_set_cfgpin(SUNXI_GPC(5), pin_function);
> > -	if (IS_ENABLED(CONFIG_MACH_SUN50I_H616) ||
> > -	    IS_ENABLED(CONFIG_MACH_SUN8I_R528))
> > -		sunxi_gpio_set_cfgpin(SUNXI_GPC(4), pin_function);
> > -
> > -	/* Older generations use PC23 for CS, newer ones use PC3. */
> > -	if (IS_ENABLED(CONFIG_MACH_SUN4I) || IS_ENABLED(CONFIG_MACH_SUN7I) ||
> > -	    IS_ENABLED(CONFIG_MACH_SUN8I_R40))
> > -		sunxi_gpio_set_cfgpin(SUNXI_GPC(23), pin_function);
> > -	else
> > -		sunxi_gpio_set_cfgpin(SUNXI_GPC(3), pin_function);
> > +	for (int i = 0; i < 4; i++)
> > +		sunxi_gpio_set_cfgpin(spi0_pc_pins[i], pin_function);
> >  }
> >  
> >  static bool is_sun6i_gen_spi(void)
> >   
> 
> 
> 
>
diff mbox series

Patch

diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
index 5f72e809952..905a7db2a77 100644
--- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
+++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
@@ -105,35 +105,28 @@ 
 
 /*
  * Allwinner A10/A20 SoCs were using pins PC0,PC1,PC2,PC23 for booting
- * from SPI Flash, everything else is using pins PC0,PC1,PC2,PC3.
- * The H6 uses PC0, PC2, PC3, PC5, the H616 PC0, PC2, PC3, PC4.
+ * from SPI Flash, later SoCs are using pins PC0,PC1,PC2,PC3.
+ * Newer SoCs are all over the place.
  */
 static void spi0_pinmux_setup(unsigned int pin_function)
 {
-	/* All chips use PC2. And all chips use PC0, except R528/T113 */
-	if (!IS_ENABLED(CONFIG_MACH_SUN8I_R528))
-		sunxi_gpio_set_cfgpin(SUNXI_GPC(0), pin_function);
-
-	sunxi_gpio_set_cfgpin(SUNXI_GPC(2), pin_function);
+	const u16 spi0_pc_pins[4] = {
+#if IS_ENABLED(CONFIG_MACH_SUN8I_R528)
+		SUNXI_GPC(2), SUNXI_GPC(3), SUNXI_GPC(4), SUNXI_GPC(5)
+#elif IS_ENABLED(CONFIG_MACH_SUN50I_H616)
+		SUNXI_GPC(0), SUNXI_GPC(2), SUNXI_GPC(3), SUNXI_GPC(4)
+#elif IS_ENABLED(CONFIG_MACH_SUN50I_H6)
+		SUNXI_GPC(0), SUNXI_GPC(2), SUNXI_GPC(3), SUNXI_GPC(5)
+#elif IS_ENABLED(CONFIG_MACH_SUN4I) || IS_ENABLED(CONFIG_MACH_SUN5I) || \
+      IS_ENABLED(CONFIG_MACH_SUN7I) || IS_ENABLED(CONFIG_MACH_SUN8I_R40)
+		SUNXI_GPC(0), SUNXI_GPC(1), SUNXI_GPC(2), SUNXI_GPC(23)
+#else
+		SUNXI_GPC(0), SUNXI_GPC(1), SUNXI_GPC(2), SUNXI_GPC(3)
+#endif
+	};
 
-	/* All chips except H6/H616/R528/T113 use PC1. */
-	if (!IS_ENABLED(CONFIG_SUN50I_GEN_H6) &&
-	    !IS_ENABLED(CONFIG_MACH_SUN8I_R528))
-		sunxi_gpio_set_cfgpin(SUNXI_GPC(1), pin_function);
-
-	if (IS_ENABLED(CONFIG_MACH_SUN50I_H6) ||
-	    IS_ENABLED(CONFIG_MACH_SUN8I_R528))
-		sunxi_gpio_set_cfgpin(SUNXI_GPC(5), pin_function);
-	if (IS_ENABLED(CONFIG_MACH_SUN50I_H616) ||
-	    IS_ENABLED(CONFIG_MACH_SUN8I_R528))
-		sunxi_gpio_set_cfgpin(SUNXI_GPC(4), pin_function);
-
-	/* Older generations use PC23 for CS, newer ones use PC3. */
-	if (IS_ENABLED(CONFIG_MACH_SUN4I) || IS_ENABLED(CONFIG_MACH_SUN7I) ||
-	    IS_ENABLED(CONFIG_MACH_SUN8I_R40))
-		sunxi_gpio_set_cfgpin(SUNXI_GPC(23), pin_function);
-	else
-		sunxi_gpio_set_cfgpin(SUNXI_GPC(3), pin_function);
+	for (int i = 0; i < 4; i++)
+		sunxi_gpio_set_cfgpin(spi0_pc_pins[i], pin_function);
 }
 
 static bool is_sun6i_gen_spi(void)