ARM: socfpga: dts: Add support for SD/MMC
PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.
v3:
- Explicitly reference synopsis-dw-mshc.txt for base bindings
- Remove "dw-mshc-ciu-div" as driver can get clock information dts from "ciu" entry.
- Fixed indentation issue
v2:
- Remove bus-width and extra line in documentation
- Merge bindings example into a single node in documentation
---
Re-send to updated device tree bindings maintainers.
--- .../devicetree/bindings/mmc/socfpga-dw-mshc.txt | 36 ++++++++++++++++++++ arch/arm/boot/dts/socfpga.dtsi | 13 ++++++- arch/arm/boot/dts/socfpga_cyclone5.dts | 12 +++++++ arch/arm/boot/dts/socfpga_vt.dts | 11 ++++++ drivers/mmc/host/dw_mmc-socfpga.c | 9 ----- 5 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +36 -0
@@ -0,0 +1,36 @@ +* Altera SOCFPGA specific extensions to the Synopsis Designware Mobile + Storage Host Controller + +The Synopsis designware mobile storage host controller is used to interface +a SoC with storage medium such as eMMC or SD/MMC cards. This file documents +differences between the core Synopsis dw mshc controller properties described +by synopsis-dw-mshc.txt and the properties used by the SOCFPGA specific +extensions to the Synopsis Designware Mobile Storage Host Controller. + +Required Properties: + +* compatible: should be + - "altr,socfpga-dw-mshc": for controllers with Altera SOCFPGA + specific extensions. + +* altr,dw-mshc-sdr-timing: Specifies the value of CIU clock phase shift value + in transmit mode and CIU clock phase shift value in receive mode for single + data rate mode operation. Refer to notes below for the order of the cells and the + valid values. + + Notes for the sdr-timing values: + + The order of the cells should be + - First Cell: CIU clock phase shift value for RX mode, smplsel bits in + the system manager SDMMC control group. + - Second Cell: CIU clock phase shift value for TX mode, drvsel bits in + the system manager SDMMC control group. + + Valid values for SDR CIU clock timing for SOCFPGA: + - valid value for tx phase shift and rx phase shift is 0 to 7. + +Example: + dwmmc0@ff704000 { + compatible = "altr,socfpga-dw-mshc", "snps,dw-mshc"; + altr,dw-mshc-sdr-timing = <0 3>; + };
arch/arm/boot/dts/socfpga.dtsi +12 -1
@@ -468,6 +468,17 @@ cache-level = <2>; }; + mmc: dwmmc0@ff704000 { + compatible = "altr,socfpga-dw-mshc"; + reg = <0xff704000 0x1000>; + interrupts = <0 139 4>; + fifo-depth = <0x400>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&l4_mp_clk>, <&sdmmc_clk>; + clock-names = "biu", "ciu"; + }; + /* Local timer */ timer@fffec600 { compatible = "arm,cortex-a9-twd-timer";
@@ -521,7 +532,7 @@ }; sysmgr@ffd08000 { - compatible = "altr,sys-mgr"; + compatible = "altr,sys-mgr", "syscon"; reg = <0xffd08000 0x4000>; }; };
arch/arm/boot/dts/socfpga_cyclone5.dts +12 -0
@@ -54,6 +54,18 @@ status = "okay"; }; + dwmmc0@ff704000 { + num-slots = <1>; + supports-highspeed; + broken-cd; + altr,dw-mshc-sdr-timing = <0 3>; + + slot@0 { + reg = <0>; + bus-width = <4>; + }; + }; + timer0@ffc08000 { clock-frequency = <100000000>; };
arch/arm/boot/dts/socfpga_vt.dts +11 -0
@@ -46,6 +46,17 @@ status = "okay"; }; + dwmmc0@ff704000 { + num-slots = <1>; + supports-highspeed; + broken-cd; + + slot@0 { + reg = <0>; + bus-width = <4>; + }; + }; + timer0@ffc08000 { clock-frequency = <7000000>; };
drivers/mmc/host/dw_mmc-socfpga.c +0 -9
@@ -31,7 +31,6 @@/* SOCFPGA implementation specific driver private data */ struct dw_mci_socfpga_priv_data { -u8 ciu_div;/* card interface unit divisor */ u32 hs_timing;/* bitmask for CIU clock phase shift */ struct regmap *sysreg;/* regmap for system manager register */ };
@@ -64,8 +63,6 @@ static int dw_mci_socfpga_setup_clock(struct dw_mci *host)regmap_write (priv->sysreg,SYSMGR_SDMMCGRP_CTRL_OFFSET , priv->hs_timing);clk_prepare_enable (host->ciu_clk); - - host->bus_hz /= (priv->ciu_div +1 );return 0 ; }
@@ -82,14 +79,8 @@ static int dw_mci_socfpga_parse_dt(struct dw_mci *host)struct dw_mci_socfpga_priv_data *priv = host->priv;struct device_node *np = host->dev->of_node;u32 timing[2 ]; -u32 div =0 ;int ret; - ret =of_property_read_u32 (np,"altr,dw-mshc-ciu-div" , &div); -if (ret) -dev_info (host->dev,"No dw-mshc-ciu-div specified, assuming 1" ); - priv->ciu_div = div; - ret =of_property_read_u32_array (np,"altr,dw-mshc-sdr-timing" , timing,2 );if (ret)
Signature · 1 line
1.7.9.5
PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
Quoted 6 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +0
-0
I think it's reasonable to define one binding as being based on another plus some additions, so, the binding,
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 36
Quoted 5 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +5
-0+Example: + dwmmc0 at ff704000 { + compatible = "altr,socfpga-dw-mshc", "snps,dw-mshc"; + altr,dw-mshc-sdr-timing = <0 3>; + };
It'd be nice to provide a complete example though, rather than only including the properties that this binding adds to the base binding.
arch/arm/boot/dts/socfpga.dtsi line 535
Quoted 5 lines
arch/arm/boot/dts/socfpga.dtsi +1
-1sysmgr at ffd08000 { - compatible = "altr,sys-mgr"; + compatible = "altr,sys-mgr", "syscon";
That seems like an unrelated change?
PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 11 lines · Stephen Warren
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
Quoted 6 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +0
-0I think it's reasonable to define one binding as being based on another plus some additions, so, the binding,
Acked-by Stephen Warren <[email protected]>
Thanks Stephen!
Quoted 9 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 36
Quoted 6 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +5
-0+Example: + dwmmc0 at ff704000 { + compatible = "altr,socfpga-dw-mshc", "snps,dw-mshc"; + altr,dw-mshc-sdr-timing = <0 3>; + };It'd be nice to provide a complete example though, rather than only including the properties that this binding adds to the base binding.
I'll add a complete binding.
Quoted 8 lines
arch/arm/boot/dts/socfpga.dtsi line 535
Quoted 6 lines
arch/arm/boot/dts/socfpga.dtsi +1
-1sysmgr at ffd08000 { - compatible = "altr,sys-mgr"; + compatible = "altr,sys-mgr", "syscon";That seems like an unrelated change?
This patch is to enable SD/MMC for SOCFPGA. The "syscon" is needed in dw_mmc-socfpga.c as the clock phases for the SD IP is controlled by registers in "altr,sys-mgr".
Thanks,
Dinh
Quoted 1 line
PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 15 lines · Dinh Nguyen
Quoted 6 lines · Stephen Warren
Quoted 4 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.Quoted 7 lines
arch/arm/boot/dts/socfpga.dtsi line 535
Quoted 5 lines
arch/arm/boot/dts/socfpga.dtsi +1
-1sysmgr at ffd08000 { - compatible = "altr,sys-mgr"; + compatible = "altr,sys-mgr", "syscon";That seems like an unrelated change?
Quoted 3 lines
This patch is to enable SD/MMC for SOCFPGA. The "syscon" is needed in
dw_mmc-socfpga.cas the clock phases for the SD IP is controlled by registers in "altr,sys-mgr".
But nothing in the dwmmc nodes refers to the syscon node, so how can the syscon driver possibly provide services to the MMC driver; the MMC driver won't be able to find it. I hope there's no back-door assumption that some syscon exists, and that there's only one of them...
PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 16 lines · Stephen Warren
Quoted 15 lines · Dinh Nguyen
Quoted 13 lines · Stephen Warren
arch/arm/boot/dts/socfpga.dtsi line 535
Quoted 10 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.arch/arm/boot/dts/socfpga.dtsi +1
-1sysmgr at ffd08000 { - compatible = "altr,sys-mgr"; + compatible = "altr,sys-mgr", "syscon";That seems like an unrelated change?
Quoted 8 lines
Quoted 3 lines
This patch is to enable SD/MMC for SOCFPGA. The "syscon" is needed in
dw_mmc-socfpga.cas the clock phases for the SD IP is controlled by registers in "altr,sys-mgr".But nothing in the dwmmc nodes refers to the syscon node, so how can the syscon driver possibly provide services to the MMC driver; the MMC driver won't be able to find it. I hope there's no back-door assumption that some syscon exists, and that there's only one of them...
In dw_mmc-socfpga.c:
priv->sysreg = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
then, uses regmap_write() to set the correct values from
"altr,dw-mshc-sdr-timing".
This was a suggestion from Arnd. It seems that this part of patch is causing alot of confusion, I should just split the patches out?
Dinh
Quoted 1 line
PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 26 lines · Dinh Nguyen
Quoted 16 lines · Stephen Warren
Quoted 15 lines · Dinh Nguyen
Quoted 13 lines · Stephen Warren
arch/arm/boot/dts/socfpga.dtsi line 535
Quoted 10 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.arch/arm/boot/dts/socfpga.dtsi +1
-1sysmgr at ffd08000 { - compatible = "altr,sys-mgr"; + compatible = "altr,sys-mgr", "syscon";That seems like an unrelated change?
Quoted 8 lines
Quoted 3 lines
This patch is to enable SD/MMC for SOCFPGA. The "syscon" is needed in
dw_mmc-socfpga.cas the clock phases for the SD IP is controlled by registers in "altr,sys-mgr".But nothing in the dwmmc nodes refers to the syscon node, so how can the syscon driver possibly provide services to the MMC driver; the MMC driver won't be able to find it. I hope there's no back-door assumption that some syscon exists, and that there's only one of them...
Quoted 6 lines
In
dw_mmc-socfpga.c:
priv->sysreg=syscon_regmap_lookup_by_compatible("altr,sys-mgr");then, uses
regmap_write()to set the correct values from
"altr,dw-mshc-sdr-timing".
Uggh. There should really be a phandle from the MMC node to the syscon node to specify the connectivity. What if there ends up being 2 HW modules that are sys-mgr?
At the very least, the binding for any device that's affected by the sys-mgr should specify that some other node must exist with that compatible value, to provide XXX services for it.
But, I wonder if the altr,dw-mshc-sdr-timing property shouldn't be part of the syscon node itself?
(Probably best to ignore my ack for now given this new information)
Quoted 2 lines
This was a suggestion from Arnd. It seems that this part of patch is causing alot of confusion, I should just split the patches out?
A separate patch to change that compatible value probably would make sense; by the sound of it, that change would also be required for a lot of other devices on the SoC; it's not just specific to MMC?