Dear Andreas Bießmann,

On 11.11.2013 12:03, Andreas Bießmann wrote:
(...)
+#include <asm/arch/hardware.h>
+#if defined(CONFIG_MACB)

I think we can include the headers unconditionally, or is there a problem?

No problem with that.

+
+       writel(1 << ATMEL_ID_PIOA | 1 << ATMEL_ID_PIOCDE, &pmc->pcer);
+
+       /* Configure RDY/BSY */
+       at91_set_gpio_input(CONFIG_SYS_NAND_READY_PIN, 1);

Could you please use the generic GPIO API here? Is it Ok for you to not
mux the pullup for the ready pin?

I've checked schematics - board has external pull-ups, so I can use generic 
GPIO.

+
+       /* Enable NandFlash */
+       at91_set_gpio_output(CONFIG_SYS_NAND_ENABLE_PIN, 1);

Here you could really use the gpio_direction_output(). I sent out an RFC
lately to define new GPIO_PIN_Px() macros. As long as that RFC is not
reworked and included I think it is Ok to use the CONFIG_ATMEL_LEGACY
defines.

Please switch to the generic GPIO API for defining GPIO direction and
switching GPIO, even on AT91. On the long run we will remove the
redundant at91_pio API, at least regarding plane GPIO functionality.
Therefore it would be good to use that API now, even if it costs some
function calls currently.

Do I understand correctly, that for all ports that I use (via generic GPIO) I should 
create "temporary" defines
(with hardcoded gpio#)?

Perhaps I'm missing something, but I currently see no other way to get GPIO# 
(It will change once your RFC is included).

Of course CONFIG_ATMEL_LEGACY has to stay, as atmel_nand uses 
CONFIG_SYS_NAND_*_PIN and is still using old API (I guess this also needs 
fixing).

+       /* Re-enable pull-up */
+       at91_set_pio_pullup(AT91_PIO_PORTC, 25, 1);
+       at91_set_pio_pullup(AT91_PIO_PORTE, 25, 1);
+       at91_set_pio_pullup(AT91_PIO_PORTE, 26, 1);
+
+       at91_macb_hw_init();

Heiko proposed a solution for unifying macb reset sequence.
+1

I will also switch usb_a9263_macb_hw_init() to generic GPIO (and remove 
re-enabling of pull-ups as they are dropped by at91_macb_hw_init).

+#ifdef CONFIG_HAS_DATAFLASH
+       at91_set_pio_output(AT91_PIO_PORTE, 20, 1);     /* select spi0 clock */

Here we could also use the generic GPIO API. Beside that, did you really
copy the at91sam9263 here? IOW, do you really have a slot for MMC/DF
here and therefore require to switch the clock?

I copied (see at91sam9263ek.c:256).
I've checked board schematics and this pin is floating, so for v3 I'll drop it.


Beside these minor complaints there is nothing to stop inclusion for
2014.01. Please stay tuned and do not send v3 immediately cause of the
ongoing change for GPIO API and consolidation of macb reset.
I think another ping from your side in about two weeks would be Ok, if
you haven't seen changes in the two mentioned points on the list.

Ping :)
Do I understand correctly, that phy_reset RFC will be soon applied, then I 
should post v3?
Or should I wait for your GPIO RFC?

In the meantime I will start playing with SPL, as u-boot image grows slowly and 
soon I'll be forced to cut another features just to keep it small enough.

Best Regards,
Mateusz
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to