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

[Daniel Wagenknecht <dwagenk@mailbox.org>]

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