usb: ehci-mx6: allow board_ehci_hcd_init to fail

PATCH New Stefan Agner 2 participants 7 messages May 5, 2016 – May 6, 2016

Subsystems (2)

USB USB ehci driver

lore

Share & tools

lore

Stefan Agner

author

2016-05-05 23:59 UTC #0 lore raw

PATCH 1/1 usb: ehci-mx6: allow board_ehci_hcd_init to fail

There could be runtime determined board specific reason why a EHCI initialization fails (e.g. ENODEV if a Port is not available). In this case, properly return the error code.
While at it, that function (board_ehci_hcd_init) has actually two documentation blocks... Use the correct function name for the documentation block of board_usb_phy_mode.

Signed-off-by Stefan Agner <[email protected]>

---
 drivers/usb/host/ehci-mx6.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/usb/host/ehci-mx6.c +5 -2

index a981b50..bb48d0d 100644
@@ -254,7 +254,7 @@ static void usb_oc_config(int index)
 }
 
 /**
- * board_ehci_hcd_init - override usb phy mode
+ * board_usb_phy_mode - override usb phy mode
  * @port:	usb host/otg port
  *
  * Target board specific, override usb_phy_mode.

@@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 #endif
 	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
 		(controller_spacing * index));
+	int ret;
 
 	if (index > 3)
 		return -EINVAL;

@@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	mdelay(1);
 
 	/* Do board specific initialization */
-	board_ehci_hcd_init(index);
+	ret = board_ehci_hcd_init(index);
+	if (ret)
+		return ret;
 
 	usb_power_config(index);
 	usb_oc_config(index);

Signature · 1 line

2.8.2

Marek Vasut

2016-05-06 00:39 UTC #1 lore raw

PATCH 1/1 usb: ehci-mx6: allow board_ehci_hcd_init to fail

Quoted 34 lines · Stefan Agner

There could be runtime determined board specific reason why a EHCI initialization fails (e.g. ENODEV if a Port is not available). In this case, properly return the error code.
While at it, that function (board_ehci_hcd_init) has actually two documentation blocks... Use the correct function name for the documentation block of board_usb_phy_mode.
Signed-off-by Stefan Agner <[email protected]>
---
 drivers/usb/host/ehci-mx6.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/host/ehci-mx6.c +2 -1
index a981b50..bb48d0d 100644
@@ -254,7 +254,7 @@ static void usb_oc_config(int index)
 }
 
 /**
- * board_ehci_hcd_init - override usb phy mode
+ * board_usb_phy_mode - override usb phy mode
  * @port:	usb host/otg port
  *
  * Target board specific, override usb_phy_mode.
@@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 #endif
 	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
 		(controller_spacing * index));
+	int ret;
 
 	if (index > 3)
 		return -EINVAL;

drivers/usb/host/ehci-mx6.c int ehci_hcd_init(int index, enum usb_init_type init, · line 323

Quoted 7 lines

drivers/usb/host/ehci-mx6.c +3 -1

@@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	mdelay(1);
 
 	/* Do board specific initialization */
-	board_ehci_hcd_init(index);
+	ret = board_ehci_hcd_init(index);
+	if (ret)
+		return ret;

Shouldn't this disable the OH3 clock , which were enabled a few lines above?

Otherwise this is nice, thanks.

Quoted 3 lines

drivers/usb/host/ehci-mx6.c +0 -0
 	usb_power_config(index);
 	usb_oc_config(index);
 

Signature · 2 lines

Best regards,
Marek Vasut

Stefan Agner

author

2016-05-06 16:12 UTC #2 lore raw

PATCH 1/1 usb: ehci-mx6: allow board_ehci_hcd_init to fail

Quoted 37 lines · Marek Vasut

Quoted 36 lines · Stefan Agner
There could be runtime determined board specific reason why a EHCI initialization fails (e.g. ENODEV if a Port is not available). In this case, properly return the error code.
While at it, that function (board_ehci_hcd_init) has actually two documentation blocks... Use the correct function name for the documentation block of board_usb_phy_mode.
Signed-off-by Stefan Agner <[email protected]>
---
 drivers/usb/host/ehci-mx6.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/host/ehci-mx6.c +2 -1
index a981b50..bb48d0d 100644
@@ -254,7 +254,7 @@ static void usb_oc_config(int index)
 }
 
 /**
- * board_ehci_hcd_init - override usb phy mode
+ * board_usb_phy_mode - override usb phy mode
  * @port:	usb host/otg port
  *
  * Target board specific, override usb_phy_mode.
@@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 #endif
 	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
 		(controller_spacing * index));
+	int ret;
 
 	if (index > 3)
 		return -EINVAL;
@@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	mdelay(1);
 

Quoted 7 lines

drivers/usb/host/ehci-mx6.c int ehci_hcd_init(int index, enum usb_init_type init, · line 323
Quoted 5 lines
drivers/usb/host/ehci-mx6.c +3 -1
 	/* Do board specific initialization */
-	board_ehci_hcd_init(index);
+	ret = board_ehci_hcd_init(index);
+	if (ret)
+		return ret;
Shouldn't this disable the OH3 clock , which were enabled a few lines above?

Hm, not sure, the clock does not take a controller index... I guess if the second instance is returning an error, that would disable the clock also for the first controller...

--
Stefan

Quoted 6 lines

Otherwise this is nice, thanks.
Quoted 3 lines
drivers/usb/host/ehci-mx6.c +0 -0
 	usb_power_config(index);
 	usb_oc_config(index);
 

Marek Vasut

2016-05-06 16:30 UTC #3 lore raw

PATCH 1/1 usb: ehci-mx6: allow board_ehci_hcd_init to fail

Quoted 46 lines · Stefan Agner

Quoted 37 lines · Marek Vasut
Quoted 36 lines · Stefan Agner
There could be runtime determined board specific reason why a EHCI initialization fails (e.g. ENODEV if a Port is not available). In this case, properly return the error code.
While at it, that function (board_ehci_hcd_init) has actually two documentation blocks... Use the correct function name for the documentation block of board_usb_phy_mode.
Signed-off-by Stefan Agner <[email protected]>
---
 drivers/usb/host/ehci-mx6.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/host/ehci-mx6.c +2 -1
index a981b50..bb48d0d 100644
@@ -254,7 +254,7 @@ static void usb_oc_config(int index)
 }
 
 /**
- * board_ehci_hcd_init - override usb phy mode
+ * board_usb_phy_mode - override usb phy mode
  * @port:	usb host/otg port
  *
  * Target board specific, override usb_phy_mode.
@@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 #endif
 	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
 		(controller_spacing * index));
+	int ret;
 
 	if (index > 3)
 		return -EINVAL;
@@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	mdelay(1);
 
Quoted 7 lines
drivers/usb/host/ehci-mx6.c int ehci_hcd_init(int index, enum usb_init_type init, · line 323
Quoted 5 lines
drivers/usb/host/ehci-mx6.c +3 -1
 	/* Do board specific initialization */
-	board_ehci_hcd_init(index);
+	ret = board_ehci_hcd_init(index);
+	if (ret)
+		return ret;
Shouldn't this disable the OH3 clock , which were enabled a few lines above?

Quoted 3 lines

Hm, not sure, the clock does not take a controller index... I guess if the second instance is returning an error, that would disable the clock also for the first controller...

Siiiiiiigh! All right, that sucks and we absolutely have no time for reworking the whole clock framework on imx for this release. You want this stuff in for this release, right ?

Best regards,
Marek Vasut

Stefan Agner

author

2016-05-06 17:11 UTC #4 lore raw

PATCH 1/1 usb: ehci-mx6: allow board_ehci_hcd_init to fail

Quoted 47 lines · Marek Vasut

Quoted 46 lines · Stefan Agner
Quoted 44 lines · Marek Vasut
Quoted 34 lines · Stefan Agner
There could be runtime determined board specific reason why a EHCI initialization fails (e.g. ENODEV if a Port is not available). In this case, properly return the error code.
While at it, that function (board_ehci_hcd_init) has actually two documentation blocks... Use the correct function name for the documentation block of board_usb_phy_mode.
Signed-off-by Stefan Agner <[email protected]>
---
 drivers/usb/host/ehci-mx6.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/host/ehci-mx6.c +2 -1
index a981b50..bb48d0d 100644
@@ -254,7 +254,7 @@ static void usb_oc_config(int index)
 }
 
 /**
- * board_ehci_hcd_init - override usb phy mode
+ * board_usb_phy_mode - override usb phy mode
  * @port:	usb host/otg port
  *
  * Target board specific, override usb_phy_mode.
@@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 #endif
 	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
 		(controller_spacing * index));
+	int ret;
 
 	if (index > 3)
 		return -EINVAL;
drivers/usb/host/ehci-mx6.c int ehci_hcd_init(int index, enum usb_init_type init, · line 323
Quoted 7 lines
drivers/usb/host/ehci-mx6.c +3 -1
@@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	mdelay(1);
 
 	/* Do board specific initialization */
-	board_ehci_hcd_init(index);
+	ret = board_ehci_hcd_init(index);
+	if (ret)
+		return ret;
Shouldn't this disable the OH3 clock , which were enabled a few lines above?

Quoted 7 lines

Quoted 3 lines
Hm, not sure, the clock does not take a controller index... I guess if the second instance is returning an error, that would disable the clock also for the first controller...
Siiiiiiigh! All right, that sucks and we absolutely have no time for reworking the whole clock framework on imx for this release. You want this stuff in for this release, right ?

Wasn't that urgent as the board which will use this is not upstream yet anyway... But still, nice to have it in.

Btw, I really like the new 2 month release cycle.

--
Stefan

Marek Vasut

2016-05-06 17:18 UTC #5 lore raw

PATCH 1/1 usb: ehci-mx6: allow board_ehci_hcd_init to fail

Quoted 52 lines · Stefan Agner

Quoted 51 lines · Marek Vasut
Quoted 49 lines · Stefan Agner
Quoted 37 lines · Marek Vasut
Quoted 36 lines · Stefan Agner
There could be runtime determined board specific reason why a EHCI initialization fails (e.g. ENODEV if a Port is not available). In this case, properly return the error code.
While at it, that function (board_ehci_hcd_init) has actually two documentation blocks... Use the correct function name for the documentation block of board_usb_phy_mode.
Signed-off-by Stefan Agner <[email protected]>
---
 drivers/usb/host/ehci-mx6.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/host/ehci-mx6.c +2 -1
index a981b50..bb48d0d 100644
@@ -254,7 +254,7 @@ static void usb_oc_config(int index)
 }
 
 /**
- * board_ehci_hcd_init - override usb phy mode
+ * board_usb_phy_mode - override usb phy mode
  * @port:	usb host/otg port
  *
  * Target board specific, override usb_phy_mode.
@@ -310,6 +310,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 #endif
 	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
 		(controller_spacing * index));
+	int ret;
 
 	if (index > 3)
 		return -EINVAL;
@@ -317,7 +318,9 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 	mdelay(1);
 
Quoted 7 lines
drivers/usb/host/ehci-mx6.c int ehci_hcd_init(int index, enum usb_init_type init, · line 323
Quoted 5 lines
drivers/usb/host/ehci-mx6.c +3 -1
 	/* Do board specific initialization */
-	board_ehci_hcd_init(index);
+	ret = board_ehci_hcd_init(index);
+	if (ret)
+		return ret;
Shouldn't this disable the OH3 clock , which were enabled a few lines above?
Hm, not sure, the clock does not take a controller index... I guess if the second instance is returning an error, that would disable the clock also for the first controller...

Quoted 8 lines

Quoted 3 lines
Siiiiiiigh! All right, that sucks and we absolutely have no time for reworking the whole clock framework on imx for this release. You want this stuff in for this release, right ?
Wasn't that urgent as the board which will use this is not upstream yet anyway... But still, nice to have it in.
Btw, I really like the new 2 month release cycle.

Heh, tell that to Tom :)

Quoted 3 lines

--
Stefan

Signature · 2 lines

Best regards,
Marek Vasut

Marek Vasut

Applied

2016-05-06 16:31 UTC #6 lore raw

PATCH 1/1 usb: ehci-mx6: allow board_ehci_hcd_init to fail

In reply to Stefan Agner

Quoted 8 lines · Stefan Agner

There could be runtime determined board specific reason why a EHCI initialization fails (e.g. ENODEV if a Port is not available). In this case, properly return the error code.
While at it, that function (board_ehci_hcd_init) has actually two documentation blocks... Use the correct function name for the documentation block of board_usb_phy_mode.
Signed-off-by Stefan Agner <[email protected]>

Applied, thanks.

Best regards,
Marek Vasut