[U-Boot] [PATCH 0/2] Tegra: MMC: Add DT support for MMC to T20 boards
This patchset adds device-tree support to the Tegra MMC driver. All device config is done via properties in the DT files instead of hard-coded config options/function arguments.
I've tested this on my Seaboard and everything works fine, including card detect. For the other T20 boards, I've used the Linux kernel DTS files for the sdhci nodes where there wasn't one already, or expanded the info that was already there. Everything builds fine, but I haven't tested anything but Seaboard.
Tom Warren (2): Tegra: fdt: Add/enhance sdhci (mmc) nodes for all T20 DT files Tegra: MMC: Add DT support to MMC driver for all T20 boards
arch/arm/dts/tegra20.dtsi | 12 ++ arch/arm/include/asm/arch-tegra/mmc.h | 2 +- arch/arm/include/asm/arch-tegra/tegra_mmc.h | 12 +- board/avionic-design/common/tamonten.c | 4 +- board/avionic-design/dts/tegra20-medcom-wide.dts | 9 + board/avionic-design/dts/tegra20-plutux.dts | 9 + board/avionic-design/dts/tegra20-tec.dts | 9 + board/compal/dts/tegra20-paz00.dts | 22 +++- board/compal/paz00/paz00.c | 14 +- board/compulab/dts/tegra20-trimslice.dts | 17 ++ board/compulab/trimslice/trimslice.c | 10 +- board/nvidia/dts/tegra20-harmony.dts | 21 +++ board/nvidia/dts/tegra20-seaboard.dts | 19 ++- board/nvidia/dts/tegra20-ventana.dts | 23 +++ board/nvidia/dts/tegra20-whistler.dts | 16 ++ board/nvidia/harmony/harmony.c | 12 +- board/nvidia/seaboard/seaboard.c | 14 +- board/nvidia/whistler/whistler.c | 4 +- board/toradex/colibri_t20_iris/colibri_t20_iris.c | 2 +- board/toradex/dts/tegra20-colibri_t20_iris.dts | 8 + drivers/mmc/tegra_mmc.c | 186 ++++++++++++++------- include/fdtdec.h | 1 + lib/fdtdec.c | 1 + 23 files changed, 322 insertions(+), 105 deletions(-)
Linux dts files were used for those boards that didn't already have sdhci info populated. If a cd-gpio was present, I set "removable = <1>", else removable = <0>.
Signed-off-by: Tom Warren twarren@nvidia.com --- arch/arm/dts/tegra20.dtsi | 12 +++++++++++ board/avionic-design/dts/tegra20-medcom-wide.dts | 9 ++++++++ board/avionic-design/dts/tegra20-plutux.dts | 9 ++++++++ board/avionic-design/dts/tegra20-tec.dts | 9 ++++++++ board/compal/dts/tegra20-paz00.dts | 22 +++++++++++++++++++- board/compulab/dts/tegra20-trimslice.dts | 17 ++++++++++++++++ board/nvidia/dts/tegra20-harmony.dts | 21 ++++++++++++++++++++ board/nvidia/dts/tegra20-seaboard.dts | 19 ++++++++++++----- board/nvidia/dts/tegra20-ventana.dts | 23 ++++++++++++++++++++++ board/nvidia/dts/tegra20-whistler.dts | 16 +++++++++++++++ board/toradex/dts/tegra20-colibri_t20_iris.dts | 8 +++++++ 11 files changed, 157 insertions(+), 8 deletions(-)
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi index 12049fd..2603277 100644 --- a/arch/arm/dts/tegra20.dtsi +++ b/arch/arm/dts/tegra20.dtsi @@ -307,23 +307,35 @@ compatible = "nvidia,tegra20-sdhci"; reg = <0xc8000000 0x200>; interrupts = < 46 >; + status = "disabled"; + /* PERIPH_ID_SDMMC1, PLLP_OUT? */ + clocks = <&tegra_car 14>; };
sdhci@c8000200 { compatible = "nvidia,tegra20-sdhci"; reg = <0xc8000200 0x200>; interrupts = < 47 >; + status = "disabled"; + /* PERIPH_ID_SDMMC2, PLLP_OUT? */ + clocks = <&tegra_car 9>; };
sdhci@c8000400 { compatible = "nvidia,tegra20-sdhci"; reg = <0xc8000400 0x200>; interrupts = < 51 >; + status = "disabled"; + /* PERIPH_ID_SDMMC3, PLLP_OUT? */ + clocks = <&tegra_car 69>; };
sdhci@c8000600 { compatible = "nvidia,tegra20-sdhci"; reg = <0xc8000600 0x200>; interrupts = < 63 >; + status = "disabled"; + /* PERIPH_ID_SDMMC4, PLLP_OUT? */ + clocks = <&tegra_car 15>; }; }; diff --git a/board/avionic-design/dts/tegra20-medcom-wide.dts b/board/avionic-design/dts/tegra20-medcom-wide.dts index e46afbe..33d174d 100644 --- a/board/avionic-design/dts/tegra20-medcom-wide.dts +++ b/board/avionic-design/dts/tegra20-medcom-wide.dts @@ -55,6 +55,15 @@ status = "disabled"; };
+ sdhci@c8000600 { + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + cd-gpios = <&gpio 58 0>; /* gpio PH2 */ + wp-gpios = <&gpio 59 0>; /* gpio PH3 */ + bus-width = <4>; + removable = <1>; + }; + lcd_panel: panel { clock = <61715000>; xres = <1366>; diff --git a/board/avionic-design/dts/tegra20-plutux.dts b/board/avionic-design/dts/tegra20-plutux.dts index 3e6cce0..3a0147f 100644 --- a/board/avionic-design/dts/tegra20-plutux.dts +++ b/board/avionic-design/dts/tegra20-plutux.dts @@ -41,4 +41,13 @@ usb@c5004000 { status = "disabled"; }; + + sdhci@c8000600 { + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + cd-gpios = <&gpio 58 0>; /* gpio PH2 */ + wp-gpios = <&gpio 59 0>; /* gpio PH3 */ + bus-width = <4>; + removable = <1>; + }; }; diff --git a/board/avionic-design/dts/tegra20-tec.dts b/board/avionic-design/dts/tegra20-tec.dts index bf3ff1d..521a255 100644 --- a/board/avionic-design/dts/tegra20-tec.dts +++ b/board/avionic-design/dts/tegra20-tec.dts @@ -66,6 +66,15 @@ status = "disabled"; };
+ sdhci@c8000600 { + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + cd-gpios = <&gpio 58 0>; /* gpio PH2 */ + wp-gpios = <&gpio 59 0>; /* gpio PH3 */ + bus-width = <4>; + removable = <1>; + }; + lcd_panel: panel { clock = <33260000>; xres = <800>; diff --git a/board/compal/dts/tegra20-paz00.dts b/board/compal/dts/tegra20-paz00.dts index 31b064d..ed709c8 100644 --- a/board/compal/dts/tegra20-paz00.dts +++ b/board/compal/dts/tegra20-paz00.dts @@ -3,11 +3,13 @@ /include/ ARCH_CPU_DTS
/ { - model = "Toshiba AC100 / Dynabook AZ"; - compatible = "compal,paz00", "nvidia,tegra20"; + model = "Toshiba AC100 / Dynabook AZ"; + compatible = "compal,paz00", "nvidia,tegra20";
aliases { usb0 = "/usb@c5008000"; + sdmmc0 = "/sdhci@c8000600"; + sdmmc1 = "/sdhci@c8000000"; };
memory { @@ -53,6 +55,22 @@ status = "disabled"; };
+ sdhci@c8000000 { + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + cd-gpios = <&gpio 173 0>; /* gpio PV5 */ + wp-gpios = <&gpio 57 0>; /* gpio PH1 */ + power-gpios = <&gpio 169 0>; /* gpio PV1 */ + bus-width = <4>; + removable = <1>; + }; + + sdhci@c8000600 { + status = "okay"; + bus-width = <8>; + removable = <0>; + }; + lcd_panel: panel { /* PAZ00 has 1024x600 */ clock = <54030000>; diff --git a/board/compulab/dts/tegra20-trimslice.dts b/board/compulab/dts/tegra20-trimslice.dts index c8a4dd4..0ab1c1d 100644 --- a/board/compulab/dts/tegra20-trimslice.dts +++ b/board/compulab/dts/tegra20-trimslice.dts @@ -9,6 +9,8 @@ aliases { usb0 = "/usb@c5008000"; usb1 = "/usb@c5000000"; + sdmmc0 = "/sdhci@c8000600"; + sdmmc1 = "/sdhci@c8000000"; };
memory { @@ -42,4 +44,19 @@ usb@c5004000 { status = "disabled"; }; + + sdhci@c8000000 { + status = "okay"; + bus-width = <4>; + removable = <0>; + }; + + sdhci@c8000600 { + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + cd-gpios = <&gpio 121 0>; /* gpio PP1 */ + wp-gpios = <&gpio 122 0>; /* gpio PP2 */ + bus-width = <4>; + removable = <1>; + }; }; diff --git a/board/nvidia/dts/tegra20-harmony.dts b/board/nvidia/dts/tegra20-harmony.dts index aeda3a1..de7ebdc 100644 --- a/board/nvidia/dts/tegra20-harmony.dts +++ b/board/nvidia/dts/tegra20-harmony.dts @@ -9,6 +9,8 @@ aliases { usb0 = "/usb@c5008000"; usb1 = "/usb@c5004000"; + sdmmc0 = "/sdhci@c8000600"; + sdmmc1 = "/sdhci@c8000200"; };
memory { @@ -52,4 +54,23 @@ usb@c5004000 { nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */ }; + + sdhci@c8000200 { + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + cd-gpios = <&gpio 69 0>; /* gpio PI5 */ + wp-gpios = <&gpio 57 0>; /* gpio PH1 */ + power-gpios = <&gpio 155 0>; /* gpio PT3 */ + bus-width = <4>; + removeable = <1>; + }; + + sdhci@c8000600 { + status = "okay"; + cd-gpios = <&gpio 58 0>; /* gpio PH2 */ + wp-gpios = <&gpio 59 0>; /* gpio PH3 */ + power-gpios = <&gpio 70 0>; /* gpio PI6 */ + bus-width = <8>; + removeable = <1>; + }; }; diff --git a/board/nvidia/dts/tegra20-seaboard.dts b/board/nvidia/dts/tegra20-seaboard.dts index 527a296..1b1f563 100644 --- a/board/nvidia/dts/tegra20-seaboard.dts +++ b/board/nvidia/dts/tegra20-seaboard.dts @@ -12,14 +12,15 @@ };
aliases { - /* This defines the order of our USB ports */ + /* This defines the order of our ports */ usb0 = "/usb@c5008000"; usb1 = "/usb@c5000000"; - i2c0 = "/i2c@7000d000"; i2c1 = "/i2c@7000c000"; i2c2 = "/i2c@7000c400"; i2c3 = "/i2c@7000c500"; + sdmmc0 = "/sdhci@c8000600"; + sdmmc1 = "/sdhci@c8000400"; };
memory { @@ -156,13 +157,19 @@ };
sdhci@c8000400 { - cd-gpios = <&gpio 69 0>; /* gpio PI5 */ - wp-gpios = <&gpio 57 0>; /* gpio PH1 */ - power-gpios = <&gpio 70 0>; /* gpio PI6 */ + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + cd-gpios = <&gpio 69 0>; /* card detect, gpio PI5 */ + wp-gpios = <&gpio 57 0>; /* write protect, gpio PH1 */ + power-gpios = <&gpio 70 3>; /* power enable, gpio PI6 */ + bus-width = <4>; + removable = <1>; };
sdhci@c8000600 { - support-8bit; + status = "okay"; + bus-width = <8>; + removable = <0>; };
lcd_panel: panel { diff --git a/board/nvidia/dts/tegra20-ventana.dts b/board/nvidia/dts/tegra20-ventana.dts index 3e5e39d..fed27ab 100644 --- a/board/nvidia/dts/tegra20-ventana.dts +++ b/board/nvidia/dts/tegra20-ventana.dts @@ -41,4 +41,27 @@ usb@c5004000 { status = "disabled"; }; + + sdhci@c8000000 { + status = "okay"; + power-gpios = <&gpio 86 0>; /* gpio PK6 */ + bus-width = <4>; + removable = <0>; + }; + + sdhci@c8000400 { + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + cd-gpios = <&gpio 69 0>; /* gpio PI5 */ + wp-gpios = <&gpio 57 0>; /* gpio PH1 */ + power-gpios = <&gpio 70 0>; /* gpio PI6 */ + bus-width = <4>; + removable = <1>; + }; + + sdhci@c8000600 { + status = "okay"; + bus-width = <8>; + removable = <0>; + }; }; diff --git a/board/nvidia/dts/tegra20-whistler.dts b/board/nvidia/dts/tegra20-whistler.dts index 4579557..ae198bd 100644 --- a/board/nvidia/dts/tegra20-whistler.dts +++ b/board/nvidia/dts/tegra20-whistler.dts @@ -9,6 +9,8 @@ aliases { i2c0 = "/i2c@7000d000"; usb0 = "/usb@c5008000"; + sdmmc0 = "/sdhci@c8000600"; + sdmmc1 = "/sdhci@c8000400"; };
memory { @@ -57,4 +59,18 @@ usb@c5004000 { status = "disabled"; }; + + sdhci@c8000400 { + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + wp-gpios = <&gpio 173 0>; /* gpio PV5 */ + bus-width = <8>; + removable = <1>; + }; + + sdhci@c8000600 { + status = "okay"; + bus-width = <8>; + removable = <0>; + }; }; diff --git a/board/toradex/dts/tegra20-colibri_t20_iris.dts b/board/toradex/dts/tegra20-colibri_t20_iris.dts index c29b43a..1689e1a 100644 --- a/board/toradex/dts/tegra20-colibri_t20_iris.dts +++ b/board/toradex/dts/tegra20-colibri_t20_iris.dts @@ -35,4 +35,12 @@ compatible = "nand-flash"; }; }; + + sdhci@c8000400 { + status = "okay"; + /* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */ + cp-gpios = <&gpio 39 0>; /* gpio PC7 */ + bus-width = <4>; + removable = <1>; + }; };
On 02/04/2013 04:48 PM, Tom Warren wrote:
Linux dts files were used for those boards that didn't already have sdhci info populated. If a cd-gpio was present, I set "removable = <1>", else removable = <0>.
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
sdhci@c8000200 { compatible = "nvidia,tegra20-sdhci"; reg = <0xc8000200 0x200>; interrupts = < 47 >;
status = "disabled";/* PERIPH_ID_SDMMC2, PLLP_OUT? */clocks = <&tegra_car 9>;
What does "PLLP_OUT?" mean?
I'm not entirely convinced it's a good idea to add this comment, since it creates a diff between the .dts files in the kernel and U-Boot.
Similarly, the status and clocks properties are in the other order in the kernel .dts files. It'd be good to be consistent to allow minimal diffs between them.
diff --git a/board/avionic-design/dts/tegra20-medcom-wide.dts b/board/avionic-design/dts/tegra20-medcom-wide.dts
I suppose that there are no aliases in this file because only one SDHCI controller is enabled. I wonder if we should add aliases to all .dts files just to be explicit? Perhaps it's not necessary because this board really will never ever get another SDHCI controller added (I assume that any SDHCI controllers the board has are already enabled, although I wonder about SDIO...), so there doesn't need to be a "hint" that there should be an alias added too?
- sdhci@c8000600 {
In the kernel, this SDHCI node is part of tegra20-tamonten.dtsi, since its parameters are defined by the carrier board. I think U-Boot .dts files should match.
The following 3 comments apply to all the .dts files (or at least the 1st and 3rd; not sure about the 2nd):
status = "okay";/* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */
I don't think that comment is particularly useful. While it's true, duplicating it every single place a GPIO is used seems wasteful. And the comment is more re: the GPIO binding that anything to do with SDHCI. Plus, it makes another diff relative to the kernel.
cd-gpios = <&gpio 58 0>; /* gpio PH2 */wp-gpios = <&gpio 59 0>; /* gpio PH3 */
The kernel appears to have a space before the comment not a TAB, so this makes another diff..
bus-width = <4>;removable = <1>;
What is "removable"? That's not in the binding documentation. There is a "non-removable" property defined in the kernel's Documentation/devicetree/bindings/mmc/mmc.txt though...
- };
diff --git a/board/nvidia/dts/tegra20-ventana.dts b/board/nvidia/dts/tegra20-ventana.dts
This file doesn't have any aliases added.
- sdhci@c8000000 {
status = "okay";power-gpios = <&gpio 86 0>; /* gpio PK6 */bus-width = <4>;removable = <0>;- };
I think that's the WiFi SDIO port, so you may not need to add it to U-Boot.
Stephen,
On Tue, Feb 5, 2013 at 12:54 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
Linux dts files were used for those boards that didn't already have sdhci info populated. If a cd-gpio was present, I set "removable = <1>", else removable = <0>.
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
sdhci@c8000200 { compatible = "nvidia,tegra20-sdhci"; reg = <0xc8000200 0x200>; interrupts = < 47 >;
status = "disabled";/* PERIPH_ID_SDMMC2, PLLP_OUT? */clocks = <&tegra_car 9>; };What does "PLLP_OUT?" mean?
The clock source used for this periph. I removed it in the I2C DT files - I'll remove it here, too, because it's up to the driver to choose that based on the freq.
I'm not entirely convinced it's a good idea to add this comment, since it creates a diff between the .dts files in the kernel and U-Boot.
Similarly, the status and clocks properties are in the other order in the kernel .dts files. It'd be good to be consistent to allow minimal diffs between them.
I used the kernel DTS files you pointed to in our internal 'Initialize SDHCI from device tree' bug: repo git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git branch for-next
The order matches arch/arm/boot/dts/tegra20.dtsi. I don't see a clocks property in that kernel file, either. I do see that the interrupts (that were already in tegra20.dtsi before I edited it) don't match the kernel dtsi for most peripherals (i2c, i2s, sdhci, gpio). I'll fix the SDHCI ones in the next patchset, though.
diff --git a/board/avionic-design/dts/tegra20-medcom-wide.dts b/board/avionic-design/dts/tegra20-medcom-wide.dts
I suppose that there are no aliases in this file because only one SDHCI controller is enabled. I wonder if we should add aliases to all .dts files just to be explicit? Perhaps it's not necessary because this board really will never ever get another SDHCI controller added (I assume that any SDHCI controllers the board has are already enabled, although I wonder about SDIO...), so there doesn't need to be a "hint" that there should be an alias added too?
If there was already an alias property in the DT file, then I tried to be consistent and add one for mmc. But adding aliases to other-than-NVIDIA boards should be up to the board maintainer/manufacturer, IMO.
sdhci@c8000600 {In the kernel, this SDHCI node is part of tegra20-tamonten.dtsi, since its parameters are defined by the carrier board. I think U-Boot .dts files should match.
Saw that, but I didn't replicate it here because, well, U-Boot's Not Linux, and IMO each board file (dts) should have its periph nodes called out explicitly. If they all happen to be exactly the same for each board, then I think the manufacturer/board maintainer can do that 'optimization' if they wish - they know better than I if they're coming out with a new board that may differ in its SDHCI properties (GPIOs, for instance).
The following 3 comments apply to all the .dts files (or at least the 1st and 3rd; not sure about the 2nd):
status = "okay";/* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */I don't think that comment is particularly useful. While it's true, duplicating it every single place a GPIO is used seems wasteful. And the comment is more re: the GPIO binding that anything to do with SDHCI. Plus, it makes another diff relative to the kernel.
Diffing the DTS files against the kernel isn't really that big a deal with a decent diff program (kompare, etc.). That GPIO comment is useful if you need to understand the 3rd param for the pwr-gpios, for instance (the cd and wp gpios almost always are input/low). And it only appears once in each DTS file, not "in every single place a GPIO is used".
cd-gpios = <&gpio 58 0>; /* gpio PH2 */wp-gpios = <&gpio 59 0>; /* gpio PH3 */The kernel appears to have a space before the comment not a TAB, so this makes another diff..
Really? That seems a little nit-picky. :/ My whitespace is consistent through-out the DTS file, and with how I always space comments on the end of a line of 'code'.
bus-width = <4>;removable = <1>;What is "removable"? That's not in the binding documentation. There is a "non-removable" property defined in the kernel's Documentation/devicetree/bindings/mmc/mmc.txt though...
These are left-over from Seaboard's original DTS file (with which I did my original development). It isn't used anywhere in U-Boot that I can see. I can remove it.
};diff --git a/board/nvidia/dts/tegra20-ventana.dts b/board/nvidia/dts/tegra20-ventana.dts
This file doesn't have any aliases added.
sdhci@c8000000 {status = "okay";power-gpios = <&gpio 86 0>; /* gpio PK6 */bus-width = <4>;removable = <0>;};I think that's the WiFi SDIO port, so you may not need to add it to U-Boot.
It's in the kernel DTS for Ventana. Won't that screw up your diff? ;) I'll remove it.
Tom
On 02/05/2013 01:29 PM, Tom Warren wrote:
Stephen,
On Tue, Feb 5, 2013 at 12:54 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
Linux dts files were used for those boards that didn't already have sdhci info populated. If a cd-gpio was present, I set "removable = <1>", else removable = <0>.
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
sdhci@c8000200 { compatible = "nvidia,tegra20-sdhci"; reg = <0xc8000200 0x200>; interrupts = < 47 >;
status = "disabled";/* PERIPH_ID_SDMMC2, PLLP_OUT? */clocks = <&tegra_car 9>; };What does "PLLP_OUT?" mean?
The clock source used for this periph. I removed it in the I2C DT files - I'll remove it here, too, because it's up to the driver to choose that based on the freq.
I'm not entirely convinced it's a good idea to add this comment, since it creates a diff between the .dts files in the kernel and U-Boot.
Similarly, the status and clocks properties are in the other order in the kernel .dts files. It'd be good to be consistent to allow minimal diffs between them.
I used the kernel DTS files you pointed to in our internal 'Initialize SDHCI from device tree' bug: repo git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git branch for-next
linux-next or the Tegra git tree have the latest additions. arm-soc hopefully will have them merged in the next day or two.
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (whatever the latest tag is there)
git://git.kernel.org/pub/scm/linux/kernel/git/swarren/linux-tegra.git (for-next)
diff --git a/board/avionic-design/dts/tegra20-medcom-wide.dts b/board/avionic-design/dts/tegra20-medcom-wide.dts
I suppose that there are no aliases in this file because only one SDHCI controller is enabled. I wonder if we should add aliases to all .dts files just to be explicit? Perhaps it's not necessary because this board really will never ever get another SDHCI controller added (I assume that any SDHCI controllers the board has are already enabled, although I wonder about SDIO...), so there doesn't need to be a "hint" that there should be an alias added too?
If there was already an alias property in the DT file, then I tried to be consistent and add one for mmc. But adding aliases to other-than-NVIDIA boards should be up to the board maintainer/manufacturer, IMO.
I don't think so; at least with the driver as-is, the code appears not to work without aliases, so not adding them causes a regression. Even ignoring that, I don't see why the code->DT conversion patch shouldn't do this for all boards.
sdhci@c8000600 {In the kernel, this SDHCI node is part of tegra20-tamonten.dtsi, since its parameters are defined by the carrier board. I think U-Boot .dts files should match.
Saw that, but I didn't replicate it here because, well, U-Boot's Not Linux, and IMO each board file (dts) should have its periph nodes called out explicitly. If they all happen to be exactly the same for each board, then I think the manufacturer/board maintainer can do that 'optimization' if they wish - they know better than I if they're coming out with a new board that may differ in its SDHCI properties (GPIOs, for instance).
No, this has nothing to do with U-Boot vs. Linux. The device tree files are (should eventually be) standard between the two, and indeed hosted outside U-Boot. Unrelated, common code is common and should be represented at a common location. In this case, the vendor for this particular file has already correctly chosen to put the SDHCI nodes in the common file, so this needs to be maintained.
The following 3 comments apply to all the .dts files (or at least the 1st and 3rd; not sure about the 2nd):
status = "okay";/* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */I don't think that comment is particularly useful. While it's true, duplicating it every single place a GPIO is used seems wasteful. And the comment is more re: the GPIO binding that anything to do with SDHCI. Plus, it makes another diff relative to the kernel.
Diffing the DTS files against the kernel isn't really that big a deal with a decent diff program (kompare, etc.). That GPIO comment is useful if you need to understand the 3rd param for the pwr-gpios, for instance (the cd and wp gpios almost always are input/low). And it only appears once in each DTS file, not "in every single place a GPIO is used".
If there is no diff at all, it's even easier.
The third parameter is already documented in the binding documentation.
cd-gpios = <&gpio 58 0>; /* gpio PH2 */wp-gpios = <&gpio 59 0>; /* gpio PH3 */The kernel appears to have a space before the comment not a TAB, so this makes another diff..
Really? That seems a little nit-picky. :/ My whitespace is consistent through-out the DTS file, and with how I always space comments on the end of a line of 'code'.
Yes, really. Why would I bother to make this review comment otherwise. As I have repeatedly specified, the idea is to reduce the diff between the kernel and U-Boot .dts files so the diff for a node is zero. There's a big difference between zero (no manual checking required at all) vs. even something trivial (always requires manual checking every time a comparison is made).
Note that at some point, all these .dts files are probably going to be removed from the kernel and U-Boot anyway, and hosted separately, so unifying them can only help there.
Sometimes I wonder why I even both reviewing things.
diff --git a/board/nvidia/dts/tegra20-ventana.dts b/board/nvidia/dts/tegra20-ventana.dts
This file doesn't have any aliases added.
sdhci@c8000000 {status = "okay";power-gpios = <&gpio 86 0>; /* gpio PK6 */bus-width = <4>;removable = <0>;};I think that's the WiFi SDIO port, so you may not need to add it to U-Boot.
It's in the kernel DTS for Ventana. Won't that screw up your diff? ;) I'll remove it.
Perhaps, but as I said before, whole nodes present/missing is a lot easier to deal with than diffs within nodes.
Right now, I believe your/Simon's policy on DT is to only include in the U-Boot .dts files what's actually needed for U-Boot. I've asked that this be done on a per-node basis rather than per-property basis in order to reduce diffs. If you want to change that, and include nodes that U-Boot doesn't need, that'd be great and assist unification, but then I'd recommend simply importing the current kernel .dts files as-is without any changes, rather than adding things piece-meal.
Hi,
On Tue, Feb 5, 2013 at 12:49 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/05/2013 01:29 PM, Tom Warren wrote:
Stephen,
On Tue, Feb 5, 2013 at 12:54 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
Linux dts files were used for those boards that didn't already have sdhci info populated. If a cd-gpio was present, I set "removable = <1>", else removable = <0>.
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
sdhci@c8000200 { compatible = "nvidia,tegra20-sdhci"; reg = <0xc8000200 0x200>; interrupts = < 47 >;
status = "disabled";/* PERIPH_ID_SDMMC2, PLLP_OUT? */clocks = <&tegra_car 9>; };What does "PLLP_OUT?" mean?
The clock source used for this periph. I removed it in the I2C DT files - I'll remove it here, too, because it's up to the driver to choose that based on the freq.
I'm not entirely convinced it's a good idea to add this comment, since it creates a diff between the .dts files in the kernel and U-Boot.
Similarly, the status and clocks properties are in the other order in the kernel .dts files. It'd be good to be consistent to allow minimal diffs between them.
I used the kernel DTS files you pointed to in our internal 'Initialize SDHCI from device tree' bug: repo git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git branch for-next
linux-next or the Tegra git tree have the latest additions. arm-soc hopefully will have them merged in the next day or two.
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (whatever the latest tag is there)
git://git.kernel.org/pub/scm/linux/kernel/git/swarren/linux-tegra.git (for-next)
diff --git a/board/avionic-design/dts/tegra20-medcom-wide.dts b/board/avionic-design/dts/tegra20-medcom-wide.dts
I suppose that there are no aliases in this file because only one SDHCI controller is enabled. I wonder if we should add aliases to all .dts files just to be explicit? Perhaps it's not necessary because this board really will never ever get another SDHCI controller added (I assume that any SDHCI controllers the board has are already enabled, although I wonder about SDIO...), so there doesn't need to be a "hint" that there should be an alias added too?
If there was already an alias property in the DT file, then I tried to be consistent and add one for mmc. But adding aliases to other-than-NVIDIA boards should be up to the board maintainer/manufacturer, IMO.
I don't think so; at least with the driver as-is, the code appears not to work without aliases, so not adding them causes a regression. Even ignoring that, I don't see why the code->DT conversion patch shouldn't do this for all boards.
sdhci@c8000600 {In the kernel, this SDHCI node is part of tegra20-tamonten.dtsi, since its parameters are defined by the carrier board. I think U-Boot .dts files should match.
Saw that, but I didn't replicate it here because, well, U-Boot's Not Linux, and IMO each board file (dts) should have its periph nodes called out explicitly. If they all happen to be exactly the same for each board, then I think the manufacturer/board maintainer can do that 'optimization' if they wish - they know better than I if they're coming out with a new board that may differ in its SDHCI properties (GPIOs, for instance).
No, this has nothing to do with U-Boot vs. Linux. The device tree files are (should eventually be) standard between the two, and indeed hosted outside U-Boot. Unrelated, common code is common and should be represented at a common location. In this case, the vendor for this particular file has already correctly chosen to put the SDHCI nodes in the common file, so this needs to be maintained.
The following 3 comments apply to all the .dts files (or at least the 1st and 3rd; not sure about the 2nd):
status = "okay";/* Parameter 3 bit 0:1=output, 0=input; bit 1:1=high, 0=low */I don't think that comment is particularly useful. While it's true, duplicating it every single place a GPIO is used seems wasteful. And the comment is more re: the GPIO binding that anything to do with SDHCI. Plus, it makes another diff relative to the kernel.
Diffing the DTS files against the kernel isn't really that big a deal with a decent diff program (kompare, etc.). That GPIO comment is useful if you need to understand the 3rd param for the pwr-gpios, for instance (the cd and wp gpios almost always are input/low). And it only appears once in each DTS file, not "in every single place a GPIO is used".
If there is no diff at all, it's even easier.
The third parameter is already documented in the binding documentation.
cd-gpios = <&gpio 58 0>; /* gpio PH2 */wp-gpios = <&gpio 59 0>; /* gpio PH3 */The kernel appears to have a space before the comment not a TAB, so this makes another diff..
Really? That seems a little nit-picky. :/ My whitespace is consistent through-out the DTS file, and with how I always space comments on the end of a line of 'code'.
Yes, really. Why would I bother to make this review comment otherwise. As I have repeatedly specified, the idea is to reduce the diff between the kernel and U-Boot .dts files so the diff for a node is zero. There's a big difference between zero (no manual checking required at all) vs. even something trivial (always requires manual checking every time a comparison is made).
Note that at some point, all these .dts files are probably going to be removed from the kernel and U-Boot anyway, and hosted separately, so unifying them can only help there.
Sometimes I wonder why I even both reviewing things.
diff --git a/board/nvidia/dts/tegra20-ventana.dts b/board/nvidia/dts/tegra20-ventana.dts
This file doesn't have any aliases added.
sdhci@c8000000 {status = "okay";power-gpios = <&gpio 86 0>; /* gpio PK6 */bus-width = <4>;removable = <0>;};I think that's the WiFi SDIO port, so you may not need to add it to U-Boot.
It's in the kernel DTS for Ventana. Won't that screw up your diff? ;) I'll remove it.
Perhaps, but as I said before, whole nodes present/missing is a lot easier to deal with than diffs within nodes.
Right now, I believe your/Simon's policy on DT is to only include in the U-Boot .dts files what's actually needed for U-Boot. I've asked that this be done on a per-node basis rather than per-property basis in order to reduce diffs. If you want to change that, and include nodes that U-Boot doesn't need, that'd be great and assist unification, but then I'd recommend simply importing the current kernel .dts files as-is without any changes, rather than adding things piece-meal.
I have to say that within reason I like the idea of bring in the DT from the kernel as is, limited perhaps to the nodes that U-Boot actually uses.
A separate repo for the DT files seems like something that should happen, but I have seen little progress on that front. Still, when it happens, it would be nice it we could drop U-Boot's files and just use the kernel's. That will be a lot easier if we head in that direction now.
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 02/05/2013 10:56:59 PM, Simon Glass wrote:
Hi,
On Tue, Feb 5, 2013 at 12:49 PM, Stephen Warren swarren@wwwdotorg.org wrote:
Right now, I believe your/Simon's policy on DT is to only include
in the
U-Boot .dts files what's actually needed for U-Boot. I've asked that this be done on a per-node basis rather than per-property basis in
order
to reduce diffs. If you want to change that, and include nodes that U-Boot doesn't need, that'd be great and assist unification, but
then
I'd recommend simply importing the current kernel .dts files as-is without any changes, rather than adding things piece-meal.
I have to say that within reason I like the idea of bring in the DT from the kernel as is, limited perhaps to the nodes that U-Boot actually uses.
A separate repo for the DT files seems like something that should happen, but I have seen little progress on that front. Still, when it happens, it would be nice it we could drop U-Boot's files and just use the kernel's. That will be a lot easier if we head in that direction now.
I think any device tree that makes assumptions about what U-Boot will be fixing up, or even what addresses U-Boot will configure devices at, belongs in the U-Boot tree. Keeping such trees in Linux has been awkward so far, especially when a change gets made to such an assumption, or when U-Boot isn't the only supported firmware.
-Scott
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
Signed-off-by: Tom Warren twarren@nvidia.com --- arch/arm/include/asm/arch-tegra/mmc.h | 2 +- arch/arm/include/asm/arch-tegra/tegra_mmc.h | 12 +- board/avionic-design/common/tamonten.c | 4 +- board/compal/paz00/paz00.c | 14 +- board/compulab/trimslice/trimslice.c | 10 +- board/nvidia/harmony/harmony.c | 12 +- board/nvidia/seaboard/seaboard.c | 14 +- board/nvidia/whistler/whistler.c | 4 +- board/toradex/colibri_t20_iris/colibri_t20_iris.c | 2 +- drivers/mmc/tegra_mmc.c | 186 ++++++++++++++------- include/fdtdec.h | 1 + lib/fdtdec.c | 1 + 12 files changed, 165 insertions(+), 97 deletions(-)
diff --git a/arch/arm/include/asm/arch-tegra/mmc.h b/arch/arm/include/asm/arch-tegra/mmc.h index 5c95047..344cdfb 100644 --- a/arch/arm/include/asm/arch-tegra/mmc.h +++ b/arch/arm/include/asm/arch-tegra/mmc.h @@ -22,6 +22,6 @@ #ifndef _TEGRA_MMC_H_ #define _TEGRA_MMC_H_
-int tegra_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio); +int tegra_mmc_init(int dev_index);
#endif /* _TEGRA_MMC_H_ */ diff --git a/arch/arm/include/asm/arch-tegra/tegra_mmc.h b/arch/arm/include/asm/arch-tegra/tegra_mmc.h index dd746ca..c229e55 100644 --- a/arch/arm/include/asm/arch-tegra/tegra_mmc.h +++ b/arch/arm/include/asm/arch-tegra/tegra_mmc.h @@ -27,6 +27,8 @@ #define TEGRA_SDMMC3_BASE 0xC8000400 #define TEGRA_SDMMC4_BASE 0xC8000600
+#define MAX_HOSTS 4 /* Max number of 'hosts'/controllers */ + #ifndef __ASSEMBLY__ struct tegra_mmc { unsigned int sysad; /* _SYSTEM_ADDRESS_0 */ @@ -119,12 +121,16 @@ struct tegra_mmc {
struct mmc_host { struct tegra_mmc *reg; + int enabled; /* 1 to enable, 0 to disable */ + int width; /* Bus Width, 1, 4 or 8 */ + int removable; /* Removable media (0 = eMMC, 1 = SD-card) */ + enum periph_id mmc_id; /* Peripheral ID: PERIPH_ID_... */ + struct fdt_gpio_state cd_gpio; /* Change Detect GPIO */ + struct fdt_gpio_state pwr_gpio; /* Power GPIO */ + struct fdt_gpio_state wp_gpio; /* Write Protect GPIO */ unsigned int version; /* SDHCI spec. version */ unsigned int clock; /* Current clock (MHz) */ unsigned int base; /* Base address, SDMMC1/2/3/4 */ - enum periph_id mmc_id; /* Peripheral ID: PERIPH_ID_... */ - int pwr_gpio; /* Power GPIO */ - int cd_gpio; /* Change Detect GPIO */ };
#endif /* __ASSEMBLY__ */ diff --git a/board/avionic-design/common/tamonten.c b/board/avionic-design/common/tamonten.c index e6a932e..5e4bd43 100644 --- a/board/avionic-design/common/tamonten.c +++ b/board/avionic-design/common/tamonten.c @@ -69,8 +69,8 @@ int board_mmc_init(bd_t *bd) /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
- /* init dev 0, SD slot, with 4-bit bus */ - tegra_mmc_init(0, 4, GPIO_PI6, GPIO_PH2); + /* init dev 0, SD slot */ + tegra_mmc_init(0);
return 0; } diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) { - debug("board_mmc_init called\n"); + debug("%s called\n", __func__);
/* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
- debug("board_mmc_init: init eMMC\n"); - /* init dev 0, eMMC chip, with 8-bit bus */ - tegra_mmc_init(0, 8, -1, -1); + debug("%s: init eMMC\n", __func__); + /* init dev 0, eMMC chip */ + tegra_mmc_init(0);
- debug("board_mmc_init: init SD slot\n"); - /* init dev 3, SD slot, with 4-bit bus */ - tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5); + debug("%s: init SD slot\n", __func__); + /* init dev 3, SD slot */ + tegra_mmc_init(3);
return 0; } diff --git a/board/compulab/trimslice/trimslice.c b/board/compulab/trimslice/trimslice.c index 8f4dd09..a1ec5e4 100644 --- a/board/compulab/trimslice/trimslice.c +++ b/board/compulab/trimslice/trimslice.c @@ -64,16 +64,16 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) { - debug("board_mmc_init called\n"); + debug("%s called\n", __func__);
/* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
- /* init dev 0 (SDMMC4), (micro-SD slot) with 4-bit bus */ - tegra_mmc_init(0, 4, -1, GPIO_PP1); + /* init dev 0 (SDMMC4), (micro-SD slot) */ + tegra_mmc_init(0);
- /* init dev 3 (SDMMC1), (SD slot) with 4-bit bus */ - tegra_mmc_init(3, 4, -1, -1); + /* init dev 3 (SDMMC1), (SD slot) */ + tegra_mmc_init(3);
return 0; } diff --git a/board/nvidia/harmony/harmony.c b/board/nvidia/harmony/harmony.c index 93430ed..cec85a3 100644 --- a/board/nvidia/harmony/harmony.c +++ b/board/nvidia/harmony/harmony.c @@ -58,18 +58,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) { - debug("board_mmc_init called\n"); + debug("%s called\n", __func__);
/* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
- debug("board_mmc_init: init SD slot J26\n"); + debug("%s: init SD slot J26\n", __func__); /* init dev 0, SD slot J26, with 8-bit bus */ - tegra_mmc_init(0, 8, GPIO_PI6, GPIO_PH2); + tegra_mmc_init(0);
- debug("board_mmc_init: init SD slot J5\n"); - /* init dev 2, SD slot J5, with 4-bit bus */ - tegra_mmc_init(2, 4, GPIO_PT3, GPIO_PI5); + debug("%s: init SD slot J5\n", __func__); + /* init dev 2 */ + tegra_mmc_init(2);
return 0; } diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c index 3e33da0..f0dad8e 100644 --- a/board/nvidia/seaboard/seaboard.c +++ b/board/nvidia/seaboard/seaboard.c @@ -65,18 +65,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) { - debug("board_mmc_init called\n"); + debug("%s called\n", __func__);
/* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
- debug("board_mmc_init: init eMMC\n"); - /* init dev 0, eMMC chip, with 8-bit bus */ - tegra_mmc_init(0, 8, -1, -1); + debug("%s: init eMMC\n", __func__); + /* init dev 0, eMMC chip */ + tegra_mmc_init(0);
- debug("board_mmc_init: init SD slot\n"); - /* init dev 1, SD slot, with 4-bit bus */ - tegra_mmc_init(1, 4, GPIO_PI6, GPIO_PI5); + debug("%s: init SD-card slot\n", __func__); + /* init dev 1, SD-card slot */ + tegra_mmc_init(1);
return 0; } diff --git a/board/nvidia/whistler/whistler.c b/board/nvidia/whistler/whistler.c index 592cd6b..194dc1b 100644 --- a/board/nvidia/whistler/whistler.c +++ b/board/nvidia/whistler/whistler.c @@ -74,10 +74,10 @@ int board_mmc_init(bd_t *bd) pin_mux_mmc();
/* init dev 0 (SDMMC4), (J29 "HSMMC") with 8-bit bus */ - tegra_mmc_init(0, 8, -1, -1); + tegra_mmc_init(0);
/* init dev 1 (SDMMC3), (J40 "SDIO3") with 8-bit bus */ - tegra_mmc_init(1, 8, -1, -1); + tegra_mmc_init(1);
return 0; } diff --git a/board/toradex/colibri_t20_iris/colibri_t20_iris.c b/board/toradex/colibri_t20_iris/colibri_t20_iris.c index e40a986..456f201 100644 --- a/board/toradex/colibri_t20_iris/colibri_t20_iris.c +++ b/board/toradex/colibri_t20_iris/colibri_t20_iris.c @@ -39,7 +39,7 @@ int board_mmc_init(bd_t *bd) funcmux_select(PERIPH_ID_SDMMC4, FUNCMUX_SDMMC4_ATB_GMA_4_BIT); pinmux_tristate_disable(PINGRP_GMB);
- tegra_mmc_init(0, 4, -1, GPIO_PC7); + tegra_mmc_init(0);
return 0; } diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c index d749ab0..24af636 100644 --- a/drivers/mmc/tegra_mmc.c +++ b/drivers/mmc/tegra_mmc.c @@ -21,6 +21,7 @@
#include <bouncebuf.h> #include <common.h> +#include <fdtdec.h> #include <asm/gpio.h> #include <asm/io.h> #include <asm/arch/clock.h> @@ -28,10 +29,14 @@ #include <asm/arch-tegra/tegra_mmc.h> #include <mmc.h>
-/* support 4 mmc hosts */ -struct mmc mmc_dev[4]; -struct mmc_host mmc_host[4]; +DECLARE_GLOBAL_DATA_PTR;
+struct mmc mmc_dev[MAX_HOSTS]; +struct mmc_host mmc_host[MAX_HOSTS]; + +#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
/** * Get the host address and peripheral ID for a device. Devices are numbered @@ -42,8 +47,60 @@ struct mmc_host mmc_host[4]; */ static void tegra_get_setup(struct mmc_host *host, int dev_index) { - debug("tegra_get_setup: dev_index = %d\n", dev_index); + debug("%s: dev_index = %d\n", __func__, dev_index); + +#ifdef CONFIG_OF_CONTROL + int count, node = 0; + int node_list[MAX_HOSTS]; + + count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc", + COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS); + debug("%s: count of nodes is %d\n", __func__, count); + + if (count < dev_index) { + printf("%s: device index %d exceeds node count (%d)!\n", + __func__, dev_index, count); + return; + } + + node = node_list[dev_index]; + + host->enabled = fdtdec_get_is_enabled(gd->fdt_blob, node); + + host->reg = (struct tegra_mmc *)fdtdec_get_addr(gd->fdt_blob, + node, "reg"); + if ((fdt_addr_t)host->reg == FDT_ADDR_T_NONE) { + debug("%s: no sdmmc base reg info found\n", __func__); + return; + } + + host->mmc_id = clock_decode_periph_id(gd->fdt_blob, node); + if (host->mmc_id == PERIPH_ID_NONE) { + debug("%s: could not decode periph id\n", __func__); + return; + } + + /* + * NOTE: mmc->bus_width is determined by mmc.c dynamically. + * Should we override it with this value? + */ + host->width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 0); + if (!host->width) + debug("%s: no sdmmc width found\n", __func__);
+ /* This flag isn't used anywhere in this driver or mmc.c */ + host->removable = fdtdec_get_int(gd->fdt_blob, node, "removable", 0); + + /* These GPIOs are optional */ + fdtdec_decode_gpio(gd->fdt_blob, node, "cd-gpios", &host->cd_gpio); + fdtdec_decode_gpio(gd->fdt_blob, node, "wp-gpios", &host->wp_gpio); + fdtdec_decode_gpio(gd->fdt_blob, node, "power-gpios", &host->pwr_gpio); + + host->base = (unsigned)host->reg; + + debug("%s: found controller at %p, width = %d, periph_id = %d\n", + __func__, host->reg, host->width, host->mmc_id); +#else switch (dev_index) { case 1: host->base = TEGRA_SDMMC3_BASE; @@ -63,8 +120,8 @@ static void tegra_get_setup(struct mmc_host *host, int dev_index) host->mmc_id = PERIPH_ID_SDMMC4; break; } - host->reg = (struct tegra_mmc *)host->base; +#endif /* !CONFIG_OF_CONTROL */ }
static void mmc_prepare_data(struct mmc_host *host, struct mmc_data *data, @@ -72,10 +129,9 @@ static void mmc_prepare_data(struct mmc_host *host, struct mmc_data *data, { unsigned char ctrl;
- - debug("buf: %p (%p), data->blocks: %u, data->blocksize: %u\n", - bbstate->bounce_buffer, bbstate->user_buffer, data->blocks, - data->blocksize); + debug("%s: buf: %p (%p), data->blocks: %u, data->blocksize: %u\n", + __func__, bbstate->bounce_buffer, bbstate->user_buffer, + data->blocks, data->blocksize);
writel((u32)bbstate->bounce_buffer, &host->reg->sysad); /* @@ -98,7 +154,7 @@ static void mmc_prepare_data(struct mmc_host *host, struct mmc_data *data, static void mmc_set_transfer_mode(struct mmc_host *host, struct mmc_data *data) { unsigned short mode; - debug(" mmc_set_transfer_mode called\n"); + debug("%s called\n", __func__); /* * TRNMOD * MUL1SIN0[5] : Multi/Single Block Select @@ -121,10 +177,8 @@ static void mmc_set_transfer_mode(struct mmc_host *host, struct mmc_data *data) writew(mode, &host->reg->trnmod); }
-static int mmc_wait_inhibit(struct mmc_host *host, - struct mmc_cmd *cmd, - struct mmc_data *data, - unsigned int timeout) +static int mmc_wait_inhibit(struct mmc_host *host, struct mmc_cmd *cmd, + struct mmc_data *data, unsigned int timeout) { /* * PRNSTS @@ -148,19 +202,18 @@ static int mmc_wait_inhibit(struct mmc_host *host, timeout--; udelay(1000); } - return 0; }
static int mmc_send_cmd_bounced(struct mmc *mmc, struct mmc_cmd *cmd, - struct mmc_data *data, struct bounce_buffer *bbstate) + struct mmc_data *data, struct bounce_buffer *bbstate) { struct mmc_host *host = (struct mmc_host *)mmc->priv; int flags, i; int result; unsigned int mask = 0; unsigned int retry = 0x100000; - debug(" mmc_send_cmd called\n"); + debug("%s called\n", __func__);
result = mmc_wait_inhibit(host, cmd, data, 10 /* ms */);
@@ -170,7 +223,7 @@ static int mmc_send_cmd_bounced(struct mmc *mmc, struct mmc_cmd *cmd, if (data) mmc_prepare_data(host, data, bbstate);
- debug("cmd->arg: %08x\n", cmd->cmdarg); + debug("%s: cmd->arg: %08x\n", __func__, cmd->cmdarg); writel(cmd->cmdarg, &host->reg->argument);
if (data) @@ -207,7 +260,7 @@ static int mmc_send_cmd_bounced(struct mmc *mmc, struct mmc_cmd *cmd, if (data) flags |= TEGRA_MMC_TRNMOD_DATA_PRESENT_SELECT_DATA_TRANSFER;
- debug("cmd: %d\n", cmd->cmdidx); + debug("%s: cmd: %d\n", __func__, cmd->cmdidx);
writew((cmd->cmdidx << 8) | flags, &host->reg->cmdreg);
@@ -229,12 +282,14 @@ static int mmc_send_cmd_bounced(struct mmc *mmc, struct mmc_cmd *cmd,
if (mask & TEGRA_MMC_NORINTSTS_CMD_TIMEOUT) { /* Timeout Error */ - debug("timeout: %08x cmd %d\n", mask, cmd->cmdidx); + debug("%s: timeout: %08x cmd %d\n", __func__, mask, + cmd->cmdidx); writel(mask, &host->reg->norintsts); return TIMEOUT; } else if (mask & TEGRA_MMC_NORINTSTS_ERR_INTERRUPT) { /* Error Interrupt */ - debug("error: %08x cmd %d\n", mask, cmd->cmdidx); + debug("%s: error: %08x cmd %d\n", __func__, mask, + cmd->cmdidx); writel(mask, &host->reg->norintsts); return -1; } @@ -251,8 +306,8 @@ static int mmc_send_cmd_bounced(struct mmc *mmc, struct mmc_cmd *cmd, cmd->response[i] |= readb(offset - 1); } - debug("cmd->resp[%d]: %08x\n", - i, cmd->response[i]); + debug("%s: cmd->resp[%d]: %08x\n", + __func__, i, cmd->response[i]); } } else if (cmd->resp_type & MMC_RSP_BUSY) { for (i = 0; i < retry; i++) { @@ -269,10 +324,12 @@ static int mmc_send_cmd_bounced(struct mmc *mmc, struct mmc_cmd *cmd, }
cmd->response[0] = readl(&host->reg->rspreg0); - debug("cmd->resp[0]: %08x\n", cmd->response[0]); + debug("%s: cmd->resp[0]: %08x\n", + __func__, cmd->response[0]); } else { cmd->response[0] = readl(&host->reg->rspreg0); - debug("cmd->resp[0]: %08x\n", cmd->response[0]); + debug("%s: cmd->resp[0]: %08x\n", + __func__, cmd->response[0]); } }
@@ -295,13 +352,13 @@ static int mmc_send_cmd_bounced(struct mmc *mmc, struct mmc_cmd *cmd, */ unsigned int address = readl(&host->reg->sysad);
- debug("DMA end\n"); + debug("%s: DMA end\n", __func__); writel(TEGRA_MMC_NORINTSTS_DMA_INTERRUPT, - &host->reg->norintsts); + &host->reg->norintsts); writel(address, &host->reg->sysad); } else if (mask & TEGRA_MMC_NORINTSTS_XFER_COMPLETE) { /* Transfer Complete */ - debug("r/w is done\n"); + debug("%s: r/w is done\n", __func__); break; } else if (get_timer(start) > 2000UL) { writel(mask, &host->reg->norintsts); @@ -325,13 +382,14 @@ static int mmc_send_cmd_bounced(struct mmc *mmc, struct mmc_cmd *cmd, }
static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, - struct mmc_data *data) + struct mmc_data *data) { void *buf; unsigned int bbflags; size_t len; struct bounce_buffer bbstate; int ret; + debug("%s: called\n", __func__);
if (data) { if (data->flags & MMC_DATA_READ) { @@ -359,8 +417,7 @@ static void mmc_change_clock(struct mmc_host *host, uint clock) int div; unsigned short clk; unsigned long timeout; - - debug(" mmc_change_clock called\n"); + debug("%s called\n", __func__);
/* * Change Tegra SDMMCx clock divisor here. Source is 216MHz, @@ -368,9 +425,8 @@ static void mmc_change_clock(struct mmc_host *host, uint clock) */ if (clock == 0) goto out; - clock_adjust_periph_pll_div(host->mmc_id, CLOCK_ID_PERIPH, clock, - &div); - debug("div = %d\n", div); + clock_adjust_periph_pll_div(host->mmc_id, CLOCK_ID_PERIPH, clock, &div); + debug("%s: div = %d\n", __func__, div);
writew(0, &host->reg->clkcon);
@@ -383,7 +439,7 @@ static void mmc_change_clock(struct mmc_host *host, uint clock) */ div >>= 1; clk = ((div << TEGRA_MMC_CLKCON_SDCLK_FREQ_SEL_SHIFT) | - TEGRA_MMC_CLKCON_INTERNAL_CLOCK_ENABLE); + TEGRA_MMC_CLKCON_INTERNAL_CLOCK_ENABLE); writew(clk, &host->reg->clkcon);
/* Wait max 10 ms */ @@ -401,7 +457,7 @@ static void mmc_change_clock(struct mmc_host *host, uint clock) clk |= TEGRA_MMC_CLKCON_SD_CLOCK_ENABLE; writew(clk, &host->reg->clkcon);
- debug("mmc_change_clock: clkcon = %08X\n", clk); + debug("%s: clkcon = %08X\n", __func__, clk);
out: host->clock = clock; @@ -411,9 +467,8 @@ static void mmc_set_ios(struct mmc *mmc) { struct mmc_host *host = mmc->priv; unsigned char ctrl; - debug(" mmc_set_ios called\n"); - - debug("bus_width: %x, clock: %d\n", mmc->bus_width, mmc->clock); + debug("%s: bus_width: %x, clock: %d\n", __func__, mmc->bus_width, + mmc->clock);
/* Change clock first */ mmc_change_clock(host, mmc->clock); @@ -436,13 +491,13 @@ static void mmc_set_ios(struct mmc *mmc) ctrl &= ~(1 << 1);
writeb(ctrl, &host->reg->hostctl); - debug("mmc_set_ios: hostctl = %08X\n", ctrl); + debug("%s: hostctl = %08X\n", __func__, ctrl); }
static void mmc_reset(struct mmc_host *host) { unsigned int timeout; - debug(" mmc_reset called\n"); + debug("%s called\n", __func__);
/* * RSTALL[0] : Software reset for all @@ -471,12 +526,11 @@ static int mmc_core_init(struct mmc *mmc) { struct mmc_host *host = (struct mmc_host *)mmc->priv; unsigned int mask; - debug(" mmc_core_init called\n");
mmc_reset(host);
host->version = readw(&host->reg->hcver); - debug("host version = %x\n", host->version); + debug("%s: host version = %x\n", __func__, host->version);
/* mask all */ writel(0xffffffff, &host->reg->norintstsen); @@ -515,44 +569,49 @@ static int mmc_core_init(struct mmc *mmc) int tegra_mmc_getcd(struct mmc *mmc) { struct mmc_host *host = (struct mmc_host *)mmc->priv; + debug("%s called, host->cd_gpio = 0x%08X\n", __func__, + (unsigned)&host->cd_gpio);
- debug("tegra_mmc_getcd called\n"); - - if (host->cd_gpio >= 0) - return !gpio_get_value(host->cd_gpio); + if (fdt_gpio_isvalid(&host->cd_gpio)) + return !fdtdec_get_gpio(&host->cd_gpio);
return 1; }
-int tegra_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio) +int tegra_mmc_init(int dev_index) { struct mmc_host *host; char gpusage[12]; /* "SD/MMCn PWR" or "SD/MMCn CD" */ struct mmc *mmc; - - debug(" tegra_mmc_init: index %d, bus width %d " - "pwr_gpio %d cd_gpio %d\n", - dev_index, bus_width, pwr_gpio, cd_gpio); + int card_det = 0;
host = &mmc_host[dev_index];
- host->clock = 0; - host->pwr_gpio = pwr_gpio; - host->cd_gpio = cd_gpio; + /* Read DT and get config */ tegra_get_setup(host, dev_index); + if (!host->enabled) + return -1; + + debug("%s: index %d, bus width %d pwr_gpio %d cd_gpio %d\n", + __func__, dev_index, host->width, + host->pwr_gpio.gpio, host->cd_gpio.gpio);
+ host->clock = 0; clock_start_periph_pll(host->mmc_id, CLOCK_ID_PERIPH, 20000000);
- if (host->pwr_gpio >= 0) { + if (fdt_gpio_isvalid(&host->pwr_gpio)) { sprintf(gpusage, "SD/MMC%d PWR", dev_index); - gpio_request(host->pwr_gpio, gpusage); - gpio_direction_output(host->pwr_gpio, 1); + gpio_request(host->pwr_gpio.gpio, gpusage); + fdtdec_set_gpio(&host->pwr_gpio, 1); + debug(" Power GPIO name = %s\n", host->pwr_gpio.name); }
- if (host->cd_gpio >= 0) { + if (fdt_gpio_isvalid(&host->cd_gpio)) { sprintf(gpusage, "SD/MMC%d CD", dev_index); - gpio_request(host->cd_gpio, gpusage); - gpio_direction_input(host->cd_gpio); + gpio_request(host->cd_gpio.gpio, gpusage); + card_det = fdtdec_get_gpio(&host->cd_gpio); + debug(" CD GPIO name = %s\n", host->cd_gpio.name); + debug("%s: CD state = %d\n", __func__, card_det); }
mmc = &mmc_dev[dev_index]; @@ -566,9 +625,10 @@ int tegra_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio)
mmc->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195; mmc->host_caps = 0; - if (bus_width == 8) + debug("%s: bus width = %d\n", __func__, host->width); + if (host->width == 8) mmc->host_caps |= MMC_MODE_8BIT; - if (bus_width >= 4) + if (host->width >= 4) mmc->host_caps |= MMC_MODE_4BIT; mmc->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_HC;
diff --git a/include/fdtdec.h b/include/fdtdec.h index f77d195..e16d6a5 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -61,6 +61,7 @@ struct fdt_memory { */ enum fdt_compat_id { COMPAT_UNKNOWN, + COMPAT_NVIDIA_TEGRA20_SDMMC, /* Tegra SDMMC controller */ COMPAT_NVIDIA_TEGRA20_USB, /* Tegra20 USB port */ COMPAT_NVIDIA_TEGRA20_I2C, /* Tegra20 i2c */ COMPAT_NVIDIA_TEGRA20_DVC, /* Tegra20 dvc (really just i2c) */ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 16921e1..4cd18a8 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -36,6 +36,7 @@ DECLARE_GLOBAL_DATA_PTR; #define COMPAT(id, name) name static const char * const compat_names[COMPAT_COUNT] = { COMPAT(UNKNOWN, "<none>"), + COMPAT(NVIDIA_TEGRA20_SDMMC, "nvidia,tegra20-sdhci"), COMPAT(NVIDIA_TEGRA20_USB, "nvidia,tegra20-ehci"), COMPAT(NVIDIA_TEGRA20_I2C, "nvidia,tegra20-i2c"), COMPAT(NVIDIA_TEGRA20_DVC, "nvidia,tegra20-i2c-dvc"),
Hi Tom,
Am Montag, 4. Februar 2013, 16:48:55 schrieb Tom Warren:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
Signed-off-by: Tom Warren twarren@nvidia.com
arch/arm/include/asm/arch-tegra/mmc.h | 2 +- arch/arm/include/asm/arch-tegra/tegra_mmc.h | 12 +- board/avionic-design/common/tamonten.c | 4 +- board/compal/paz00/paz00.c | 14 +- board/compulab/trimslice/trimslice.c | 10 +- board/nvidia/harmony/harmony.c | 12 +- board/nvidia/seaboard/seaboard.c | 14 +- board/nvidia/whistler/whistler.c | 4 +- board/toradex/colibri_t20_iris/colibri_t20_iris.c | 2 +- drivers/mmc/tegra_mmc.c | 186 ++++++++++++++------- include/fdtdec.h | 1 + lib/fdtdec.c | 1 + 12 files changed, 165 insertions(+), 97 deletions(-)
[...]
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
- debug("board_mmc_init called\n");
debug("%s called\n", __func__);
/* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
- debug("board_mmc_init: init eMMC\n");
- /* init dev 0, eMMC chip, with 8-bit bus */
- tegra_mmc_init(0, 8, -1, -1);
- debug("%s: init eMMC\n", __func__);
- /* init dev 0, eMMC chip */
- tegra_mmc_init(0);
This looks wrong because the sd is on sdmmc0
- debug("board_mmc_init: init SD slot\n");
- /* init dev 3, SD slot, with 4-bit bus */
- tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
- debug("%s: init SD slot\n", __func__);
- /* init dev 3, SD slot */
- tegra_mmc_init(3);
and the emmc on sdmmc3. The DTS is correct.
Not your fault as it seems to be wrong in the original code already. I guess it didn't made large difference but may in the future. I wonder how to test this though.
Marc
Marc,
On Tue, Feb 5, 2013 at 2:28 AM, Marc Dietrich marvin24@gmx.de wrote:
Hi Tom,
Am Montag, 4. Februar 2013, 16:48:55 schrieb Tom Warren:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
Signed-off-by: Tom Warren twarren@nvidia.com
arch/arm/include/asm/arch-tegra/mmc.h | 2 +- arch/arm/include/asm/arch-tegra/tegra_mmc.h | 12 +- board/avionic-design/common/tamonten.c | 4 +- board/compal/paz00/paz00.c | 14 +- board/compulab/trimslice/trimslice.c | 10 +- board/nvidia/harmony/harmony.c | 12 +- board/nvidia/seaboard/seaboard.c | 14 +- board/nvidia/whistler/whistler.c | 4 +- board/toradex/colibri_t20_iris/colibri_t20_iris.c | 2 +- drivers/mmc/tegra_mmc.c | 186 ++++++++++++++------- include/fdtdec.h | 1 + lib/fdtdec.c | 1 + 12 files changed, 165 insertions(+), 97 deletions(-)
[...]
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);This looks wrong because the sd is on sdmmc0
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3);and the emmc on sdmmc3. The DTS is correct.
Not your fault as it seems to be wrong in the original code already. I guess it didn't made large difference but may in the future. I wonder how to test this though.
Marc
OK, so just the comments are wrong in paz00.c - I can fix that if I have to do a V2 patchset, or when I apply the patches to u-boot-tegra.
As to testing, just stop at the command prompt and select each device (mmc dev 0, etc.) and run mmcinfo. You should be able to tell from the data displayed whether you are on an SD-card or eMMC chip. You can also eject the SD-card and you should get a warning about card presence due to the CD GPIO.
Tom
On Tuesday 05 February 2013 08:31:03 Tom Warren wrote:
Marc,
On Tue, Feb 5, 2013 at 2:28 AM, Marc Dietrich marvin24@gmx.de wrote:
[...]
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void)
/* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);This looks wrong because the sd is on sdmmc0
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3);and the emmc on sdmmc3. The DTS is correct.
Not your fault as it seems to be wrong in the original code already. I guess it didn't made large difference but may in the future. I wonder how to test this though.
Marc
OK, so just the comments are wrong in paz00.c - I can fix that if I have to do a V2 patchset, or when I apply the patches to u-boot-tegra.
ah no, this is weird!
index 3 maps to sdmmc1 index 2 maps to sdmmc2 index 1 maps to sdmmc3 index 0 maps to sdmmc4
so all is fine, nearly ...
As to testing, just stop at the command prompt and select each device (mmc dev 0, etc.) and run mmcinfo. You should be able to tell from the data displayed whether you are on an SD-card or eMMC chip. You can also eject the SD-card and you should get a warning about card presence due to the CD GPIO.
the sd card is not detected because:
TEGRA20 Board: Compal Paz00 DRAM: 512 MiB MMC: tegra_get_setup: dev_index = 0 tegra_get_setup: count of nodes is 2 tegra_get_setup: found controller at c8000600, width = 8, periph_id = 15 tegra_mmc_init: index 0, bus width 8 pwr_gpio -1 cd_gpio -1 tegra_mmc_init: bus width = 8 tegra_get_setup: dev_index = 3 tegra_get_setup: count of nodes is 2 tegra_get_setup: device index 3 exceeds node count (2)!
If I understand correctly, you are counting the aliases only, not the controllers..., so index 3 (the sdcard) is not initialized at all. Arrr, debugging stole all of my time, but I guess this needs fixing.
Marc
Marc,
On Tue, Feb 5, 2013 at 1:06 PM, Marc Dietrich marvin24@gmx.de wrote:
On Tuesday 05 February 2013 08:31:03 Tom Warren wrote:
Marc,
On Tue, Feb 5, 2013 at 2:28 AM, Marc Dietrich marvin24@gmx.de wrote:
[...]
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void)
/* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);This looks wrong because the sd is on sdmmc0
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3);and the emmc on sdmmc3. The DTS is correct.
Not your fault as it seems to be wrong in the original code already. I guess it didn't made large difference but may in the future. I wonder how to test this though.
Marc
OK, so just the comments are wrong in paz00.c - I can fix that if I have to do a V2 patchset, or when I apply the patches to u-boot-tegra.
ah no, this is weird!
index 3 maps to sdmmc1 index 2 maps to sdmmc2 index 1 maps to sdmmc3 index 0 maps to sdmmc4so all is fine, nearly ...
As to testing, just stop at the command prompt and select each device (mmc dev 0, etc.) and run mmcinfo. You should be able to tell from the data displayed whether you are on an SD-card or eMMC chip. You can also eject the SD-card and you should get a warning about card presence due to the CD GPIO.
the sd card is not detected because:
TEGRA20 Board: Compal Paz00 DRAM: 512 MiB MMC: tegra_get_setup: dev_index = 0 tegra_get_setup: count of nodes is 2 tegra_get_setup: found controller at c8000600, width = 8, periph_id = 15 tegra_mmc_init: index 0, bus width 8 pwr_gpio -1 cd_gpio -1 tegra_mmc_init: bus width = 8 tegra_get_setup: dev_index = 3 tegra_get_setup: count of nodes is 2 tegra_get_setup: device index 3 exceeds node count (2)!
If I understand correctly, you are counting the aliases only, not the controllers..., so index 3 (the sdcard) is not initialized at all. Arrr, debugging stole all of my time, but I guess this needs fixing.
Yep, I am checking the aliases to get a node count (just like the Tegra SPI, SLINK, and I2C drivers, and the Exynos SPI and S3C I2C drivers).
I left the Paz00 tegra_mmc_init(3) call the same as originally (w/o the width and GPIO params, of course). The device numbering is kind of arbitrary - if there are only 2 MMC devices present, I'd expect dev 0 to be eMMC and dev 1 to be SD (again, like my T20 reference board, Seaboard). I don't see that Paz uses mmc anywhere in its config files for a boot script - does it have to have mmc dev 3 be SD? or would dev 1 work? Note that the tegra20-common-post.h file that all T20 boards inherit uses dev 0 and dev 1.
Let me look into it - wish I had a Paz00 board here to debug with. I'll try to simulate this on my Seaboard, maybe with all 4 MMC device addresses in the alias.
Thanks for the help,
Tom
Marc
On 02/05/2013 01:41 PM, Tom Warren wrote: ...
I left the Paz00 tegra_mmc_init(3) call the same as originally (w/o the width and GPIO params, of course). The device numbering is kind of arbitrary - if there are only 2 MMC devices present, I'd expect dev 0 to be eMMC and dev 1 to be SD (again, like my T20 reference board, Seaboard). I don't see that Paz uses mmc anywhere in its config files for a boot script - does it have to have mmc dev 3 be SD? or would dev 1 work? Note that the tegra20-common-post.h file that all T20 boards inherit uses dev 0 and dev 1.
This board currently uses U-Boot MMC device ID 0 for the eMMC and U-Boot MMC device ID 1 for the SD card slot. The U-Boot device ID's aren't directly related to the Tegra HW instance IDs.
Tom,
On Tuesday 05 February 2013 13:41:21 Tom Warren wrote:
Marc,
On Tue, Feb 5, 2013 at 1:06 PM, Marc Dietrich marvin24@gmx.de wrote:
On Tuesday 05 February 2013 08:31:03 Tom Warren wrote:
Marc,
On Tue, Feb 5, 2013 at 2:28 AM, Marc Dietrich marvin24@gmx.de wrote:
[...]
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void)
/* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);This looks wrong because the sd is on sdmmc0
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3);and the emmc on sdmmc3. The DTS is correct.
Not your fault as it seems to be wrong in the original code already. I guess it didn't made large difference but may in the future. I wonder how to test this though.
Marc
OK, so just the comments are wrong in paz00.c - I can fix that if I have to do a V2 patchset, or when I apply the patches to u-boot-tegra.
ah no, this is weird!
index 3 maps to sdmmc1 index 2 maps to sdmmc2 index 1 maps to sdmmc3 index 0 maps to sdmmc4so all is fine, nearly ...
As to testing, just stop at the command prompt and select each device (mmc dev 0, etc.) and run mmcinfo. You should be able to tell from the data displayed whether you are on an SD-card or eMMC chip. You can also eject the SD-card and you should get a warning about card presence due to the CD GPIO.
the sd card is not detected because:
TEGRA20 Board: Compal Paz00 DRAM: 512 MiB MMC: tegra_get_setup: dev_index = 0 tegra_get_setup: count of nodes is 2 tegra_get_setup: found controller at c8000600, width = 8, periph_id = 15 tegra_mmc_init: index 0, bus width 8 pwr_gpio -1 cd_gpio -1 tegra_mmc_init: bus width = 8 tegra_get_setup: dev_index = 3 tegra_get_setup: count of nodes is 2 tegra_get_setup: device index 3 exceeds node count (2)!
If I understand correctly, you are counting the aliases only, not the controllers..., so index 3 (the sdcard) is not initialized at all. Arrr, debugging stole all of my time, but I guess this needs fixing.
Yep, I am checking the aliases to get a node count (just like the Tegra SPI, SLINK, and I2C drivers, and the Exynos SPI and S3C I2C drivers).
I left the Paz00 tegra_mmc_init(3) call the same as originally (w/o the width and GPIO params, of course). The device numbering is kind of arbitrary - if there are only 2 MMC devices present, I'd expect dev 0 to be eMMC and dev 1 to be SD (again, like my T20 reference board, Seaboard). I don't see that Paz uses mmc anywhere in its config files for a boot script - does it have to have mmc dev 3 be SD? or would dev 1 work? Note that the tegra20-common-post.h file that all T20 boards inherit uses dev 0 and dev 1.
U-boot scans all devices during boot, so no need to specify a specific one. I think what Stephen is suggesting is the right way. Forget all this dev ids and let the device tree control everything. The aliases can be used for enumbering if someone really needs it.
Let me look into it - wish I had a Paz00 board here to debug with. I'll try to simulate this on my Seaboard, maybe with all 4 MMC device addresses in the alias.
He, if you ever come to old Europe, there are still some shops selling them ;-)
Marc
Marc,
On Tue, Feb 5, 2013 at 1:54 PM, Marc Dietrich marvin24@gmx.de wrote:
Tom,
On Tuesday 05 February 2013 13:41:21 Tom Warren wrote:
Marc,
On Tue, Feb 5, 2013 at 1:06 PM, Marc Dietrich marvin24@gmx.de wrote:
On Tuesday 05 February 2013 08:31:03 Tom Warren wrote:
Marc,
On Tue, Feb 5, 2013 at 2:28 AM, Marc Dietrich marvin24@gmx.de wrote:
[...]
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void)
/* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);This looks wrong because the sd is on sdmmc0
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3);and the emmc on sdmmc3. The DTS is correct.
Not your fault as it seems to be wrong in the original code already. I guess it didn't made large difference but may in the future. I wonder how to test this though.
Marc
OK, so just the comments are wrong in paz00.c - I can fix that if I have to do a V2 patchset, or when I apply the patches to u-boot-tegra.
ah no, this is weird!
index 3 maps to sdmmc1 index 2 maps to sdmmc2 index 1 maps to sdmmc3 index 0 maps to sdmmc4so all is fine, nearly ...
As to testing, just stop at the command prompt and select each device (mmc dev 0, etc.) and run mmcinfo. You should be able to tell from the data displayed whether you are on an SD-card or eMMC chip. You can also eject the SD-card and you should get a warning about card presence due to the CD GPIO.
the sd card is not detected because:
TEGRA20 Board: Compal Paz00 DRAM: 512 MiB MMC: tegra_get_setup: dev_index = 0 tegra_get_setup: count of nodes is 2 tegra_get_setup: found controller at c8000600, width = 8, periph_id = 15 tegra_mmc_init: index 0, bus width 8 pwr_gpio -1 cd_gpio -1 tegra_mmc_init: bus width = 8 tegra_get_setup: dev_index = 3 tegra_get_setup: count of nodes is 2 tegra_get_setup: device index 3 exceeds node count (2)!
If I understand correctly, you are counting the aliases only, not the controllers..., so index 3 (the sdcard) is not initialized at all. Arrr, debugging stole all of my time, but I guess this needs fixing.
Yep, I am checking the aliases to get a node count (just like the Tegra SPI, SLINK, and I2C drivers, and the Exynos SPI and S3C I2C drivers).
I left the Paz00 tegra_mmc_init(3) call the same as originally (w/o the width and GPIO params, of course). The device numbering is kind of arbitrary - if there are only 2 MMC devices present, I'd expect dev 0 to be eMMC and dev 1 to be SD (again, like my T20 reference board, Seaboard). I don't see that Paz uses mmc anywhere in its config files for a boot script - does it have to have mmc dev 3 be SD? or would dev 1 work? Note that the tegra20-common-post.h file that all T20 boards inherit uses dev 0 and dev 1.
U-boot scans all devices during boot, so no need to specify a specific one. I think what Stephen is suggesting is the right way. Forget all this dev ids and let the device tree control everything. The aliases can be used for enumbering if someone really needs it.
Yeah, I just now realized the the tegra_mmc driver used this code to assign base addresses to dev IDs:
switch (dev_index) {
case 1:
host->base = TEGRA_SDMMC3_BASE;
host->mmc_id = PERIPH_ID_SDMMC3;
break;
case 2:
host->base = TEGRA_SDMMC2_BASE;
host->mmc_id = PERIPH_ID_SDMMC2;
break;
case 3:
host->base = TEGRA_SDMMC1_BASE;
host->mmc_id = PERIPH_ID_SDMMC1;
break;
case 0:
default:
host->base = TEGRA_SDMMC4_BASE;
host->mmc_id = PERIPH_ID_SDMMC4;
break;
}
which puts SDMMC4 at dev 0 (eMMC, I knew that) and the other 3 (usually SDIO) in reverse order, SDMMC1 = dev 3, etc. I didn't know that.
As you say, letting the DT control it all, with only a single 'mmc_init' call from the board file is the way to go. I'll redo and submit a V2 later in the week.
Thanks again!
Let me look into it - wish I had a Paz00 board here to debug with. I'll try to simulate this on my Seaboard, maybe with all 4 MMC device addresses in the alias.
He, if you ever come to old Europe, there are still some shops selling them ;-)
Unfortunately, no plans to travel to Europe in the near future. :(
Marc
On 02/04/2013 04:48 PM, Tom Warren wrote:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
- debug("board_mmc_init called\n");
debug("%s called\n", __func__);
/* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
- debug("board_mmc_init: init eMMC\n");
- /* init dev 0, eMMC chip, with 8-bit bus */
- tegra_mmc_init(0, 8, -1, -1);
- debug("%s: init eMMC\n", __func__);
- /* init dev 0, eMMC chip */
- tegra_mmc_init(0);
- debug("board_mmc_init: init SD slot\n");
- /* init dev 3, SD slot, with 4-bit bus */
- tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);
/* init dev 3, SD slot */
tegra_mmc_init(3);
return 0;
}
That doesn't look right. The board code still has knowledge of which SDHCI controllers are in use by the board. Instead, the board should just call tegra_mmc_init() with no parameters at all, and the MMC driver should scan the device tree for all present-and-enabled SDHCI nodes, and instantiate a U-Boot SDHCI device. Without this, the device tree isn't in control of the whole process, so there's little point doing the conversion; a new board couldn't be supported /just/ by creating a new device tree file.
diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
So CONFIG_OF_CONTROL must be enabled ...
static void tegra_get_setup(struct mmc_host *host, int dev_index) {
- debug("tegra_get_setup: dev_index = %d\n", dev_index);
- debug("%s: dev_index = %d\n", __func__, dev_index);
+#ifdef CONFIG_OF_CONTROL
... so there's no need for that ifdef
- int count, node = 0;
- int node_list[MAX_HOSTS];
- count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",
COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);- debug("%s: count of nodes is %d\n", __func__, count);
- if (count < dev_index) {
printf("%s: device index %d exceeds node count (%d)!\n",__func__, dev_index, count);return;- }
This requires that an alias exist in order for the SDHCI node to be found/processed. This isn't correct; the SDHCI nodes must be enumerated themselves, and then the aliases (if any are present) provide a naming hint for them, but even without aliases, the SDHCI nodes must be processed.
- /*
* NOTE: mmc->bus_width is determined by mmc.c dynamically.* Should we override it with this value?*/- host->width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 0);
- if (!host->width)
debug("%s: no sdmmc width found\n", __func__);
It should be possible to inform the MMC core of the width of the bus in terms of wires on the PCB. It should only probe the connected device up to that width. If that feature is missing, it can be added later though, outside the scope of this patch set.
You didn't Cc the MMC maintainer; they should be Cc'd since this code is in drivers/mmc/.
Stephen,
On Tue, Feb 5, 2013 at 1:03 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3); return 0;}
That doesn't look right. The board code still has knowledge of which SDHCI controllers are in use by the board. Instead, the board should just call tegra_mmc_init() with no parameters at all, and the MMC driver should scan the device tree for all present-and-enabled SDHCI nodes, and instantiate a U-Boot SDHCI device. Without this, the device tree isn't in control of the whole process, so there's little point doing the conversion; a new board couldn't be supported /just/ by creating a new device tree file.
diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
So CONFIG_OF_CONTROL must be enabled ...
static void tegra_get_setup(struct mmc_host *host, int dev_index) {
debug("tegra_get_setup: dev_index = %d\n", dev_index);
debug("%s: dev_index = %d\n", __func__, dev_index);+#ifdef CONFIG_OF_CONTROL
... so there's no need for that ifdef
I took Allen's recent SPI/SLINK driver(s) as an example, but as you point out, if CONFIG_OF_CONTROL isn't enabled, you get build errors anyway.
int count, node = 0;int node_list[MAX_HOSTS];count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);debug("%s: count of nodes is %d\n", __func__, count);if (count < dev_index) {printf("%s: device index %d exceeds node count (%d)!\n",__func__, dev_index, count);return;}This requires that an alias exist in order for the SDHCI node to be found/processed. This isn't correct; the SDHCI nodes must be enumerated themselves, and then the aliases (if any are present) provide a naming hint for them, but even without aliases, the SDHCI nodes must be processed.
Again, I used Allen's SLINK driver for as a template here. In fact, it looks like our I2C and SPI/SLINK drivers do this, as well as the Exynos SPI and S3C24x0 I2C driver all do this. Can you point out a U-Boot driver that does it the right way (preferably with more than 1 node, like MMC)? I can take a look at that code to use as an example of what you're proposing above.
/** NOTE: mmc->bus_width is determined by mmc.c dynamically.* Should we override it with this value?*/host->width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 0);if (!host->width)debug("%s: no sdmmc width found\n", __func__);It should be possible to inform the MMC core of the width of the bus in terms of wires on the PCB. It should only probe the connected device up to that width. If that feature is missing, it can be added later though, outside the scope of this patch set.
You didn't Cc the MMC maintainer; they should be Cc'd since this code is in drivers/mmc/.
Thanks, added Andy Fleming to CC.
Tom
On 02/05/2013 02:02 PM, Tom Warren wrote:
Stephen,
On Tue, Feb 5, 2013 at 1:03 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c
...
@@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd)
...
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);
...
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3);That doesn't look right. The board code still has knowledge of which SDHCI controllers are in use by the board. Instead, the board should just call tegra_mmc_init() with no parameters at all, and the MMC driver should scan the device tree for all present-and-enabled SDHCI nodes, and instantiate a U-Boot SDHCI device. Without this, the device tree isn't in control of the whole process, so there's little point doing the conversion; a new board couldn't be supported /just/ by creating a new device tree file.
...
static void tegra_get_setup(struct mmc_host *host, int dev_index)
...
int count, node = 0;int node_list[MAX_HOSTS];count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);debug("%s: count of nodes is %d\n", __func__, count);if (count < dev_index) {printf("%s: device index %d exceeds node count (%d)!\n",__func__, dev_index, count);return;}This requires that an alias exist in order for the SDHCI node to be found/processed. This isn't correct; the SDHCI nodes must be enumerated themselves, and then the aliases (if any are present) provide a naming hint for them, but even without aliases, the SDHCI nodes must be processed.
Again, I used Allen's SLINK driver for as a template here. In fact, it looks like our I2C and SPI/SLINK drivers do this, as well as the Exynos SPI and S3C24x0 I2C driver all do this. Can you point out a U-Boot driver that does it the right way (preferably with more than 1 node, like MMC)? I can take a look at that code to use as an example of what you're proposing above.
Tegra's I2C driver has just a single i2c_init_board() function, which calls fdtdec_find_aliases_for_id() to find all the DT nodes for a given compatible value (and associated aliases if there are any, I believe), then calls process_nodes() to loop over them all, and initialize/register them.
Hi,
On Tue, Feb 5, 2013 at 1:02 PM, Tom Warren twarren.nvidia@gmail.com wrote:
Stephen,
On Tue, Feb 5, 2013 at 1:03 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3); return 0;}
That doesn't look right. The board code still has knowledge of which SDHCI controllers are in use by the board. Instead, the board should just call tegra_mmc_init() with no parameters at all, and the MMC driver should scan the device tree for all present-and-enabled SDHCI nodes, and instantiate a U-Boot SDHCI device. Without this, the device tree isn't in control of the whole process, so there's little point doing the conversion; a new board couldn't be supported /just/ by creating a new device tree file.
diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
So CONFIG_OF_CONTROL must be enabled ...
static void tegra_get_setup(struct mmc_host *host, int dev_index) {
debug("tegra_get_setup: dev_index = %d\n", dev_index);
debug("%s: dev_index = %d\n", __func__, dev_index);+#ifdef CONFIG_OF_CONTROL
... so there's no need for that ifdef
I took Allen's recent SPI/SLINK driver(s) as an example, but as you point out, if CONFIG_OF_CONTROL isn't enabled, you get build errors anyway.
int count, node = 0;int node_list[MAX_HOSTS];count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);debug("%s: count of nodes is %d\n", __func__, count);if (count < dev_index) {printf("%s: device index %d exceeds node count (%d)!\n",__func__, dev_index, count);return;}This requires that an alias exist in order for the SDHCI node to be found/processed. This isn't correct; the SDHCI nodes must be enumerated themselves, and then the aliases (if any are present) provide a naming hint for them, but even without aliases, the SDHCI nodes must be processed.
Again, I used Allen's SLINK driver for as a template here. In fact, it looks like our I2C and SPI/SLINK drivers do this, as well as the Exynos SPI and S3C24x0 I2C driver all do this. Can you point out a U-Boot driver that does it the right way (preferably with more than 1 node, like MMC)? I can take a look at that code to use as an example of what you're proposing above.
You have it correct already. Stephen please take another look and let me know what problem you see with this approach. I'm very sorry that I am so late to this discussion.
/** NOTE: mmc->bus_width is determined by mmc.c dynamically.* Should we override it with this value?*/host->width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 0);if (!host->width)debug("%s: no sdmmc width found\n", __func__);It should be possible to inform the MMC core of the width of the bus in terms of wires on the PCB. It should only probe the connected device up to that width. If that feature is missing, it can be added later though, outside the scope of this patch set.
You didn't Cc the MMC maintainer; they should be Cc'd since this code is in drivers/mmc/.
Thanks, added Andy Fleming to CC.
Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon
Simon,
On Tue, Feb 12, 2013 at 11:07 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On Tue, Feb 5, 2013 at 1:02 PM, Tom Warren twarren.nvidia@gmail.com wrote:
Stephen,
On Tue, Feb 5, 2013 at 1:03 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3); return 0;}
That doesn't look right. The board code still has knowledge of which SDHCI controllers are in use by the board. Instead, the board should just call tegra_mmc_init() with no parameters at all, and the MMC driver should scan the device tree for all present-and-enabled SDHCI nodes, and instantiate a U-Boot SDHCI device. Without this, the device tree isn't in control of the whole process, so there's little point doing the conversion; a new board couldn't be supported /just/ by creating a new device tree file.
diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
So CONFIG_OF_CONTROL must be enabled ...
static void tegra_get_setup(struct mmc_host *host, int dev_index) {
debug("tegra_get_setup: dev_index = %d\n", dev_index);
debug("%s: dev_index = %d\n", __func__, dev_index);+#ifdef CONFIG_OF_CONTROL
... so there's no need for that ifdef
I took Allen's recent SPI/SLINK driver(s) as an example, but as you point out, if CONFIG_OF_CONTROL isn't enabled, you get build errors anyway.
int count, node = 0;int node_list[MAX_HOSTS];count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);debug("%s: count of nodes is %d\n", __func__, count);if (count < dev_index) {printf("%s: device index %d exceeds node count (%d)!\n",__func__, dev_index, count);return;}This requires that an alias exist in order for the SDHCI node to be found/processed. This isn't correct; the SDHCI nodes must be enumerated themselves, and then the aliases (if any are present) provide a naming hint for them, but even without aliases, the SDHCI nodes must be processed.
Again, I used Allen's SLINK driver for as a template here. In fact, it looks like our I2C and SPI/SLINK drivers do this, as well as the Exynos SPI and S3C24x0 I2C driver all do this. Can you point out a U-Boot driver that does it the right way (preferably with more than 1 node, like MMC)? I can take a look at that code to use as an example of what you're proposing above.
You have it correct already. Stephen please take another look and let me know what problem you see with this approach. I'm very sorry that I am so late to this discussion.
PTAL at the current patchset (v2) - I changed it to look for 'sdhci', which names the node and the aliases, and seem to work fine. I also have one function that parses all the nodes at once, so only one call to tegra_mmc_init() is needed from the board level.
/** NOTE: mmc->bus_width is determined by mmc.c dynamically.* Should we override it with this value?*/host->width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 0);if (!host->width)debug("%s: no sdmmc width found\n", __func__);It should be possible to inform the MMC core of the width of the bus in terms of wires on the PCB. It should only probe the connected device up to that width. If that feature is missing, it can be added later though, outside the scope of this patch set.
You didn't Cc the MMC maintainer; they should be Cc'd since this code is in drivers/mmc/.
Thanks, added Andy Fleming to CC.
Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon
Hi Tom,
On Tue, Feb 12, 2013 at 11:05 AM, Tom Warren twarren.nvidia@gmail.com wrote:
Simon,
On Tue, Feb 12, 2013 at 11:07 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On Tue, Feb 5, 2013 at 1:02 PM, Tom Warren twarren.nvidia@gmail.com wrote:
Stephen,
On Tue, Feb 5, 2013 at 1:03 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3); return 0;}
That doesn't look right. The board code still has knowledge of which SDHCI controllers are in use by the board. Instead, the board should just call tegra_mmc_init() with no parameters at all, and the MMC driver should scan the device tree for all present-and-enabled SDHCI nodes, and instantiate a U-Boot SDHCI device. Without this, the device tree isn't in control of the whole process, so there's little point doing the conversion; a new board couldn't be supported /just/ by creating a new device tree file.
diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
So CONFIG_OF_CONTROL must be enabled ...
static void tegra_get_setup(struct mmc_host *host, int dev_index) {
debug("tegra_get_setup: dev_index = %d\n", dev_index);
debug("%s: dev_index = %d\n", __func__, dev_index);+#ifdef CONFIG_OF_CONTROL
... so there's no need for that ifdef
I took Allen's recent SPI/SLINK driver(s) as an example, but as you point out, if CONFIG_OF_CONTROL isn't enabled, you get build errors anyway.
int count, node = 0;int node_list[MAX_HOSTS];count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);debug("%s: count of nodes is %d\n", __func__, count);if (count < dev_index) {printf("%s: device index %d exceeds node count (%d)!\n",__func__, dev_index, count);return;}This requires that an alias exist in order for the SDHCI node to be found/processed. This isn't correct; the SDHCI nodes must be enumerated themselves, and then the aliases (if any are present) provide a naming hint for them, but even without aliases, the SDHCI nodes must be processed.
Again, I used Allen's SLINK driver for as a template here. In fact, it looks like our I2C and SPI/SLINK drivers do this, as well as the Exynos SPI and S3C24x0 I2C driver all do this. Can you point out a U-Boot driver that does it the right way (preferably with more than 1 node, like MMC)? I can take a look at that code to use as an example of what you're proposing above.
You have it correct already. Stephen please take another look and let me know what problem you see with this approach. I'm very sorry that I am so late to this discussion.
PTAL at the current patchset (v2) - I changed it to look for 'sdhci', which names the node and the aliases, and seem to work fine. I also have one function that parses all the nodes at once, so only one call to tegra_mmc_init() is needed from the board level.
Great. Yes I think that was the version I looked at (above, in this thread).
Regards, Simon
/** NOTE: mmc->bus_width is determined by mmc.c dynamically.* Should we override it with this value?*/host->width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 0);if (!host->width)debug("%s: no sdmmc width found\n", __func__);It should be possible to inform the MMC core of the width of the bus in terms of wires on the PCB. It should only probe the connected device up to that width. If that feature is missing, it can be added later though, outside the scope of this patch set.
You didn't Cc the MMC maintainer; they should be Cc'd since this code is in drivers/mmc/.
Thanks, added Andy Fleming to CC.
Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon
On 02/12/2013 11:07 AM, Simon Glass wrote:
Hi,
On Tue, Feb 5, 2013 at 1:02 PM, Tom Warren twarren.nvidia@gmail.com wrote:
Stephen,
On Tue, Feb 5, 2013 at 1:03 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3); return 0;}
That doesn't look right. The board code still has knowledge of which SDHCI controllers are in use by the board. Instead, the board should just call tegra_mmc_init() with no parameters at all, and the MMC driver should scan the device tree for all present-and-enabled SDHCI nodes, and instantiate a U-Boot SDHCI device. Without this, the device tree isn't in control of the whole process, so there's little point doing the conversion; a new board couldn't be supported /just/ by creating a new device tree file.
diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
So CONFIG_OF_CONTROL must be enabled ...
static void tegra_get_setup(struct mmc_host *host, int dev_index) {
debug("tegra_get_setup: dev_index = %d\n", dev_index);
debug("%s: dev_index = %d\n", __func__, dev_index);+#ifdef CONFIG_OF_CONTROL
... so there's no need for that ifdef
I took Allen's recent SPI/SLINK driver(s) as an example, but as you point out, if CONFIG_OF_CONTROL isn't enabled, you get build errors anyway.
int count, node = 0;int node_list[MAX_HOSTS];count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);debug("%s: count of nodes is %d\n", __func__, count);if (count < dev_index) {printf("%s: device index %d exceeds node count (%d)!\n",__func__, dev_index, count);return;}This requires that an alias exist in order for the SDHCI node to be found/processed. This isn't correct; the SDHCI nodes must be enumerated themselves, and then the aliases (if any are present) provide a naming hint for them, but even without aliases, the SDHCI nodes must be processed.
Again, I used Allen's SLINK driver for as a template here. In fact, it looks like our I2C and SPI/SLINK drivers do this, as well as the Exynos SPI and S3C24x0 I2C driver all do this. Can you point out a U-Boot driver that does it the right way (preferably with more than 1 node, like MMC)? I can take a look at that code to use as an example of what you're proposing above.
You have it correct already. Stephen please take another look and let me know what problem you see with this approach. I'm very sorry that I am so late to this discussion.
Could you explain how this works then? The code I looked at (a) was driven by board files not DT enumerationk (b) errored out if no alias ID was present.
But given there's a V2, I'll go look at that and check again.
Hi Stephen,
On Tue, Feb 12, 2013 at 12:13 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/12/2013 11:07 AM, Simon Glass wrote:
Hi,
On Tue, Feb 5, 2013 at 1:02 PM, Tom Warren twarren.nvidia@gmail.com wrote:
Stephen,
On Tue, Feb 5, 2013 at 1:03 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3); return 0;}
That doesn't look right. The board code still has knowledge of which SDHCI controllers are in use by the board. Instead, the board should just call tegra_mmc_init() with no parameters at all, and the MMC driver should scan the device tree for all present-and-enabled SDHCI nodes, and instantiate a U-Boot SDHCI device. Without this, the device tree isn't in control of the whole process, so there's little point doing the conversion; a new board couldn't be supported /just/ by creating a new device tree file.
diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
So CONFIG_OF_CONTROL must be enabled ...
static void tegra_get_setup(struct mmc_host *host, int dev_index) {
debug("tegra_get_setup: dev_index = %d\n", dev_index);
debug("%s: dev_index = %d\n", __func__, dev_index);+#ifdef CONFIG_OF_CONTROL
... so there's no need for that ifdef
I took Allen's recent SPI/SLINK driver(s) as an example, but as you point out, if CONFIG_OF_CONTROL isn't enabled, you get build errors anyway.
int count, node = 0;int node_list[MAX_HOSTS];count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);debug("%s: count of nodes is %d\n", __func__, count);if (count < dev_index) {printf("%s: device index %d exceeds node count (%d)!\n",__func__, dev_index, count);return;}This requires that an alias exist in order for the SDHCI node to be found/processed. This isn't correct; the SDHCI nodes must be enumerated themselves, and then the aliases (if any are present) provide a naming hint for them, but even without aliases, the SDHCI nodes must be processed.
Again, I used Allen's SLINK driver for as a template here. In fact, it looks like our I2C and SPI/SLINK drivers do this, as well as the Exynos SPI and S3C24x0 I2C driver all do this. Can you point out a U-Boot driver that does it the right way (preferably with more than 1 node, like MMC)? I can take a look at that code to use as an example of what you're proposing above.
You have it correct already. Stephen please take another look and let me know what problem you see with this approach. I'm very sorry that I am so late to this discussion.
Could you explain how this works then? The code I looked at (a) was driven by board files not DT enumerationk (b) errored out if no alias ID was present.
But given there's a V2, I'll go look at that and check again.
I'm really talking about the function fdtdec_add_aliases_for_id(). It is designed to do exactly what you wanted, from memory, except that it only supports a single compatible ID. If there is no alias node, then it should still find all the compatible nodes. The alias node is only used to order them.
In terms of being driver by board files, it is best if the drivers do this, so that the enumeration is independent of any board file.
Regards, Simon
Hi Stephen,
On Tue, Feb 5, 2013 at 12:03 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/04/2013 04:48 PM, Tom Warren wrote:
tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc. Tested on Seaboard, fully functional.
diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c index 1447f47..5cee91a 100644 --- a/board/compal/paz00/paz00.c +++ b/board/compal/paz00/paz00.c @@ -55,18 +55,18 @@ static void pin_mux_mmc(void) /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) {
debug("board_mmc_init called\n");
debug("%s called\n", __func__); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc();
debug("board_mmc_init: init eMMC\n");/* init dev 0, eMMC chip, with 8-bit bus */tegra_mmc_init(0, 8, -1, -1);
debug("%s: init eMMC\n", __func__);/* init dev 0, eMMC chip */tegra_mmc_init(0);
debug("board_mmc_init: init SD slot\n");/* init dev 3, SD slot, with 4-bit bus */tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
debug("%s: init SD slot\n", __func__);/* init dev 3, SD slot */tegra_mmc_init(3); return 0;}
That doesn't look right. The board code still has knowledge of which SDHCI controllers are in use by the board. Instead, the board should just call tegra_mmc_init() with no parameters at all, and the MMC driver should scan the device tree for all present-and-enabled SDHCI nodes, and instantiate a U-Boot SDHCI device. Without this, the device tree isn't in control of the whole process, so there's little point doing the conversion; a new board couldn't be supported /just/ by creating a new device tree file.
Agreed.
diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
+#ifndef CONFIG_OF_CONTROL +#error "Please enable device tree support to use this driver" +#endif
So CONFIG_OF_CONTROL must be enabled ...
static void tegra_get_setup(struct mmc_host *host, int dev_index) {
debug("tegra_get_setup: dev_index = %d\n", dev_index);
debug("%s: dev_index = %d\n", __func__, dev_index);+#ifdef CONFIG_OF_CONTROL
... so there's no need for that ifdef
int count, node = 0;int node_list[MAX_HOSTS];count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);debug("%s: count of nodes is %d\n", __func__, count);if (count < dev_index) {printf("%s: device index %d exceeds node count (%d)!\n",__func__, dev_index, count);return;}This requires that an alias exist in order for the SDHCI node to be found/processed. This isn't correct; the SDHCI nodes must be enumerated themselves, and then the aliases (if any are present) provide a naming hint for them, but even without aliases, the SDHCI nodes must be processed.
I believe this is quite incorrect. Please take a look at the function, which deals with aliases if they are present, but works in any case. There is a test in fdtdec_test.c which handles the cases that we discussed at the time. Quite a bit of effort went into this function at the time.
So I believe this function should always be used when enumerating multiple devices. If there is a bug in the function, let's find out what it is and fix it, and add a new test.
/** NOTE: mmc->bus_width is determined by mmc.c dynamically.* Should we override it with this value?*/host->width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 0);if (!host->width)debug("%s: no sdmmc width found\n", __func__);It should be possible to inform the MMC core of the width of the bus in terms of wires on the PCB. It should only probe the connected device up to that width. If that feature is missing, it can be added later though, outside the scope of this patch set.
You didn't Cc the MMC maintainer; they should be Cc'd since this code is in drivers/mmc/. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon
Folks with Tegra20-based boards (Compulab, Avionics Design, etc.) - please check my DT node settings for address, GPIOs (CD, WP, and power, if utilized), and width. I tried to keep everything the same as the tegra_mmc_init() function args that were removed, plus integrating what was in the Linux kernel DTS files for your board(s). Please also do a quick functional test to ensure everything still works as before with the MMC devices on your board(s).
Stephen, please check the boards you are familiar with (Whistler, PAZ00, Trimslice, Harmony) that I may not have here in AZ.
Thanks,
Tom
On Mon, Feb 4, 2013 at 4:48 PM, Tom Warren twarren.nvidia@gmail.com wrote:
This patchset adds device-tree support to the Tegra MMC driver. All device config is done via properties in the DT files instead of hard-coded config options/function arguments.
I've tested this on my Seaboard and everything works fine, including card detect. For the other T20 boards, I've used the Linux kernel DTS files for the sdhci nodes where there wasn't one already, or expanded the info that was already there. Everything builds fine, but I haven't tested anything but Seaboard.
Tom Warren (2): Tegra: fdt: Add/enhance sdhci (mmc) nodes for all T20 DT files Tegra: MMC: Add DT support to MMC driver for all T20 boards
arch/arm/dts/tegra20.dtsi | 12 ++ arch/arm/include/asm/arch-tegra/mmc.h | 2 +- arch/arm/include/asm/arch-tegra/tegra_mmc.h | 12 +- board/avionic-design/common/tamonten.c | 4 +- board/avionic-design/dts/tegra20-medcom-wide.dts | 9 + board/avionic-design/dts/tegra20-plutux.dts | 9 + board/avionic-design/dts/tegra20-tec.dts | 9 + board/compal/dts/tegra20-paz00.dts | 22 +++- board/compal/paz00/paz00.c | 14 +- board/compulab/dts/tegra20-trimslice.dts | 17 ++ board/compulab/trimslice/trimslice.c | 10 +- board/nvidia/dts/tegra20-harmony.dts | 21 +++ board/nvidia/dts/tegra20-seaboard.dts | 19 ++- board/nvidia/dts/tegra20-ventana.dts | 23 +++ board/nvidia/dts/tegra20-whistler.dts | 16 ++ board/nvidia/harmony/harmony.c | 12 +- board/nvidia/seaboard/seaboard.c | 14 +- board/nvidia/whistler/whistler.c | 4 +- board/toradex/colibri_t20_iris/colibri_t20_iris.c | 2 +- board/toradex/dts/tegra20-colibri_t20_iris.dts | 8 + drivers/mmc/tegra_mmc.c | 186 ++++++++++++++------- include/fdtdec.h | 1 + lib/fdtdec.c | 1 + 23 files changed, 322 insertions(+), 105 deletions(-)
On Mon, Feb 04, 2013 at 04:48:53PM -0700, Tom Warren wrote:
This patchset adds device-tree support to the Tegra MMC driver. All device config is done via properties in the DT files instead of hard-coded config options/function arguments.
I've tested this on my Seaboard and everything works fine, including card detect. For the other T20 boards, I've used the Linux kernel DTS files for the sdhci nodes where there wasn't one already, or expanded the info that was already there. Everything builds fine, but I haven't tested anything but Seaboard.
Tom Warren (2): Tegra: fdt: Add/enhance sdhci (mmc) nodes for all T20 DT files Tegra: MMC: Add DT support to MMC driver for all T20 boards
I've tested on TEC only, but since Medcom-Wide and Plutux are also based on Tamonten they should be good as well, so:
Tested-by: Thierry Reding thierry.reding@avionic-design.de
Thierry,
On Tue, Feb 5, 2013 at 3:21 AM, Thierry Reding thierry.reding@avionic-design.de wrote:
On Mon, Feb 04, 2013 at 04:48:53PM -0700, Tom Warren wrote:
This patchset adds device-tree support to the Tegra MMC driver. All device config is done via properties in the DT files instead of hard-coded config options/function arguments.
I've tested this on my Seaboard and everything works fine, including card detect. For the other T20 boards, I've used the Linux kernel DTS files for the sdhci nodes where there wasn't one already, or expanded the info that was already there. Everything builds fine, but I haven't tested anything but Seaboard.
Tom Warren (2): Tegra: fdt: Add/enhance sdhci (mmc) nodes for all T20 DT files Tegra: MMC: Add DT support to MMC driver for all T20 boards
I've tested on TEC only, but since Medcom-Wide and Plutux are also based on Tamonten they should be good as well, so:
Tested-by: Thierry Reding thierry.reding@avionic-design.de
Great! Thanks
Tom
participants (6)
-
Marc Dietrich -
Scott Wood -
Simon Glass -
Stephen Warren -
Thierry Reding -
Tom Warren