[U-Boot] [PATCH v4] at91: Update MEESC board support

[Daniel Gorsulowski]

Hello Tom,

Tom wrote:
> Daniel Gorsulowski wrote:
>> This patch implements several updates:
>> -disable CONFIG_ENV_OVERWRITE
>> -add new hardware style variants and set the arch numbers appropriate
>> -pass the serial# and hardware revision to the kernel
>> -removed unused macros from include/configs/meesc.h
>> -fixed multiline comment style
>>
>> Signed-off-by: Daniel Gorsulowski <Daniel.Gorsulowski at esd.eu>
>> ---
>> v2: - don't write the ethernet address to the EMAC module anymore
>>
>> v3: - removed function meesc_set_arch_number and moved code to checkboard()
>>     - reworked function get_board_serial()
>>     - removed unused macros from include/configs/meesc.h
>>
>> v4: - fixed commit message (to many characters)
>>     - fixed indentation in switch-case statement
>>     - fixed multiline comment style
>>
>>  board/esd/meesc/meesc.c |   65 +++++++++++++++++++++++++++++++++++++++++-----
>>  include/configs/meesc.h |   25 ++++++------------
>>  2 files changed, 66 insertions(+), 24 deletions(-)
>>
>> diff --git a/board/esd/meesc/meesc.c b/board/esd/meesc/meesc.c
>> index 636d0ed..7cdc04a 100644
>> --- a/board/esd/meesc/meesc.c
>> +++ b/board/esd/meesc/meesc.c
>> @@ -126,8 +126,10 @@ static void meesc_ethercat_hw_init(void)
>>  		AT91_SMC_NRDPULSE_(4) | AT91_SMC_NCS_RDPULSE_(9));
>>  	at91_sys_write(AT91_SMC1_CYCLE(0),
>>  		AT91_SMC_NWECYCLE_(10) | AT91_SMC_NRDCYCLE_(5));
>> -	/* Configure behavior at external wait signal, byte-select mode, 16 bit
>> -	data bus width, none data float wait states and TDF optimization */
>> +	/*
>> +	 * Configure behavior at external wait signal, byte-select mode, 16 bit
>> +	 * data bus width, none data float wait states and TDF optimization
>> +	 */
>>  	at91_sys_write(AT91_SMC1_MODE(0),
>>  		AT91_SMC_READMODE | AT91_SMC_EXNWMODE_READY |
>>  		AT91_SMC_BAT_SELECT | AT91_SMC_DBW_16 | AT91_SMC_TDF_(0) |
>> @@ -156,8 +158,32 @@ int board_eth_init(bd_t *bis)
>>  int checkboard(void)
>>  {
>>  	char str[32];
>> -
>> -	puts("Board: esd CAN-EtherCAT Gateway");
>> +	u_char hw_type;	/* hardware type */
>> +
>> +	/* read the "Type" register of the ET1100 controller */
>> +	hw_type = readb(CONFIG_ET1100_BASE);
>> +
>> +	switch (hw_type) {
>> +	case 0x11:
>> +	case 0x3F:
>> +		/* ET1100 present, arch number of MEESC-Board */
>> +		gd->bd->bi_arch_number = MACH_TYPE_MEESC;
>> +		puts("Board: CAN-EtherCAT Gateway");
>> +		break;
>> +	case 0xFF:
>> +		/* no ET1100 present, arch number of EtherCAN/2-Board */
>> +		gd->bd->bi_arch_number = MACH_TYPE_ETHERCAN2;
>> +		puts("Board: EtherCAN/2 Gateway");
>> +		/* switch on LED1D */
>> +		at91_set_gpio_output(AT91_PIN_PB12, 1);
>> +		break;
>> +	default:
>> +		/* assume, no ET1100 present, arch number of EtherCAN/2-Board */
>> +		gd->bd->bi_arch_number = MACH_TYPE_ETHERCAN2;
>> +		printf("FATAL! Read invalid hw_type: %02X\n", hw_type);
>> +		puts("Board: EtherCAN/2 Gateway");
> 
> You may want to soften this "FATAL" to "ERROR"
> I would expect a "FATAL" to be followed by a call to hang()
> 
Ok, I'll change this to "ERROR"
>> +		break;
>> +	}
>>  	if (getenv_r("serial#", str, sizeof(str)) > 0) {
>>  		puts(", serial# ");
>>  		puts(str);
>> @@ -167,6 +193,32 @@ int checkboard(void)
>>  	return 0;
>>  }
>>  
>> +#ifdef CONFIG_SERIAL_TAG
>> +void get_board_serial(struct tag_serialnr *serialnr)
>> +{
>> +	char *str;
>> +
>> +	char *serial = getenv("serial#");
>> +	if (serial) {
>> +		str = strchr(serial, '_');
>> +		if (str && (strlen(str) >= 4)) {
>> +			serialnr->high = (*(str + 1) << 8) | *(str + 2);
>> +			serialnr->low = simple_strtoul(str + 3, NULL, 16);
> 
> How is serial# set?
> It seems like this is available if the user explicitly does a `setenv 
> serial`
> So at best this is a placeholder for future function.
> The logic that decodes the variable assumes a format that a user is
> not going, in general, to use.
The serial# is set by factory start-up operation, so the expected format is
guaranteed.
And because of disabled CONFIG_ENV_OVERWRITE, it can never be changed
again by the user. (Moreover, the user has no access to the serial console)
> 
> 
> Tom

Regards, Daniel