From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mateusz Kulikowski <mateusz.kulikowski@gmail.com>
Date: Thu, 28 Nov 2013 22:29:48 +0100
Subject: [U-Boot] [PATCH v2] arm: at91: support for the Calao USB-A9263
 board (based on AT91SAM9263)
In-Reply-To: <5280B99C.1070702@gmail.com>
References: <526F7BFE.9050900@gmail.com>
	<1383347981-4707-1-git-send-email-mateusz.kulikowski@gmail.com>
	<5280B99C.1070702@gmail.com>
Message-ID: <5297B5CC.4040300@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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