| Message ID | 1337953588-20696-1-git-send-email-thierry.reding@avionic-design.de |
|---|---|
| State | Accepted |
| Commit | ed900c55c35a653a0d2ce5489cdeea9a4dd18b5b |
| Headers | show |
On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: > This commit uses the common Tegra board implementation instead of > duplicating a lot of the code. In addition, the Plutux and Medcom > specific board files can be removed as the MMC/SD setup is common > among all Tamonten-based boards. > > ... > diff --git a/board/avionic-design/medcom/Makefile b/board/avionic-design/medcom/Makefile > index b0c318c..d96d043 100644 > --- a/board/avionic-design/medcom/Makefile > +++ b/board/avionic-design/medcom/Makefile > @@ -26,12 +26,12 @@ > include $(TOPDIR)/config.mk > > ifneq ($(OBJTREE),$(SRCTREE)) > -$(shell mkdir -p $(obj)../common) > +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > endif > > LIB = $(obj)lib$(BOARD).o > This breaks with my patchset to split out the arm7 code to an SPL because even though SRCDIR and OBJDIR are the same, in the SPL build $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). How about the following instead which seems more to the point: ifeq ($(wildcard $(obj)../common),) $(shell mkdir -p $(obj)../common) endif ifeq ($(wildcard $(obj)../../nvidia/common),) $(shell mkdir -p $(obj)../../nvidia/common) endif -Allen
On 06/08/2012 02:01 PM, Allen Martin wrote: > On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: >> This commit uses the common Tegra board implementation instead of >> duplicating a lot of the code. In addition, the Plutux and Medcom >> specific board files can be removed as the MMC/SD setup is common >> among all Tamonten-based boards. >> >> ... >> diff --git a/board/avionic-design/medcom/Makefile b/board/avionic-design/medcom/Makefile >> index b0c318c..d96d043 100644 >> --- a/board/avionic-design/medcom/Makefile >> +++ b/board/avionic-design/medcom/Makefile >> @@ -26,12 +26,12 @@ >> include $(TOPDIR)/config.mk >> >> ifneq ($(OBJTREE),$(SRCTREE)) >> -$(shell mkdir -p $(obj)../common) >> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) >> endif >> >> LIB = $(obj)lib$(BOARD).o >> > > This breaks with my patchset to split out the arm7 code to an SPL > because even though SRCDIR and OBJDIR are the same, in the SPL build > $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). > > How about the following instead which seems more to the point: > > ifeq ($(wildcard $(obj)../common),) > $(shell mkdir -p $(obj)../common) > endif > ifeq ($(wildcard $(obj)../../nvidia/common),) > $(shell mkdir -p $(obj)../../nvidia/common) > endif Maybe I'm just not reading it right, but isn't that just running the exact same mkdir commands, just splitting it into two commands, and making them optional based on the $(wildcard)? I'm still not clear what the problem is. Anyway, if this is an issue, then compal/paz00 and compulabl/trimslice will need to be fixed for the SPL changes in the same way.
On Fri, Jun 08, 2012 at 02:07:25PM -0700, Stephen Warren wrote: > On 06/08/2012 02:01 PM, Allen Martin wrote: > > On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: > >> This commit uses the common Tegra board implementation instead of > >> duplicating a lot of the code. In addition, the Plutux and Medcom > >> specific board files can be removed as the MMC/SD setup is common > >> among all Tamonten-based boards. > >> > >> ... > >> diff --git a/board/avionic-design/medcom/Makefile b/board/avionic-design/medcom/Makefile > >> index b0c318c..d96d043 100644 > >> --- a/board/avionic-design/medcom/Makefile > >> +++ b/board/avionic-design/medcom/Makefile > >> @@ -26,12 +26,12 @@ > >> include $(TOPDIR)/config.mk > >> > >> ifneq ($(OBJTREE),$(SRCTREE)) > >> -$(shell mkdir -p $(obj)../common) > >> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > >> endif > >> > >> LIB = $(obj)lib$(BOARD).o > >> > > > > This breaks with my patchset to split out the arm7 code to an SPL > > because even though SRCDIR and OBJDIR are the same, in the SPL build > > $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). > > > > How about the following instead which seems more to the point: > > > > ifeq ($(wildcard $(obj)../common),) > > $(shell mkdir -p $(obj)../common) > > endif > > ifeq ($(wildcard $(obj)../../nvidia/common),) > > $(shell mkdir -p $(obj)../../nvidia/common) > > endif > > Maybe I'm just not reading it right, but isn't that just running the > exact same mkdir commands, just splitting it into two commands, and > making them optional based on the $(wildcard)? I'm still not clear what > the problem is. > > Anyway, if this is an issue, then compal/paz00 and compulabl/trimslice > will need to be fixed for the SPL changes in the same way. The problem is this: > >> ifneq ($(OBJTREE),$(SRCTREE)) is true, but these directories: > >> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) don't exist because $(obj) is not under $(OBJTREE) in the SPL build, it's under $(SPLTREE) -Allen
On 06/08/2012 03:27 PM, Allen Martin wrote: > On Fri, Jun 08, 2012 at 02:07:25PM -0700, Stephen Warren wrote: >> On 06/08/2012 02:01 PM, Allen Martin wrote: >>> On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: >>>> This commit uses the common Tegra board implementation instead of >>>> duplicating a lot of the code. In addition, the Plutux and Medcom >>>> specific board files can be removed as the MMC/SD setup is common >>>> among all Tamonten-based boards. >>>> >>>> ... >>>> diff --git a/board/avionic-design/medcom/Makefile b/board/avionic-design/medcom/Makefile >>>> index b0c318c..d96d043 100644 >>>> --- a/board/avionic-design/medcom/Makefile >>>> +++ b/board/avionic-design/medcom/Makefile >>>> @@ -26,12 +26,12 @@ >>>> include $(TOPDIR)/config.mk >>>> >>>> ifneq ($(OBJTREE),$(SRCTREE)) >>>> -$(shell mkdir -p $(obj)../common) >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) >>>> endif >>>> >>>> LIB = $(obj)lib$(BOARD).o >>>> >>> >>> This breaks with my patchset to split out the arm7 code to an SPL >>> because even though SRCDIR and OBJDIR are the same, in the SPL build >>> $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). >>> >>> How about the following instead which seems more to the point: >>> >>> ifeq ($(wildcard $(obj)../common),) >>> $(shell mkdir -p $(obj)../common) >>> endif >>> ifeq ($(wildcard $(obj)../../nvidia/common),) >>> $(shell mkdir -p $(obj)../../nvidia/common) >>> endif >> >> Maybe I'm just not reading it right, but isn't that just running the >> exact same mkdir commands, just splitting it into two commands, and >> making them optional based on the $(wildcard)? I'm still not clear what >> the problem is. >> >> Anyway, if this is an issue, then compal/paz00 and compulabl/trimslice >> will need to be fixed for the SPL changes in the same way. > > The problem is this: > >>>> ifneq ($(OBJTREE),$(SRCTREE)) > > is true, but these directories: > >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > > don't exist because $(obj) is not under $(OBJTREE) in the SPL build, > it's under $(SPLTREE) OK, so I think the issue is just that $(obj) isn't the correct place to mkdir these, not that we shouldn't attempt to mkdir any directory. In other words, don't you need to switch between mkdir in the object tree or the SPL tree rather than switching between doing a mkdir and not? It's quite possible this only affects builds where OBJDIR!=SRCDIR, so a regular build might not show up the problem I /think/ exists in the code quoted a couple of emails above.
On Fri, Jun 08, 2012 at 10:28:57PM -0700, Stephen Warren wrote: > On 06/08/2012 03:27 PM, Allen Martin wrote: > > On Fri, Jun 08, 2012 at 02:07:25PM -0700, Stephen Warren wrote: > >> On 06/08/2012 02:01 PM, Allen Martin wrote: > >>> On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: > >>>> This commit uses the common Tegra board implementation instead of > >>>> duplicating a lot of the code. In addition, the Plutux and Medcom > >>>> specific board files can be removed as the MMC/SD setup is common > >>>> among all Tamonten-based boards. > >>>> > >>>> ... > >>>> diff --git a/board/avionic-design/medcom/Makefile b/board/avionic-design/medcom/Makefile > >>>> index b0c318c..d96d043 100644 > >>>> --- a/board/avionic-design/medcom/Makefile > >>>> +++ b/board/avionic-design/medcom/Makefile > >>>> @@ -26,12 +26,12 @@ > >>>> include $(TOPDIR)/config.mk > >>>> > >>>> ifneq ($(OBJTREE),$(SRCTREE)) > >>>> -$(shell mkdir -p $(obj)../common) > >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > >>>> endif > >>>> > >>>> LIB = $(obj)lib$(BOARD).o > >>>> > >>> > >>> This breaks with my patchset to split out the arm7 code to an SPL > >>> because even though SRCDIR and OBJDIR are the same, in the SPL build > >>> $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). > >>> > >>> How about the following instead which seems more to the point: > >>> > >>> ifeq ($(wildcard $(obj)../common),) > >>> $(shell mkdir -p $(obj)../common) > >>> endif > >>> ifeq ($(wildcard $(obj)../../nvidia/common),) > >>> $(shell mkdir -p $(obj)../../nvidia/common) > >>> endif > >> > >> Maybe I'm just not reading it right, but isn't that just running the > >> exact same mkdir commands, just splitting it into two commands, and > >> making them optional based on the $(wildcard)? I'm still not clear what > >> the problem is. > >> > >> Anyway, if this is an issue, then compal/paz00 and compulabl/trimslice > >> will need to be fixed for the SPL changes in the same way. > > > > The problem is this: > > > >>>> ifneq ($(OBJTREE),$(SRCTREE)) > > > > is true, but these directories: > > > >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > > > > don't exist because $(obj) is not under $(OBJTREE) in the SPL build, > > it's under $(SPLTREE) > > OK, so I think the issue is just that $(obj) isn't the correct place to > mkdir these, not that we shouldn't attempt to mkdir any directory. In > other words, don't you need to switch between mkdir in the object tree > or the SPL tree rather than switching between doing a mkdir and not? > It's quite possible this only affects builds where OBJDIR!=SRCDIR, so a > regular build might not show up the problem I /think/ exists in the code > quoted a couple of emails above. $(obj) is always the place where the object files are going to be placed, that may be in $(OBJTREE) or in $(SRCTREE) or in $(SPLTREE) depending on what you're building and what you have set. I think the $(wildcard) suggestion I made is the most straightforward, it basically says "we're about to spit out an object file to here, so make sure the directory exists first". In the "normal" case where your object files are in your source tree the directory will already exist so it won't do anything. -Allen
Allen Martin <amartin@nvidia.com> wrote: >On Fri, Jun 08, 2012 at 10:28:57PM -0700, Stephen Warren wrote: >> On 06/08/2012 03:27 PM, Allen Martin wrote: >> > On Fri, Jun 08, 2012 at 02:07:25PM -0700, Stephen Warren wrote: >> >> On 06/08/2012 02:01 PM, Allen Martin wrote: >> >>> On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: >> >>>> This commit uses the common Tegra board implementation instead >of >> >>>> duplicating a lot of the code. In addition, the Plutux and >Medcom >> >>>> specific board files can be removed as the MMC/SD setup is >common >> >>>> among all Tamonten-based boards. >> >>>> >> >>>> ... >> >>>> diff --git a/board/avionic-design/medcom/Makefile >b/board/avionic-design/medcom/Makefile >> >>>> index b0c318c..d96d043 100644 >> >>>> --- a/board/avionic-design/medcom/Makefile >> >>>> +++ b/board/avionic-design/medcom/Makefile >> >>>> @@ -26,12 +26,12 @@ >> >>>> include $(TOPDIR)/config.mk >> >>>> >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) >> >>>> -$(shell mkdir -p $(obj)../common) >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) >> >>>> endif >> >>>> >> >>>> LIB = $(obj)lib$(BOARD).o >> >>>> >> >>> >> >>> This breaks with my patchset to split out the arm7 code to an SPL >> >>> because even though SRCDIR and OBJDIR are the same, in the SPL >build >> >>> $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). >> >>> >> >>> How about the following instead which seems more to the point: >> >>> >> >>> ifeq ($(wildcard $(obj)../common),) >> >>> $(shell mkdir -p $(obj)../common) >> >>> endif >> >>> ifeq ($(wildcard $(obj)../../nvidia/common),) >> >>> $(shell mkdir -p $(obj)../../nvidia/common) >> >>> endif >> >> >> >> Maybe I'm just not reading it right, but isn't that just running >the >> >> exact same mkdir commands, just splitting it into two commands, >and >> >> making them optional based on the $(wildcard)? I'm still not clear >what >> >> the problem is. >> >> >> >> Anyway, if this is an issue, then compal/paz00 and >compulabl/trimslice >> >> will need to be fixed for the SPL changes in the same way. >> > >> > The problem is this: >> > >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) >> > >> > is true, but these directories: >> > >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) >> > >> > don't exist because $(obj) is not under $(OBJTREE) in the SPL >build, >> > it's under $(SPLTREE) >> >> OK, so I think the issue is just that $(obj) isn't the correct place >to >> mkdir these, not that we shouldn't attempt to mkdir any directory. In >> other words, don't you need to switch between mkdir in the object >tree >> or the SPL tree rather than switching between doing a mkdir and not? >> It's quite possible this only affects builds where OBJDIR!=SRCDIR, so >a >> regular build might not show up the problem I /think/ exists in the >code >> quoted a couple of emails above. > >$(obj) is always the place where the object files are going to be >placed, that may be in $(OBJTREE) or in $(SRCTREE) or in $(SPLTREE) >depending on what you're building and what you have set. > >I think the $(wildcard) suggestion I made is the most straightforward, >it basically says "we're about to spit out an object file to here, so >make sure the directory exists first". In the "normal" case where >your object files are in your source tree the directory will already >exist so it won't do anything. Ah right, I understand now. But why not make it super-simple and just run the mkdir unconditionally; it won't cause any harm for an in-tree object location, but removes the need for anyone to think about the conditional logic. (I hope this is legible due to email client!)
* Stephen Warren wrote: > Allen Martin <amartin@nvidia.com> wrote: > >On Fri, Jun 08, 2012 at 10:28:57PM -0700, Stephen Warren wrote: > >> On 06/08/2012 03:27 PM, Allen Martin wrote: > >> > On Fri, Jun 08, 2012 at 02:07:25PM -0700, Stephen Warren wrote: > >> >> On 06/08/2012 02:01 PM, Allen Martin wrote: > >> >>> On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: > >> >>>> This commit uses the common Tegra board implementation instead > >of > >> >>>> duplicating a lot of the code. In addition, the Plutux and > >Medcom > >> >>>> specific board files can be removed as the MMC/SD setup is > >common > >> >>>> among all Tamonten-based boards. > >> >>>> > >> >>>> ... > >> >>>> diff --git a/board/avionic-design/medcom/Makefile > >b/board/avionic-design/medcom/Makefile > >> >>>> index b0c318c..d96d043 100644 > >> >>>> --- a/board/avionic-design/medcom/Makefile > >> >>>> +++ b/board/avionic-design/medcom/Makefile > >> >>>> @@ -26,12 +26,12 @@ > >> >>>> include $(TOPDIR)/config.mk > >> >>>> > >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) > >> >>>> -$(shell mkdir -p $(obj)../common) > >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > >> >>>> endif > >> >>>> > >> >>>> LIB = $(obj)lib$(BOARD).o > >> >>>> > >> >>> > >> >>> This breaks with my patchset to split out the arm7 code to an SPL > >> >>> because even though SRCDIR and OBJDIR are the same, in the SPL > >build > >> >>> $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). > >> >>> > >> >>> How about the following instead which seems more to the point: > >> >>> > >> >>> ifeq ($(wildcard $(obj)../common),) > >> >>> $(shell mkdir -p $(obj)../common) > >> >>> endif > >> >>> ifeq ($(wildcard $(obj)../../nvidia/common),) > >> >>> $(shell mkdir -p $(obj)../../nvidia/common) > >> >>> endif > >> >> > >> >> Maybe I'm just not reading it right, but isn't that just running > >the > >> >> exact same mkdir commands, just splitting it into two commands, > >and > >> >> making them optional based on the $(wildcard)? I'm still not clear > >what > >> >> the problem is. > >> >> > >> >> Anyway, if this is an issue, then compal/paz00 and > >compulabl/trimslice > >> >> will need to be fixed for the SPL changes in the same way. > >> > > >> > The problem is this: > >> > > >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) > >> > > >> > is true, but these directories: > >> > > >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > >> > > >> > don't exist because $(obj) is not under $(OBJTREE) in the SPL > >build, > >> > it's under $(SPLTREE) > >> > >> OK, so I think the issue is just that $(obj) isn't the correct place > >to > >> mkdir these, not that we shouldn't attempt to mkdir any directory. In > >> other words, don't you need to switch between mkdir in the object > >tree > >> or the SPL tree rather than switching between doing a mkdir and not? > >> It's quite possible this only affects builds where OBJDIR!=SRCDIR, so > >a > >> regular build might not show up the problem I /think/ exists in the > >code > >> quoted a couple of emails above. > > > >$(obj) is always the place where the object files are going to be > >placed, that may be in $(OBJTREE) or in $(SRCTREE) or in $(SPLTREE) > >depending on what you're building and what you have set. > > > >I think the $(wildcard) suggestion I made is the most straightforward, > >it basically says "we're about to spit out an object file to here, so > >make sure the directory exists first". In the "normal" case where > >your object files are in your source tree the directory will already > >exist so it won't do anything. > > Ah right, I understand now. > > But why not make it super-simple and just run the mkdir unconditionally; > it won't cause any harm for an in-tree object location, but removes the > need for anyone to think about the conditional logic. Shouldn't this really be handled by the commands that create files in those directories? They could check that the directories exist, and create them if necessary, before executing their actual commands. Thierry
On Mon, Jun 11, 2012 at 02:29:59AM -0700, Thierry Reding wrote: > * PGP Signed by an unknown key > > * Stephen Warren wrote: > > Allen Martin <amartin@nvidia.com> wrote: > > >On Fri, Jun 08, 2012 at 10:28:57PM -0700, Stephen Warren wrote: > > >> On 06/08/2012 03:27 PM, Allen Martin wrote: > > >> > On Fri, Jun 08, 2012 at 02:07:25PM -0700, Stephen Warren wrote: > > >> >> On 06/08/2012 02:01 PM, Allen Martin wrote: > > >> >>> On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: > > >> >>>> This commit uses the common Tegra board implementation instead > > >of > > >> >>>> duplicating a lot of the code. In addition, the Plutux and > > >Medcom > > >> >>>> specific board files can be removed as the MMC/SD setup is > > >common > > >> >>>> among all Tamonten-based boards. > > >> >>>> > > >> >>>> ... > > >> >>>> diff --git a/board/avionic-design/medcom/Makefile > > >b/board/avionic-design/medcom/Makefile > > >> >>>> index b0c318c..d96d043 100644 > > >> >>>> --- a/board/avionic-design/medcom/Makefile > > >> >>>> +++ b/board/avionic-design/medcom/Makefile > > >> >>>> @@ -26,12 +26,12 @@ > > >> >>>> include $(TOPDIR)/config.mk > > >> >>>> > > >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) > > >> >>>> -$(shell mkdir -p $(obj)../common) > > >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > > >> >>>> endif > > >> >>>> > > >> >>>> LIB = $(obj)lib$(BOARD).o > > >> >>>> > > >> >>> > > >> >>> This breaks with my patchset to split out the arm7 code to an SPL > > >> >>> because even though SRCDIR and OBJDIR are the same, in the SPL > > >build > > >> >>> $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). > > >> >>> > > >> >>> How about the following instead which seems more to the point: > > >> >>> > > >> >>> ifeq ($(wildcard $(obj)../common),) > > >> >>> $(shell mkdir -p $(obj)../common) > > >> >>> endif > > >> >>> ifeq ($(wildcard $(obj)../../nvidia/common),) > > >> >>> $(shell mkdir -p $(obj)../../nvidia/common) > > >> >>> endif > > >> >> > > >> >> Maybe I'm just not reading it right, but isn't that just running > > >the > > >> >> exact same mkdir commands, just splitting it into two commands, > > >and > > >> >> making them optional based on the $(wildcard)? I'm still not clear > > >what > > >> >> the problem is. > > >> >> > > >> >> Anyway, if this is an issue, then compal/paz00 and > > >compulabl/trimslice > > >> >> will need to be fixed for the SPL changes in the same way. > > >> > > > >> > The problem is this: > > >> > > > >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) > > >> > > > >> > is true, but these directories: > > >> > > > >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > > >> > > > >> > don't exist because $(obj) is not under $(OBJTREE) in the SPL > > >build, > > >> > it's under $(SPLTREE) > > >> > > >> OK, so I think the issue is just that $(obj) isn't the correct place > > >to > > >> mkdir these, not that we shouldn't attempt to mkdir any directory. In > > >> other words, don't you need to switch between mkdir in the object > > >tree > > >> or the SPL tree rather than switching between doing a mkdir and not? > > >> It's quite possible this only affects builds where OBJDIR!=SRCDIR, so > > >a > > >> regular build might not show up the problem I /think/ exists in the > > >code > > >> quoted a couple of emails above. > > > > > >$(obj) is always the place where the object files are going to be > > >placed, that may be in $(OBJTREE) or in $(SRCTREE) or in $(SPLTREE) > > >depending on what you're building and what you have set. > > > > > >I think the $(wildcard) suggestion I made is the most straightforward, > > >it basically says "we're about to spit out an object file to here, so > > >make sure the directory exists first". In the "normal" case where > > >your object files are in your source tree the directory will already > > >exist so it won't do anything. > > > > Ah right, I understand now. > > > > But why not make it super-simple and just run the mkdir unconditionally; > > it won't cause any harm for an in-tree object location, but removes the > > need for anyone to think about the conditional logic. > > Shouldn't this really be handled by the commands that create files in those > directories? They could check that the directories exist, and create them if > necessary, before executing their actual commands. The normal rules make sure that $(obj) exists, it's only because these boards generate object files in $(obj)../common and $(obj)../../nvidia/common that there's a problem and we need to do something special. Stephen's idea of just the unconditional mkdir makes the most sense, we don't really save anything by skipping it if the directory already exists. -Allen
On Mon, Jun 11, 2012 at 02:29:59AM -0700, Thierry Reding wrote: > * PGP Signed by an unknown key > > * Stephen Warren wrote: > > Allen Martin <amartin@nvidia.com> wrote: > > >On Fri, Jun 08, 2012 at 10:28:57PM -0700, Stephen Warren wrote: > > >> On 06/08/2012 03:27 PM, Allen Martin wrote: > > >> > On Fri, Jun 08, 2012 at 02:07:25PM -0700, Stephen Warren wrote: > > >> >> On 06/08/2012 02:01 PM, Allen Martin wrote: > > >> >>> On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: > > >> >>>> This commit uses the common Tegra board implementation instead > > >of > > >> >>>> duplicating a lot of the code. In addition, the Plutux and > > >Medcom > > >> >>>> specific board files can be removed as the MMC/SD setup is > > >common > > >> >>>> among all Tamonten-based boards. > > >> >>>> > > >> >>>> ... > > >> >>>> diff --git a/board/avionic-design/medcom/Makefile > > >b/board/avionic-design/medcom/Makefile > > >> >>>> index b0c318c..d96d043 100644 > > >> >>>> --- a/board/avionic-design/medcom/Makefile > > >> >>>> +++ b/board/avionic-design/medcom/Makefile > > >> >>>> @@ -26,12 +26,12 @@ > > >> >>>> include $(TOPDIR)/config.mk > > >> >>>> > > >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) > > >> >>>> -$(shell mkdir -p $(obj)../common) > > >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > > >> >>>> endif > > >> >>>> > > >> >>>> LIB = $(obj)lib$(BOARD).o > > >> >>>> > > >> >>> > > >> >>> This breaks with my patchset to split out the arm7 code to an SPL > > >> >>> because even though SRCDIR and OBJDIR are the same, in the SPL > > >build > > >> >>> $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). > > >> >>> > > >> >>> How about the following instead which seems more to the point: > > >> >>> > > >> >>> ifeq ($(wildcard $(obj)../common),) > > >> >>> $(shell mkdir -p $(obj)../common) > > >> >>> endif > > >> >>> ifeq ($(wildcard $(obj)../../nvidia/common),) > > >> >>> $(shell mkdir -p $(obj)../../nvidia/common) > > >> >>> endif > > >> >> > > >> >> Maybe I'm just not reading it right, but isn't that just running > > >the > > >> >> exact same mkdir commands, just splitting it into two commands, > > >and > > >> >> making them optional based on the $(wildcard)? I'm still not clear > > >what > > >> >> the problem is. > > >> >> > > >> >> Anyway, if this is an issue, then compal/paz00 and > > >compulabl/trimslice > > >> >> will need to be fixed for the SPL changes in the same way. > > >> > > > >> > The problem is this: > > >> > > > >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) > > >> > > > >> > is true, but these directories: > > >> > > > >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > > >> > > > >> > don't exist because $(obj) is not under $(OBJTREE) in the SPL > > >build, > > >> > it's under $(SPLTREE) > > >> > > >> OK, so I think the issue is just that $(obj) isn't the correct place > > >to > > >> mkdir these, not that we shouldn't attempt to mkdir any directory. In > > >> other words, don't you need to switch between mkdir in the object > > >tree > > >> or the SPL tree rather than switching between doing a mkdir and not? > > >> It's quite possible this only affects builds where OBJDIR!=SRCDIR, so > > >a > > >> regular build might not show up the problem I /think/ exists in the > > >code > > >> quoted a couple of emails above. > > > > > >$(obj) is always the place where the object files are going to be > > >placed, that may be in $(OBJTREE) or in $(SRCTREE) or in $(SPLTREE) > > >depending on what you're building and what you have set. > > > > > >I think the $(wildcard) suggestion I made is the most straightforward, > > >it basically says "we're about to spit out an object file to here, so > > >make sure the directory exists first". In the "normal" case where > > >your object files are in your source tree the directory will already > > >exist so it won't do anything. > > > > Ah right, I understand now. > > > > But why not make it super-simple and just run the mkdir unconditionally; > > it won't cause any harm for an in-tree object location, but removes the > > need for anyone to think about the conditional logic. > > Shouldn't this really be handled by the commands that create files in those > directories? They could check that the directories exist, and create them if > necessary, before executing their actual commands. The normal rules make sure $(obj) exists, it's only because these boards generate files in $(obj)../common and $(obj)../../nvidia/common that there's a problem and we need to do something special. I think Stephen's idea of the unconditional mkdir makes the most sense. We don't really buy anything by skipping it if the direcotry already exists. -Allen
* Allen Martin wrote: > On Mon, Jun 11, 2012 at 02:29:59AM -0700, Thierry Reding wrote: > > * PGP Signed by an unknown key > > > > * Stephen Warren wrote: > > > Allen Martin <amartin@nvidia.com> wrote: > > > >On Fri, Jun 08, 2012 at 10:28:57PM -0700, Stephen Warren wrote: > > > >> On 06/08/2012 03:27 PM, Allen Martin wrote: > > > >> > On Fri, Jun 08, 2012 at 02:07:25PM -0700, Stephen Warren wrote: > > > >> >> On 06/08/2012 02:01 PM, Allen Martin wrote: > > > >> >>> On Fri, May 25, 2012 at 06:46:22AM -0700, Thierry Reding wrote: > > > >> >>>> This commit uses the common Tegra board implementation instead > > > >of > > > >> >>>> duplicating a lot of the code. In addition, the Plutux and > > > >Medcom > > > >> >>>> specific board files can be removed as the MMC/SD setup is > > > >common > > > >> >>>> among all Tamonten-based boards. > > > >> >>>> > > > >> >>>> ... > > > >> >>>> diff --git a/board/avionic-design/medcom/Makefile > > > >b/board/avionic-design/medcom/Makefile > > > >> >>>> index b0c318c..d96d043 100644 > > > >> >>>> --- a/board/avionic-design/medcom/Makefile > > > >> >>>> +++ b/board/avionic-design/medcom/Makefile > > > >> >>>> @@ -26,12 +26,12 @@ > > > >> >>>> include $(TOPDIR)/config.mk > > > >> >>>> > > > >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) > > > >> >>>> -$(shell mkdir -p $(obj)../common) > > > >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > > > >> >>>> endif > > > >> >>>> > > > >> >>>> LIB = $(obj)lib$(BOARD).o > > > >> >>>> > > > >> >>> > > > >> >>> This breaks with my patchset to split out the arm7 code to an SPL > > > >> >>> because even though SRCDIR and OBJDIR are the same, in the SPL > > > >build > > > >> >>> $(obj) is a subdir of $(SPLTREE) not $(OBJTREE) (not my doing). > > > >> >>> > > > >> >>> How about the following instead which seems more to the point: > > > >> >>> > > > >> >>> ifeq ($(wildcard $(obj)../common),) > > > >> >>> $(shell mkdir -p $(obj)../common) > > > >> >>> endif > > > >> >>> ifeq ($(wildcard $(obj)../../nvidia/common),) > > > >> >>> $(shell mkdir -p $(obj)../../nvidia/common) > > > >> >>> endif > > > >> >> > > > >> >> Maybe I'm just not reading it right, but isn't that just running > > > >the > > > >> >> exact same mkdir commands, just splitting it into two commands, > > > >and > > > >> >> making them optional based on the $(wildcard)? I'm still not clear > > > >what > > > >> >> the problem is. > > > >> >> > > > >> >> Anyway, if this is an issue, then compal/paz00 and > > > >compulabl/trimslice > > > >> >> will need to be fixed for the SPL changes in the same way. > > > >> > > > > >> > The problem is this: > > > >> > > > > >> >>>> ifneq ($(OBJTREE),$(SRCTREE)) > > > >> > > > > >> > is true, but these directories: > > > >> > > > > >> >>>> +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) > > > >> > > > > >> > don't exist because $(obj) is not under $(OBJTREE) in the SPL > > > >build, > > > >> > it's under $(SPLTREE) > > > >> > > > >> OK, so I think the issue is just that $(obj) isn't the correct place > > > >to > > > >> mkdir these, not that we shouldn't attempt to mkdir any directory. In > > > >> other words, don't you need to switch between mkdir in the object > > > >tree > > > >> or the SPL tree rather than switching between doing a mkdir and not? > > > >> It's quite possible this only affects builds where OBJDIR!=SRCDIR, so > > > >a > > > >> regular build might not show up the problem I /think/ exists in the > > > >code > > > >> quoted a couple of emails above. > > > > > > > >$(obj) is always the place where the object files are going to be > > > >placed, that may be in $(OBJTREE) or in $(SRCTREE) or in $(SPLTREE) > > > >depending on what you're building and what you have set. > > > > > > > >I think the $(wildcard) suggestion I made is the most straightforward, > > > >it basically says "we're about to spit out an object file to here, so > > > >make sure the directory exists first". In the "normal" case where > > > >your object files are in your source tree the directory will already > > > >exist so it won't do anything. > > > > > > Ah right, I understand now. > > > > > > But why not make it super-simple and just run the mkdir unconditionally; > > > it won't cause any harm for an in-tree object location, but removes the > > > need for anyone to think about the conditional logic. > > > > Shouldn't this really be handled by the commands that create files in those > > directories? They could check that the directories exist, and create them if > > necessary, before executing their actual commands. > > The normal rules make sure that $(obj) exists, it's only because > these boards generate object files in $(obj)../common and > $(obj)../../nvidia/common that there's a problem and we need to do > something special. Yes, but the rules that create the .o files from .c or .S could also be modified to create the directory part of the target if it doesn't exist. The same workaround is used in a number of places already and is likely to be needed again and again in the future. > Stephen's idea of just the unconditional mkdir makes the most sense, > we don't really save anything by skipping it if the directory already > exists. That's certainly the easiest way to fix it. Thierry
diff --git a/board/avionic-design/common/tamonten.c b/board/avionic-design/common/tamonten.c index f23b657..d9ecd23 100644 --- a/board/avionic-design/common/tamonten.c +++ b/board/avionic-design/common/tamonten.c @@ -1,7 +1,7 @@ /* * (C) Copyright 2010,2011 * NVIDIA Corporation <www.nvidia.com> - * (C) Copyright 2011 + * (C) Copyright 2011-2012 * Avionic Design GmbH <www.avionic-design.de> * * See file CREDITS for list of people who contributed to this @@ -36,25 +36,17 @@ #include <asm/arch/pinmux.h> #include <asm/arch/uart.h> #include <asm/arch/mmc.h> -#include "tamonten.h" #ifdef CONFIG_TEGRA2_MMC #include <mmc.h> #endif -DECLARE_GLOBAL_DATA_PTR; - -const struct tegra2_sysinfo sysinfo = { - CONFIG_TEGRA2_BOARD_STRING -}; - /* - * Routine: timer_init - * Description: init the timestamp and lastinc value + * Routine: gpio_config_uart + * Description: Does nothing on Tamonten - no conflict w/SPI. */ -int timer_init(void) +void gpio_config_uart(void) { - return 0; } #ifdef CONFIG_TEGRA2_MMC @@ -65,46 +57,19 @@ int timer_init(void) static void pin_mux_mmc(void) { funcmux_select(PERIPH_ID_SDMMC4, FUNCMUX_SDMMC4_ATB_GMA_GME_8_BIT); + /* for CD GPIO PH2 */ + pinmux_tristate_disable(PINGRP_ATD); } -#endif - -/* - * Routine: board_init - * Description: Early hardware init. - */ -int board_init(void) -{ - clock_init(); - clock_verify(); - - /* boot param addr */ - gd->bd->bi_boot_params = (NV_PA_SDRAM_BASE + 0x100); - return 0; -} - -#ifdef CONFIG_TEGRA2_MMC /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) { - debug("board_mmc_init called\n"); /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc(); - gpio_config_mmc(); - debug("board_mmc_init: init eMMC\n"); - /* init dev 0, eMMC chip, with 4-bit bus */ + /* init dev 0, SD slot, with 4-bit bus */ tegra2_mmc_init(0, 4, -1, GPIO_PH2); return 0; } #endif - -#ifdef CONFIG_BOARD_EARLY_INIT_F -int board_early_init_f(void) -{ - /* Initialize selected UARTs */ - board_init_uart_f(); - return 0; -} -#endif /* EARLY_INIT */ diff --git a/board/avionic-design/common/tamonten.h b/board/avionic-design/common/tamonten.h deleted file mode 100644 index 0e60b0f..0000000 --- a/board/avionic-design/common/tamonten.h +++ /dev/null @@ -1,32 +0,0 @@ -/* - * (C) Copyright 2010,2011 - * NVIDIA Corporation <www.nvidia.com> - * (C) Copyright 2011 - * Avionic Design GmbH <www.avionic-design.de> - * - * See file CREDITS for list of people who contributed to this - * project. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; either version 2 of - * the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, - * MA 02111-1307 USA - */ - -#ifndef _TAMONTEN_H_ -#define _TAMONTEN_H_ - -void tegra2_start(void); -void gpio_config_mmc(void); - -#endif /* TAMONTEN_H */ diff --git a/board/avionic-design/medcom/Makefile b/board/avionic-design/medcom/Makefile index b0c318c..d96d043 100644 --- a/board/avionic-design/medcom/Makefile +++ b/board/avionic-design/medcom/Makefile @@ -1,7 +1,7 @@ # # (C) Copyright 2010,2011 # NVIDIA Corporation <www.nvidia.com> -# (C) Copyright 2011 +# (C) Copyright 2011,2012 # Avionic Design GmbH <www.avionic-design.de> # # See file CREDITS for list of people who contributed to this @@ -26,12 +26,12 @@ include $(TOPDIR)/config.mk ifneq ($(OBJTREE),$(SRCTREE)) -$(shell mkdir -p $(obj)../common) +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) endif LIB = $(obj)lib$(BOARD).o -COBJS := $(BOARD).o +COBJS := ../../nvidia/common/board.o COBJS += ../common/tamonten.o SRCS := $(COBJS:.o=.c) diff --git a/board/avionic-design/medcom/medcom.c b/board/avionic-design/medcom/medcom.c deleted file mode 100644 index 42c8094..0000000 --- a/board/avionic-design/medcom/medcom.c +++ /dev/null @@ -1,45 +0,0 @@ -/* - * (C) Copyright 2010,2011 - * NVIDIA Corporation <www.nvidia.com> - * (C) Copyright 2011 - * Avionic Design GmbH <www.avionic-design.de> - * - * See file CREDITS for list of people who contributed to this - * project. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; either version 2 of - * the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, - * MA 02111-1307 USA - */ - -#include <common.h> -#include <asm/io.h> -#include <asm/gpio.h> -#include <asm/arch/tegra2.h> -#ifdef CONFIG_TEGRA2_MMC -#include <mmc.h> -#endif - -#ifdef CONFIG_TEGRA2_MMC -/* - * Routine: gpio_config_mmc - * Description: Set GPIOs for SD card - */ -void gpio_config_mmc(void) -{ - /* configure pin as input for card detect */ - gpio_request(GPIO_PH2, "SD4 CD"); - gpio_direction_input(GPIO_PH2); -} -#endif diff --git a/board/avionic-design/plutux/Makefile b/board/avionic-design/plutux/Makefile index b0c318c..d96d043 100644 --- a/board/avionic-design/plutux/Makefile +++ b/board/avionic-design/plutux/Makefile @@ -1,7 +1,7 @@ # # (C) Copyright 2010,2011 # NVIDIA Corporation <www.nvidia.com> -# (C) Copyright 2011 +# (C) Copyright 2011,2012 # Avionic Design GmbH <www.avionic-design.de> # # See file CREDITS for list of people who contributed to this @@ -26,12 +26,12 @@ include $(TOPDIR)/config.mk ifneq ($(OBJTREE),$(SRCTREE)) -$(shell mkdir -p $(obj)../common) +$(shell mkdir -p $(obj)../common $(obj)../../nvidia/common) endif LIB = $(obj)lib$(BOARD).o -COBJS := $(BOARD).o +COBJS := ../../nvidia/common/board.o COBJS += ../common/tamonten.o SRCS := $(COBJS:.o=.c) diff --git a/board/avionic-design/plutux/plutux.c b/board/avionic-design/plutux/plutux.c deleted file mode 100644 index 42c8094..0000000 --- a/board/avionic-design/plutux/plutux.c +++ /dev/null @@ -1,45 +0,0 @@ -/* - * (C) Copyright 2010,2011 - * NVIDIA Corporation <www.nvidia.com> - * (C) Copyright 2011 - * Avionic Design GmbH <www.avionic-design.de> - * - * See file CREDITS for list of people who contributed to this - * project. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; either version 2 of - * the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, - * MA 02111-1307 USA - */ - -#include <common.h> -#include <asm/io.h> -#include <asm/gpio.h> -#include <asm/arch/tegra2.h> -#ifdef CONFIG_TEGRA2_MMC -#include <mmc.h> -#endif - -#ifdef CONFIG_TEGRA2_MMC -/* - * Routine: gpio_config_mmc - * Description: Set GPIOs for SD card - */ -void gpio_config_mmc(void) -{ - /* configure pin as input for card detect */ - gpio_request(GPIO_PH2, "SD4 CD"); - gpio_direction_input(GPIO_PH2); -} -#endif
This commit uses the common Tegra board implementation instead of duplicating a lot of the code. In addition, the Plutux and Medcom specific board files can be removed as the MMC/SD setup is common among all Tamonten-based boards. Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de> --- Changes in v2: - new patch board/avionic-design/common/tamonten.c | 49 +++++--------------------------- board/avionic-design/common/tamonten.h | 32 --------------------- board/avionic-design/medcom/Makefile | 6 ++-- board/avionic-design/medcom/medcom.c | 45 ----------------------------- board/avionic-design/plutux/Makefile | 6 ++-- board/avionic-design/plutux/plutux.c | 45 ----------------------------- 6 files changed, 13 insertions(+), 170 deletions(-) delete mode 100644 board/avionic-design/common/tamonten.h delete mode 100644 board/avionic-design/medcom/medcom.c delete mode 100644 board/avionic-design/plutux/plutux.c