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:

quoted

Hi Rob,

On 2016/11/10 2:24, Rob Herring wrote:  

quoted

On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:    

quoted

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 <redacted>
---
 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?
  

quoted

quoted

+
+- 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