| Message ID | 1347016499-29354-5-git-send-email-lee.jones@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday 07 September 2012, Lee Jones wrote: > + // External Micro SD slot > + sdi@80126000 { > + arm,primecell-periphid = <0x10480180>; > + max-frequency = <50000000>; > + bus-width = <8>; > + mmc-cap-sd-highspeed; > + mmc-cap-mmc-highspeed; > + vmmc-supply = <&ab8500_ldo_aux3_reg>; > + > + #gpio-cells = <1>; > + cd-gpios = <&gpio2 31 0x4>; // 95 > + > + status = "okay"; > + }; > + The #gpio-cells property seems misplaced here. Arnd
On Fri, Sep 07, 2012 at 12:29:31PM +0000, Arnd Bergmann wrote: > On Friday 07 September 2012, Lee Jones wrote: > > + // External Micro SD slot > > + sdi@80126000 { > > + arm,primecell-periphid = <0x10480180>; > > + max-frequency = <50000000>; > > + bus-width = <8>; > > + mmc-cap-sd-highspeed; > > + mmc-cap-mmc-highspeed; > > + vmmc-supply = <&ab8500_ldo_aux3_reg>; > > + > > + #gpio-cells = <1>; > > + cd-gpios = <&gpio2 31 0x4>; // 95 > > + > > + status = "okay"; > > + }; > > + > > The #gpio-cells property seems misplaced here. Are you reading my patches in reverse order? ;) If not, good spot. :)
On Fri, Sep 7, 2012 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote: I really want Ulf, Per or Johan to have a look at this, so Cc:ing them.. > + // External Micro SD slot Are C99 comments OK in DTS files? Else /* comment like that */ (See Documentation/CodingStyle) Ah well, nitpick, I don't care really. > + sdi@80126000 { Not just sdi@ please, these have names in the current board code that comes from the DB8500 reference manual. In this case 80120000 is the per1 (peripheral group one), and offset 6000 in that group is SDI0. So the quick fix is to name it like: sdi0_per1@80126000 But basically we have a deeper problem here. I think the ux500 Device Tree should be arranged after peripheral block so it actually reflects the hardware (yeah, a big fat sorry for not realizing that before....) So it should *actually* be like: per_group_1 { sdi0@80126000 { }; }; per_group_2 { sdi4@80114000 { }; sdi1@80118000 { }; sdi3@80119000 { }; }; Then I do not know if it's also possible to use the peripheral group offsets for the registers? Can you do this thing and have it working? per_group_1 { reg = <0x80110000 0x10000>; sdi0@80126000 { reg = <0x6000 0x10000>; }; }; I.e. so getting the regs for sdi0 would give 0x80116000? Any of the above proper solutions require a heavy patch on the SoC .dtsi pushing all peripherals to their peripheral group to go in first of course. > + arm,primecell-periphid = <0x10480180>; > + max-frequency = <50000000>; > + bus-width = <8>; Isn't *THIS* (sdi0) the one which is 4 bits? (SD cards only have 4 lines...) > + mmc-cap-sd-highspeed; > + mmc-cap-mmc-highspeed; > + vmmc-supply = <&ab8500_ldo_aux3_reg>; > + > + #gpio-cells = <1>; Arnd already complained about this... > + cd-gpios = <&gpio2 31 0x4>; // 95 95? 95 what? Lightning McQueen? http://en.wikipedia.org/wiki/Lightning_McQueen Elaborate or drop ;-) > + > + status = "okay"; > + }; > + > + // WLAN SDIO channel > + sdi@80118000 { > + arm,primecell-periphid = <0x10480180>; > + max-frequency = <50000000>; > + bus-width = <4>; Isn't this 8 bit capable? > + > + status = "okay"; > + }; > + > + // PoP:ed eMMC > + sdi@80005000 { > + arm,primecell-periphid = <0x10480180>; > + max-frequency = <50000000>; > + bus-width = <8>; > + mmc-cap-mmc-highspeed; > + > + status = "okay"; > + }; > + > + // On-board eMMC > + sdi@80114000 { > + arm,primecell-periphid = <0x10480180>; > + max-frequency = <50000000>; > + bus-width = <8>; > + mmc-cap-mmc-highspeed; > + vmmc-supply = <&ab8500_ldo_aux2_reg>; > + > + status = "okay"; > + }; > }; > }; Inspecting board-mop500-sdi.c I see missing pieces that gets me worried about whether this device tree even works, and which I am sure has regressions in highly demanding usecases because these things are there for a reason: - .sigdir is defined with some flags for the MMCI platform data for sdi0 (SD card), and no DT bindings are available for it. I think Ulf had a patch for pushing this into the driver that will need to go in first. - SDI0 has a callback .ios_handler that I guess must be passed in using platform data. If you don't have this, level-shifting cards will not be run at optimal voltage, symptom: you cannot gear up signals level and will get bad blocks at some high speeds on some cards. (The .ocr_mask is surplus though!) I really think the above needs to be resolved (preferrably by pusing some of Ulfs MMCI patches) before we can move on with MMC DT. Yours, Linus Walleij
diff --git a/arch/arm/boot/dts/hrefv60plus.dts b/arch/arm/boot/dts/hrefv60plus.dts index 32cd7a0..62b2ac1 100644 --- a/arch/arm/boot/dts/hrefv60plus.dts +++ b/arch/arm/boot/dts/hrefv60plus.dts @@ -32,5 +32,50 @@ uart@80007000 { status = "okay"; }; + + // External Micro SD slot + sdi@80126000 { + arm,primecell-periphid = <0x10480180>; + max-frequency = <50000000>; + bus-width = <8>; + mmc-cap-sd-highspeed; + mmc-cap-mmc-highspeed; + vmmc-supply = <&ab8500_ldo_aux3_reg>; + + #gpio-cells = <1>; + cd-gpios = <&gpio2 31 0x4>; // 95 + + status = "okay"; + }; + + // WLAN SDIO channel + sdi@80118000 { + arm,primecell-periphid = <0x10480180>; + max-frequency = <50000000>; + bus-width = <4>; + + status = "okay"; + }; + + // PoP:ed eMMC + sdi@80005000 { + arm,primecell-periphid = <0x10480180>; + max-frequency = <50000000>; + bus-width = <8>; + mmc-cap-mmc-highspeed; + + status = "okay"; + }; + + // On-board eMMC + sdi@80114000 { + arm,primecell-periphid = <0x10480180>; + max-frequency = <50000000>; + bus-width = <8>; + mmc-cap-mmc-highspeed; + vmmc-supply = <&ab8500_ldo_aux2_reg>; + + status = "okay"; + }; }; };
Here we add the Device Tree nodes which are required to successfully probe the MMCI driver which will enable the four cards available on ST-Ericsson's HREF hardware development platform. Signed-off-by: Lee Jones <lee.jones@linaro.org> --- arch/arm/boot/dts/hrefv60plus.dts | 45 +++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)