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 Nelsoneric.nelson@boundarydevices.com >>> Acked-by: Dirk Behmedirk.behme@de.bosch.com >>> Acked-by: Stefano Babicsbabic@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