Dear Wolfgang Denk,
Wolfgang Denk wrote:
> Dear Daniel Gorsulowski,
>
> In message <12524805241911-git-send-email-daniel.gorsulow...@esd.eu> you
> wrote:
>> This patch implements several updates:
>> -Disable CONFIG_ENV_OVERWRITE
>> -Add new hardware style variants and set the arch numbers appropriate
>> (autodetect)
>> -Pass the serial# and hardware revision to the kernel
>>
>> Signed-off-by: Daniel Gorsulowski <daniel.gorsulow...@esd.eu>
>> ---
>
> You should indicate that this is version 2 of an earlier patch, and
> describe what has been changed compared to earlier versions.
>
> And as it's a single patch, it makes no sense to number it, i. e.
> please omit the "1/1" part.
>
I'll do so.
>> +static void meesc_set_arch_number(void)
>> +{
>> + /* 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;
>> + break;
>> + case 0xFF:
>> + /* no ET1100 present,
>> + arch number of EtherCAN/2-Board */
>> + gd->bd->bi_arch_number = MACH_TYPE_ETHERCAN2;
>> + break;
>> + default:
>> + /* assume, no ET1100 present,
>> + arch number of EtherCAN/2-Board */
>> + gd->bd->bi_arch_number = MACH_TYPE_ETHERCAN2;
>> + break;
>> + }
>
> You have the same switch() in checkboard() - maybe you move this code
> there, so you can avoid the whole function?
>
Good idea...
>> +#ifdef CONFIG_SERIAL_TAG
>> +void get_board_serial(struct tag_serialnr *serialnr)
>> +{
>> + char *str;
>> +
>> + str = strchr(getenv("serial#"), '_');
>> + if (str) {
>> + serialnr->high = (*(str + 1) << 8) | *(str + 2);
>> + serialnr->low = simple_strtoul(str + 3, NULL, 16);
>
> Hm... that looks dangerous to me. Who tells you that the value of the
> "serial#" envrionment variable has that many characters?
>
You are right, I'll rework it.
>> int board_init(void)
>> {
>> /* Peripheral Clock Enable Register */
>> @@ -174,8 +234,15 @@ int board_init(void)
>> 1 << AT91SAM9263_ID_PIOB |
>> 1 << AT91SAM9263_ID_PIOCDE);
>>
>> - /* arch number of MEESC-Board */
>> - gd->bd->bi_arch_number = MACH_TYPE_MEESC;
>> + /* initialize ET1100 Controller */
>> + meesc_ethercat_hw_init();
>
> I thought we had agreed not to initialize the Ethernet hardware if it
> not used by U-Boot?
>
We had, but this does not initialize unused hardware. This is needed for
detecting hw_type and setting correct arch_number.
>
> Best regards,
>
> Wolfgang Denk
>
Kind regards,
Daniel Gorsulowski
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
