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

Commit Message

Gregory CLEMENT Oct. 31, 2016, 11:09 a.m. UTC

From: Ziji Hu <[e-mail]>

Marvell Xenon SDHC can support eMMC/SD/SDIO.
Add Xenon-specific properties.
Also add properties for Xenon PHY setting.

Signed-off-by: Hu Ziji <[e-mail]>
Signed-off-by: Gregory CLEMENT <[e-mail]>
---
 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
 MAINTAINERS                                                   |   1 +-
 2 files changed, 162 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt

Comments

Rob Herring (Arm) Nov. 9, 2016, 6:24 p.m. UTC | #1

On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:
> From: Ziji Hu <[e-mail]>
> 
> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> Add Xenon-specific properties.
> Also add properties for Xenon PHY setting.
> 
> Signed-off-by: Hu Ziji <[e-mail]>
> Signed-off-by: Gregory CLEMENT <[e-mail]>
> ---
>  Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
>  MAINTAINERS                                                   |   1 +-
>  2 files changed, 162 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> 
> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> new file mode 100644
> index 000000000000..0d2d139494d3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> @@ -0,0 +1,161 @@
> +Marvell's Xenon SDHCI Controller device tree bindings
> +This file documents differences between the core mmc properties
> +described by mmc.txt and the properties used by the Xenon implementation.
> +
> +A single Xenon IP can support multiple slots.
> +Each slot acts as an independent SDHC. It owns independent resources, such
> +as register sets clock and PHY.
> +Each slot should have an independent device tree node.
> +
> +Required Properties:
> +- compatible: should be one of the following
> +  - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
> +  Must provide a second register area and marvell,pad-type.
> +  - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
> +  Armada-3700.

Need SoC specific compatible strings.

> +
> +- clocks:
> +  Array of clocks required for SDHCI.
> +  Requires at least one for Xenon IP core.
> +  Some SOCs require additional clock for AXI bus.
> +
> +- clock-names:
> +  Array of names corresponding to clocks property.
> +  The input clock for Xenon IP core should be named as "core".
> +  The optional AXI clock should be named as "axi".

When is AXI clock optional? This should be required for ?? compatible 
strings.

> +
> +- reg:
> +  * For "marvell,xenon-sdhci", one register area for Xenon IP.
> +
> +  * For "marvell,armada-3700-sdhci", two register areas.
> +    The first one for Xenon IP register. The second one for the Armada 3700 SOC
> +    PHY PAD Voltage Control register.
> +    Please follow the examples with compatible "marvell,armada-3700-sdhci"
> +    in below.
> +    Please also check property marvell,pad-type in below.
> +
> +Optional Properties:
> +- marvell,xenon-slotno:

Multiple slots should be represented as child nodes IMO. I think some 
other bindings already do this.

> +  Indicate the corresponding bit index of current Xenon SDHC slot in
> +  SDHC System Operation Control Register Bit[7:0].
> +  Set/clear the corresponding bit to enable/disable current Xenon SDHC
> +  slot.
> +  If this property is not provided, Xenon IP should contain only one
> +  slot.
> +
> +- marvell,xenon-phy-type:
> +  Xenon support mutilple types of PHYs.
> +  To select eMMC 5.1 PHY, set:
> +  marvell,xenon-phy-type = "emmc 5.1 phy"
> +  eMMC 5.1 PHY is the default choice if this property is not provided.
> +  To select eMMC 5.0 PHY, set:
> +  marvell,xenon-phy-type = "emmc 5.0 phy"
> +  To select SDH PHY, set:
> +  marvell,xenon-phy-type = "sdh phy"
> +  Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
> +  eMMC only.

Does this vary per instance on a single SoC? If not, then an SoC 
specific compatible should determine this.

Also, the " phy" part is redundant.

> +
> +- marvell,xenon-phy-znr:
> +  Set PHY ZNR value.
> +  Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
> +  valid range = [0:0x1F].
> +  ZNR is set as 0xF by default if this property is not provided.
> +
> +- marvell,xenon-phy-zpr:
> +  Set PHY ZPR value.
> +  Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
> +  valid range = [0:0x1F].
> +  ZPR is set as 0xF by default if this property is not provided.
> +
> +- marvell,xenon-phy-nr-success-tun:
> +  Set the number of required consecutive successful sampling points used to
> +  identify a valid sampling window, in tuning process.
> +  Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
> +
> +- marvell,xenon-phy-tun-step-divider:
> +  Set the divider for calculating TUN_STEP.
> +  Set as 64 by default if this property is not provided.
> +
> +- marvell,xenon-phy-slow-mode:
> +  Force PHY into slow mode.
> +  Only available when bus frequency lower than 50MHz in SDR mde.
> +  Disabled by default. Please do not enable it unless it is necessary.
> +
> +- marvell,xenon-mask-conflict-err:
> +  Mask Conflict Error alert on some SOC. Disabled by default.
> +
> +- marvell,xenon-tun-count:
> +  Xenon SDHC SOC usually doesn't provide re-tuning counter in
> +  Capabilities Register 3 Bit[11:8].
> +  This property provides the re-tuning counter.
> +  If this property is not set, default re-tuning counter will
> +  be set as 0x9 in driver.
> +
> +- marvell,pad-type:
> +  Type of Armada 3700 SOC PHY PAD Voltiage Controller register.
> +  Only valid when "marvell,armada-3700-sdhci" is selected.
> +  Two types: "sd" and "fixed-1-8v".
> +  If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
> +  switched to 1.8V when SD in UHS-I.
> +  If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
> +  Please follow the examples with compatible "marvell,armada-3700-sdhci"
> +  in below.
> +
> +Example:
> +- For eMMC slot:
> +
> +	sdhci@aa0000 {
> +		compatible = "marvell,xenon-sdhci";
> +		reg = <0xaa0000 0x1000>;
> +		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> +		clocks = <&emmc_clk>, <&axi_clock>;
> +		clock-names = "core", "axi";
> +		bus-width = <8>;
> +		marvell,xenon-emmc;

Not documented. If we need to specify the type of slot/card, then we 
need to come up with a standard property. This was either already done 
or attempted IIRC.

> +		marvell,xenon-slotno = <0>;
> +		marvell,xenon-phy-type = "emmc 5.1 phy";
> +		marvell,xenon-tun-count = <11>;
> +	};
> +
> +- For SD/SDIO slot:
> +
> +	sdhci@ab0000 {
> +		compatible = "marvell,xenon-sdhci";
> +		reg = <0xab0000 0x1000>;
> +		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> +		vqmmc-supply = <&sd_regulator>;
> +		clocks = <&sdclk>;
> +		clock-names = "core";
> +		bus-width = <4>;
> +		marvell,xenon-tun-count = <9>;
> +	};
> +
> +- For eMMC slot with compatible "marvell,armada-3700-sdhci":
> +
> +	sdhci@aa0000 {
> +		compatible = "marvell,armada-3700-sdhci";
> +		reg = <0xaa0000 0x1000>,
> +		      <phy_addr 0x4>;
> +		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> +		clocks = <&emmcclk>;
> +		clock-names = "core";
> +		bus-width = <8>;
> +		marvell,xenon-emmc;
> +
> +		marvell,pad-type = "fixed-1-8v";
> +	};
> +
> +- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
> +
> +	sdhci@ab0000 {
> +		compatible = "marvell,armada-3700-sdhci";
> +		reg = <0xab0000 0x1000>,
> +		      <phy_addr 0x4>;
> +		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> +		vqmmc-supply = <&sd_regulator>;
> +		clocks = <&sdclk>;
> +		clock-names = "core";
> +		bus-width = <4>;
> +
> +		marvell,pad-type = "sd";
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1a5c4c30ea24..850a0afb0c8d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
>  M:	Ziji Hu <[e-mail]>
>  L:	[e-mail]
>  S:	Supported
> +F:	Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>  
>  MATROX FRAMEBUFFER DRIVER
>  L:	[e-mail]
> -- 
> git-series 0.8.10
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hu Ziji Nov. 10, 2016, 11:44 a.m. UTC | #2

Hi Rob,

On 2016/11/10 2:24, Rob Herring wrote:
> On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:
>> From: Ziji Hu <[e-mail]>
>>
>> Marvell Xenon SDHC can support eMMC/SD/SDIO.
>> Add Xenon-specific properties.
>> Also add properties for Xenon PHY setting.
>>
>> Signed-off-by: Hu Ziji <[e-mail]>
>> Signed-off-by: Gregory CLEMENT <[e-mail]>
>> ---
>>  Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
>>  MAINTAINERS                                                   |   1 +-
>>  2 files changed, 162 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>> new file mode 100644
>> index 000000000000..0d2d139494d3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>> @@ -0,0 +1,161 @@
>> +Marvell's Xenon SDHCI Controller device tree bindings
>> +This file documents differences between the core mmc properties
>> +described by mmc.txt and the properties used by the Xenon implementation.
>> +
>> +A single Xenon IP can support multiple slots.
>> +Each slot acts as an independent SDHC. It owns independent resources, such
>> +as register sets clock and PHY.
>> +Each slot should have an independent device tree node.
>> +
>> +Required Properties:
>> +- compatible: should be one of the following
>> +  - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
>> +  Must provide a second register area and marvell,pad-type.
>> +  - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
>> +  Armada-3700.
> 
> Need SoC specific compatible strings.
> 

	Xenon SDHC is a common IP for all Marvell SOCs.
	It is difficult to use a single SOC specific compatible to represent Xenon SDHC.
	There will be so many SOC compatible strings in list if each specific SOC owns a compatible.
	Actually only few SOCs require special properties.
	Any suggestion please?

>> +
>> +- clocks:
>> +  Array of clocks required for SDHCI.
>> +  Requires at least one for Xenon IP core.
>> +  Some SOCs require additional clock for AXI bus.
>> +
>> +- clock-names:
>> +  Array of names corresponding to clocks property.
>> +  The input clock for Xenon IP core should be named as "core".
>> +  The optional AXI clock should be named as "axi".
> 
> When is AXI clock optional? This should be required for ?? compatible 
> strings.
> 
	It is required on some SOCs.
	I will double check if a suitable compatible string can be determined for those SOCs.

>> +
>> +- reg:
>> +  * For "marvell,xenon-sdhci", one register area for Xenon IP.
>> +
>> +  * For "marvell,armada-3700-sdhci", two register areas.
>> +    The first one for Xenon IP register. The second one for the Armada 3700 SOC
>> +    PHY PAD Voltage Control register.
>> +    Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> +    in below.
>> +    Please also check property marvell,pad-type in below.
>> +
>> +Optional Properties:
>> +- marvell,xenon-slotno:
> 
> Multiple slots should be represented as child nodes IMO. I think some 
> other bindings already do this.
> 

	All the slots are entirely independent.
	I prefer to consider it as multiple independent SDHCs placed in a single IP, instead of that a IP contains multiple child slots.

	It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
	If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
	In my very own opinion, it is inconvenient and unnecessary.

>> +  Indicate the corresponding bit index of current Xenon SDHC slot in
>> +  SDHC System Operation Control Register Bit[7:0].
>> +  Set/clear the corresponding bit to enable/disable current Xenon SDHC
>> +  slot.
>> +  If this property is not provided, Xenon IP should contain only one
>> +  slot.
>> +
>> +- marvell,xenon-phy-type:
>> +  Xenon support mutilple types of PHYs.
>> +  To select eMMC 5.1 PHY, set:
>> +  marvell,xenon-phy-type = "emmc 5.1 phy"
>> +  eMMC 5.1 PHY is the default choice if this property is not provided.
>> +  To select eMMC 5.0 PHY, set:
>> +  marvell,xenon-phy-type = "emmc 5.0 phy"
>> +  To select SDH PHY, set:
>> +  marvell,xenon-phy-type = "sdh phy"
>> +  Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
>> +  eMMC only.
> 
> Does this vary per instance on a single SoC? If not, then an SoC 
> specific compatible should determine this.
> 
> Also, the " phy" part is redundant.
> 

	Yes. Some SOCs might have multiple Xenon PHY types.

	This property is only the name/type of PHY. It doesn't stand for the entire SDHC property.
	"emmc 5.1 PHY" doesn't mean that this Xenon SDHC only support eMMC 5.1.
	Xenon SDHC with "sdh PHY" can also support eMMC. 

>> +
>> +- marvell,xenon-phy-znr:
>> +  Set PHY ZNR value.
>> +  Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
>> +  valid range = [0:0x1F].
>> +  ZNR is set as 0xF by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-zpr:
>> +  Set PHY ZPR value.
>> +  Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
>> +  valid range = [0:0x1F].
>> +  ZPR is set as 0xF by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-nr-success-tun:
>> +  Set the number of required consecutive successful sampling points used to
>> +  identify a valid sampling window, in tuning process.
>> +  Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-tun-step-divider:
>> +  Set the divider for calculating TUN_STEP.
>> +  Set as 64 by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-slow-mode:
>> +  Force PHY into slow mode.
>> +  Only available when bus frequency lower than 50MHz in SDR mde.
>> +  Disabled by default. Please do not enable it unless it is necessary.
>> +
>> +- marvell,xenon-mask-conflict-err:
>> +  Mask Conflict Error alert on some SOC. Disabled by default.
>> +
>> +- marvell,xenon-tun-count:
>> +  Xenon SDHC SOC usually doesn't provide re-tuning counter in
>> +  Capabilities Register 3 Bit[11:8].
>> +  This property provides the re-tuning counter.
>> +  If this property is not set, default re-tuning counter will
>> +  be set as 0x9 in driver.
>> +
>> +- marvell,pad-type:
>> +  Type of Armada 3700 SOC PHY PAD Voltiage Controller register.
>> +  Only valid when "marvell,armada-3700-sdhci" is selected.
>> +  Two types: "sd" and "fixed-1-8v".
>> +  If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
>> +  switched to 1.8V when SD in UHS-I.
>> +  If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
>> +  Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> +  in below.
>> +
>> +Example:
>> +- For eMMC slot:
>> +
>> +	sdhci@aa0000 {
>> +		compatible = "marvell,xenon-sdhci";
>> +		reg = <0xaa0000 0x1000>;
>> +		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> +		clocks = <&emmc_clk>, <&axi_clock>;
>> +		clock-names = "core", "axi";
>> +		bus-width = <8>;
>> +		marvell,xenon-emmc;
> 
> Not documented. If we need to specify the type of slot/card, then we 
> need to come up with a standard property. This was either already done 
> or attempted IIRC.

	Sorry to lost this property in above.
	I will add it in above.

	Thank you.

Best regards,
Hu Ziji

> 
>> +		marvell,xenon-slotno = <0>;
>> +		marvell,xenon-phy-type = "emmc 5.1 phy";
>> +		marvell,xenon-tun-count = <11>;
>> +	};
>> +
>> +- For SD/SDIO slot:
>> +
>> +	sdhci@ab0000 {
>> +		compatible = "marvell,xenon-sdhci";
>> +		reg = <0xab0000 0x1000>;
>> +		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> +		vqmmc-supply = <&sd_regulator>;
>> +		clocks = <&sdclk>;
>> +		clock-names = "core";
>> +		bus-width = <4>;
>> +		marvell,xenon-tun-count = <9>;
>> +	};
>> +
>> +- For eMMC slot with compatible "marvell,armada-3700-sdhci":
>> +
>> +	sdhci@aa0000 {
>> +		compatible = "marvell,armada-3700-sdhci";
>> +		reg = <0xaa0000 0x1000>,
>> +		      <phy_addr 0x4>;
>> +		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> +		clocks = <&emmcclk>;
>> +		clock-names = "core";
>> +		bus-width = <8>;
>> +		marvell,xenon-emmc;
>> +
>> +		marvell,pad-type = "fixed-1-8v";
>> +	};
>> +
>> +- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
>> +
>> +	sdhci@ab0000 {
>> +		compatible = "marvell,armada-3700-sdhci";
>> +		reg = <0xab0000 0x1000>,
>> +		      <phy_addr 0x4>;
>> +		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> +		vqmmc-supply = <&sd_regulator>;
>> +		clocks = <&sdclk>;
>> +		clock-names = "core";
>> +		bus-width = <4>;
>> +
>> +		marvell,pad-type = "sd";
>> +	};
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1a5c4c30ea24..850a0afb0c8d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
>>  M:	Ziji Hu <[e-mail]>
>>  L:	[e-mail]
>>  S:	Supported
>> +F:	Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>>  
>>  MATROX FRAMEBUFFER DRIVER
>>  L:	[e-mail]
>> -- 
>> git-series 0.8.10
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jisheng Zhang Nov. 11, 2016, 3:22 a.m. UTC | #3

Hi Rob, Ziji,

On Thu, 10 Nov 2016 19:44:19 +0800 Ziji Hu wrote:

> Hi Rob,
> 
> On 2016/11/10 2:24, Rob Herring wrote:
> > On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:  
> >> From: Ziji Hu <[e-mail]>
> >>
> >> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> >> Add Xenon-specific properties.
> >> Also add properties for Xenon PHY setting.
> >>
> >> Signed-off-by: Hu Ziji <[e-mail]>
> >> Signed-off-by: Gregory CLEMENT <[e-mail]>
> >> ---
> >>  Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
> >>  MAINTAINERS                                                   |   1 +-
> >>  2 files changed, 162 insertions(+), 0 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >> new file mode 100644
> >> index 000000000000..0d2d139494d3
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >> @@ -0,0 +1,161 @@
> >> +Marvell's Xenon SDHCI Controller device tree bindings
> >> +This file documents differences between the core mmc properties
> >> +described by mmc.txt and the properties used by the Xenon implementation.
> >> +
> >> +A single Xenon IP can support multiple slots.
> >> +Each slot acts as an independent SDHC. It owns independent resources, such
> >> +as register sets clock and PHY.
> >> +Each slot should have an independent device tree node.
> >> +
> >> +Required Properties:
> >> +- compatible: should be one of the following
> >> +  - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
> >> +  Must provide a second register area and marvell,pad-type.
> >> +  - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
> >> +  Armada-3700.  
> > 
> > Need SoC specific compatible strings.
> >   
> 
> 	Xenon SDHC is a common IP for all Marvell SOCs.
> 	It is difficult to use a single SOC specific compatible to represent Xenon SDHC.
> 	There will be so many SOC compatible strings in list if each specific SOC owns a compatible.
> 	Actually only few SOCs require special properties.
> 	Any suggestion please?
> 
> >> +
> >> +- clocks:
> >> +  Array of clocks required for SDHCI.
> >> +  Requires at least one for Xenon IP core.
> >> +  Some SOCs require additional clock for AXI bus.
> >> +
> >> +- clock-names:
> >> +  Array of names corresponding to clocks property.
> >> +  The input clock for Xenon IP core should be named as "core".
> >> +  The optional AXI clock should be named as "axi".  
> > 
> > When is AXI clock optional? This should be required for ?? compatible 
> > strings.
> >   
> 	It is required on some SOCs.
> 	I will double check if a suitable compatible string can be determined for those SOCs.

Besides the core clk, berlin SoCs have one AXI clock. Usually, we have two
solutions:

solA: as current patch does, take "marvell,xenon-sdhci" as compatible string
and make the AXI clock property optional. Usually for berlin SoCs, we don't need
special properties.

PS: this solution is also what sdhci-pxav3.c takes

solB: As Rob said, add extra SoC compatible strings, so we'll have
something like:

static const struct of_device_id sdhci_xenon_of_match[] = {
	{ .compatible = "marvell,armada-3700-sdhci", },
	{ .compatible = "marvell,berlin4ct-sdhci", },
	...
	{ .compatible = "marvell,berlinxxx-mmc", },
}

then we take care the AXI clk for berlin SoCs in the code.


Which solution do you prefer?

Thanks,
Jisheng
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jisheng Zhang Nov. 11, 2016, 3:33 a.m. UTC | #4

On Fri, 11 Nov 2016 11:22:43 +0800 Jisheng Zhang wrote:

> Hi Rob, Ziji,
> 
> On Thu, 10 Nov 2016 19:44:19 +0800 Ziji Hu wrote:
> 
> > Hi Rob,
> > 
> > On 2016/11/10 2:24, Rob Herring wrote:  
> > > On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:    
> > >> From: Ziji Hu <[e-mail]>
> > >>
> > >> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> > >> Add Xenon-specific properties.
> > >> Also add properties for Xenon PHY setting.
> > >>
> > >> Signed-off-by: Hu Ziji <[e-mail]>
> > >> Signed-off-by: Gregory CLEMENT <[e-mail]>
> > >> ---
> > >>  Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
> > >>  MAINTAINERS                                                   |   1 +-
> > >>  2 files changed, 162 insertions(+), 0 deletions(-)
> > >>  create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> > >> new file mode 100644
> > >> index 000000000000..0d2d139494d3
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> > >> @@ -0,0 +1,161 @@
> > >> +Marvell's Xenon SDHCI Controller device tree bindings
> > >> +This file documents differences between the core mmc properties
> > >> +described by mmc.txt and the properties used by the Xenon implementation.
> > >> +
> > >> +A single Xenon IP can support multiple slots.
> > >> +Each slot acts as an independent SDHC. It owns independent resources, such
> > >> +as register sets clock and PHY.
> > >> +Each slot should have an independent device tree node.
> > >> +
> > >> +Required Properties:
> > >> +- compatible: should be one of the following
> > >> +  - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
> > >> +  Must provide a second register area and marvell,pad-type.
> > >> +  - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
> > >> +  Armada-3700.    
> > > 
> > > Need SoC specific compatible strings.
> > >     
> > 
> > 	Xenon SDHC is a common IP for all Marvell SOCs.
> > 	It is difficult to use a single SOC specific compatible to represent Xenon SDHC.
> > 	There will be so many SOC compatible strings in list if each specific SOC owns a compatible.
> > 	Actually only few SOCs require special properties.
> > 	Any suggestion please?
> >   
> > >> +
> > >> +- clocks:
> > >> +  Array of clocks required for SDHCI.
> > >> +  Requires at least one for Xenon IP core.
> > >> +  Some SOCs require additional clock for AXI bus.
> > >> +
> > >> +- clock-names:
> > >> +  Array of names corresponding to clocks property.
> > >> +  The input clock for Xenon IP core should be named as "core".
> > >> +  The optional AXI clock should be named as "axi".    
> > > 
> > > When is AXI clock optional? This should be required for ?? compatible 
> > > strings.
> > >     
> > 	It is required on some SOCs.
> > 	I will double check if a suitable compatible string can be determined for those SOCs.  
> 
> Besides the core clk, berlin SoCs have one AXI clock. Usually, we have two
> solutions:
> 
> solA: as current patch does, take "marvell,xenon-sdhci" as compatible string
> and make the AXI clock property optional. Usually for berlin SoCs, we don't need
> special properties.

Personally, I prefer solA: use the IP name as compatible string. This is IP
specific rather than SoC specific. The HW itself supports two clks

Thanks,
Jisheng

> 
> PS: this solution is also what sdhci-pxav3.c takes
> 
> solB: As Rob said, add extra SoC compatible strings, so we'll have
> something like:
> 
> static const struct of_device_id sdhci_xenon_of_match[] = {
> 	{ .compatible = "marvell,armada-3700-sdhci", },
> 	{ .compatible = "marvell,berlin4ct-sdhci", },
> 	...
> 	{ .compatible = "marvell,berlinxxx-mmc", },
> }
> 
> then we take care the AXI clk for berlin SoCs in the code.
> 
> 
> Which solution do you prefer?
> 
> Thanks,
> Jisheng

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Gregory CLEMENT Nov. 22, 2016, 5:23 p.m. UTC | #5

Hi Rob,
 
 On jeu., nov. 10 2016, Ziji Hu <[e-mail]> wrote:

[...]

>>> +
>>> +- reg:
>>> +  * For "marvell,xenon-sdhci", one register area for Xenon IP.
>>> +
>>> +  * For "marvell,armada-3700-sdhci", two register areas.
>>> +    The first one for Xenon IP register. The second one for the Armada 3700 SOC
>>> +    PHY PAD Voltage Control register.
>>> +    Please follow the examples with compatible "marvell,armada-3700-sdhci"
>>> +    in below.
>>> +    Please also check property marvell,pad-type in below.
>>> +
>>> +Optional Properties:
>>> +- marvell,xenon-slotno:
>> 
>> Multiple slots should be represented as child nodes IMO. I think some 
>> other bindings already do this.
>> 
>
> 	All the slots are entirely independent.
> 	I prefer to consider it as multiple independent SDHCs placed in
> 	a single IP, instead of that a IP contains multiple child slots.

It was indeed what I tried to show in my answer for the 1st version:
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461860.html

Maybe you missed it.

You also mentioned other bindings using child nodes, but for this one
we have one controller with only one set of register with multiple slots
(Atmel is an example). Here each slot have it own set of register.

Actually giving the fact that each slot is controlled by a different set
of register I wonder why the hardware can't also deduce the slot number
from the address register. For me it looks like an hardware bug but we
have to deal with it.

Do you still think we needchild node here?

>
> 	It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
> 	If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
> 	In my very own opinion, it is inconvenient and unnecessary.


Gregory

Ulf Hansson Nov. 24, 2016, 9:05 a.m. UTC | #6

On 22 November 2016 at 18:23, Gregory CLEMENT
<[e-mail]> wrote:
> Hi Rob,
>
>  On jeu., nov. 10 2016, Ziji Hu <[e-mail]> wrote:
>
> [...]
>
>>>> +
>>>> +- reg:
>>>> +  * For "marvell,xenon-sdhci", one register area for Xenon IP.
>>>> +
>>>> +  * For "marvell,armada-3700-sdhci", two register areas.
>>>> +    The first one for Xenon IP register. The second one for the Armada 3700 SOC
>>>> +    PHY PAD Voltage Control register.
>>>> +    Please follow the examples with compatible "marvell,armada-3700-sdhci"
>>>> +    in below.
>>>> +    Please also check property marvell,pad-type in below.
>>>> +
>>>> +Optional Properties:
>>>> +- marvell,xenon-slotno:
>>>
>>> Multiple slots should be represented as child nodes IMO. I think some
>>> other bindings already do this.
>>>
>>
>>       All the slots are entirely independent.
>>       I prefer to consider it as multiple independent SDHCs placed in
>>       a single IP, instead of that a IP contains multiple child slots.
>
> It was indeed what I tried to show in my answer for the 1st version:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461860.html
>
> Maybe you missed it.
>
> You also mentioned other bindings using child nodes, but for this one
> we have one controller with only one set of register with multiple slots
> (Atmel is an example). Here each slot have it own set of register.
>
> Actually giving the fact that each slot is controlled by a different set
> of register I wonder why the hardware can't also deduce the slot number
> from the address register. For me it looks like an hardware bug but we
> have to deal with it.
>
> Do you still think we needchild node here?

Using child-nodes for slots like what's done in the atmel case, is
currently broken. I would recommend to avoid using child-nodes for
slots, if possible.

To give you some more background, currently the mmc core treats child
nodes as embedded non-removable cards or SDIO funcs. However, we can
change to make child-nodes also allowed to describe slots, but it
requires a specific compatible for "slots" and of course then we also
need to update the DT parsing of the child-nodes in the mmc core.

Documentation/devicetree/bindings/mmc/mmc.txt
Documentation/devicetree/bindings/mmc/mmc-card.txt

>
>>
>>       It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
>>       If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
>>       In my very own opinion, it is inconvenient and unnecessary.
>

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Arnd Bergmann Nov. 24, 2016, 9:11 a.m. UTC | #7

On Thursday, November 24, 2016 10:05:45 AM CET Ulf Hansson wrote:
> > You also mentioned other bindings using child nodes, but for this one
> > we have one controller with only one set of register with multiple slots
> > (Atmel is an example). Here each slot have it own set of register.
> >
> > Actually giving the fact that each slot is controlled by a different set
> > of register I wonder why the hardware can't also deduce the slot number
> > from the address register. For me it looks like an hardware bug but we
> > have to deal with it.
> >
> > Do you still think we needchild node here?
> 
> Using child-nodes for slots like what's done in the atmel case, is
> currently broken. I would recommend to avoid using child-nodes for
> slots, if possible.
> 
> To give you some more background, currently the mmc core treats child
> nodes as embedded non-removable cards or SDIO funcs. However, we can
> change to make child-nodes also allowed to describe slots, but it
> requires a specific compatible for "slots" and of course then we also
> need to update the DT parsing of the child-nodes in the mmc core.
> 
> Documentation/devicetree/bindings/mmc/mmc.txt
> Documentation/devicetree/bindings/mmc/mmc-card.txt

I don't see anything wrong with having child nodes for the slots
even with the current binding, under one condition:

The mmc.txt binding above must refer only to the child node, while
the parent node conceptually becomes a plain bus or MFD that
happens to encapsulate multiple MMC host controllers, and possibly
provides some shared registers to them.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Gregory CLEMENT Nov. 24, 2016, 9:22 a.m. UTC | #8

Hi Arnd,
 
 On jeu., nov. 24 2016, Arnd Bergmann <[e-mail]> wrote:

> On Thursday, November 24, 2016 10:05:45 AM CET Ulf Hansson wrote:
>> > You also mentioned other bindings using child nodes, but for this one
>> > we have one controller with only one set of register with multiple slots
>> > (Atmel is an example). Here each slot have it own set of register.
>> >
>> > Actually giving the fact that each slot is controlled by a different set
>> > of register I wonder why the hardware can't also deduce the slot number
>> > from the address register. For me it looks like an hardware bug but we
>> > have to deal with it.
>> >
>> > Do you still think we needchild node here?
>> 
>> Using child-nodes for slots like what's done in the atmel case, is
>> currently broken. I would recommend to avoid using child-nodes for
>> slots, if possible.
>> 
>> To give you some more background, currently the mmc core treats child
>> nodes as embedded non-removable cards or SDIO funcs. However, we can
>> change to make child-nodes also allowed to describe slots, but it
>> requires a specific compatible for "slots" and of course then we also
>> need to update the DT parsing of the child-nodes in the mmc core.
>> 
>> Documentation/devicetree/bindings/mmc/mmc.txt
>> Documentation/devicetree/bindings/mmc/mmc-card.txt
>
> I don't see anything wrong with having child nodes for the slots
> even with the current binding, under one condition:
>
> The mmc.txt binding above must refer only to the child node, while
> the parent node conceptually becomes a plain bus or MFD that
> happens to encapsulate multiple MMC host controllers, and possibly
> provides some shared registers to them.


I don't have an option for mmc in general, but using child node do not
fit at all the xenon controller.

For this controller each slot has its own set of register, so there is
no common ressource to share so no advantage to use it. Using child node
in our case will just make the code more complex for no benefit.

Gregory

>
> 	Arnd

Arnd Bergmann Nov. 24, 2016, 9:34 a.m. UTC | #9

On Thursday, November 24, 2016 10:22:31 AM CET Gregory CLEMENT wrote:
> 
> I don't have an option for mmc in general, but using child node do not
> fit at all the xenon controller.
> 
> For this controller each slot has its own set of register, so there is
> no common ressource to share so no advantage to use it. Using child node
> in our case will just make the code more complex for no benefit.

If every slot has its own registers, what is it that makes up the
'controller'? It sounds to me that you just have to adjust the terminology
and talk about multiple controllers then, with one slot per controller.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thomas Petazzoni Nov. 24, 2016, 9:48 a.m. UTC | #10

Hello,

On Thu, 24 Nov 2016 10:44:48 +0100, Gregory CLEMENT wrote:

> "A single Xenon IP can support multiple slots.
> Each slot acts as an independent SDHC. It owns independent resources, such
> as register sets clock and PHY.
> Each slot should have an independent device tree node."

I think this wording is still very confusing, and continues to cause
confusion.

We should just state that each Xenon controller supports a single slot,
and that's it.

The text still says "a single Xenon IP can support multiple slots",
which continues to cause confusion.

Best regards,

Thomas

Marcin Wojtas Nov. 24, 2016, 9:49 a.m. UTC | #11

Hi Gregory,

2016-11-24 10:44 GMT+01:00 Gregory CLEMENT <[e-mail]>:
> Hi Arnd,
>
>  On jeu., nov. 24 2016, Arnd Bergmann <[e-mail]> wrote:
>
>> On Thursday, November 24, 2016 10:22:31 AM CET Gregory CLEMENT wrote:
>>>
>>> I don't have an option for mmc in general, but using child node do not
>>> fit at all the xenon controller.
>>>
>>> For this controller each slot has its own set of register, so there is
>>> no common ressource to share so no advantage to use it. Using child node
>>> in our case will just make the code more complex for no benefit.
>>
>> If every slot has its own registers, what is it that makes up the
>> 'controller'? It sounds to me that you just have to adjust the terminology
>> and talk about multiple controllers then, with one slot per controller.
>>
>
> I agree and actually there were some words about in at the begining of
> the binding:
>
> "A single Xenon IP can support multiple slots.
> Each slot acts as an independent SDHC. It owns independent resources, such
> as register sets clock and PHY.
> Each slot should have an independent device tree node."
>
> All the confusion came from the fact that we still need to identify a
> slot ID. For an obscure reason the hardware can't guess the slot ID from
> the address register."
>

How about to avoid confusion, by simply renaming this number to
port-id/xenon-id or anything else but slot? I guess this may allow to
avoid some misunderstandings.

Best regards,
Marcin
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Arnd Bergmann Nov. 24, 2016, 10:04 a.m. UTC | #12

On Thursday, November 24, 2016 10:48:58 AM CET Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 24 Nov 2016 10:44:48 +0100, Gregory CLEMENT wrote:
> 
> > "A single Xenon IP can support multiple slots.
> > Each slot acts as an independent SDHC. It owns independent resources, such
> > as register sets clock and PHY.
> > Each slot should have an independent device tree node."
> 
> I think this wording is still very confusing, and continues to cause
> confusion.
> 
> We should just state that each Xenon controller supports a single slot,
> and that's it.
> 
> The text still says "a single Xenon IP can support multiple slots",
> which continues to cause confusion.

Agreed. Ideally we'd find out why exactly the slot number must
be used for accessing some of the registers to have a better
explanation to put in there, aside from stating that only one
slot is supported but the number must be set.

Could it be that this is some form of pinmuxing, i.e. that each
controller could in theory be used for any of the slots but you
have to pick one of them?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thomas Petazzoni Nov. 24, 2016, 10:10 a.m. UTC | #13

Hello,

On Thu, 24 Nov 2016 10:49:23 +0100, Marcin Wojtas wrote:

> How about to avoid confusion, by simply renaming this number to
> port-id/xenon-id or anything else but slot? I guess this may allow to
> avoid some misunderstandings.

Agreed.

Thomas

Hu Ziji Nov. 24, 2016, 10:38 a.m. UTC | #14

Hi all,

On 2016/11/24 18:10, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 24 Nov 2016 10:49:23 +0100, Marcin Wojtas wrote:
> 
>> How about to avoid confusion, by simply renaming this number to
>> port-id/xenon-id or anything else but slot? I guess this may allow to
>> avoid some misunderstandings.
> 
	We borrow the term "slot" from PCIe interface from SD spec.
	According to Appendix C in SD spec 3.0, slot means an independent set of register from the view of SW.

	I can avoid using "slot" and replace "slot index" with "sdhc-id".
	Thanks for the suggestions.

	Thank you.

Best regards,
Hu Ziji

> Agreed.
> 
> Thomas
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
new file mode 100644
index 000000000000..0d2d139494d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
@@ -0,0 +1,161 @@ 
+Marvell's Xenon SDHCI Controller device tree bindings
+This file documents differences between the core mmc properties
+described by mmc.txt and the properties used by the Xenon implementation.
+
+A single Xenon IP can support multiple slots.
+Each slot acts as an independent SDHC. It owns independent resources, such
+as register sets clock and PHY.
+Each slot should have an independent device tree node.
+
+Required Properties:
+- compatible: should be one of the following
+  - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
+  Must provide a second register area and marvell,pad-type.
+  - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
+  Armada-3700.
+
+- clocks:
+  Array of clocks required for SDHCI.
+  Requires at least one for Xenon IP core.
+  Some SOCs require additional clock for AXI bus.
+
+- clock-names:
+  Array of names corresponding to clocks property.
+  The input clock for Xenon IP core should be named as "core".
+  The optional AXI clock should be named as "axi".
+
+- reg:
+  * For "marvell,xenon-sdhci", one register area for Xenon IP.
+
+  * For "marvell,armada-3700-sdhci", two register areas.
+    The first one for Xenon IP register. The second one for the Armada 3700 SOC
+    PHY PAD Voltage Control register.
+    Please follow the examples with compatible "marvell,armada-3700-sdhci"
+    in below.
+    Please also check property marvell,pad-type in below.
+
+Optional Properties:
+- marvell,xenon-slotno:
+  Indicate the corresponding bit index of current Xenon SDHC slot in
+  SDHC System Operation Control Register Bit[7:0].
+  Set/clear the corresponding bit to enable/disable current Xenon SDHC
+  slot.
+  If this property is not provided, Xenon IP should contain only one
+  slot.
+
+- marvell,xenon-phy-type:
+  Xenon support mutilple types of PHYs.
+  To select eMMC 5.1 PHY, set:
+  marvell,xenon-phy-type = "emmc 5.1 phy"
+  eMMC 5.1 PHY is the default choice if this property is not provided.
+  To select eMMC 5.0 PHY, set:
+  marvell,xenon-phy-type = "emmc 5.0 phy"
+  To select SDH PHY, set:
+  marvell,xenon-phy-type = "sdh phy"
+  Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
+  eMMC only.
+
+- marvell,xenon-phy-znr:
+  Set PHY ZNR value.
+  Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
+  valid range = [0:0x1F].
+  ZNR is set as 0xF by default if this property is not provided.
+
+- marvell,xenon-phy-zpr:
+  Set PHY ZPR value.
+  Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
+  valid range = [0:0x1F].
+  ZPR is set as 0xF by default if this property is not provided.
+
+- marvell,xenon-phy-nr-success-tun:
+  Set the number of required consecutive successful sampling points used to
+  identify a valid sampling window, in tuning process.
+  Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
+
+- marvell,xenon-phy-tun-step-divider:
+  Set the divider for calculating TUN_STEP.
+  Set as 64 by default if this property is not provided.
+
+- marvell,xenon-phy-slow-mode:
+  Force PHY into slow mode.
+  Only available when bus frequency lower than 50MHz in SDR mde.
+  Disabled by default. Please do not enable it unless it is necessary.
+
+- marvell,xenon-mask-conflict-err:
+  Mask Conflict Error alert on some SOC. Disabled by default.
+
+- marvell,xenon-tun-count:
+  Xenon SDHC SOC usually doesn't provide re-tuning counter in
+  Capabilities Register 3 Bit[11:8].
+  This property provides the re-tuning counter.
+  If this property is not set, default re-tuning counter will
+  be set as 0x9 in driver.
+
+- marvell,pad-type:
+  Type of Armada 3700 SOC PHY PAD Voltiage Controller register.
+  Only valid when "marvell,armada-3700-sdhci" is selected.
+  Two types: "sd" and "fixed-1-8v".
+  If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
+  switched to 1.8V when SD in UHS-I.
+  If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
+  Please follow the examples with compatible "marvell,armada-3700-sdhci"
+  in below.
+
+Example:
+- For eMMC slot:
+
+	sdhci@aa0000 {
+		compatible = "marvell,xenon-sdhci";
+		reg = <0xaa0000 0x1000>;
+		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
+		clocks = <&emmc_clk>, <&axi_clock>;
+		clock-names = "core", "axi";
+		bus-width = <8>;
+		marvell,xenon-emmc;
+		marvell,xenon-slotno = <0>;
+		marvell,xenon-phy-type = "emmc 5.1 phy";
+		marvell,xenon-tun-count = <11>;
+	};
+
+- For SD/SDIO slot:
+
+	sdhci@ab0000 {
+		compatible = "marvell,xenon-sdhci";
+		reg = <0xab0000 0x1000>;
+		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
+		vqmmc-supply = <&sd_regulator>;
+		clocks = <&sdclk>;
+		clock-names = "core";
+		bus-width = <4>;
+		marvell,xenon-tun-count = <9>;
+	};
+
+- For eMMC slot with compatible "marvell,armada-3700-sdhci":
+
+	sdhci@aa0000 {
+		compatible = "marvell,armada-3700-sdhci";
+		reg = <0xaa0000 0x1000>,
+		      <phy_addr 0x4>;
+		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
+		clocks = <&emmcclk>;
+		clock-names = "core";
+		bus-width = <8>;
+		marvell,xenon-emmc;
+
+		marvell,pad-type = "fixed-1-8v";
+	};
+
+- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
+
+	sdhci@ab0000 {
+		compatible = "marvell,armada-3700-sdhci";
+		reg = <0xab0000 0x1000>,
+		      <phy_addr 0x4>;
+		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
+		vqmmc-supply = <&sd_regulator>;
+		clocks = <&sdclk>;
+		clock-names = "core";
+		bus-width = <4>;
+
+		marvell,pad-type = "sd";
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 1a5c4c30ea24..850a0afb0c8d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7608,6 +7608,7 @@  MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
 M:	Ziji Hu <[e-mail]>
 L:	[e-mail]
 S:	Supported
+F:	Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
 
 MATROX FRAMEBUFFER DRIVER
 L:	[e-mail]