ARM: socfpga: dts: Add support for SD/MMC
Subsystems (2)
PATCH 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.
v4:
- Fixed grammar in documentation file
v3:
- Add Acked-by: Jaehoon Chung
v2:
- Fixed misspellings extentions->extensions
---
Re-send to updated device tree bindings maintainers.
--- .../devicetree/bindings/mmc/socfpga-dw-mshc.txt | 60 ++++++++++++++++++++ arch/arm/boot/dts/socfpga.dtsi | 13 ++++- arch/arm/boot/dts/socfpga_cyclone5.dts | 13 +++++ arch/arm/boot/dts/socfpga_vt.dts | 12 ++++ 4 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +60 -0
@@ -0,0 +1,60 @@ +* Altera 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-ciu-div: Specifies the divider value for the card interface + unit (ciu) clock. The value should be (n-1). For Altera's SOCFPGA, the divider + value is fixed at 3, which means parent_clock/4. + +* 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. + +Required properties for a slot: + +* bus-width: Data width for card slot. 4-bit or 8-bit data. + +Example: + + The MSHC controller node can be split into two portions, SoC specific and + board specific portions, as listed below. + + dwmmc0 at ff704000 { + compatible = "altr,socfpga-dw-mshc"; + reg = <0xff704000 0x1000>; + interrupts = <0 139 4>; + #address-cells = <1>; + #size-cells = <0>; + }; + + dwmmc0 at ff704000 { + num-slots = <1>; + supports-highspeed; + broken-cd; + fifo-depth = <0x400>; + altr,dw-mshc-ciu-div = <3>; + altr,dw-mshc-sdr-timing = <0 3>; + + slot at 0 { + reg = <0>; + bus-width = <4>; + }; + }; +
arch/arm/boot/dts/socfpga.dtsi +12 -1
@@ -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";
@@ -521,7 +532,7 @@ }; sysmgr at ffd08000 { - compatible = "altr,sys-mgr"; + compatible = "altr,sys-mgr", "syscon"; reg = <0xffd08000 0x4000>; }; };
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,dw-mshc-ciu-div = <3>; + 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 +12 -0
@@ -46,6 +46,18 @@ status = "okay"; }; + dwmmc0 at ff704000 { + num-slots = <1>; + supports-highspeed; + broken-cd; + altr,dw-mshc-ciu-div = <3>; + + slot at 0 { + reg = <0>; + bus-width = <4>; + }; + }; + timer0 at ffc08000 { clock-frequency = <7000000>; };
Signature · 1 line
1.7.9.5
PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
Hello Ding,
Excuse me if the questions below were already asked and feel free to point me at the appropriate mail archive...
Quoted 2 lines · dinguyen at altera.com
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.
Are those two related? As in: what does the "syscon" bit have to do with "Add(ing) support for SD/MMC"? Should those two be separated?
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 12
Quoted 3 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +3
-0+* altr,dw-mshc-ciu-div: Specifies the divider value for the card interface + unit (ciu) clock. The value should be (n-1). For Altera's SOCFPGA, the divider + value is fixed at 3, which means parent_clock/4.
In what circumstances would this be different than 3? Is the interface in question member of the "hard part" of the SOFPGA, or is it supposed to be synthesized in the FPGA?
Quoted 5 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +5
-0+* 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. +
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 28
Quoted 10 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +10
-0+ 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.
How does one pick those value? Do they depend on the board design? The FPGA synthesis options?
I am not trying to be picky, just trying to establish if those value are "hardware" enough to be present in the tree at all...
I've also noticed that Exynos defines almost identical bindings:
Quoted 3 lines
samsung,dw-mshc-ciu-div
samsung,dw-mshc-sdr-timing
samsung,dw-mshc-ddr-timing
Aren't you both using the same "Synopsis Designware Mobile Storage Host Controller" by any chance? Are you sharing a driver? And if not, why? ;-) If the timings really must be parametrised, would it be possible to come up with a common set of "synopsis" properties, instead of "samsung" and "altr" ones?
Thanks for your time!
Pawel
PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
Hi Pawel,
Quoted 1 line · Pawel Moll
Hello Ding,
Dinh please...
Quoted 10 lines
Excuse me if the questions below were already asked and feel free to point me at the appropriate mail archive...
Quoted 2 lines · dinguyen at altera.com
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.Are those two related? As in: what does the "syscon" bit have to do with "Add(ing) support for SD/MMC"? Should those two be separated?
You can reference these 2 threads:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/168470.html
https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/035227.html
I hope that you will address your question for syscon.
Quoted 19 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 12
Quoted 4 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +3
-0+* altr,dw-mshc-ciu-div: Specifies the divider value for the card interface + unit (ciu) clock. The value should be (n-1). For Altera's SOCFPGA, the divider + value is fixed at 3, which means parent_clock/4.In what circumstances would this be different than 3? Is the interface in question member of the "hard part" of the SOFPGA, or is it supposed to be synthesized in the FPGA?
Quoted 10 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +10
-0+* 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.
Quoted 8 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 28
Quoted 5 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +5
-0+ - 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.How does one pick those value? Do they depend on the board design? The FPGA synthesis options?
The sd/mmc is not in the FPGA at all, it is a hardened IP. The values are implementation specific on how the IP is put down.
Quoted 3 lines
I am not trying to be picky, just trying to establish if those value are "hardware" enough to be present in the tree at all...
It is very much tied to the hardware.
Quoted 6 lines
I've also noticed that Exynos defines almost identical bindings:
Quoted 3 lines
samsung,dw-mshc-ciu-div
samsung,dw-mshc-sdr-timing
samsung,dw-mshc-ddr-timing
Yes, I agree.
Quoted 6 lines
Aren't you both using the same "Synopsis Designware Mobile Storage Host Controller" by any chance? Are you sharing a driver? And if not, why? ;-) If the timings really must be parametrised, would it be possible to come up with a common set of "synopsis" properties, instead of "samsung" and "altr" ones?
We are using the same driver. This is just a platform specifc entries for how the IP can be implemented. I also agree that we can come up with a shared set of properties for these.
But since the platform-driver part has already been picked into the master tree, can I work on a common set after this patch? That way it enables SD/MMC to work on SocFPGA for the time being.
Thanks alot for the review.
Dinh
Quoted 7 lines
Thanks for your time!
Pawel
PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 1 line · Dinh Nguyen
Dinh please...
Uh, accept my apologies. I know exactly how it feels ;-)
Quoted 14 lines
Quoted 5 lines
I've also noticed that Exynos defines almost identical bindings:
Quoted 3 lines
samsung,dw-mshc-ciu-div
samsung,dw-mshc-sdr-timing
samsung,dw-mshc-ddr-timingYes, I agree.
Quoted 6 lines
Aren't you both using the same "Synopsis Designware Mobile Storage Host Controller" by any chance? Are you sharing a driver? And if not, why? ;-) If the timings really must be parametrised, would it be possible to come up with a common set of "synopsis" properties, instead of "samsung" and "altr" ones?
Quoted 7 lines
We are using the same driver. This is just a platform specifc entries for how the IP can be implemented. I also agree that we can come up with a shared set of properties for these.
But since the platform-driver part has already been picked into the master tree, can I work on a common set after this patch? That way it enables SD/MMC to work on SocFPGA for the time being.
Ah, I see. You should have included the binding documentation in the driver using the bindings, not in the patch adding the required nodes to DTS files... Than no one would have any comments on this patch ;-)
So yes, if the driver part got merged I will shut up now, providing you promise to work on "consolidating" the bindings for the next cycle. The sooner, the better. Does it sound like a deal?
Pawel
PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 27 lines · Pawel Moll
Quoted 1 line · Dinh Nguyen
Dinh please...
Uh, accept my apologies. I know exactly how it feels ;-)
Quoted 14 lines
Quoted 5 lines
I've also noticed that Exynos defines almost identical bindings:
Quoted 3 lines
samsung,dw-mshc-ciu-div
samsung,dw-mshc-sdr-timing
samsung,dw-mshc-ddr-timingYes, I agree.
Quoted 6 lines
Aren't you both using the same "Synopsis Designware Mobile Storage Host Controller" by any chance? Are you sharing a driver? And if not, why? ;-) If the timings really must be parametrised, would it be possible to come up with a common set of "synopsis" properties, instead of "samsung" and "altr" ones?
Quoted 7 lines
We are using the same driver. This is just a platform specifc entries for how the IP can be implemented. I also agree that we can come up with a shared set of properties for these.
But since the platform-driver part has already been picked into the master tree, can I work on a common set after this patch? That way it enables SD/MMC to work on SocFPGA for the time being.
Quoted 7 lines
Ah, I see. You should have included the binding documentation in the driver using the bindings, not in the patch adding the required nodes to DTS files... Than no one would have any comments on this patch ;-)
So yes, if the driver part got merged I will shut up now, providing you promise to work on "consolidating" the bindings for the next cycle. The sooner, the better. Does it sound like a deal?
Yes, you have my promise on the consolidating work! I was also thinking the same thing when I enabled this for SOCFPGA, but since I don't have any Exynos HW, I didn't have a way to test. But I will work on a patch soon and send to the Exynos people to test.
Thanks,
Dinh
Quoted 5 lines
Pawel
PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
In reply to dinguyen at altera.comDocumentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 2
Quoted 9 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 +2
-0+* Altera SOCFPGA specific extensions to the Synopsis Designware Mobile + Storage Host Controller
If these are extensions to an existing binding, it feels like the documentation should be part of that binding, although suppose it's reasonable to create a HW-specific "sub-class" of an existing binding.
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 8
Quoted 5 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +5
-0+Required Properties: + +* compatible: should be + - "altr,socfpga-dw-mshc": for controllers with Altera SOCFPGA + specific extensions.
The "altr" vendor prefix doesn't appear in Documentation/devicetree/bindings/vendor-prefixes.txt. Is there another patch in flight to add it?
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 12
Quoted 3 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +3
-0+* altr,dw-mshc-ciu-div: Specifies the divider value for the card interface + unit (ciu) clock. The value should be (n-1). For Altera's SOCFPGA, the divider + value is fixed at 3, which means parent_clock/4.
Should the clocks be represented using the common clock DT binding? If this register is something that will always be handled entirely internally to the HW module, then perhaps there's no need. If the driver is going to need to call clk_set_rate() at all, then representing the clock using the standard bindings seems better, unless you expect the driver to call clk_set_rate(desired_rate * internal_divider) instead of clk_set_rate(desired_rate) everywhere. That might not be unreasonable though.
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 32
Quoted 3 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +3
-0+Required properties for a slot: + +* bus-width: Data width for card slot. 4-bit or 8-bit data.
Isn't that already part of the standard MMC bindings, and hence not something you need to duplicate here?
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 37
Quoted 4 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +4
-0+Example: + + The MSHC controller node can be split into two portions, SoC specific and + board specific portions, as listed below.
That doesn't sound like a good idea. There should be one DT node for each logical block. The internal construction of the Linux drivers (presumably you have entirely separate code to handle the two nodes in Linux so far?) should not influence the DT construction at all.
Quoted 12 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +12
-0+ dwmmc0 at ff704000 { + compatible = "altr,socfpga-dw-mshc"; + reg = <0xff704000 0x1000>; + interrupts = <0 139 4>; + #address-cells = <1>; + #size-cells = <0>; + }; + + dwmmc0 at ff704000 { + num-slots = <1>; + supports-highspeed; + broken-cd;
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 60
Quoted 10 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +10
-0+ fifo-depth = <0x400>; + altr,dw-mshc-ciu-div = <3>; + altr,dw-mshc-sdr-timing = <0 3>; + + slot at 0 { + reg = <0>; + bus-width = <4>; + }; + }; +
You might want to trim that trailing blank line.
PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 15 lines · Stephen Warren
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 2
Quoted 9 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 +2
-0+* Altera SOCFPGA specific extensions to the Synopsis Designware Mobile + Storage Host ControllerIf these are extensions to an existing binding, it feels like the documentation should be part of that binding, although suppose it's reasonable to create a HW-specific "sub-class" of an existing binding.
Quoted 9 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 8
Quoted 5 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +5
-0+Required Properties: + +* compatible: should be + - "altr,socfpga-dw-mshc": for controllers with Altera SOCFPGA + specific extensions.The "altr" vendor prefix doesn't appear in
Documentation/devicetree/bindings/vendor-prefixes.txt. Is there another patch in flight to add it?
I will send one shortly.
Quoted 5 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 12
Quoted 4 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +3
-0+* altr,dw-mshc-ciu-div: Specifies the divider value for the card interface + unit (ciu) clock. The value should be (n-1). For Altera's SOCFPGA, the divider + value is fixed at 3, which means parent_clock/4.
Quoted 8 lines
Should the clocks be represented using the common clock DT binding? If this register is something that will always be handled entirely internally to the HW module, then perhaps there's no need. If the driver is going to need to call
clk_set_rate()at all, then representing the clock using the standard bindings seems better, unless you expect the driver to callclk_set_rate(desired_rate * internal_divider)instead ofclk_set_rate(desired_rate)everywhere. That might not be unreasonable though.
I will have to explore this option. But yes, this register is handled entirely in the HW module in the case of Exynos, and in SOCFPGA's case its in another HW block that is not clock related at all.
Since I've based this code on the exynos implementation, I've CCed a few of those guys to see if this is possible.
Quoted 7 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 32
Quoted 4 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +3
-0+Required properties for a slot: + +* bus-width: Data width for card slot. 4-bit or 8-bit data.Isn't that already part of the standard MMC bindings, and hence not something you need to duplicate here?
Yes, I will remove.
Quoted 10 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 37
Quoted 5 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +4
-0+Example: + + The MSHC controller node can be split into two portions, SoC specific and + board specific portions, as listed below.That doesn't sound like a good idea. There should be one DT node for each logical block. The internal construction of the Linux drivers (presumably you have entirely separate code to handle the two nodes in Linux so far?) should not influence the DT construction at all.
In the end, there is only 1 DT node for each logical block:
dwmmc0 at ff704000 {
compatible = "altr,socfpga-dw-mshc";
reg = <0xff704000 0x00001000>;
interrupts = <0x00000000 0x0000008b 0x00000004>;
fifo-depth = <0x00000400>;
#address-cells = <0x00000001>;
#size-cells = <0x00000000>;
clocks = <0x00000016 0x00000017>;
clock-names = "biu", "ciu";
num-slots = <0x00000001>;
supports-highspeed;
broken-cd;
altr,dw-mshc-ciu-div = <0x00000003>;
altr,dw-mshc-sdr-timing = <0x00000000 0x00000003>;
slot at 0 {
reg = <0x00000000>;
bus-width = <0x00000004>;
};
};Quoted 17 lines
Quoted 17 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +16
-0+ dwmmc0 at ff704000 { + compatible = "altr,socfpga-dw-mshc"; + reg = <0xff704000 0x1000>; + interrupts = <0 139 4>; + #address-cells = <1>; + #size-cells = <0>; + }; + + dwmmc0 at ff704000 { + num-slots = <1>; + supports-highspeed; + broken-cd; + fifo-depth = <0x400>; + altr,dw-mshc-ciu-div = <3>; + altr,dw-mshc-sdr-timing = <0 3>; +
Quoted 9 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 60
Quoted 6 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +6
-0+ slot at 0 { + reg = <0>; + bus-width = <4>; + }; + }; +You might want to trim that trailing blank line.
Will do. Thanks for reviewing this.
Dinh
PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 19 lines · Dinh Nguyen
Quoted 8 lines · Stephen Warren
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
-0Quoted 9 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 37
Quoted 4 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +4
-0+Example: + + The MSHC controller node can be split into two portions, SoC specific and + board specific portions, as listed below.That doesn't sound like a good idea. There should be one DT node for each logical block. The internal construction of the Linux drivers (presumably you have entirely separate code to handle the two nodes in Linux so far?) should not influence the DT construction at all.
Quoted 1 line
In the end, there is only 1 DT node for each logical block:
Oh right, I see you were intending to show the distinction between the SoC .dtsi and board .dts file. I hadn't realized that. I don't think it's common to do that in the examples, so I would recommend just merging the whole example together myself.
Quoted 2 lines
dwmmc0 at ff704000 {
compatible = "altr,socfpga-dw-mshc";That should include the baseline synopsis compatible value too.
Quoted 17 lines
reg = <0xff704000 0x00001000>;
interrupts = <0x00000000 0x0000008b 0x00000004>;
fifo-depth = <0x00000400>;
#address-cells = <0x00000001>;
#size-cells = <0x00000000>;
clocks = <0x00000016 0x00000017>;
clock-names = "biu", "ciu";
num-slots = <0x00000001>;
supports-highspeed;
broken-cd;
altr,dw-mshc-ciu-div = <0x00000003>;
altr,dw-mshc-sdr-timing = <0x00000000 0x00000003>;
slot at 0 {
reg = <0x00000000>;
bus-width = <0x00000004>;
};
};PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 20 lines · Stephen Warren
Quoted 19 lines · Dinh Nguyen
Quoted 17 lines · Stephen Warren
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 37
Quoted 11 lines · On 07/25/2013 04:04 PM, [email protected] wrote:
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 +4
-0+Example: + + The MSHC controller node can be split into two portions, SoC specific and + board specific portions, as listed below.That doesn't sound like a good idea. There should be one DT node for each logical block. The internal construction of the Linux drivers (presumably you have entirely separate code to handle the two nodes in Linux so far?) should not influence the DT construction at all.
Quoted 6 lines
Quoted 1 line
In the end, there is only 1 DT node for each logical block:
Oh right, I see you were intending to show the distinction between the SoC .dtsi and board .dts file. I hadn't realized that. I don't think it's common to do that in the examples, so I would recommend just merging the whole example together myself.
I'll merge it.
Quoted 5 lines
Quoted 3 lines
dwmmc0@ff704000 { compatible = "altr,socfpga-dw-mshc";That should include the baseline synopsis compatible value too.
We don't need the baseline synopsis compatible because of dw_mci_pltfm_register() call.
Thanks,
Dinh
Quoted 20 lines
Quoted 18 lines
reg = <0xff704000 0x00001000>; interrupts = <0x00000000 0x0000008b 0x00000004>; fifo-depth = <0x00000400>; #address-cells = <0x00000001>; #size-cells = <0x00000000>; clocks = <0x00000016 0x00000017>; clock-names = "biu", "ciu"; num-slots = <0x00000001>; supports-highspeed; broken-cd; altr,dw-mshc-ciu-div = <0x00000003>; altr,dw-mshc-sdr-timing = <0x00000000 0x00000003>; slot@0 { reg = <0x00000000>; bus-width = <0x00000004>; }; };
PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 31 lines · Dinh Nguyen
Quoted 20 lines · Stephen Warren
Quoted 19 lines · Dinh Nguyen
Quoted 17 lines · Stephen Warren
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 37
Quoted 11 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 +4
-0+Example: + + The MSHC controller node can be split into two portions, SoC specific and + board specific portions, as listed below.That doesn't sound like a good idea. There should be one DT node for each logical block. The internal construction of the Linux drivers (presumably you have entirely separate code to handle the two nodes in Linux so far?) should not influence the DT construction at all.
Quoted 6 lines
Quoted 1 line
In the end, there is only 1 DT node for each logical block:
Oh right, I see you were intending to show the distinction between the SoC .dtsi and board .dts file. I hadn't realized that. I don't think it's common to do that in the examples, so I would recommend just merging the whole example together myself.
I'll merge it.
Quoted 1 line
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
dwmmc0 at ff704000 { compatible = "altr,socfpga-dw-mshc";That should include the baseline synopsis compatible value too.
We don't need the baseline synopsis compatible because of
dw_mci_pltfm_register()call.
It's not a matter of whether it's strictly necessary for the SW to work right now. The compatible property should include entries for everything that the HW is actually compatible with.
Of course, if a plain driver for the raw synopsis controller/binding wouldn't actually work on this HW at all, without explicit knowledge of the extra details of the more HW-specific binding, then that's a good argument for leaving it out of compatible.
PATCH 1/1 ARM: socfpga: dts: Add support for SD/MMC
Quoted 35 lines · Stephen Warren
Quoted 34 lines · Dinh Nguyen
Quoted 20 lines · Stephen Warren
Quoted 19 lines · Dinh Nguyen
Quoted 17 lines · Stephen Warren
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 37
Quoted 11 lines · On 07/25/2013 04:04 PM, [email protected] wrote:
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 +4
-0+Example: + + The MSHC controller node can be split into two portions, SoC specific and + board specific portions, as listed below.That doesn't sound like a good idea. There should be one DT node for each logical block. The internal construction of the Linux drivers (presumably you have entirely separate code to handle the two nodes in Linux so far?) should not influence the DT construction at all.
Quoted 6 lines
Quoted 1 line
In the end, there is only 1 DT node for each logical block:
Oh right, I see you were intending to show the distinction between the SoC .dtsi and board .dts file. I hadn't realized that. I don't think it's common to do that in the examples, so I would recommend just merging the whole example together myself.
I'll merge it.
Quoted 4 lines
Quoted 3 lines
dwmmc0@ff704000 { compatible = "altr,socfpga-dw-mshc";
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
That should include the baseline synopsis compatible value too.
We don't need the baseline synopsis compatible because of
dw_mci_pltfm_register()call.It's not a matter of whether it's strictly necessary for the SW to work right now. The compatible property should include entries for everything that the HW is actually compatible with.
Ah ok..will add the baseline compatible binding. But this also sounds like there's clean-up work that can be done that I have already _promised_ Pawell I would do for the next cycle. Along with finding common attributes for the clocks, I think we can also move all the platform compatible entries into a single of_device_id struct.
Dinh
Quoted 11 lines
Of course, if a plain driver for the raw synopsis controller/binding wouldn't actually work on this HW at all, without explicit knowledge of the extra details of the more HW-specific binding, then that's a good argument for leaving it out of compatible.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel