[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Valentin Longchamp]

On various mxc boards, the intial ULPI reads resulted in a timeout
which prevented the transceiver to be identified and thus the ehci
device to be probed.

Initializing the hardware lines connected to the transceiver (through
pdata->init call) before actually enabling clocks and configuring
registers in the devices fixes this problem.

Signed-off-by: Valentin Longchamp <valentin.longchamp@epfl.ch>
---
 drivers/usb/host/ehci-mxc.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c
index 35c56f4..689b683 100644
--- a/drivers/usb/host/ehci-mxc.c
+++ b/drivers/usb/host/ehci-mxc.c
@@ -162,6 +162,15 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
 		goto err_ioremap;
 	}
 
+	/* call platform specific init function */
+	if (pdata->init) {
+		ret = pdata->init(pdev);
+		if (ret) {
+			dev_err(dev, "platform init failed\n");
+			goto err_init;
+		}
+	}
+
 	/* enable clocks */
 	priv->usbclk = clk_get(dev, "usb");
 	if (IS_ERR(priv->usbclk)) {
@@ -192,15 +201,6 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_init;
 
-	/* call platform specific init function */
-	if (pdata->init) {
-		ret = pdata->init(pdev);
-		if (ret) {
-			dev_err(dev, "platform init failed\n");
-			goto err_init;
-		}
-	}
-
 	/* most platforms need some time to settle changed IO settings */
 	mdelay(10);
 
-- 
1.6.3.3

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Daniel Mack]

On Wed, Dec 02, 2009 at 05:13:01PM +0100, Valentin Longchamp wrote:
> On various mxc boards, the intial ULPI reads resulted in a timeout
> which prevented the transceiver to be identified and thus the ehci
> device to be probed.
> 
> Initializing the hardware lines connected to the transceiver (through
> pdata->init call) before actually enabling clocks and configuring
> registers in the devices fixes this problem.

Hmm, glad to hear it fixed your problem :) However, there is no real
ULPI communication done on the viewports before the board specific
init function is called, and the timeouts that are reported come from
the transceiver probing which is called at a later point.

Even with that patch applied, one board I have here fails to initialize
the OTG port.

> Signed-off-by: Valentin Longchamp <valentin.longchamp@epfl.ch>

As it fixes a real world problem and doesn't seem to be harmful to
anyone else:

Acked-by: Daniel Mack <daniel@caiaq.de>

>  drivers/usb/host/ehci-mxc.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c
> index 35c56f4..689b683 100644
> --- a/drivers/usb/host/ehci-mxc.c
> +++ b/drivers/usb/host/ehci-mxc.c

[...]

> @@ -192,15 +201,6 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_init;
>  
> -	/* call platform specific init function */
> -	if (pdata->init) {
> -		ret = pdata->init(pdev);
> -		if (ret) {
> -			dev_err(dev, "platform init failed\n");
> -			goto err_init;
> -		}
> -	}
> -
>  	/* most platforms need some time to settle changed IO settings */
>  	mdelay(10);

You should probably also move the mdelay() and the comment then, right?
And as you're on it, the delay make more sense inside the
'if (pdata->init)' block ...

Thanks,
Daniel

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Valentin Longchamp]

Daniel Mack wrote:
> On Wed, Dec 02, 2009 at 05:13:01PM +0100, Valentin Longchamp wrote:
>> On various mxc boards, the intial ULPI reads resulted in a timeout
>> which prevented the transceiver to be identified and thus the ehci
>> device to be probed.
>>
>> Initializing the hardware lines connected to the transceiver (through
>> pdata->init call) before actually enabling clocks and configuring
>> registers in the devices fixes this problem.
> 
> Hmm, glad to hear it fixed your problem :) However, there is no real
> ULPI communication done on the viewports before the board specific
> init function is called, and the timeouts that are reported come from
> the transceiver probing which is called at a later point.

Yeah I know, there is no reason why it would help, that's why it seemed 
strange to me too. I tried to have to the same sequence as I have with 
the ISP1504 coupled with the fsl_usb2_udc where I NEVER have a problem 
with the exact same driver and it did the trick (at least for me).

Maybe there is a bad reset or something like this earlier, that causes 
the initializations on the viewport to put the transceiver in a state 
where it cannot answer the probing.
> 
> Even with that patch applied, one board I have here fails to initialize
> the OTG port.

I have not tested it on the OTG port, only on the Host 2 (that's the 
only one I use as host on our hardware).

> 
>> Signed-off-by: Valentin Longchamp <valentin.longchamp@epfl.ch>
> 
> As it fixes a real world problem and doesn't seem to be harmful to
> anyone else:
> 
> Acked-by: Daniel Mack <daniel@caiaq.de>
> 
>>  drivers/usb/host/ehci-mxc.c |   18 +++++++++---------
>>  1 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c
>> index 35c56f4..689b683 100644
>> --- a/drivers/usb/host/ehci-mxc.c
>> +++ b/drivers/usb/host/ehci-mxc.c
> 
> [...]
> 
>> @@ -192,15 +201,6 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
>>  	if (ret < 0)
>>  		goto err_init;
>>  
>> -	/* call platform specific init function */
>> -	if (pdata->init) {
>> -		ret = pdata->init(pdev);
>> -		if (ret) {
>> -			dev_err(dev, "platform init failed\n");
>> -			goto err_init;
>> -		}
>> -	}
>> -
>>  	/* most platforms need some time to settle changed IO settings */
>>  	mdelay(10);
> 
> You should probably also move the mdelay() and the comment then, right?
> And as you're on it, the delay make more sense inside the
> 'if (pdata->init)' block ...

Ok, will do that

> 
> Thanks,
> Daniel
> 

Thanks for the comments and the ack, new version coming soon.

Val

-- 
Valentin Longchamp, PhD Student, EPFL-STI-LSRO1
valentin.longchamp at epfl.ch, Phone: +41216937827
http://people.epfl.ch/valentin.longchamp
MEA3485, Station 9, CH-1015 Lausanne

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Valentin Longchamp]

On various mxc boards, the intial ULPI reads resulted in a timeout
which prevented the transceiver to be identified and thus the ehci
device to be probed.

Initializing the hardware lines connected to the transceiver (through
pdata->init call) before actually enabling clocks and configuring
registers in the devices fixes this problem.

Signed-off-by: Valentin Longchamp <valentin.longchamp@epfl.ch>
---
 drivers/usb/host/ehci-mxc.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c
index 35c56f4..23cd917 100644
--- a/drivers/usb/host/ehci-mxc.c
+++ b/drivers/usb/host/ehci-mxc.c
@@ -162,6 +162,17 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
 		goto err_ioremap;
 	}
 
+	/* call platform specific init function */
+	if (pdata->init) {
+		ret = pdata->init(pdev);
+		if (ret) {
+			dev_err(dev, "platform init failed\n");
+			goto err_init;
+		}
+		/* platforms need some time to settle changed IO settings */
+		mdelay(10);
+	}
+
 	/* enable clocks */
 	priv->usbclk = clk_get(dev, "usb");
 	if (IS_ERR(priv->usbclk)) {
@@ -192,18 +203,6 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_init;
 
-	/* call platform specific init function */
-	if (pdata->init) {
-		ret = pdata->init(pdev);
-		if (ret) {
-			dev_err(dev, "platform init failed\n");
-			goto err_init;
-		}
-	}
-
-	/* most platforms need some time to settle changed IO settings */
-	mdelay(10);
-
 	/* Initialize the transceiver */
 	if (pdata->otg) {
 		pdata->otg->io_priv = hcd->regs + ULPI_VIEWPORT_OFFSET;
-- 
1.6.3.3

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Daniel Mack]

On Thu, Dec 03, 2009 at 11:35:58AM +0100, Valentin Longchamp wrote:
> On various mxc boards, the intial ULPI reads resulted in a timeout
> which prevented the transceiver to be identified and thus the ehci
> device to be probed.
> 
> Initializing the hardware lines connected to the transceiver (through
> pdata->init call) before actually enabling clocks and configuring
> registers in the devices fixes this problem.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@epfl.ch>

Acked-by: Daniel Mack <daniel@caiaq.de>


> ---
>  drivers/usb/host/ehci-mxc.c |   23 +++++++++++------------
>  1 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c
> index 35c56f4..23cd917 100644
> --- a/drivers/usb/host/ehci-mxc.c
> +++ b/drivers/usb/host/ehci-mxc.c
> @@ -162,6 +162,17 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
>  		goto err_ioremap;
>  	}
>  
> +	/* call platform specific init function */
> +	if (pdata->init) {
> +		ret = pdata->init(pdev);
> +		if (ret) {
> +			dev_err(dev, "platform init failed\n");
> +			goto err_init;
> +		}
> +		/* platforms need some time to settle changed IO settings */
> +		mdelay(10);
> +	}
> +
>  	/* enable clocks */
>  	priv->usbclk = clk_get(dev, "usb");
>  	if (IS_ERR(priv->usbclk)) {
> @@ -192,18 +203,6 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_init;
>  
> -	/* call platform specific init function */
> -	if (pdata->init) {
> -		ret = pdata->init(pdev);
> -		if (ret) {
> -			dev_err(dev, "platform init failed\n");
> -			goto err_init;
> -		}
> -	}
> -
> -	/* most platforms need some time to settle changed IO settings */
> -	mdelay(10);
> -
>  	/* Initialize the transceiver */
>  	if (pdata->otg) {
>  		pdata->otg->io_priv = hcd->regs + ULPI_VIEWPORT_OFFSET;
> -- 
> 1.6.3.3
> 

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Eric Bénard]

Daniel Mack a ?crit :
> On Wed, Dec 02, 2009 at 05:13:01PM +0100, Valentin Longchamp wrote:
>> On various mxc boards, the intial ULPI reads resulted in a timeout
>> which prevented the transceiver to be identified and thus the ehci
>> device to be probed.
>>
>> Initializing the hardware lines connected to the transceiver (through
>> pdata->init call) before actually enabling clocks and configuring
>> registers in the devices fixes this problem.
> 
> Hmm, glad to hear it fixed your problem :) However, there is no real
> ULPI communication done on the viewports before the board specific
> init function is called, and the timeouts that are reported come from
> the transceiver probing which is called at a later point.
> 
> Even with that patch applied, one board I have here fails to initialize
> the OTG port.
> 
can you probe the DIR signal (pin 19) of the ISP1504 when you have the 
problem ?

Eric

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Alan Carvalho de Assis]

Hi Valentin,

On 12/2/09, Valentin Longchamp <valentin.longchamp@epfl.ch> wrote:
> On various mxc boards, the intial ULPI reads resulted in a timeout
> which prevented the transceiver to be identified and thus the ehci
> device to be probed.
>
> Initializing the hardware lines connected to the transceiver (through
> pdata->init call) before actually enabling clocks and configuring
> registers in the devices fixes this problem.
>

Unfortunately it didn't solve the issues on my not working board.

Also I noticed on "working" board the PSW_N pin is not going to low.
I created an ulpi with USB_OTG_DRV_VBUS and USB_OTG_DRV_VBUS_EXT.

Then I forced VBUS to high directly (connecting it to VCC, I know it
is very *dangerous*) in order power on a mouse connected to USB port.
It detected a device was attached, but didn't enumerate it:

mx27# usb 2-1: new low speed USB device using mxc-ehci and address 2
hub 2-0:1.0: unable to enumerate USB device on port 1

What you think about it?

Best Regards,

Alan

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Andy Green]

On 12/02/09 18:58, Somebody in the thread at some point said:

Hi -

> mx27# usb 2-1: new low speed USB device using mxc-ehci and address 2
> hub 2-0:1.0: unable to enumerate USB device on port 1
>
> What you think about it?

What did you do with your chip select line on the ULPI PHY?

The first iMX LiteKIT board I had was an old revision where they had 
tied the PHY chip select pin asserted.  The ULPI PHY acted braindamaged 
because glitchy junk from during poweron / reset was taken as real data 
and it never recovered during the session.

Later versions of the LiteKIT changed this to be pulled deasserted and a 
GPIO can drive it asserted after reset is all done.  After I got a later 
revision with that mod, and drove the GPIO, suddenly all was well.

Never going to get that week back though.

-Andy

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Alan Carvalho de Assis]

Hi Andy,

On 12/2/09, Andy Green <andy@warmcat.com> wrote:
>
> What did you do with your chip select line on the ULPI PHY?
>

I turn this line (CS_N) low before initializing ISP1504.

> The first iMX LiteKIT board I had was an old revision where they had
> tied the PHY chip select pin asserted.  The ULPI PHY acted braindamaged
> because glitchy junk from during poweron / reset was taken as real data
> and it never recovered during the session.
>
> Later versions of the LiteKIT changed this to be pulled deasserted and a
> GPIO can drive it asserted after reset is all done.  After I got a later
> revision with that mod, and drove the GPIO, suddenly all was well.
>

I was thinking it could be some hw issues, but this is not, because
using old kernel everything works fine.

Best Regards,

Alan

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Andy Green]

On 12/02/09 20:25, Somebody in the thread at some point said:

Hi Alan -

>> What did you do with your chip select line on the ULPI PHY?
>
> I turn this line (CS_N) low before initializing ISP1504.

It's more a question about what that input on the PHY chip sees during 
powerup / reset... it needs to be nicely and definitely high the whole 
time so it will ignore what happens then.  Is it pulled up, or just 
connected to the iMX GPIO?  iMX GPIO alone may not be enough.  They 
mostly default to 100K pullup but even that may not be enabled early in 
the powerup action.

A 10K to your IO voltage rail on CS_N solved it for the LiteKIT and on 
the boards we have made.

>> The first iMX LiteKIT board I had was an old revision where they had
>> tied the PHY chip select pin asserted.  The ULPI PHY acted braindamaged
>> because glitchy junk from during poweron / reset was taken as real data
>> and it never recovered during the session.
>>
>> Later versions of the LiteKIT changed this to be pulled deasserted and a
>> GPIO can drive it asserted after reset is all done.  After I got a later
>> revision with that mod, and drove the GPIO, suddenly all was well.
>
> I was thinking it could be some hw issues, but this is not, because
> using old kernel everything works fine.

OK.

But on the old LiteKIT where it was tied low, it was erratic: worked 
sometimes and not others depending on the random details of what went 
where in what order during powerup each time.  If you didn't see it work 
on this older kernel over a bunch of powercycles keep the idea in a back 
pocket :-)

-Andy

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Valentin Longchamp]

Hi all,

Andy Green wrote:
> On 12/02/09 20:25, Somebody in the thread at some point said:
> 
>>> What did you do with your chip select line on the ULPI PHY?
>> I turn this line (CS_N) low before initializing ISP1504.
> 
> It's more a question about what that input on the PHY chip sees during 
> powerup / reset... it needs to be nicely and definitely high the whole 
> time so it will ignore what happens then.  Is it pulled up, or just 
> connected to the iMX GPIO?  iMX GPIO alone may not be enough.  They 
> mostly default to 100K pullup but even that may not be enabled early in 
> the powerup action.
> 
> A 10K to your IO voltage rail on CS_N solved it for the LiteKIT and on 
> the boards we have made.
> 

I just wanted to point out that we also have a 10K pull-up on the CS_N 
on our boards, it helps a lot.

Val

-- 
Valentin Longchamp, PhD Student, EPFL-STI-LSRO1
valentin.longchamp at epfl.ch, Phone: +41216937827
http://people.epfl.ch/valentin.longchamp
MEA3485, Station 9, CH-1015 Lausanne

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Alan Carvalho de Assis]

Hi Andy

On 12/2/09, Andy Green <andy@warmcat.com> wrote:
> It's more a question about what that input on the PHY chip sees during
> powerup / reset... it needs to be nicely and definitely high the whole
> time so it will ignore what happens then.  Is it pulled up, or just
> connected to the iMX GPIO?  iMX GPIO alone may not be enough.  They
> mostly default to 100K pullup but even that may not be enabled early in
> the powerup action.
>

The CS_N pin is connected directly to GPIO (PA24) with no pull-up.

> A 10K to your IO voltage rail on CS_N solved it for the LiteKIT and on
> the boards we have made.
>

I place a 10K resistor to pull-up and changed CS_N to low at usb
platform init and it worked fine!

Thank you very much!

Best Regards,

Alan

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Andy Green]

On 12/03/09 14:45, Somebody in the thread at some point said:

Hi Alan -

> On 12/2/09, Andy Green<andy@warmcat.com>  wrote:
>> It's more a question about what that input on the PHY chip sees during
>> powerup / reset... it needs to be nicely and definitely high the whole
>> time so it will ignore what happens then.  Is it pulled up, or just
>> connected to the iMX GPIO?  iMX GPIO alone may not be enough.  They
>> mostly default to 100K pullup but even that may not be enabled early in
>> the powerup action.
>
> The CS_N pin is connected directly to GPIO (PA24) with no pull-up.
>
>> A 10K to your IO voltage rail on CS_N solved it for the LiteKIT and on
>> the boards we have made.
>
> I place a 10K resistor to pull-up and changed CS_N to low at usb
> platform init and it worked fine!
>
> Thank you very much!

You're super welcome... glad that time spent wandering in the desert was 
able to help someone else in the end :-)

-Andy

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Alan Carvalho de Assis]

On 12/3/09, Andy Green <andy@warmcat.com> wrote:
>
> You're super welcome... glad that time spent wandering in the desert was
> able to help someone else in the end :-)
>

Yes, it was a big saga, but finally it is working very nice.

Now we need few things to get a good imx27 support in the kernel. I
think the camera and the VPU support are the most important drivers
missing.

I think mx1_camera.c could be modified to support mx27 CSI, but to the
VPU I have no idea.

Best Regards,

Alan

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Daniel Mack]

On Thu, Dec 03, 2009 at 02:21:17PM -0200, Alan Carvalho de Assis wrote:
> On 12/3/09, Andy Green <andy@warmcat.com> wrote:
> >
> > You're super welcome... glad that time spent wandering in the desert was
> > able to help someone else in the end :-)
> >
> 
> Yes, it was a big saga, but finally it is working very nice.

Good to hear :)

> Now we need few things to get a good imx27 support in the kernel. I
> think the camera and the VPU support are the most important drivers
> missing.

Do you have any patches ready for MX27 EHCI?

Daniel

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Alan Carvalho de Assis]

Hi Daniel,

On 12/3/09, Daniel Mack <daniel@caiaq.de> wrote:
> On Thu, Dec 03, 2009 at 02:21:17PM -0200, Alan Carvalho de Assis wrote:
>> On 12/3/09, Andy Green <andy@warmcat.com> wrote:
>> >
>> > You're super welcome... glad that time spent wandering in the desert was
>> > able to help someone else in the end :-)
>> >
>>
>> Yes, it was a big saga, but finally it is working very nice.
>
> Good to hear :)
>
>> Now we need few things to get a good imx27 support in the kernel. I
>> think the camera and the VPU support are the most important drivers
>> missing.
>
> Do you have any patches ready for MX27 EHCI?
>

Yes, I'm preparing a patch to add usb support on TD60.

Best Regards,

Alan

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[Robert Schwebel]

On Thu, Dec 03, 2009 at 02:21:17PM -0200, Alan Carvalho de Assis wrote:
> On 12/3/09, Andy Green <andy@warmcat.com> wrote:
> >
> > You're super welcome... glad that time spent wandering in the desert was
> > able to help someone else in the end :-)
> >
> 
> Yes, it was a big saga, but finally it is working very nice.
> 
> Now we need few things to get a good imx27 support in the kernel. I
> think the camera and the VPU support are the most important drivers
> missing.
> 
> I think mx1_camera.c could be modified to support mx27 CSI, but to the
> VPU I have no idea.

Sascha has CSI drivers for the MX27, but I'm not sure in which state
they are.

rsc
-- 
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 |

[PATCH 1/1] fix occasional ULPI timeouts with ehci-mxc

[javier Martin]

> >
> > I think mx1_camera.c could be modified to support mx27 CSI, but to the
> > VPU I have no idea.
>
> Sascha has CSI drivers for the MX27, but I'm not sure in which state
> they are.
>

I think a good start point would be this driver "mx2_camera.c" that can be
found in phytec GIT repository. I do not know what exactly are the lacks for
this driver not going mainline, what I can say is that this driver uses PrP
channel 1 which can be useful for capturing video. However if you want to
encode using VPU this driver should be modified to choose PrP channel 2 so
that it uses YUV planar format needed by VPU.

VPU support is more complicated. I don't think there is a simmilar case
already in mainline. Maybe integrating VPU support in a host camera driver
(mx2_camera.c) would be admitted, although AFAIK V4L does not have H.264
data type it could be added if needed.

-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20091204/830e6a24/attachment-0001.htm>