Skip to contentSkip to conversation

ppc4xx/Canyonlands added USB board callbacks

PATCH New Rupjyoti Sarmah 3 participants 3 messages Jun 16, 2010 – Jun 17, 2010

lore
Share & tools
lore
#0 lore raw

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
index 23874d2..2e30a32 100644
@@ -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;
+};
+
 #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
index ac9b3c5..6a70160 100644
@@ -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
#1 lore raw

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.

---
 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
index 23874d2..2e30a32 100644
@@ -34,6 +34,18 @@ extern flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* info for FLASH ch
 
 DECLARE_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
#2 lore raw

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.

---
 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 -0
 DECLARE_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