[linux-sunxi] [PATCH 2/3] arm64: allwinner: h6: add device tree nodes for MMC controllers

André Przywara
2018-04-27T21:25:49Z

On 27/04/18 10:23, Icenowy Zheng wrote:
>
>
> ? 2018?4?27? GMT+08:00 ??5:18:23, Andre Przywara <andre.przywara@arm.com> ??:
>> Hi,
>>
>> On 27/04/18 09:36, Icenowy Zheng wrote:
>>>
>>>
>>> ? 2018?4?27? GMT+08:00 ??12:45:38, Andre Przywara
>> <andre.przywara@arm.com> ??:
>>>> Hi,
>>>>
>>>> On 26/04/18 15:07, Icenowy Zheng wrote:
>>>>> The Allwinner H6 SoC have 3 MMC controllers.
>>>>>
>>>>> Add device tree nodes for them.
>>>>>
>>>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>>>> ---
>>>>> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 56
>>>> ++++++++++++++++++++++++++++
>>>>> 1 file changed, 56 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>>>> b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>>>>> index 4debc3962830..3cbfc035c979 100644
>>>>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>>>>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>>>>> @@ -124,12 +124,68 @@
>>>>> interrupt-controller;
>>>>> #interrupt-cells = <3>;
>>>>>
>>>>> + mmc0_pins: mmc0-pins {
>>>>> + pins = "PF0", "PF1", "PF2", "PF3",
>>>>> + "PF4", "PF5";
>>>>> + function = "mmc0";
>>>>> + drive-strength = <30>;
>>>>> + bias-pull-up;
>>>>> + };
>>>>> +
>>>>> + mmc2_pins: mmc2-pins {
>>>>> + pins = "PC1", "PC4", "PC5", "PC6",
>>>>> + "PC7", "PC8", "PC9", "PC10",
>>>>> + "PC11", "PC12", "PC13", "PC14";
>>>>> + function = "mmc2";
>>>>> + drive-strength = <30>;
>>>>> + bias-pull-up;
>>>>> + };
>>>>> +
>>>>> uart0_ph_pins: uart0-ph {
>>>>> pins = "PH0", "PH1";
>>>>> function = "uart0";
>>>>> };
>>>>> };
>>>>>
>>>>> + mmc0: mmc at 4020000 {
>>>>> + compatible = "allwinner,sun50i-h6-mmc";
>>>>
>>>> This should be:
>>>> compatible = "allwinner,sun50i-h6-mmc",
>>>> "allwinner,sun50i-a64-mmc";
>>>
>>> I'm intended to not add A64 compatible, as
>>> H6 is a quite new design
>>> (new process) and there might be different behavior, even on mmc0/1.
>>
>> But as your patch proves, it is fully backwards compatible: An A64
>> driver works with this device.
>
> No, my patch only proves "the current A64 driver works
> with this device", not "Any A64 driver works with device", as
> the current driver doesn't fully use the capability provided
> by A64 MMC cobtrollers.

Good point, but I still believe every A64 driver would be capable of
driving an H6 MMC controller, ....

>> And this is what this compatible string list says: If your system does
>> not have a specific H6 driver, you can use an A64 driver.
>> You might not get all the (potentially) new features, but it covers
>> everything the A64 has.
>>
>> And a new silicon process doesn't matter here, since the software
>> interface is unchanged. *If* we find bugs, we can add quirks matching
>
> I think there's timing parameters for higher speed bins which
> are different among chips. As we have currently no support
> for speed bins higher than DDR50, they're not added yet.

True, but what are those differences? I compared the A64 and H6 manuals
side by side, the differences I found are:
SMHC_FIFOTH[+0x40]:
BSIZE_OF_TRANS[30:28]:
- H6 supports 16 transfers for SMHC0 also.
other parameters:
- H6 recommends better values for SMHC0 also
SMHC_CSDC[+0x54]:
- H6 doesn't mention restriction to SMHC2
(though this might be a mistake)
SMHC_NTSR_REG[+0x5C]:
- H6 defines fields for bits[24:8]
SMHC_EMCE[+0x64] and SMHC_EMCE_DBG[+0x68]:
- H6 adds, for EMCE support
EMMC_DDR_SBIT_DET_REG[0x10c]:
- A64 doesn't mention restriction to SMHC2,
but I believe this is a mistake
SMHC_EMCE_BMn[0x150 + 0x4 * 0..31]
- H6 adds, for EMCE support

All those pieces are only *additions* to the H6 over the A64, so don't
affect backwards compatibility.

>> on
>> the H6 compatible string - that's why we put it here already, despite
>> having a matching string in the kernel at the moment.
>
> Device tree is not driver data but hardware description, so
> it should follow "how the device is formed" rather than
> "how the device works".

True, but as shown above, the compatibility is really at the device level.
Unless you have any other information ...

Cheers,
Andre.