HI Magnus Lilja: Best Regards Fred
2009/9/23, Magnus Lilja <lilja.mag...@gmail.com>: > > Hi > > 2009/9/22 Fred Fan <fanyef...@gmail.com>: > > Hi Magnus Liljia: > > Thanks for your comments. > > Best Regards > > Fred > > > > 2009/9/22, Magnus Lilja <lilja.mag...@gmail.com>: > >> > >> Hi > >> > >> > >> I've scanned the patch briefly and have some comments below. > >> > >> gareat...@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 >
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot