Dear Rupjyoti Sarmah,
In message 201006161734.o5GHYa6I014963@amcc.com you wrote:
Functions added to support board callbacks for USB init. This isolates USB manipulations such that it is only touched if USB is used by U-Boot.
Signed-off-by: Dave Mitchell dmitchell@appliedmicro.com Signed-off-by: Rupjyoti Sarmah rsarmah@appliedmicro.com
board/amcc/canyonlands/canyonlands.c | 79 +++++++++++++++++++++++++++------- include/configs/canyonlands.h | 1 + 2 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/board/amcc/canyonlands/canyonlands.c b/board/amcc/canyonlands/canyonlands.c index 23874d2..2e30a32 100644 --- a/board/amcc/canyonlands/canyonlands.c +++ b/board/amcc/canyonlands/canyonlands.c @@ -34,6 +34,18 @@ extern flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH ch
DECLARE_GLOBAL_DATA_PTR;
+struct ep460c_bcsr {
- u8 cpld_rev,
led_user,board_status,reset_ctrl,flash_ctrl,eth_ctrl,usb_ctrl,irq_ctrl;+};
Please use standard style to describe structs, and mind indetation, i. e. write:
struct ep460c_bcsr { u8 cpld_rev, u8 led_user, u8 board_status, ... u8 irq_ctrl, };
#define CONFIG_SYS_BCSR3_PCIE 0x10
CONFIG_SYS_* variable sshould not be defined in board code; they are intended to be used in board config files only.
+#define BCSR_CPLDREV 0x00 +#define BCSR_BRDSTS 0x03 +#define BCSR_FLASHCTRL 0x05 +#define BCSR_ETHCTRL 0x06 +#define BCSR_USBCTRL 0x07 +#define BCSR_USBCTRL_OTG_RST 0x32 +#define BCSR_USBCTRL_HOST_RST 0x01
Please add comments what these magic numbers do.
#if !defined(CONFIG_ARCHES) /* Enable ethernet and take out of reset */
- out_8((void *)CONFIG_SYS_BCSR_BASE + 6, 0);
Remove this blank line, please.
/* Remove NOR-FLASH, NAND-FLASH & EEPROM hardware write protection */
- out_8((void *)CONFIG_SYS_BCSR_BASE + 5, 0);
- /* Enable USB host & USB-OTG */
- out_8((void *)CONFIG_SYS_BCSR_BASE + 7, 0);
- bcsr_data->flash_ctrl = 0;
NAK. Please always use proper I/O accessors to access device registers.
- if (pvr_460ex()) {
/** Configure USB-STP pins as alternate and not GPIO* It seems to be neccessary to configure the STP pins as GPIO* input at powerup (perhaps while USB reset is asserted). So* we configure those pins to their "real" function now.*/gpio_config(16, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);gpio_config(19, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);- }
I am not sure if this is really a good idea to remove this code here.
- /* Enable USB host & USB-OTG */
- bcsr_data->usb_ctrl &= ~(BCSR_USBCTRL_OTG_RST | BCSR_USBCTRL_HOST_RST);
Please always use proper I/O accessors! Fix globally!
- /*
* Configure USB-STP pins as alternate and not GPIO.*/- gpio_config(16, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
- gpio_config(19, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
Isn't this too late? And maybe you want to keep the comment that explains what is being done, and why?
Best regards,
Wolfgang Denk