[PATCH 1/1] sunxi: SPI: fix pinmuxing for Allwinner H6 SoCs

[PATCH 1/1] sunxi: SPI: fix pinmuxing for Allwinner H6 SoCs

[Daniel Wagenknecht]

The driver for SPI0 on Allwinner H6 SoCs did not use the correct define
SUN50I_GPC_SPI0 for the pin function, but one for a different Allwinner
SoC series.

Fix the conditionals to use the correct define for H6 SoCs. This matches
the conditional logic in the SPL spi driver.

Tested by probing the spi-flash on a pine64_h64-model-b board with
adapted device-tree (disable mmc2, enable spi0).

Signed-off-by: Daniel Wagenknecht <dwagenk@mailbox.org>
---
 drivers/spi/spi-sunxi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index bc2f544e86..6a8ee8d542 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -249,7 +249,8 @@ static int sun4i_spi_parse_pins(struct udevice *dev)
 			if (pin < 0)
 				break;
 
-			if (IS_ENABLED(CONFIG_MACH_SUN50I))
+			if (IS_ENABLED(CONFIG_MACH_SUN50I) ||
+			    IS_ENABLED(CONFIG_MACH_SUN50I_H6))
 				sunxi_gpio_set_cfgpin(pin, SUN50I_GPC_SPI0);
 			else
 				sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_SPI0);
-- 
2.31.1

Re: [PATCH 1/1] sunxi: SPI: fix pinmuxing for Allwinner H6 SoCs

[Andre Przywara]

On Sun, 12 Dec 2021 20:29:06 +0100
Daniel Wagenknecht <dwagenk@mailbox.org> wrote:

Hi Daniel,

please don't just resend without saying why. If you messed up the mail,
or forgot to CC: the list, then please put "RESEND" in the subject, and
briefly say why you resend.
If you want to ping, please reply to the cover letter, but please give
people some time to react. We are all volunteers, and not all patches
seem immediately important to jump on them. Plus you might just annoy
people ;-)
If you are unsure whether your email made it out, check the mailing
list archive.

> The driver for SPI0 on Allwinner H6 SoCs did not use the correct define
> SUN50I_GPC_SPI0 for the pin function, but one for a different Allwinner
> SoC series.
> 
> Fix the conditionals to use the correct define for H6 SoCs. This matches
> the conditional logic in the SPL spi driver.
> 
> Tested by probing the spi-flash on a pine64_h64-model-b board with
> adapted device-tree (disable mmc2, enable spi0).

Yeah, this was probably the reason no one noticed:  It doesn't work
without hacks, and also the PineH64 seems to be the only (supported) H6
board with SPI.

If you have some spare cycles ;-) you can look whether we can fix up
the DT when no eMMC is detected, and how to make this work so this not
only applies to the kernel, but also to U-Boot itself.

> 
> Signed-off-by: Daniel Wagenknecht <dwagenk@mailbox.org>
> ---
>  drivers/spi/spi-sunxi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
> index bc2f544e86..6a8ee8d542 100644
> --- a/drivers/spi/spi-sunxi.c
> +++ b/drivers/spi/spi-sunxi.c
> @@ -249,7 +249,8 @@ static int sun4i_spi_parse_pins(struct udevice *dev)
>  			if (pin < 0)
>  				break;
>  
> -			if (IS_ENABLED(CONFIG_MACH_SUN50I))
> +			if (IS_ENABLED(CONFIG_MACH_SUN50I) ||
> +			    IS_ENABLED(CONFIG_MACH_SUN50I_H6))

Indeed this was missing. And as you already figured, this applies to the
H616 as well, where SPI is actually usable (just a clash with SDC2_DS,
which is only needed for HS400).
So please put CONFIG_SUN50I_GEN_H6 in there.

Jernej, Samuel, can you please test this on the OPi Zero2? My board
seems to be in a bad mood :-(

But also please note that this is fragile anyway, as this only applies
to SPI0, and other SoCs use many more multiplexes as well. Please have
a look at Samuel's pinctrl series to see if it fixes your problem, and
possibly help reviewing this:
https://lists.denx.de/pipermail/u-boot/2021-October/464248.html

Cheers,
Andre

>  				sunxi_gpio_set_cfgpin(pin, SUN50I_GPC_SPI0);
>  			else
>  				sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_SPI0);

Re: [PATCH 1/1] sunxi: SPI: fix pinmuxing for Allwinner H6 SoCs

[Daniel Wagenknecht]

Hi Andre,

On 12/15/21 12:54 AM, Andre Przywara wrote:

> please don't just resend without saying why. If you messed up the mail,
> or forgot to CC: the list, then please put "RESEND" in the subject, and
> briefly say why you resend.

yes, this was a RESEND. The original submission was rejected by the
mailinglist because I wasn't a subscriber yet. You and the other CC'd
people got the mail multiple times without an obvious reason, sorry.

> If you have some spare cycles ;-) you can look whether we can fix up
> the DT when no eMMC is detected, and how to make this work so this not
> only applies to the kernel, but also to U-Boot itself.

I'm not sure if implementing some magic here would be good. I did
experiment with adapting the fdt in the u-boot shell, but if implemented
correctly this would probably live in the board initialization code,
right? I haven't looked for references of similar implementations in the
u-boot tree yet.
The sunxi mmc and spi driver both initialize the pins before starting a
transfer, correct? So activating both in the u-boot fdt would probably
work due to the single-threaded execution. But when that same fdt would
be passed to the linux kernel (as with UEFI on U-Boot) it would cause
conflict there.

Like I mentioned above: I wouldn't want to implement too much magic here.

> Please have
> a look at Samuel's pinctrl series to see if it fixes your problem, and
> possibly help reviewing this:
> https://lists.denx.de/pipermail/u-boot/2021-October/464248.html

That looks promising, [PATCH
22/23](https://lists.denx.de/pipermail/u-boot/2021-October/464267.html)
defines the correct pinctrl value for spi0:
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> index d80886269c..b3b5228214 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> @@ -15,6 +15,7 @@ static const struct sunxi_pinctrl_function sun50i_h6_pinctrl_functions[] = {
>  	{ "mmc0",	2 },	/* PF0-PF5 */
>  	{ "mmc1",	2 },	/* PG0-PG5 */
>  	{ "mmc2",	3 },	/* PC1-PC14 */
> +	{ "spi0",	4 },	/* PC0-PC7 */
>  #if IS_ENABLED(CONFIG_UART0_PORT_F)
>  	{ "uart0",	3 },	/* PF2-PF4 */
>  #else

I'll give it a try and provide feedback in that thread. If this get's
included my fix becomes obsolete.

>>  
>> -			if (IS_ENABLED(CONFIG_MACH_SUN50I))
>> +			if (IS_ENABLED(CONFIG_MACH_SUN50I) ||
>> +			    IS_ENABLED(CONFIG_MACH_SUN50I_H6))
> [...]
> So please put CONFIG_SUN50I_GEN_H6 in there.

I'll send a fixup anyhow!

Best Wishes
Daniel

[PATCH v2] sunxi: SPI: fix pinmuxing for Allwinner H6 SoCs

[Daniel Wagenknecht]

The driver for SPI0 on Allwinner H6 SoCs did not use the correct define
SUN50I_GPC_SPI0 for the pin function, but one for a different Allwinner
SoC series.

Fix the conditionals to use the correct define for H6 SoCs. This matches
the conditional logic in the SPL spi driver.

Tested by probing the spi-flash on a pine64_h64-model-b board with
adapted device-tree (disable mmc2, enable spi0).

Signed-off-by: Daniel Wagenknecht <dwagenk@mailbox.org>
---

Adapted to use CONFIG_SUN50I_GEN_H6 as requested.


 drivers/spi/spi-sunxi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index bc2f544e86..6a8ee8d542 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -249,7 +249,8 @@ static int sun4i_spi_parse_pins(struct udevice *dev)
 			if (pin < 0)
 				break;
 
-			if (IS_ENABLED(CONFIG_MACH_SUN50I))
+			if (IS_ENABLED(CONFIG_MACH_SUN50I) ||
+			    IS_ENABLED(CONFIG_SUN50I_GEN_H6))
 				sunxi_gpio_set_cfgpin(pin, SUN50I_GPC_SPI0);
 			else
 				sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_SPI0);
-- 
2.31.1