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

[Eric Nelson]

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?