[U-Boot] [PATCH v2 1/2] Tegra: fdt: Add/enhance sdhci (mmc) nodes for all T20 DT files

Tom Warren
2013-02-12T17:36:14Z

Thierry,

On Tue, Feb 12, 2013 at 3:53 AM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Tue, Feb 12, 2013 at 11:41:09AM +0100, Thierry Reding wrote:
>> On Tue, Feb 12, 2013 at 07:51:55AM +0100, Thierry Reding wrote:
>> > On Mon, Feb 11, 2013 at 12:21:59PM -0700, Tom Warren wrote:
>> > > Thierry/Lucas,
>> > >
>> > > On Mon, Feb 11, 2013 at 12:11 PM, Thierry Reding
>> > > <thierry.reding@avionic-design.de> wrote:
>> > > > On Mon, Feb 11, 2013 at 10:56:33AM -0700, Tom Warren wrote:
>> > > >> Lucas,
>> > > >>
>> > > >> On Mon, Feb 11, 2013 at 10:28 AM, Lucas Stach <dev@lynxeye.de> wrote:
>> > > >> > Hi Tom,
>> > > >> >
>> > > >> > Am Montag, den 11.02.2013, 10:17 -0700 schrieb Tom Warren:
>> > > >> >> Linux dts files were used for those boards that didn't already
>> > > >> >> have sdhci info populated. Tamonten has their own dtsi file with
>> > > >> >> common sdhci nodes (sourced from Linux).
>> > > >> >>
>> > > >> >> Signed-off-by: Tom Warren <twarren@nvidia.com>
>> > > >> >> ---
>> > > >> >> v2:
>> > > >> >> - cleanup comments in dts files/match w/kernel files
>> > > >> >> - add sdhci aliases in all dts files
>> > > >> >> - use tegra20-tamonten.dtsi from the kernel for AD boards
>> > > >> >>
>> > > >> >> arch/arm/dts/tegra20-tamonten.dtsi | 489 ++++++++++++++++++++++
>> > > >> >
>> > > >> > I'm not sure if pushing the whole file in this patch is the right thing
>> > > >> > to do.
>> > > >>
>> > > >> I didn't want to edit it since we seem to be moving towards using the
>> > > >> Linux DTS files in toto in U-Boot (as per Stephen & Simon). Does it do
>> > > >> any harm to have the whole thing here? Saves some work later. Thierry
>> > > >> - what do you think?
>> > > >
>> > > > Given that it isn't used anywhere I don't think we really need it right
>> > > > now. We can always add it later when we can make better use of it.
>> > >
>> > > It actually is used (for SDMMC/sdhci) now, Thierry. That's why it's in
>> > > this patchset.
>> >
>> > Right, I hadn't looked at that patch yet.
>> >
>> > > I had originally put the sdhci node for Avionic Design
>> > > boards in their respective .dts files, but Stephen pointed out that
>> > > the kernel had a tegra20-tamonten.dtsi file with common info, which
>> > > included the sdhci node, and asked that I use it, instead, so we echo
>> > > the kernel layout. So I pulled that file into my MMC DT patchset, and
>> > > used it in all AD board builds (medcom/tec/plutux) - it's pulled in
>> > > via an override of CONFIG_ARCH_DEVICE_TREE in the config files.
>> > >
>> > > So the options seem to be:
>> > >
>> > > a) Don't use the tamonton dtsi file, and put the sdhci nodes in the AD
>> > > dts files, just like all other boards (this was my V1 approach).
>> > > Vetoed by Stephen.
>> > > b) Use tegra20-tamonten.dtsi as is, identical to the kernel file. If
>> > > necessary, I can move it's inclusion to a separate patch, independent
>> > > of the MMC DT patchset, as suggested by Lucas.
>> > > c) Use tegra20-tamonten.dtsi, but just with the sdhci node (is this
>> > > what you're suggesting, Thierry?). I'd still pull it in via a
>> > > CONFIG_ARCH_DEVICE_TREE #define in the AD config files.
>> > >
>> > > Let me know ASAP - I'd like to get V3 upstreamed soon so I can move on
>> > > to work on the T30/T114 MMC patches.
>> >
>> > I think option b) sounds fine given that we want to move to the same DTS
>> > as the kernel eventually anyway. So for the Tamonten (and AD board)
>> > pieces, consider this:
>> >
>> > Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
>> >
>> > I can't give you a Tested-by because I have a bunch of other things to
>> > take care of and I probably won't get to testing this for a few days.
>>
>> So it turned out that I need to touch U-Boot anyway, so I decided to
>> give this a spin. I noticed that overriding CONFIG_ARCH_DEVICE_TREE from
>> the board configuration file doesn't work currently. What happens is
>> that the autoconf.mk (which is derived from the board configuration) is
>> included before the CPU config.mk which sets CONFIG_ARCH_DEVICE_TREE to
>> tegra20 (or tegra30, tegra114). I came up with the attached patch to set
>> the variable if not set previously (by the board configuration file).
>>
>> Feel free to squash that in your patch series if you deem it a proper
>> solution. I can also provide a proper separate patch if you prefer.
>>
>> Thierry
>
>> diff --git a/arch/arm/cpu/armv7/tegra114/config.mk b/arch/arm/cpu/armv7/tegra114/config.mk
>> index cb1a19d..e7c22c0 100644
>> --- a/arch/arm/cpu/armv7/tegra114/config.mk
>> +++ b/arch/arm/cpu/armv7/tegra114/config.mk
>> @@ -16,4 +16,4 @@
>> # You should have received a copy of the GNU General Public License
>> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>> #
>> -CONFIG_ARCH_DEVICE_TREE := tegra114
>> +CONFIG_ARCH_DEVICE_TREE ?= tegra114
>> diff --git a/arch/arm/cpu/armv7/tegra20/config.mk b/arch/arm/cpu/armv7/tegra20/config.mk
>> index 6432e75..9042664 100644
>> --- a/arch/arm/cpu/armv7/tegra20/config.mk
>> +++ b/arch/arm/cpu/armv7/tegra20/config.mk
>> @@ -23,4 +23,4 @@
>> # Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> # MA 02111-1307 USA
>> #
>> -CONFIG_ARCH_DEVICE_TREE := tegra20
>> +CONFIG_ARCH_DEVICE_TREE ?= tegra20
>> diff --git a/arch/arm/cpu/armv7/tegra30/config.mk b/arch/arm/cpu/armv7/tegra30/config.mk
>> index 719ca81..0035bc5 100644
>> --- a/arch/arm/cpu/armv7/tegra30/config.mk
>> +++ b/arch/arm/cpu/armv7/tegra30/config.mk
>> @@ -16,4 +16,4 @@
>> # You should have received a copy of the GNU General Public License
>> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>> #
>> -CONFIG_ARCH_DEVICE_TREE := tegra30
>> +CONFIG_ARCH_DEVICE_TREE ?= tegra30
>
> I forgot: with that patch applied:
>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
>
> Thierry
Thanks for the patch - I never would have seen that, since it all
built OK w/MAKEALL.

Tom