[U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select

[Marek Vasut]

> On 01/29/2012 07:36 PM, Marek Vasut wrote:
> >> 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 at boundarydevices.com>
> >>>>>>>> Acked-by: Dirk Behme<dirk.behme at de.bosch.com>
> >>>>>>>> Acked-by: Stefano Babic<sbabic at 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.
> 
> Hmmm. I just noticed that the IMX_GPIO_NR() macro isn't in Stefano's tree.
> My previous patches were against Dirk's	tree on GitHub, which has a 
patch
> from Troy:
> 	https://github.com/dirkbehme/u-boot-
imx6/commit/c8b2870efd041f820a91eebcb8
> 88c84a4f79f1f6
> 
> If we move this macro into arch-mx6/imx-regs.h, we avoid the #if.
> 
> Looking at the remaining content of gpio.h, it seems that it's
> driver-specific anyway (only the driver should be worried about the
> register layout of the GPIO controller).
> 
> If there are no objections, I'll forward a separate patch to define the
> macro.
> 
> Should this be based on Stefano's tree?

Definitelly. Please rebase to u-boot-imx.

Thanks for the patches and good work so far!

M
> 
> Please advise,
> 
> 
> Eric