| Message ID | 1360021735-13286-2-git-send-email-twarren@nvidia.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Warren |
| Headers | show |
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
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>; + }; };
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(-)