unbreak ehci-mxc on otg port of i.MX27

PATCH New Eric Bénard 5 participants 9 messages Jan 13, 2011

Subsystems (3)

USB Arm i.MX and Layerscape USB ehci driver

lore

Share & tools

lore

Eric Bénard

author

2011-01-13 09:06 UTC #0 lore raw

PATCH 1/1 unbreak ehci-mxc on otg port of i.MX27

commit 711669e5b80b6f2d88f61ed8a9681f83d8cbd201 fixed port 0 support for i.MX51 but broke it for (at least) i.MX27 which doesn't have a usb_phy1 clock but has a pdev->id 0.

Signed-off-by Eric B?nard <[email protected]>

Cc Arnaud Patard <[email protected]>

Cc Sascha Hauer <[email protected]>

---
 drivers/usb/host/ehci-mxc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/usb/host/ehci-mxc.c +1 -1

index fa59b26..18eb597 100644
@@ -178,7 +178,7 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
 	}
 
 	/* "dr" device has its own clock */
-	if (pdev->id == 0) {
+	if ((cpu_is_mx51()) & (pdev->id == 0)) {
 		priv->phy1clk = clk_get(dev, "usb_phy1");
 		if (IS_ERR(priv->phy1clk)) {
 			ret = PTR_ERR(priv->phy1clk);

Signature · 1 line

1.7.3.4

Sascha Hauer

maintainer

2011-01-13 09:12 UTC #1 lore raw

PATCH 1/1 unbreak ehci-mxc on otg port of i.MX27

Quoted 3 lines · Eric B?nard

commit 711669e5b80b6f2d88f61ed8a9681f83d8cbd201 fixed port 0 support for i.MX51 but broke it for (at least) i.MX27 which doesn't have a usb_phy1 clock but has a pdev->id 0.

Shame on me for not reviewing the patch properly :(

Quoted 13 lines

Signed-off-by Eric B?nard <[email protected]>
Cc Arnaud Patard <[email protected]>
Cc Sascha Hauer <[email protected]>
---
 drivers/usb/host/ehci-mxc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/host/ehci-mxc.c +0 -0
index fa59b26..18eb597 100644

drivers/usb/host/ehci-mxc.c static int ehci_mxc_drv_probe(struct platform_device *pdev) · line 181

Quoted 5 lines

drivers/usb/host/ehci-mxc.c +1 -1

@@ -178,7 +178,7 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
 	}
 
 	/* "dr" device has its own clock */
-	if (pdev->id == 0) {
+	if ((cpu_is_mx51()) & (pdev->id == 0)) {

should be && and please remove the unnecessary braces around cpu_is_mx51(). Otherwise ok.

Sascha

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Eric Bénard

author

2011-01-13 09:17 UTC #2 lore raw

PATCH 1/1 unbreak ehci-mxc on otg port of i.MX27

Hi Sascha,

Quoted 8 lines · Sascha Hauer

Review
Quoted 4 lines
/* "dr" device has its own clock */
(quoted diff) +1 -1
-	if (pdev->id == 0) {
+	if ((cpu_is_mx51())&  (pdev->id == 0)) {
should be&& and please remove the unnecessary braces around cpu_is_mx51(). Otherwise ok.

shame on me for the &, a wrong copy and paste from my working tree removed the second one, While I'm here does MX53 or MX50 need this ?

Eric

Eric Bénard

author

2011-01-13 13:53 UTC #3 lore raw

PATCH v2 1/1 unbreak ehci-mxc on otg port of i.MX27

In reply to Sascha Hauer

commit 711669e5b80b6f2d88f61ed8a9681f83d8cbd201 fixed port 0 support for i.MX51 but broke it for (at least) i.MX27 which doesn't have a usb_phy1 clock but has a pdev->id 0.

Signed-off-by Eric B?nard <[email protected]>

Cc Arnaud Patard <[email protected]>

Cc Sascha Hauer <[email protected]>

---
v2: fix missing &, remove parenthesis and add i.MX51 in the comment

 drivers/usb/host/ehci-mxc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/usb/host/ehci-mxc.c +2 -2

index fa59b26..d443fbd 100644
@@ -177,8 +177,8 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
 		clk_enable(priv->ahbclk);
 	}
 
-	/* "dr" device has its own clock */
-	if (pdev->id == 0) {
+	/* "dr" device has its own clock on i.MX51 */
+	if (cpu_is_mx51() && (pdev->id == 0)) {
 		priv->phy1clk = clk_get(dev, "usb_phy1");
 		if (IS_ERR(priv->phy1clk)) {
 			ret = PTR_ERR(priv->phy1clk);

Signature · 1 line

1.7.3.4

Sergei Shtylyov

2011-01-13 17:48 UTC #4 lore raw

PATCH v2 1/1 unbreak ehci-mxc on otg port of i.MX27

Hello.

Quoted 1 line · Eric B?nard

commit 711669e5b80b6f2d88f61ed8a9681f83d8cbd201 fixed port 0 support

Please also specify the commit summary in parens, as asked by Linus. I think
I have already asked you to do so.

Quoted 6 lines

for i.MX51 but broke it for (at least) i.MX27 which doesn't have a usb_phy1 clock but has a pdev->id 0.
Signed-off-by Eric B?nard <[email protected]>
Cc Arnaud Patard <[email protected]>
Cc Sascha Hauer <[email protected]>

WBR, Sergei

Greg KH

2011-01-13 18:02 UTC #5 lore raw

PATCH v2 1/1 unbreak ehci-mxc on otg port of i.MX27

Quoted 8 lines · Sergei Shtylyov

Hello.
Quoted 1 line · Eric B?nard
commit 711669e5b80b6f2d88f61ed8a9681f83d8cbd201 fixed port 0 support
Please also specify the commit summary in parens, as asked by
Linus. I think I have already asked you to do so.

No need, I can handle it.

thanks,

greg k-h

Arnaud Patard (Rtp)

2011-01-13 10:48 UTC #6 lore raw

PATCH 1/1 unbreak ehci-mxc on otg port of i.MX27

In reply to Eric Bénard

Quoted 3 lines · Eric B?nard

commit 711669e5b80b6f2d88f61ed8a9681f83d8cbd201 fixed port 0 support for i.MX51 but broke it for (at least) i.MX27 which doesn't have a usb_phy1 clock but has a pdev->id 0.

ouch. Sorry for this. I didn't thought that imx51 was an exception rather than the rule. Also, I think that imx53 may deserve same treatment but I've neither a imx53 nor its datasheet at hand to check that.

thanks,
Arnaud

Sergei Shtylyov

2011-01-13 12:17 UTC #7 lore raw

PATCH 1/1 unbreak ehci-mxc on otg port of i.MX27

In reply to Eric Bénard

Hello.

Quoted 16 lines · Eric B?nard

commit 711669e5b80b6f2d88f61ed8a9681f83d8cbd201 fixed port 0 support for i.MX51 but broke it for (at least) i.MX27 which doesn't have a usb_phy1 clock but has a pdev->id 0.
Signed-off-by Eric B?nard<[email protected]>
Cc Arnaud Patard<[email protected]>
Cc Sascha Hauer<[email protected]>
---
  drivers/usb/host/ehci-mxc.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/host/ehci-mxc.c +0 -0
index fa59b26..18eb597 100644

drivers/usb/host/ehci-mxc.c static int ehci_mxc_drv_probe(struct platform_device *pdev) · line 181

Quoted 5 lines

drivers/usb/host/ehci-mxc.c +1 -1

@@ -178,7 +178,7 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
  	}
 
  	/* "dr" device has its own clock */
-	if (pdev->id == 0) {
+	if ((cpu_is_mx51()) & (pdev->id == 0)) {

Why enclose the function call in parens at all? You also don't need parens
around 'pdev->id == 0'...

WBR, Sergei

Sergei Shtylyov

2011-01-13 12:22 UTC #8 lore raw

PATCH 1/1 unbreak ehci-mxc on otg port of i.MX27

In reply to Eric Bénard

Quoted 1 line · Eric B?nard

commit 711669e5b80b6f2d88f61ed8a9681f83d8cbd201 fixed port 0 support

Forgot to say that Linus has asked to also specify the commit summary in
parens after the commit ID.

Quoted 15 lines

for i.MX51 but broke it for (at least) i.MX27 which doesn't have a usb_phy1 clock but has a pdev->id 0.
Signed-off-by Eric B?nard<[email protected]>
Cc Arnaud Patard<[email protected]>
Cc Sascha Hauer<[email protected]>
---
  drivers/usb/host/ehci-mxc.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/host/ehci-mxc.c +0 -0
index fa59b26..18eb597 100644

drivers/usb/host/ehci-mxc.c static int ehci_mxc_drv_probe(struct platform_device *pdev) · line 181

Quoted 5 lines

drivers/usb/host/ehci-mxc.c +1 -1

@@ -178,7 +178,7 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
  	}
 
  	/* "dr" device has its own clock */
-	if (pdev->id == 0) {
+	if ((cpu_is_mx51())&  (pdev->id == 0)) {

    And it should be &&, not &.

WBR, Sergei