Hi Bo,
On 29.06.2012 11:28, Bo Shen wrote:
Hi Andreas,
On 6/27/2012 21:47, Andreas Bießmann wrote:
On 22.05.2012 12:06, Bo Shen wrote:
<snip>
+/*
- User Peripheral physical base addresses.
- */
+#define ATMEL_BASE_SPI0 0xf0000000 +#define ATMEL_BASE_SPI1 0xf0004000 +#define ATMEL_BASE_HSMCI0 0xf0008000 +#define ATMEL_BASE_HSMCI1 0xf000c000
Tabstop is 8 char, can you please allign these here for better readability?
The tabstop is 8 char, when I use vi to edit the file, it is aligned. But when use git send patch, it display like this. May I need to fix this issue?
I'm sorry, I've read the patch which adds a single char at beginning of line ('+'). After applying the patch it is aligned correctly!
<snip>
+#endif /* __ASSEMBLY__ */
+#define AT91_MATRIX_ULBT_INFINITE (0 << 0) +#define AT91_MATRIX_ULBT_SINGLE (1 << 0) +#define AT91_MATRIX_ULBT_FOUR (2 << 0) +#define AT91_MATRIX_ULBT_EIGHT (3 << 0) +#define AT91_MATRIX_ULBT_SIXTEEN (4 << 0) +#define AT91_MATRIX_ULBT_THIRTYTWO (5 << 0) +#define AT91_MATRIX_ULBT_SIXTYFOUR (6 << 0) +#define AT91_MATRIX_ULBT_128 (7 << 0)
+#define AT91_MATRIX_DEFMSTR_TYPE_NONE (0 << 16) +#define AT91_MATRIX_DEFMSTR_TYPE_LAST (1 << 16) +#define AT91_MATRIX_DEFMSTR_TYPE_FIXED (2 << 16)
please remove the tab between 'define' and the definitions above.
Ok, I will fix this at next version patch.
Great, and can you please use the same indention in the whole file (some have tabs and some have blanks).
<snip>
+#ifdef CONFIG_RESET_PHY_R +void reset_phy(void) +{ +#ifdef CONFIG_MACB
- /*
* Initialize ethernet HW addr prior to starting Linux,* needed for nfsroot*/- eth_init(gd->bd);
Isn't there a generic solution?
I will try to find a generic solution.
Well that does not mean that you should implement something here. I thought there is some generic mechanism already in u-boot and therefore it is not required to call eth_init() here to get the MAC-addr written into PHY. But I'm not that familiar with it. Take it as a question, give it a try and remove the line, ask some network custodian ;) AFAIR other boards have written the HW address correctly without this line.
<snip>
+void lcd_enable(void) +{
- if (has_lcdc())
Isn't has_lcd() cpu specific? I can not understand why one should enable CONFIG_LCD if the cpu can not provide ... but that is ok with me.
sam9x5 is a series SoC (9G15, 9G25, 9G35, 9X25, 9X35) with different peripherals. Try to use one configuration file, so it need to decide whether SoC supports?
Well, you could add a config parameter in boards.cfg to switch the feature on/off. But you can leave it as is, I have no strong meaning about that particular piece of code, it was just a question.
<snip>
+/* serial console */ +#define CONFIG_ATMEL_USART +#define CONFIG_USART_BASE ATMEL_BASE_DBGU +#define CONFIG_USART_ID ATMEL_ID_SYS
--------------------------------^ fix alignment here
The tabstop is 8 char, when I use vi to edit the file, it is aligned. But when use git send patch, it display like this. May I need to fix this issue?
You are right. See comment above, maybe I should have applied the patch before reading.
+#define CONFIG_BOOTARGS "mem=128M console=ttyS0,115200 " \
"mtdparts=atmel_nand:" \"8M(bootstrap/uboot/kernel)ro,-(rootfs) " \"root=/dev/mtdblock1 rw " \"rootfstype=ubifs ubi.mtd=1 root=ubi0:rootfs"You could provide MTDPARTS_DEFAULT and MTDIDS_DEFAULT to be able to use e.g. ubifs in u-boot (FYI, see also CONFIG_CMD_MTDPARTS).
Ok, I will fix this at next version patch.
Well, as I said before: This is for your information. I feel it is very useful especially when working with ubifs in u-boot (just to mention that, no need to change it).
Best regards
Andreas Bießmann