Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1512482 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-10-31 12:20 +0100
    Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD  Host Controller Rob Herring <robh@kernel.org> - 2016-11-09 19:30 +0100
      Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host  Controller Ziji Hu <huziji@marvell.com> - 2016-11-10 12:50 +0100
        Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD  Host Controller Jisheng Zhang <jszhang@marvell.com> - 2016-11-11 04:30 +0100
          Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD  Host Controller Jisheng Zhang <jszhang@marvell.com> - 2016-11-11 04:40 +0100

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

From: Ziji Hu <huziji@marvell.com>

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

Signed-off-by: Hu Ziji <huziji@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 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.
+
+- 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 <huziji@marvell.com>
 L:	linux-mmc@vger.kernel.org
 S:	Supported
+F:	Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
 
 MATROX FRAMEBUFFER DRIVER
 L:	linux-fbdev@vger.kernel.org
-- 
git-series 0.8.10

[toc] | [next] | [standalone]

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

On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:
> From: Ziji Hu <huziji@marvell.com>
> 
> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> Add Xenon-specific properties.
> Also add properties for Xenon PHY setting.
> 
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  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 <huziji@marvell.com>
>  L:	linux-mmc@vger.kernel.org
>  S:	Supported
> +F:	Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>  
>  MATROX FRAMEBUFFER DRIVER
>  L:	linux-fbdev@vger.kernel.org
> -- 
> git-series 0.8.10

[toc] | [prev] | [next] | [standalone]

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

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 <huziji@marvell.com>
>>
>> Marvell Xenon SDHC can support eMMC/SD/SDIO.
>> Add Xenon-specific properties.
>> Also add properties for Xenon PHY setting.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>>  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 <huziji@marvell.com>
>>  L:	linux-mmc@vger.kernel.org
>>  S:	Supported
>> +F:	Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>>  
>>  MATROX FRAMEBUFFER DRIVER
>>  L:	linux-fbdev@vger.kernel.org
>> -- 
>> git-series 0.8.10

[toc] | [prev] | [next] | [standalone]

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

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 <huziji@marvell.com>
> >>
> >> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> >> Add Xenon-specific properties.
> >> Also add properties for Xenon PHY setting.
> >>
> >> Signed-off-by: Hu Ziji <huziji@marvell.com>
> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >> ---
> >>  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

[toc] | [prev] | [next] | [standalone]

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

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 <huziji@marvell.com>
> > >>
> > >> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> > >> Add Xenon-specific properties.
> > >> Also add properties for Xenon PHY setting.
> > >>
> > >> Signed-off-by: Hu Ziji <huziji@marvell.com>
> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > >> ---
> > >>  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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web