arm: socfpga: dts: Add a syscon binding for sys-mgr
PATCH v4 1/3 arm: socfpga: dts: Add a syscon binding for sys-mgr
From: Dinh Nguyen <[email protected]>
The system manager block in SOCFPGA contains registers that control other IPs in the SoC. Using the mfd/syscon driver is the best mechanism to link the system manager to the other IPs.
--- arch/arm/boot/dts/socfpga.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/socfpga.dtsi +2 -2
@@ -520,8 +520,8 @@ reg = <0xffd05000 0x1000>; }; - sysmgr at ffd08000 { - compatible = "altr,sys-mgr"; + system_mgr: sysmgr at ffd08000 { + compatible = "altr,sys-mgr", "syscon"; reg = <0xffd08000 0x4000>; }; };
Signature · 1 line
1.7.9.5
PATCH v4 2/3 ARM: socfpga: dts: Add support for SD/MMC
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
v4:
- Add a complete binding example in documentations
- Add a phandle entry for "altr,sysmgr" which links the system manager to the SD/MMC IP block that controls the SDR timings.
- Split up patches
1/3 - Add syscon binding to sys-mgr node
2/3 - DTS bindings and documentation for SD/MMC on SOCFPGA 3/3 - Driver changes to use the bindings
v3:
- Explicitly reference synopsis-dw-mshc.txt for base bindings
- Remove "dw-mshc-ciu-div" as driver can get clock information dts "ciu" entry
- Fixed indentation issue
v2:
- Remove bus-width and extra line in documentation
- Merge bindings example into a single node in documentation
--- .../devicetree/bindings/mmc/socfpga-dw-mshc.txt | 56 ++++++++++++++++++++ arch/arm/boot/dts/socfpga.dtsi | 11 ++++ arch/arm/boot/dts/socfpga_cyclone5.dts | 13 +++++ arch/arm/boot/dts/socfpga_vt.dts | 11 ++++ 4 files changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +56 -0
@@ -0,0 +1,56 @@ +* 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. + +* altr,sysmgr: Should be the phandle to the system_mgr node. As this is where + this where the register that controls the CIU clock phases + reside. + +Example: + dwmmc0 at ff704000 { + compatible = "altr,socfpga-dw-mshc", "snps,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"; + num-slots = <1>; + supports-highspeed; + broken-cd; + altr,sysmgr = <&system_mgr>; + altr,dw-mshc-sdr-timing = <0 3>; + + slot at 0 { + reg = <0>; + bus-width = <4>; + }; + };
arch/arm/boot/dts/socfpga.dtsi +11 -0
@@ -468,6 +468,17 @@ cache-level = <2>; }; + mmc: dwmmc0 at 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 at fffec600 { compatible = "arm,cortex-a9-twd-timer";
arch/arm/boot/dts/socfpga_cyclone5.dts +13 -0
@@ -54,6 +54,19 @@ status = "okay"; }; + dwmmc0 at ff704000 { + num-slots = <1>; + supports-highspeed; + broken-cd; + altr,sysmgr = <&system_mgr>; + altr,dw-mshc-sdr-timing = <0 3>; + + slot at 0 { + reg = <0>; + bus-width = <4>; + }; + }; + timer0 at ffc08000 { clock-frequency = <100000000>; };
arch/arm/boot/dts/socfpga_vt.dts +11 -0
@@ -46,6 +46,17 @@ status = "okay"; }; + dwmmc0 at ff704000 { + num-slots = <1>; + supports-highspeed; + broken-cd; + + slot at 0 { + reg = <0>; + bus-width = <4>; + }; + }; + timer0 at ffc08000 { clock-frequency = <7000000>; };
Signature · 1 line
1.7.9.5
PATCH v4 2/3 ARM: socfpga: dts: Add support for SD/MMC
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 34
Quoted 9 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +3
-0+* altr,sysmgr: Should be the phandle to the system_mgr node. As this is where + this where the register that controls the CIU clock phases + reside.
On the surface, this binding series seems OK, but I do have a question:
how is the sysmgr phandle used?
I assume there's some register in this syscon device that resets or enables or otherwise controls this MSHC module. How does the code know which register it is? The phandle in the altr,sysmgr property would usually be followed by a/some cell(s) that encode this information, so that the MSHC driver doesn't have to know anything about the layout of the syscon registers, and so the sysconf driver doesn't have to know anything about the identity of the MSHC client device.
That way, the MSHC driver will work fine if a HW designer has dropped the MSHC IP block into a completely different SoC with a different syscon register layout.
PATCH v4 2/3 ARM: socfpga: dts: Add support for SD/MMC
Quoted 14 lines · Stephen Warren
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 34
Quoted 9 lines · On 08/14/2013 10:48 AM, [email protected] wrote:
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +3
-0+* altr,sysmgr: Should be the phandle to the system_mgr node. As this is where + this where the register that controls the CIU clock phases + reside.On the surface, this binding series seems OK, but I do have a question:
how is the sysmgr phandle used?
Quoted 7 lines
I assume there's some register in this syscon device that resets or enables or otherwise controls this MSHC module. How does the code know which register it is? The phandle in the altr,sysmgr property would usually be followed by a/some cell(s) that encode this information, so that the MSHC driver doesn't have to know anything about the layout of the syscon registers, and so the sysconf driver doesn't have to know anything about the identity of the MSHC client device.
There is a #define SYSMGR_SDMMCGRP_CTRL_OFFSET that is in dw_mmc-socfpga.c. This defines the offset from the base address that the sysmgr phandle will give me.
Quoted 10 lines
That way, the MSHC driver will work fine if a HW designer has dropped the MSHC IP block into a completely different SoC with a different syscon register layout.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v4 2/3 ARM: socfpga: dts: Add support for SD/MMC
Quoted 23 lines · Dinh Nguyen
Quoted 14 lines · Stephen Warren
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 34
Quoted 9 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +3
-0+* altr,sysmgr: Should be the phandle to the system_mgr node. As this is where + this where the register that controls the CIU clock phases + reside.On the surface, this binding series seems OK, but I do have a question:
how is the sysmgr phandle used?Quoted 7 lines
I assume there's some register in this syscon device that resets or enables or otherwise controls this MSHC module. How does the code know which register it is? The phandle in the altr,sysmgr property would usually be followed by a/some cell(s) that encode this information, so that the MSHC driver doesn't have to know anything about the layout of the syscon registers, and so the sysconf driver doesn't have to know anything about the identity of the MSHC client device.
Quoted 3 lines
There is a #define
SYSMGR_SDMMCGRP_CTRL_OFFSETthat is indw_mmc-socfpga.c. This defines the offset from the base address that the sysmgr phandle will give me.
Hmmm. That doesn't sound good. That means that the SDMMC driver knows internal details about some other HW module. It'd be better if either:
a) The sysmgr driver was required to provide an API to the SDMMC driver to set up the CIU register as requested.
or:
b) The CIU register details were represented in DT.
Either of these would allow the SDMMC driver to operate unchanged on an SoC with a different sysmgr register layout.
PATCH v4 2/3 ARM: socfpga: dts: Add support for SD/MMC
Quoted 34 lines · Stephen Warren
Quoted 23 lines · Dinh Nguyen
Quoted 14 lines · Stephen Warren
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 34
Quoted 9 lines · On 08/14/2013 10:48 AM, [email protected] wrote:
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +3
-0+* altr,sysmgr: Should be the phandle to the system_mgr node. As this is where + this where the register that controls the CIU clock phases + reside.On the surface, this binding series seems OK, but I do have a question:
how is the sysmgr phandle used?Quoted 7 lines
I assume there's some register in this syscon device that resets or enables or otherwise controls this MSHC module. How does the code know which register it is? The phandle in the altr,sysmgr property would usually be followed by a/some cell(s) that encode this information, so that the MSHC driver doesn't have to know anything about the layout of the syscon registers, and so the sysconf driver doesn't have to know anything about the identity of the MSHC client device.
Quoted 3 lines
There is a #define
SYSMGR_SDMMCGRP_CTRL_OFFSETthat is indw_mmc-socfpga.c. This defines the offset from the base address that the sysmgr phandle will give me.Hmmm. That doesn't sound good. That means that the SDMMC driver knows internal details about some other HW module. It'd be better if either:
a) The sysmgr driver was required to provide an API to the SDMMC driver to set up the CIU register as requested.
Quoted 6 lines
or:
b) The CIU register details were represented in DT.
Either of these would allow the SDMMC driver to operate unchanged on an SoC with a different sysmgr register layout.
Thanks Stephen...will rework accordingly.
Dinh
Quoted 1 line
PATCH v4 3/3 mmc: dw_mmc: Use phandle to get SDR timing values from sys-mgr
In reply to dinguyen at altera.comFrom: Dinh Nguyen <[email protected]>
Update the driver to get the system manager node from a phandle. Also, the driver can get the correct clock value from the common clock API, thus the "altr,dw-mshc-ciu-div" binding is not needed at all.
--- drivers/mmc/host/dw_mmc-socfpga.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)
drivers/mmc/host/dw_mmc-socfpga.c +3 -11
@@ -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 */ };
@@ -39,6 +38,7 @@ struct dw_mci_socfpga_priv_data {static int dw_mci_socfpga_priv_init (struct dw_mci *host) {struct dw_mci_socfpga_priv_data *priv; +struct device_node *np = host->dev->of_node; priv =devm_kzalloc (host->dev,sizeof (*priv),GFP_KERNEL );if (!priv) {
@@ -46,9 +46,9 @@ static int dw_mci_socfpga_priv_init(struct dw_mci *host)return -ENOMEM ; } - priv->sysreg =syscon_regmap_lookup_by_compatible ("altr,sys-mgr" ); + priv->sysreg =syscon_regmap_lookup_by_phandle (np,"altr,sysmgr" );if (IS_ERR (priv->sysreg)) { -dev_err (host->dev,"regmap for altr,sys-mgr lookup failed.\n" ); +dev_err (host->dev,"No sysmgr phandle specified!\n" );return PTR_ERR (priv->sysreg); } host->priv = priv;
@@ -64,8 +64,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 +80,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