[U-Boot] [PATCH] imx51:Add support basic boot code of freescale imx51 bbg board

[Fred Fan]

HI Magnus Lilja:
    Best Regards
Fred


2009/9/23, Magnus Lilja <lilja.magnus at gmail.com>:
>
> Hi
>
> 2009/9/22 Fred Fan <fanyefeng at gmail.com>:
> > Hi Magnus Liljia:
> >     Thanks for your comments.
> >      Best Regards
> > Fred
> >
> > 2009/9/22, Magnus Lilja <lilja.magnus at gmail.com>:
> >>
> >> Hi
> >>
> >>
> >> I've scanned the patch briefly and have some comments below.
> >>
> >> gareatech at gmail.com wrote:
> >> > diff --git a/MAKEALL b/MAKEALL
> >> > index edebaea..ed8c437 100755
> >> > --- a/MAKEALL
> >> > +++ b/MAKEALL
> >> <snip>
> >
> >
> > Does means use new board name?
>
> Heh, no. <snip> just means that I've removed parts of your patch
> (those parts which I don't have any comments on). Sorry for the
> confusion.
> OK
> >> > +++ b/board/freescale/imx51/Makefile
> >> <snip>
> >> Does means use new board name?
>
> The board name should be used instead of the imx51 name.

OK

>> > +     mxc_request_iomux(MX51_PIN_UART1_RXD, IOMUX_CONFIG_ALT0);
> >> > +     mxc_iomux_set_pad(MX51_PIN_UART1_RXD, pad | PAD_CTL_SRE_FAST);
> >> > +     mxc_request_iomux(MX51_PIN_UART1_TXD, IOMUX_CONFIG_ALT0);
> >> > +     mxc_iomux_set_pad(MX51_PIN_UART1_TXD, pad | PAD_CTL_SRE_FAST);
> >> > +     mxc_request_iomux(MX51_PIN_UART1_RTS, IOMUX_CONFIG_ALT0);
> >> > +     mxc_iomux_set_pad(MX51_PIN_UART1_RTS, pad);
> >> > +     mxc_request_iomux(MX51_PIN_UART1_CTS, IOMUX_CONFIG_ALT0);
> >> > +     mxc_iomux_set_pad(MX51_PIN_UART1_CTS, pad);
> >> > +}
> >> > +
> >> > +void setup_nfc(void)
> >> > +{
> >> > +     /* Enable NFC IOMUX */
> >> > +     mxc_request_iomux(MX51_PIN_NANDF_CS0, IOMUX_CONFIG_ALT0);
> >> > +     mxc_request_iomux(MX51_PIN_NANDF_CS1, IOMUX_CONFIG_ALT0);
> >> > +     mxc_request_iomux(MX51_PIN_NANDF_CS2, IOMUX_CONFIG_ALT0);
> >> > +     mxc_request_iomux(MX51_PIN_NANDF_CS3, IOMUX_CONFIG_ALT0);
> >> > +     mxc_request_iomux(MX51_PIN_NANDF_CS4, IOMUX_CONFIG_ALT0);
> >> > +     mxc_request_iomux(MX51_PIN_NANDF_CS5, IOMUX_CONFIG_ALT0);
> >> > +     mxc_request_iomux(MX51_PIN_NANDF_CS6, IOMUX_CONFIG_ALT0);
> >> > +     mxc_request_iomux(MX51_PIN_NANDF_CS7, IOMUX_CONFIG_ALT0);
> >> > +}
> >>
> >> Since it's very likely that setup_nfc() and setup_uart() will be used in
> >> other i.MX51 boards as well it's a good idea to
> >> place these functions in cpu/arm_cortexa8/mx51/devices.c (or something
> >> similar).
> >> I think it should be board level. Some board based on i.MX51 maybe has
> >> differenent choice.
>
> That can  be handed with #ifdef's just like in the i.MX31 case.



> If we do like i.MX31, the code in soc level has the details of board level.

   we should reduce the block of #ifdef.

> <...>
> >> > +#define MXC_SRPGC_EMI_SRPGCR (MXC_SRPGC_EMI_BASE + 0x0)
> >> > +#define MXC_SRPGC_EMI_PUPSCR (MXC_SRPGC_EMI_BASE + 0x4)
> >> > +#define MXC_SRPGC_EMI_PDNSCR (MXC_SRPGC_EMI_BASE + 0x8)
> >> > +
> >>
> >> Are all of the above #defines needed/expected to be needed by U-boot?
> >
> >   No. It just copy from linux kernel code. Does need remove them?
>
> Don't no what the policy is.


we prefer to keep the sync with the file in kernel source code.

>> > diff --git a/cpu/arm_cortexa8/mx51/interrupts.c
> >> > b/cpu/arm_cortexa8/mx51/interrupts.c
> >> > new file mode 100644
> >> > index 0000000..c0d70ac
> >> > --- /dev/null
> >> > +++ b/cpu/arm_cortexa8/mx51/interrupts.c
> >> <snip>
> >> What's means?
>
> Ignore the <snip> comments.
>
> >> > diff --git a/cpu/arm_cortexa8/mx51/serial.c
> >> > b/cpu/arm_cortexa8/mx51/serial.c
> >> > new file mode 100644
> >> > index 0000000..580ac13
> >> > --- /dev/null
> >> > +++ b/cpu/arm_cortexa8/mx51/serial.c
> >>
> >> I haven't looked in the details of the serial driver, but would it be
> >> possible to use drivers/serial/serial_mxc.c
> >> instead? It looks very similar.
> >> I don't like the implement of this driver. It contains the chip details
> in
> >> driver code.
> >>
> >> But I will do what as your said. And restructure this driver in another
> >> patch.
>
> That sounds like a good idea if you want to improve the serial driver.
> Create a separate standalone patch so people can review and test that.
>
> Regards, Magnus
>