Patchwork [U-Boot,6/9] imx: usb: There is no such register

login
register
mail settings
Submitter Timo Ketola
Date April 18, 2012, 7:57 a.m.
Message ID <1334735852-23415-7-git-send-email-timo@exertus.fi>
Download mbox | patch
Permalink /patch/153429/
State Superseded
Delegated to: Stefano Babic
Headers show

Comments

Timo Ketola - April 18, 2012, 7:57 a.m.
The reference manual of i.MX25 (nor i.MX31) does not define such register.
This seems to access read only UH2_CAPLENGTH register (if
CONFIG_MXC_USB_PORT is zero).

Signed-off-by: Timo Ketola <timo@exertus.fi>
---
 drivers/usb/host/ehci-mxc.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
Stefano Babic - April 18, 2012, 9:05 a.m.
On 18/04/2012 09:57, Timo Ketola wrote:
> The reference manual of i.MX25 (nor i.MX31) does not define such register.
> This seems to access read only UH2_CAPLENGTH register (if
> CONFIG_MXC_USB_PORT is zero).
> 
> Signed-off-by: Timo Ketola <timo@exertus.fi>

Hi Timo,

> ---
>  drivers/usb/host/ehci-mxc.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c
> index 65f40a4..6f4df58 100644
> --- a/drivers/usb/host/ehci-mxc.c
> +++ b/drivers/usb/host/ehci-mxc.c
> @@ -126,8 +126,6 @@ int ehci_hcd_init(void)
>  			HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
>  	setbits_le32(&ehci->usbmode, CM_HOST);
>  #if defined(CONFIG_MX31) || defined(CONFIG_MX25)

As far as I can see, only MX31 and MX25 boards are using this file.
Other i.MX have its own initialization file. So #if defined(CONFIG_MX31)
|| defined(CONFIG_MX25) is always true.

However, where is this code ? In current u-boot I see only #if
defined(CONFIG_MX31) at this line. Is it your patch correct ?

> -	setbits_le32(&ehci->control, USB_EN);
> -

As far as I can see, it tries to overwrite a capability register, that
is for our luck read-only. Good catch !

Best regards,
Stefano Babic
Timo Ketola - April 18, 2012, 9:15 a.m.
On 18.04.2012 12:05, Stefano Babic wrote:
> As far as I can see, only MX31 and MX25 boards are using this file.
> Other i.MX have its own initialization file. So #if defined(CONFIG_MX31)
> || defined(CONFIG_MX25) is always true.

So, would it be OK to remove this check altogether?

> However, where is this code ? In current u-boot I see only #if
> defined(CONFIG_MX31) at this line. Is it your patch correct ?

My previous patch 5 touched that one.

>> -	setbits_le32(&ehci->control, USB_EN);
>> -
>
> As far as I can see, it tries to overwrite a capability register, that
> is for our luck read-only. Good catch !

Thanks.

--

Timo
Stefano Babic - April 18, 2012, 10:32 a.m.
On 18/04/2012 11:15, Timo Ketola wrote:
> On 18.04.2012 12:05, Stefano Babic wrote:
>> As far as I can see, only MX31 and MX25 boards are using this file.
>> Other i.MX have its own initialization file. So #if defined(CONFIG_MX31)
>> || defined(CONFIG_MX25) is always true.
> 
> So, would it be OK to remove this check altogether?

Yes, I think so - if the file is compiled only by i.MX25 or i.MX31
boards, it makes no sense.

Best regards,
Stefano Babic

Patch

diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c
index 65f40a4..6f4df58 100644
--- a/drivers/usb/host/ehci-mxc.c
+++ b/drivers/usb/host/ehci-mxc.c
@@ -126,8 +126,6 @@  int ehci_hcd_init(void)
 			HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
 	setbits_le32(&ehci->usbmode, CM_HOST);
 #if defined(CONFIG_MX31) || defined(CONFIG_MX25)
-	setbits_le32(&ehci->control, USB_EN);
-
 	__raw_writel(CONFIG_MXC_USB_PORTSC, &ehci->portsc);
 #endif
 	mxc_set_usbcontrol(CONFIG_MXC_USB_PORT, CONFIG_MXC_USB_FLAGS);