On 30.01.2012 19:10, Eric Nelson wrote:
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/c8b2870efd041f820a91eebcb888...
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.
Yes, please. I have this on my todo list, too. But I haven't found the time to look at the consequences trying to mainline this patch. I.e. if we try to mainline this, we have to touch all gpio_xxx() functions to use this new macro? At least for i.MX6? Or does this macro apply for more i.MX SoCs? If yes, do we have to find out for which it will work? And move it to a i.MX common gpio header? And then touch all i.MX code it applies to?
Anyway, many thanks for your good work!
Best regards
Dirk
Btw.: It looks to me that the patch series to introduce the i.MX6 SPI driver increases from each version of the patch series to the next one. I'm not sure if this is ok? Or if we should try to split it to smaller chunks which would be easier to get them merged?