From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marek.vasut@gmail.com>
Date: Mon, 30 Jan 2012 03:36:22 +0100
Subject: [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default
	serial flash bus and chip-select
In-Reply-To: <4F25D069.4080806@boundarydevices.com>
References: <1327863595-1524-1-git-send-email-eric.nelson@boundarydevices.com>
	<201201292316.26332.marek.vasut@gmail.com>
	<4F25D069.4080806@boundarydevices.com>
Message-ID: <201201300336.22615.marek.vasut@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

> On 01/29/2012 03:16 PM, Marek Vasut wrote:
> >> On 01/29/2012 01:11 PM, Marek Vasut wrote:
> >>>> On 01/29/2012 12:18 PM, Marek Vasut wrote:
> >>>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >>>>>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
> >>>>>> Acked-by: Stefano Babic<sbabic@denx.de>
> >>>>>> ---
> >>>>>> 
> >>>>>>     include/configs/mx6qsabrelite.h |    3 +++
> >>>>>>     1 files changed, 3 insertions(+), 0 deletions(-)
> >>>>>> 
> >>>>>> diff --git a/include/configs/mx6qsabrelite.h
> >>>>>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
> >>>>>> --- a/include/configs/mx6qsabrelite.h
> >>>>>> +++ b/include/configs/mx6qsabrelite.h
> >>>>>> @@ -46,9 +46,12 @@
> >>>>>> 
> >>>>>>     #define CONFIG_CMD_SF
> >>>>>>     #ifdef CONFIG_CMD_SF
> >>>>>> 
> >>>>>> +#define GPIO_3_19 ((2*32)+19)
> >>>>> 
> >>>>> I'd expect this to be in platform headers?
> >>>> 
> >>>> This is a choice made in the SabreLite design. I don't think
> >>>> the same choice has been made for other i.MX6 boards.
> >>> 
> >>> I mean the definition of the GPIO_3_19 ...
> >> 
> >> I don't think we want to set precedent for defining
> >> constants for the 100s of GPIO numbers.
> >> 
> >> That said, I could achieve my objective of clarifying
> >> what the number meant (that the constant refers to a GP) by
> >> 
> >> changing this:
> >> 	#define CONFIG_SF_DEFAULT_CS   (0|(GPIO_3_19<<8))
> >> 
> >> to this
> >> 
> >> 	#define CONFIG_SF_DEFAULT_CS   (0|(IMX_GPIO_NR(3,19)<<8))
> > 
> > Why the (0| part ? Anyway, that indeed looks better, more standard even.
> > 
> > And I think for MX5, there was even stuff defining the GPIO numbers
> > imported from Linux.
> > 
> > M
> > 
> >> There's a bit of an issue with this. The IMX_GPIO_NR() macro
> >> is defined in arch-mx6/gpio.h which is normally included after
> >> mx6qsabrelite.h because the latter defines the machine.
> > 
> > And the CPP will choke on that ?
> 
> Assembler or linker. IMX_GPIO_NR will be undefined and treated as an
> external unless we add this nested include into
> include/configs/mx6qsabrelite.h:
> 
> #ifndef __ASSEMBLY__
> #include <asm/arch/gpio.h>
> #endif
> 
> arch-mx6/gpio.h isn't directly ASM-friendly.
> 
> This seems like a lot of #include-foo for giving a name to a constant,
> don't you think?

Better than redefining stuff, which will eventually lead to errors and breakage.