Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller

[Gregory CLEMENT]

Hi Rob,
 
 On mar., oct. 11 2016, Ziji Hu <huziji@xxxxxxxxxxx> wrote:
[...]

>>> +  Different Xenon SDHC release has different register set size.
>>> +  The specific size should also refer to the SOC implementation.
>>> +
>>> +Optional Properties:
>>> +- Slot Index
>>> +  A single Xenon IP can support multiple slots.
>>> +  During initialization, each slot should set corresponding setting bit in
>>> +  some Xenon-specific registers. The corresponding bit is determined by
>>> +  this property.
>>> +  - xenon,slotno = <slot_index>;
>> 
>> Slots should probably be represented as child nodes with the reg 
>> property being the slot number.
>
> 	Since each SDHC slot is independent, I find it is more
> convenient to implement each one as independent SD host/MMC host
> instant.
> 	Otherwise, a main function should loop and initialize each
> slot, like sdhci-pci. I prefer to avoiding such a unnecessary main
> function.
>
> 	It is very hard to determine the slot number by reg property.
> 	Xenon slots are likely to be different types. 1st slot might
> be eMMC and 2nd one might be SD. They might have different register
> size.
> 	The register size might also varies in different Xenon versions.
>

Something that took me a while to figure out is that even it is the same
hardware block which handle multiple SoCs.

Each slots is managed by its own set of register. From the point of view
of the OS, it is as if we have an independent controller for each
slot.

But for an obscure reason, some command need to know which slot is
used. That's why we ended with this property.

With some example what you had in mind was something like that:
sdhci@aa0000 {
		compatible = "marvell,armada-3700-sdhci";
		reg = <0xaa0000 0x1000>;
[...]
                slot0 {
                      /* slot0 is an eMMC */
                      reg = <0>;
                      bus-width = <8>;
                      xenon,pad-type = "fixed-1-8v";

                }
                slot1 {
                      /* slot1 is an SD Card */
                      reg = <1>;
                      bus-width = <4>;
                      xenon,pad-type = "fixed-1-8v";

                }
	};

But it won't work as each slot uses its own address registers, that why we
ended with this:
sdhci@aa0000 {
                /* slot0 is an eMMC */
		compatible = "marvell,armada-3700-sdhci";
		reg = <0xaa0000 0x1000>;
[...]
                xenon,slotno = <0>;
                bus-width = <8>;
                xenon,pad-type = "fixed-1-8v";
	};
sdhci@bb0000 {
                /* slot1 is an SD Card */
		compatible = "marvell,armada-3700-sdhci";
		reg = <0xbb0000 0x1000>;
[...]
                xenon,slotno = <1>;
                bus-width = <4>;
                xenon,pad-type = "fixed-1-8v";
	};

I hope it is more clear now.

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com