ARM: mx5/mx51_babbage: Move GPIO initialization for USB PHY Reset line to commom place

PATCH Superseded Fabio Estevam 3 participants 5 messages Apr 28, 2011

Subsystems (2)

Arm (32-bit) Arm sub-architectures

lore

Share & tools

lore

Fabio Estevam

author

2011-04-28 00:44 UTC #0 lore raw

PATCH 1/2 ARM: mx5/mx51_babbage: Move GPIO initialization for USB PHY Reset line to commom place

The USB PHY Reset GPIO can be configured in the same place as the other GPIOs.

While at it rename the pin as BABBAGE_USB_PHY_RESET to make clearer its purpose.

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/mach-mx5/board-mx51_babbage.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

arch/arm/mach-mx5/board-mx51_babbage.c +6 -5

index c7b3fab..7b919bc 100644
@@ -36,7 +36,7 @@
 
 #define BABBAGE_USB_HUB_RESET	IMX_GPIO_NR(1, 7)
 #define BABBAGE_USBH1_STP	IMX_GPIO_NR(1, 27)
-#define BABBAGE_PHY_RESET	IMX_GPIO_NR(2, 5)
+#define BABBAGE_USB_PHY_RESET	IMX_GPIO_NR(2, 5)
 #define BABBAGE_FEC_PHY_RESET	IMX_GPIO_NR(2, 14)
 #define BABBAGE_POWER_KEY	IMX_GPIO_NR(2, 21)
 #define BABBAGE_ECSPI1_CS0	IMX_GPIO_NR(4, 24)

@@ -110,6 +110,9 @@ static iomux_v3_cfg_t mx51babbage_pads[] = {
 	/* USB HUB reset line*/
 	MX51_PAD_GPIO1_7__GPIO1_7,
 
+	/* USB PHY reset line */
+	MX51_PAD_EIM_D21__GPIO2_5,
+
 	/* FEC */
 	MX51_PAD_EIM_EB2__FEC_MDIO,
 	MX51_PAD_EIM_EB3__FEC_RDATA1,

@@ -172,7 +175,6 @@ static struct imxi2c_platform_data babbage_hsi2c_data = {
 static int gpio_usbh1_active(void)
 {
 	iomux_v3_cfg_t usbh1stp_gpio = MX51_PAD_USBH1_STP__GPIO1_27;
-	iomux_v3_cfg_t phyreset_gpio = MX51_PAD_EIM_D21__GPIO2_5;
 	int ret;
 
 	/* Set USBH1_STP to GPIO and toggle it */

@@ -189,14 +191,13 @@ static int gpio_usbh1_active(void)
 	gpio_free(BABBAGE_USBH1_STP);
 
 	/* De-assert USB PHY RESETB */
-	mxc_iomux_v3_setup_pad(phyreset_gpio);
-	ret = gpio_request(BABBAGE_PHY_RESET, "phy_reset");
+	ret = gpio_request(BABBAGE_USB_PHY_RESET, "phy_reset");
 
 	if (ret) {
 		pr_debug("failed to get MX51_PAD_EIM_D21__GPIO_2_5: %d\n", ret);
 		return ret;
 	}
-	gpio_direction_output(BABBAGE_PHY_RESET, 1);
+	gpio_direction_output(BABBAGE_USB_PHY_RESET, 1);
 	return 0;
 }

Signature · 1 line

1.6.0.4

Fabio Estevam

author

2011-04-28 00:44 UTC #1 lore raw

PATCH 2/2 ARM: mx5/mx51_babbage: Use gpio_request_one when possible

Instead of using gpio_request followed by gpio_direction_output use gpio_request_one.

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/mach-mx5/board-mx51_babbage.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

arch/arm/mach-mx5/board-mx51_babbage.c +4 -4

index 7b919bc..00203fb 100644
@@ -179,25 +179,25 @@ static int gpio_usbh1_active(void)
 
 	/* Set USBH1_STP to GPIO and toggle it */
 	mxc_iomux_v3_setup_pad(usbh1stp_gpio);
-	ret = gpio_request(BABBAGE_USBH1_STP, "usbh1_stp");
+	ret = gpio_request_one(BABBAGE_USBH1_STP,
+					GPIOF_OUT_INIT_LOW, "usbh1_stp");
 
 	if (ret) {
 		pr_debug("failed to get MX51_PAD_USBH1_STP__GPIO_1_27: %d\n", ret);
 		return ret;
 	}
-	gpio_direction_output(BABBAGE_USBH1_STP, 0);
 	gpio_set_value(BABBAGE_USBH1_STP, 1);
 	msleep(100);
 	gpio_free(BABBAGE_USBH1_STP);
 
 	/* De-assert USB PHY RESETB */
-	ret = gpio_request(BABBAGE_USB_PHY_RESET, "phy_reset");
+	ret = gpio_request_one(BABBAGE_USB_PHY_RESET,
+					GPIOF_OUT_INIT_HIGH, "phy_reset");
 
 	if (ret) {
 		pr_debug("failed to get MX51_PAD_EIM_D21__GPIO_2_5: %d\n", ret);
 		return ret;
 	}
-	gpio_direction_output(BABBAGE_USB_PHY_RESET, 1);
 	return 0;
 }

Signature · 1 line

1.6.0.4

Uwe Kleine-König

2011-04-28 07:55 UTC #2 lore raw

PATCH 2/2 ARM: mx5/mx51_babbage: Use gpio_request_one when possible

Hello,

ah, this patch makes my review comment to your patch 1 nil.

Quoted 12 lines · Fabio Estevam

Instead of using gpio_request followed by gpio_direction_output use gpio_request_one.
Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/mach-mx5/board-mx51_babbage.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/mach-mx5/board-mx51_babbage.c +0 -0
index 7b919bc..00203fb 100644

arch/arm/mach-mx5/board-mx51_babbage.c static int gpio_usbh1_active(void) · line 181

Quoted 3 lines

arch/arm/mach-mx5/board-mx51_babbage.c +0 -0

@@ -179,25 +179,25 @@ static int gpio_usbh1_active(void)
 
 	/* Set USBH1_STP to GPIO and toggle it */
 	mxc_iomux_v3_setup_pad(usbh1stp_gpio);

This cannot go into the global list of pins to mux?

arch/arm/mach-mx5/board-mx51_babbage.c static int gpio_usbh1_active(void) · line 190

Quoted 11 lines

arch/arm/mach-mx5/board-mx51_babbage.c +2 -2

-	ret = gpio_request(BABBAGE_USBH1_STP, "usbh1_stp");
+	ret = gpio_request_one(BABBAGE_USBH1_STP,
+					GPIOF_OUT_INIT_LOW, "usbh1_stp");
 
 	if (ret) {
 		pr_debug("failed to get MX51_PAD_USBH1_STP__GPIO_1_27: %d\n", ret);
 		return ret;
 	}
-	gpio_direction_output(BABBAGE_USBH1_STP, 0);
 	gpio_set_value(BABBAGE_USBH1_STP, 1);
 	msleep(100);

Why is the sleep between set_value(1) and free? I would have expected it between request_one(INIT_LOW) and set_value(1)?

Quoted 19 lines

arch/arm/mach-mx5/board-mx51_babbage.c +2 -2

 	gpio_free(BABBAGE_USBH1_STP);
 
 	/* De-assert USB PHY RESETB */
-	ret = gpio_request(BABBAGE_USB_PHY_RESET, "phy_reset");
+	ret = gpio_request_one(BABBAGE_USB_PHY_RESET,
+					GPIOF_OUT_INIT_HIGH, "phy_reset");
 
 	if (ret) {
 		pr_debug("failed to get MX51_PAD_EIM_D21__GPIO_2_5: %d\n", ret);
 		return ret;
 	}
-	gpio_direction_output(BABBAGE_USB_PHY_RESET, 1);
 	return 0;
 }

Signature · 1 line

1.6.0.4

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Fabio Estevam

2011-04-28 16:03 UTC #3 lore raw

PATCH 2/2 ARM: mx5/mx51_babbage: Use gpio_request_one when possible

Hi Uwe,

On 4/28/2011 4:55 AM, Uwe Kleine-K?nig wrote:
...

Quoted 3 lines

arch/arm/mach-mx5/board-mx51_babbage.c static int gpio_usbh1_active(void) · line 181
Quoted 2 lines
arch/arm/mach-mx5/board-mx51_babbage.c +0 -0
 	/* Set USBH1_STP to GPIO and toggle it */
 	mxc_iomux_v3_setup_pad(usbh1stp_gpio);
This cannot go into the global list of pins to mux?

This pin alternates its mode(GPIO and USB STP), so I think we can leave it defined here.

Quoted 5 lines

arch/arm/mach-mx5/board-mx51_babbage.c +2 -1

-	ret = gpio_request(BABBAGE_USBH1_STP, "usbh1_stp");
+	ret = gpio_request_one(BABBAGE_USBH1_STP,
+					GPIOF_OUT_INIT_LOW, "usbh1_stp");

Quoted 9 lines

arch/arm/mach-mx5/board-mx51_babbage.c static int gpio_usbh1_active(void) · line 190
Quoted 7 lines
arch/arm/mach-mx5/board-mx51_babbage.c +0 -1
 	if (ret) {
 		pr_debug("failed to get MX51_PAD_USBH1_STP__GPIO_1_27: %d\n", ret);
 		return ret;
 	}
-	gpio_direction_output(BABBAGE_USBH1_STP, 0);
 	gpio_set_value(BABBAGE_USBH1_STP, 1);
 	msleep(100);
Why is the sleep between set_value(1) and free? I would have expected it between request_one(INIT_LOW) and set_value(1)?

Agreed. I haven?t changed the logic on this patch. Will send a third patch for fixing this.

Thanks,

Fabio Estevam

Uwe Kleine-König

2011-04-28 07:51 UTC #4 lore raw

PATCH 1/2 ARM: mx5/mx51_babbage: Move GPIO initialization for USB PHY Reset line to commom place

In reply to Fabio Estevam

Hello Fabio,

Quoted 46 lines · Fabio Estevam

The USB PHY Reset GPIO can be configured in the same place as the other GPIOs.

While at it rename the pin as BABBAGE_USB_PHY_RESET to make clearer its purpose.

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/mach-mx5/board-mx51_babbage.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

arch/arm/mach-mx5/board-mx51_babbage.c +4 -4

index c7b3fab..7b919bc 100644
@@ -36,7 +36,7 @@
 
 #define BABBAGE_USB_HUB_RESET	IMX_GPIO_NR(1, 7)
 #define BABBAGE_USBH1_STP	IMX_GPIO_NR(1, 27)
-#define BABBAGE_PHY_RESET	IMX_GPIO_NR(2, 5)
+#define BABBAGE_USB_PHY_RESET	IMX_GPIO_NR(2, 5)
 #define BABBAGE_FEC_PHY_RESET	IMX_GPIO_NR(2, 14)
 #define BABBAGE_POWER_KEY	IMX_GPIO_NR(2, 21)
 #define BABBAGE_ECSPI1_CS0	IMX_GPIO_NR(4, 24)

@@ -110,6 +110,9 @@ static iomux_v3_cfg_t mx51babbage_pads[] = {
 	/* USB HUB reset line*/
 	MX51_PAD_GPIO1_7__GPIO1_7,
 
+	/* USB PHY reset line */
+	MX51_PAD_EIM_D21__GPIO2_5,
+
 	/* FEC */
 	MX51_PAD_EIM_EB2__FEC_MDIO,
 	MX51_PAD_EIM_EB3__FEC_RDATA1,

@@ -172,7 +175,6 @@ static struct imxi2c_platform_data babbage_hsi2c_data = {
 static int gpio_usbh1_active(void)
 {
 	iomux_v3_cfg_t usbh1stp_gpio = MX51_PAD_USBH1_STP__GPIO1_27;
-	iomux_v3_cfg_t phyreset_gpio = MX51_PAD_EIM_D21__GPIO2_5;
 	int ret;
 
 	/* Set USBH1_STP to GPIO and toggle it */

@@ -189,14 +191,13 @@ static int gpio_usbh1_active(void)
 	gpio_free(BABBAGE_USBH1_STP);
 
 	/* De-assert USB PHY RESETB */
-	mxc_iomux_v3_setup_pad(phyreset_gpio);
-	ret = gpio_request(BABBAGE_PHY_RESET, "phy_reset");

arch/arm/mach-mx5/board-mx51_babbage.c static int gpio_usbh1_active(void) · line 202

Quoted 10 lines

arch/arm/mach-mx5/board-mx51_babbage.c +2 -1

+	ret = gpio_request(BABBAGE_USB_PHY_RESET, "phy_reset");
 
 	if (ret) {
 		pr_debug("failed to get MX51_PAD_EIM_D21__GPIO_2_5: %d\n", ret);
 		return ret;
 	}
-	gpio_direction_output(BABBAGE_PHY_RESET, 1);
+	gpio_direction_output(BABBAGE_USB_PHY_RESET, 1);
 	return 0;
 }

this looks like a candidate for gpio_request_one, doesn't it?

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |