> 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.nel...@boundarydevices.com> > >>>>>>>> Acked-by: Dirk Behme<dirk.be...@de.bosch.com> > >>>>>>>> Acked-by: Stefano Babic<sba...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
