Re: [PATCH 1/2] arm64: dts: sun50i: H6: Add SPI controllers nodes and pinmuxes

[André Przywara <andre.przywara@xxxxxxx>]

On 11/01/2020 17:26, Maxime Ripard wrote:

Hi Maxime,

> On Wed, Jan 08, 2020 at 10:10:05AM +0000, Andre Przywara wrote:
>> The Allwinner H6 SoC contains two SPI controllers similar to the H3/A64,
>> but with the added capability of 3-wire and 4-wire operation modes.
>> For now the driver does not support those, but the SPI registers are
>> fully backwards-compatible, just adding bits and registers which were
>> formerly reserved. So we can use the existing driver for the "normal" SPI
>> modes, for instance to access the SPI NOR flash soldered on the PineH64
>> board.
>> We use an H6 specific compatible string in addition to the existing H3
>> string, so when the driver later gains Quad SPI support, it should work
>> automatically without any DT changes.
>>
>> Tested by accessing the SPI flash on a Pine H64 board (SPI0), also
>> connecting another SPI flash to the SPI1 header pins.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>> ---
>>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 54 ++++++++++++++++++++
>>  1 file changed, 54 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> index 3329283e38ab..40835850893e 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> @@ -338,6 +338,30 @@
>>  				bias-pull-up;
>>  			};
>>
>> +			/omit-if-no-ref/
>> +			spi0_pins: spi0-pins {
>> +				pins = "PC0", "PC2", "PC3";
>> +				function = "spi0";
>> +			};
>> +
>> +			/omit-if-no-ref/
>> +			spi0_cs_pin: spi0-cs-pin {
>> +				pins = "PC5";
>> +				function = "spi0";
>> +			};
> 
> It seems suspicious to use it in the Pine H64, since PC5 is also used
> by the eMMC (and this prevents either the SPI or the emmc controller
> to probe, depending on which probed first).

Argh, good catch! I saw that AW changed the pin sharing between SPI and
MMC2 slightly, but didn't actually check that they made it worse :-(
Because this time it's the MMC CMD pin affected, and not the somewhat
optional DS pin as in the A64.
So I see we can't really have both at the same time. So what about this:

We keep the SPI flash chip described as in patch 2/2 (as it's soldered
on every board), but mark it as disabled and explain this in a comment.
This way we can't access it under Linux, but keep a potential eMMC chip
accessible.

In U-Boot's DT copy we could deviate and mark it as "okay", as U-Boot
doesn't use both eMMC and SPI at the same time. I need to check whether
this works or we would need to move the pinmux setup out of the probe
routine into something later.

And we could go one step further: If U-Boot detects an eMMC connected
(it's on a socket and so optional), it changes the SPI flash status to
"disabled", to allow EFI apps and kernels using this DT to access the
eMMC - which is far more useful than the SPI flash.
Otherwise (no eMMC connected) it can stay at "okay", as there would be
no conflict.

Does this make sense?

Cheers,
Andre.