Dear Wolfgang Denk,
>
> In message <4b7c14ec.7050...@taskit.de> you wrote:
>> Signed-off-by: Achim Ehrlich <aehrl...@taskit.de>
>> ---
> ...
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2906,6 +2906,15 @@ TNY_A9260_config : unconfig
>> @echo "#define CONFIG_$(@:_config=) 1" >$(obj)include/config.h
>> @$(MKCONFIG) -a tny_a9260 arm arm926ejs tny_a9260 calao at91
>>
>> +portuxG20_config \
>> +stamp9G20_config : unconfig
>> + @mkdir -p $(obj)include
>> + @if [ "$(findstring portux,$@)" ] ; then \
>> + echo "#define CONFIG_PORTUXG20 1" >$(obj)include/config.h; \
>> + $(XECHO) "... PortuxG20";\
>> + fi;
>
> NAK. We don't accept such scripting in the Makefile any more. Please
> use the available mkconfig features instead.
>
Sorry, I overlooked that feature and will change that.
>
>> diff --git a/board/taskit/stamp9G20/partition.c
>> b/board/taskit/stamp9G20/partition.c
>> new file mode 100644
>> index 0000000..2629c67
>> --- /dev/null
>> +++ b/board/taskit/stamp9G20/partition.c
>
> So we are adding yet another copy of this file. We already have 12 of
> these. Mostly identical.
>
> Isn't there a more intelligent way to handle this?
>
The file has not much content anyway. I can get rid of it completely.
> ...
>> +int dram_init(void)
>> +{
>> + gd->bd->bi_dram[0].start = PHYS_SDRAM;
>> + gd->bd->bi_dram[0].size = PHYS_SDRAM_SIZE;
>> + return 0;
>> +}
>
> Please consider using get_ram_size() for memory auto-sizing and
> testing.
Thanks for the hint, i will test it.
>
>> +#ifdef CONFIG_RESET_PHY_R
>> +void reset_phy(void)
>> +{
>> +#ifdef CONFIG_MACB
>
>
> CONFIG_MACB seems to be an undocumented variable. What is it supposed
> to do?
>
It switches on the pin initialization for the Ethernet MAC.
Actually, i just copied this part from the AT91SAM9260ek board code.
>
>> diff --git a/include/configs/stamp9G20.h b/include/configs/stamp9G20.h
>> new file mode 100644
>> index 0000000..5008554
>> --- /dev/null
>> +++ b/include/configs/stamp9G20.h
> ...
>> +/* ARM asynchronous clock */
>> +#define AT91_MAIN_CLOCK 18432000 /* 18.432 MHz crystal */
>
> This should be changed into a CONFIG_SYS_ (globally).
>
Do you mean, I should change that for all AT91-Boards in u-boot?
How or who is going to test that?
>
>> +#define AT91_CPU_NAME "AT91SAM9G20"
>
> Ditto.
>
>> +#define CONFIG_ATMEL_USART 1
>> +#undef CONFIG_USART0
>> +#undef CONFIG_USART1
>> +#undef CONFIG_USART2
>> +#define CONFIG_USART3 1 /* USART 3 is DBGU */
>
> Do not undefine what is not defined anyway.
>
>> +/* LED */
>> +
>> +#undef CONFIG_AT91_LED
>
> Ditto.
>
>> +/*
>> + * Command line configuration.
>> + */
>> +#include <config_cmd_default.h>
>> +#undef CONFIG_CMD_BDI
>> +#undef CONFIG_CMD_FPGA
>> +#undef CONFIG_CMD_IMI
>> +#undef CONFIG_CMD_IMLS
>> +#undef CONFIG_CMD_SOURCE
>
> Is there any specific reason for disabling the "bdi", "imi", "imls"
> and "source" commands? They are pretty useful, and you don't seem to
> be especially short on resources...
>
> ...
>> +/* USB */
>> +#define CONFIG_USB_ATMEL
>> +#define CONFIG_USB_OHCI_NEW 1
>> +#define CONFIG_DOS_PARTITION 1
>> +#define CONFIG_SYS_USB_OHCI_CPU_INIT 1
>> +#define CONFIG_SYS_USB_OHCI_REGS_BASE 0x00500000 /*
>> AT91SAM9260_UHP_BASE */
>
> Line too long. Please fix globally.
>
> ...
>> + "basicargs=console=ttyS0,115200 mem=64M\0" \
>> + ""
>> +
>> +
>> +
>
> Excessive white space. Drop 3 of the empty lines.
I will fix these as well.
Thank you for your corrections.
Kind regards
Achim
--
product manager
email:aehrl...@taskit.de
Tel.: ++49 30 611295-25
Fax: ++49 30 611295-11
--
taskit GmbH
Seelenbinderstr. 33 | D-12555 Berlin
web:http://www.taskit.de
Amtsgericht Charlottenburg: 93HRB39014
Managing director: Thorsten Raulfs
--
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
