[U-Boot] [PATCH 3/5] i.MX6: nitrogen6x/sabrelite: override set_board_name()

[Eric Nelson]

Hi Stefano,

On 11/18/2013 03:57 AM, Stefano Babic wrote:
> Hi Eric,
>
> On 17/11/2013 18:17, Eric Nelson wrote:
>> Since the nitrogen6x board file auto-detects Nitrogen6x and
>> SABRE Lite boards, override set_board_name to produce one
>> of two values for board_name.
>>
>> Signed-off-by: Eric Nelson <eric.nelson at boundarydevices.com>
>> ---
>>   board/boundary/nitrogen6x/nitrogen6x.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c
>> index 616ad55..aa9717a 100644
>> --- a/board/boundary/nitrogen6x/nitrogen6x.c
>> +++ b/board/boundary/nitrogen6x/nitrogen6x.c
>> @@ -756,9 +756,14 @@ int board_init(void)
>>   	return 0;
>>   }
>>
>> +static inline int is_n6x(void)
>> +{
>> +	return gpio_get_value(WL12XX_WL_IRQ_GP);
>> +}
>> +
>>   int checkboard(void)
>>   {
>> -	if (gpio_get_value(WL12XX_WL_IRQ_GP))
>> +	if (is_n6x())
>>   		puts("Board: Nitrogen6X\n");
>>   	else
>>   		puts("Board: SABRE Lite\n");
>> @@ -766,6 +771,13 @@ int checkboard(void)
>>   	return 0;
>>   }
>>
>> +void set_board_name(void)user
>> +{
>> +	char *old = getenv("board_name");
>
> Agree on the name: board_name was already introduced in u-boot.
>
>> +	if (!old)
>> +		setenv("board_name", is_n6x() ? "nitrogen6x" : "sabrelite");
>
> I have a major question: if it is possible to detect at runtime, as you
> have already implemented, which is the board where code is running, why
> is it possible to override it for the operator ?
 >
First off, I want to clarify things. There are two levels of override
possible here:
	- weak functions can be over-ridden by board files
	- environment variables can be overridden through saveenv()

The first is to allow auto-detection of a "board" as shown in
nitrogen6x.c. I assume you're okay with that bit.

 > I agree that forcing environment variables inside code is bad, but
 > in this case it is a hardware related stuff. It is like to the
 > processor type or the serial-id of the processor (variable dieid#
 > on OMAP). Overriding seems weird.
>
There is a reason for this, and it mostly involves custom pin-muxing.

All of our boards support a wide variety of peripherals, and we make
assumptions about what the various connectors will be used for.

For instance, we define a particular connector for use as a parallel
(RGB) display.

Lots of customers have other needs though, and through the magic of
pin-muxing, they build their own "board" files in the kernel and
use the parallel display pins for connecting SPI devices or additional
I2C or RS-232 pins. Or simply as GPIOs.

The easiest, most maintainable way to do that is by cloning our board
files and editing the pin muxes.

With the addition of device tree support, this becomes even easier.

So is it a different board? Maybe not, but it's useful to name things
differently in the kernel tree so you can easily perform a diff
against the original or boot to the original DTB for comparison.

SOM customers blur the lines even further, and it's not uncommon
for someone to boot a different carrier with our stock U-Boot if
the changes are minor or their needs are modest.

Re-reading the patch, it seems that there's no good reason to have
set_imx_type(void) declared __weak.

Regards,


Eric