All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mateusz Kulikowski <mateusz.kulikowski@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] arm: at91: support for the Calao USB-A9263 board (based on AT91SAM9263)
Date: Thu, 28 Nov 2013 22:29:48 +0100	[thread overview]
Message-ID: <5297B5CC.4040300@gmail.com> (raw)
In-Reply-To: <5280B99C.1070702@gmail.com>

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

  parent reply	other threads:[~2013-11-28 21:29 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-27 19:34 [U-Boot] [PATCH] arm: at91: support for the Calao USB-A9263 board (based on AT91SAM9263) Mateusz Kulikowski
2013-10-28  4:57 ` Bo Shen
2013-10-28 21:30   ` Mateusz Kulikowski
2013-10-29  5:20     ` Heiko Schocher
2013-10-30 18:03       ` Mateusz Kulikowski
2013-10-29  5:24     ` Bo Shen
2013-10-29  5:54       ` Heiko Schocher
2013-10-30 18:17       ` Mateusz Kulikowski
2013-10-29  6:26     ` Andreas Bießmann
2013-10-29  6:35       ` Heiko Schocher
2013-10-29  9:12   ` Andreas Bießmann
2013-11-01 19:26     ` Mateusz Kulikowski
2013-11-04  9:36       ` Andreas Bießmann
2013-11-01 23:19     ` [U-Boot] [PATCH v2] " Mateusz Kulikowski
2013-11-11 11:03       ` Andreas Bießmann
2013-11-12  8:53         ` Heiko Schocher
2013-11-28 21:29         ` Mateusz Kulikowski [this message]
2013-11-29 11:31           ` Andreas Bießmann
2013-12-02 22:30             ` [U-Boot] [PATCH v3] " Mateusz Kulikowski
2013-12-09 12:39               ` [U-Boot] [U-Boot, " Andreas Bießmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5297B5CC.4040300@gmail.com \
    --to=mateusz.kulikowski@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.