ppc4xx/Canyonlands added USB board callbacks
PATCH 1/1 ppc4xx/Canyonlands added USB board callbacks
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.
--- board/amcc/canyonlands/canyonlands.c | 79 +++++++++++++++++++++++++++------- include/configs/canyonlands.h | 1 + 2 files changed, 64 insertions(+), 16 deletions(-)
board/amcc/canyonlands/canyonlands.c +63 -16
@@ -34,6 +34,18 @@ extern flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH chDECLARE_GLOBAL_DATA_PTR ; + +struct ep460c_bcsr { +u8 cpld_rev, + led_user, + board_status, + reset_ctrl, + flash_ctrl, + eth_ctrl, + usb_ctrl, + irq_ctrl; +}; +#define CONFIG_SYS_BCSR3_PCIE 0x10#define BOARD_CANYONLANDS_PCIE 1
@@ -41,6 +53,14 @@ DECLARE_GLOBAL_DATA_PTR;#define BOARD_GLACIER 3#define BOARD_ARCHES 4 +#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 +/* * Override the default functions in arch/powerpc/cpu/ppc4xx/44x_spd_ddr2.c with * board specific values.
@@ -112,6 +132,9 @@ int board_early_init_f(void) {#if !defined(CONFIG_ARCHES )u32 sdr0_cust0; +struct ep460c_bcsr *bcsr_data = + (struct ep460c_bcsr *)CONFIG_SYS_BCSR_BASE ; +#endif /*
@@ -172,13 +195,11 @@ int board_early_init_f(void)#if !defined(CONFIG_ARCHES )/* Enable ethernet and take out of reset */ +out_8 ((void *)CONFIG_SYS_BCSR_BASE +6 ,0 );/* 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 ;mtsdr (SDR0_SRST1 ,0 );/* Pull AHB out of reset default=1 */
@@ -186,21 +207,45 @@ int board_early_init_f(void)mtdcr (AHB_TOP ,0x8000004B );mtdcr (AHB_BOT ,0x8000004B ); -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 ); - }#endif return 0 ; } +#if defined(CONFIG_USB_OHCI_NEW ) && defined(CONFIG_SYS_USB_OHCI_BOARD_INIT ) +int usb_board_init (void ) +{ +struct ep460c_bcsr *bcsr_data = + (struct ep460c_bcsr *)CONFIG_SYS_BCSR_BASE ; + +/* Enable USB host & USB-OTG */ + bcsr_data->usb_ctrl &= ~(BCSR_USBCTRL_OTG_RST |BCSR_USBCTRL_HOST_RST ); + +/* +* 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 ); + +return 0 ; +} + +int usb_board_stop (void ) +{ +struct ep460c_bcsr *bcsr_data = + (struct ep460c_bcsr *)CONFIG_SYS_BCSR_BASE ; + +/* Disable USB host & USB-OTG */ + bcsr_data->usb_ctrl |= (BCSR_USBCTRL_OTG_RST |BCSR_USBCTRL_HOST_RST ); +return 0 ; +} + +int usb_board_init_fail (void ) +{ +return usb_board_stop (); +} +#endif /* CONFIG_USB_OHCI_NEW && CONFIG_SYS_USB_OHCI_BOARD_INIT */ +#if !defined(CONFIG_ARCHES )static void canyonlands_sata_init (int board_type) {
@@ -244,11 +289,13 @@ int get_cpu_num(void)#if !defined(CONFIG_ARCHES )int checkboard (void ) { +struct ep460c_bcsr *bcsr_data = + (struct ep460c_bcsr *)CONFIG_SYS_BCSR_BASE ;char *s =getenv ("serial#" );if (pvr_460ex ()) {printf ("Board: Canyonlands - AMCC PPC460EX Evaluation Board" ); -if (in_8 ((void *)(CONFIG_SYS_BCSR_BASE +3 )) &CONFIG_SYS_BCSR3_PCIE ) +if (bcsr_data->board_status &CONFIG_SYS_BCSR3_PCIE ) gd->board_type =BOARD_CANYONLANDS_PCIE ;else gd->board_type =BOARD_CANYONLANDS_SATA ;
@@ -268,7 +315,7 @@ int checkboard(void)break ; } -printf (", Rev. %X" ,in_8 ((void *)(CONFIG_SYS_BCSR_BASE +0 ))); +printf (", Rev. %X" , bcsr_data->cpld_rev);if (s !=NULL ) {puts (", serial# " );
include/configs/canyonlands.h +1 -0
@@ -417,6 +417,7 @@#define CONFIG_SYS_USB_OHCI_REGS_BASE (CONFIG_SYS_AHB_BASE | 0xd0000)#define CONFIG_SYS_USB_OHCI_SLOT_NAME "ppc440"#define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS 15 +#define CONFIG_SYS_USB_OHCI_BOARD_INIT #endif /*
Signature · 1 line
1.5.6.3
PATCH 1/1 ppc4xx/Canyonlands added USB board callbacks
Dear Rupjyoti Sarmah,
Quoted 21 lines · In message <[email protected]> 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 <[email protected]>Signed-off-by Rupjyoti Sarmah <[email protected]>--- board/amcc/canyonlands/canyonlands.c | 79 +++++++++++++++++++++++++++------- include/configs/canyonlands.h | 1 + 2 files changed, 64 insertions(+), 16 deletions(-)board/amcc/canyonlands/canyonlands.c +1
-0@@ -34,6 +34,18 @@ extern flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH chDECLARE_GLOBAL_DATA_PTR ; +
board/amcc/canyonlands/canyonlands.c extern flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH ch · line 47
Quoted 10 lines
board/amcc/canyonlands/canyonlands.c +10
-0+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,
};board/amcc/canyonlands/canyonlands.c extern flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH ch · line 49
Quoted 1 line
board/amcc/canyonlands/canyonlands.c +0
-0#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.
board/amcc/canyonlands/canyonlands.c DECLARE_GLOBAL_DATA_PTR; · line 62
Quoted 7 lines
board/amcc/canyonlands/canyonlands.c +7
-0+#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.
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void) · line 199
Quoted 4 lines
#if !defined(CONFIG_ARCHES)
board/amcc/canyonlands/canyonlands.c +1
-0/* Enable ethernet and take out of reset */ +out_8 ((void *)CONFIG_SYS_BCSR_BASE +6 ,0 );
Remove this blank line, please.
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void) · line 202
Quoted 6 lines
board/amcc/canyonlands/canyonlands.c +1
-4/* 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.
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void) · line 198
Quoted 10 lines
board/amcc/canyonlands/canyonlands.c +0
-10-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.
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void) · line 222
Quoted 2 lines
board/amcc/canyonlands/canyonlands.c +2
-0+/* 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!
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void) · line 228
Quoted 5 lines
board/amcc/canyonlands/canyonlands.c +5
-0+/* +* 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
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "Well I don't see why I have to make one man miserable when I can make so many men happy." - Ellyn Mustard, about marriage
PATCH 1/1 ppc4xx/Canyonlands added USB board callbacks
Thanks for your inputs. I will do the necessary changes are resubmit.
Regards,
Rup
-----Original Message-----
From: Wolfgang Denk [mailto:wd at denx.de]
Sent: Thursday, June 17, 2010 1:24 AM
To: Rupjyoti Sarmah
Cc: u-boot at lists.denx.de; rupjyoti.sarmah at gmail.com; sr at denx.de; rsarmah at apm.com
Subject: Re: [U-Boot] [PATCH]ppc4xx/Canyonlands added USB board callbacks
Dear Rupjyoti Sarmah,
Quoted 9 lines · In message <[email protected]> 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 <[email protected]>Signed-off-by Rupjyoti Sarmah <[email protected]>--- board/amcc/canyonlands/canyonlands.c | 79
+++++++++++++++++++++++++++-------
Review
Quoted 4 lines
include/configs/canyonlands.h | 1 + 2 files changed, 64 insertions(+), 16 deletions(-)(quoted diff) +0
-0
b/board/amcc/canyonlands/canyonlands.c
board/amcc/canyonlands/canyonlands.c extern flash_info_t
Quoted 4 lines
index 23874d2..2e30a32 100644
board/amcc/canyonlands/canyonlands.c +0
-0@@ -34,6 +34,18 @@ extern flash_info_t
flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH ch
Quoted 4 lines
board/amcc/canyonlands/canyonlands.c +1
-0DECLARE_GLOBAL_DATA_PTR ; +
board/amcc/canyonlands/canyonlands.c extern flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH ch · line 47
Quoted 10 lines
board/amcc/canyonlands/canyonlands.c +10
-0+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,
};board/amcc/canyonlands/canyonlands.c extern flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH ch · line 49
Quoted 1 line
board/amcc/canyonlands/canyonlands.c +0
-0#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.
board/amcc/canyonlands/canyonlands.c DECLARE_GLOBAL_DATA_PTR; · line 62
Quoted 7 lines
board/amcc/canyonlands/canyonlands.c +7
-0+#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.
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void) · line 199
Quoted 4 lines
#if !defined(CONFIG_ARCHES)
board/amcc/canyonlands/canyonlands.c +1
-0/* Enable ethernet and take out of reset */ +out_8 ((void *)CONFIG_SYS_BCSR_BASE +6 ,0 );
Remove this blank line, please.
Quoted 1 line
/* Remove NOR-FLASH, NAND-FLASH & EEPROM hardware write protection
*/
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void) · line 202
Quoted 5 lines
board/amcc/canyonlands/canyonlands.c +1
-4-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.
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void)
Quoted 4 lines
board/amcc/canyonlands/canyonlands.c +0
-4-if (pvr_460ex ()) { -/* -* Configure USB-STP pins as alternate and not GPIO -* It seems to be neccessary to configure the STP pins as
GPIO
board/amcc/canyonlands/canyonlands.c
Quoted 1 line
board/amcc/canyonlands/canyonlands.c +0
-1-* input at powerup (perhaps while USB reset is asserted).
So
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void) · line 198
Quoted 5 lines
board/amcc/canyonlands/canyonlands.c +0
-5-* 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.
board/amcc/canyonlands/canyonlands.c
Quoted 2 lines
board/amcc/canyonlands/canyonlands.c +2
-0+/* 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!
board/amcc/canyonlands/canyonlands.c int board_early_init_f(void) · line 228
Quoted 5 lines
board/amcc/canyonlands/canyonlands.c +5
-0+/* +* 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
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "Well I don't see why I have to make one man miserable when I can make so many men happy." - Ellyn Mustard, about marriage