[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Nikolay Dimitrov]

Hi Igor,

My personal opinion is that unless you intend to run the binary on 
multiple IMX6 variants, there's no
need to do expensive checks in runtime, when you can do the same at 
compile-time. For me it's the
same as choosing puts() vs printf() - you know at compile time whether 
you need to print arguments
or not, so same with the USB controller base address - you know in 
advance that you target a specific
CPU variant and not the other ones.

To a certain extent I agree that it would be awesome to have the same 
code running on all IMX6
variants, but the run-time checks will increase somewhat the binary 
footprint and U-Boot community has
already gone to great efforts to remove unnecessary bloat. My personal 
assumption is that such generic
approach would be more tolerated in the Linux kernel.

Kind regards,
Nikolay


On 6/16/2014 1:00 PM, u-boot-request at lists.denx.de wrote:
> Message: 30 Date: Mon, 16 Jun 2014 10:05:08 +0300 From: Igor Grinberg 
> <grinberg@compulab.co.il> Subject: Re: [U-Boot] [PATCH 1/6] usb: ehci: 
> mx6: Add support for i.MX6SL To: Otavio Salvador 
> <otavio@ossystems.com.br>, U-Boot Mailing List <u-boot@lists.denx.de> 
> Cc: Marek Vasut <marex@denx.de>, Fabio Estevam 
> <fabio.estevam@freescale.com> Message-ID: 
> <539E9724.4020809@compulab.co.il> Content-Type: text/plain; 
> charset=ISO-8859-1 Hmmm... Can't this be done dynamically? I mean... 
> you can check the SoC type in runtime, right? And then substitute the 
> correct address in a variable or so... I would really prefer such kind 
> of things done in runtime.
> -- Regards, Igor.

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Marek Vasut]

On Monday, June 16, 2014 at 04:33:09 PM, Nikolay Dimitrov wrote:
> Hi Igor,

Please do not top-post.

> My personal opinion is that unless you intend to run the binary on
> multiple IMX6 variants, there's no
> need to do expensive checks in runtime, when you can do the same at
> compile-time.

With the upcoming DM implementation, that's what we plan. Moreover, the checks 
are not expensive, you just decide the address based on the CPU model and assign 
it into some variable. You then access the registers via that variable + offset, 
as usual.

> For me it's the
> same as choosing puts() vs printf() - you know at compile time whether
> you need to print arguments
> or not, so same with the USB controller base address - you know in
> advance that you target a specific
> CPU variant and not the other ones.
> 
> To a certain extent I agree that it would be awesome to have the same
> code running on all IMX6
> variants, but the run-time checks will increase somewhat the binary
> footprint and U-Boot community has
> already gone to great efforts to remove unnecessary bloat. My personal
> assumption is that such generic
> approach would be more tolerated in the Linux kernel.
> 
> Kind regards,
> Nikolay
> 
> On 6/16/2014 1:00 PM, u-boot-request at lists.denx.de wrote:
> > Message: 30 Date: Mon, 16 Jun 2014 10:05:08 +0300 From: Igor Grinberg
> > <grinberg@compulab.co.il> Subject: Re: [U-Boot] [PATCH 1/6] usb: ehci:
> > mx6: Add support for i.MX6SL To: Otavio Salvador
> > <otavio@ossystems.com.br>, U-Boot Mailing List <u-boot@lists.denx.de>
> > Cc: Marek Vasut <marex@denx.de>, Fabio Estevam
> > <fabio.estevam@freescale.com> Message-ID:
> > <539E9724.4020809@compulab.co.il> Content-Type: text/plain;
> > charset=ISO-8859-1 Hmmm... Can't this be done dynamically? I mean...
> > you can check the SoC type in runtime, right? And then substitute the
> > correct address in a variable or so... I would really prefer such kind
> > of things done in runtime.
> > -- Regards, Igor.

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Nikolay Dimitrov]

Hi Marek,

On 6/16/2014 5:53 PM, Marek Vasut wrote:
> Please do not top-post. 
Please excuse me - I was unable to find posting guidelines for the 
mailing list, so just posted by habit (last time I had to post to 
another list, they wanted me to top-post).

> With the upcoming DM implementation, that's what we plan. Moreover, 
> the checks are not expensive, you just decide the address based on the 
> CPU model and assign it into some variable. You then access the 
> registers via that variable + offset, as usual. 
I see. Would be great to see this compatibility coming.

> Best regards,
> Marek Vasut
Thanks for taking your time to reply.

Kind regards,
Nikolay

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Marek Vasut]

On Tuesday, June 17, 2014 at 06:21:05 PM, Nikolay Dimitrov wrote:
> Hi Marek,
> 
> On 6/16/2014 5:53 PM, Marek Vasut wrote:
> > Please do not top-post.
> 
> Please excuse me - I was unable to find posting guidelines for the
> mailing list, so just posted by habit (last time I had to post to
> another list, they wanted me to top-post).

Then follow RFC1855, quote:

"
    - If you are sending a reply to a message or a posting be sure you
      summarize the original at the top of the message, or include just
      enough text of the original to give a context.  This will make
      sure readers understand when they start to read your response.
"
;-)

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Igor Grinberg]

Hi Nikolay,

On 06/16/14 17:33, Nikolay Dimitrov wrote:
> Hi Igor,
> 
> My personal opinion is that unless you intend to run the binary on multiple IMX6 variants,

That is exactly what we do already (code is on the way) and IMO what
we should aim for.

> there's no
> need to do expensive checks in runtime, when you can do the same at compile-time.

Why do you think it is expensive? Any benchmarks?
Also, the solution is what Marek already said, in short:
do the check once and store the result for future use...

> For me it's the
> same as choosing puts() vs printf() - you know at compile time whether you need to print arguments
> or not, so same with the USB controller base address - you know in advance that you target a specific
> CPU variant and not the other ones.

For me it is just an artificial complication which prevents single binary for
i.MX6 based boards.
Don't get me wrong, I think that in your board code you can choose which
approach you want, whether it will be single or multi binary, but this
is i.MX6 (and possibly future i.MX*) USB code which can be used on many
i.MX6 boards.

> 
> To a certain extent I agree that it would be awesome to have the same code running on all IMX6
> variants, but the run-time checks will increase somewhat the binary footprint and U-Boot community has
> already gone to great efforts to remove unnecessary bloat.

Again, what are we talking about? A couple of bytes?

> My personal assumption is that such generic
> approach would be more tolerated in the Linux kernel.

For Linux kernel this is the only acceptable way now.


-- 
Regards,
Igor.

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Nikolay Dimitrov]

Hi Igor,

On 6/17/2014 9:26 AM, Igor Grinberg wrote:
> That is exactly what we do already (code is on the way) and IMO what 
> we should aim for.
I really didn't knew this in the beginning before seeing your answers, 
this would be definitely easier
to support.

> For me it is just an artificial complication which prevents single binary for
> i.MX6 based boards.
> Don't get me wrong, I think that in your board code you can choose which
> approach you want, whether it will be single or multi binary, but this
> is i.MX6 (and possibly future i.MX*) USB code which can be used on many
> i.MX6 boards.
I definitely like the idea, no argument on this.

> Again, what are we talking about? A couple of bytes? 
I think it will cost 22 additional bytes per check. Here's a rough example:

(Code that just assigns the address to the struct pointer -> 10 bytes)
     p = (mx6_usb_t*)0x55667788;
     8380:       f247 7388       movw    r3, #30600      ; 0x7788
     8384:       f2c5 5366       movt    r3, #21862      ; 0x5566
     8388:       60fb            str     r3, [r7, #12]


(Code that checks in runtime the CPU type -> 32 bytes)
     if (is_imx6q())
     8380:       f7ff ffee       bl      8360 <is_imx6q>
     8384:       4603            mov     r3, r0
     8386:       2b00            cmp     r3, #0
     8388:       d005            beq.n   8396 <main+0x26>
     {
         p = (mx6_usb_t*)0x11223344;
     838a:       f243 3344       movw    r3, #13124      ; 0x3344
     838e:       f2c1 1322       movt    r3, #4386       ; 0x1122
     8392:       60fb            str     r3, [r7, #12]
     8394:       e004            b.n     83a0 <main+0x30>
     }
     else
     {
         p = (mx6_usb_t*)0x55667788;
     8396:       f247 7388       movw    r3, #30600      ; 0x7788
     839a:       f2c5 5366       movt    r3, #21862      ; 0x5566
     839e:       60fb            str     r3, [r7, #12]
     }

If I assume that we have 20 sub-systems in U-Boot, and each needs to 
check the CPU type
in 10 places, this makes 4400 bytes difference, which is roughly the 
size of a moderately
small driver in U-Boot. I need to say that I'm in no way expert in ARM 
assembly, so please
feel free to point out any mistakes in my assumptions.

Please don't get me wrong - I don't want to argue at all. I was just 
wondering about this topic
and decided to check with you guys just to be sure.

Kind regards,
Nikolay

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Igor Grinberg]

Hi Nikolay,

On 06/17/14 20:49, Nikolay Dimitrov wrote:
> Hi Igor,
> 
> On 6/17/2014 9:26 AM, Igor Grinberg wrote:
>> That is exactly what we do already (code is on the way) and IMO what we should aim for.
> I really didn't knew this in the beginning before seeing your answers, this would be definitely easier
> to support.
> 
>> For me it is just an artificial complication which prevents single binary for
>> i.MX6 based boards.
>> Don't get me wrong, I think that in your board code you can choose which
>> approach you want, whether it will be single or multi binary, but this
>> is i.MX6 (and possibly future i.MX*) USB code which can be used on many
>> i.MX6 boards.
> I definitely like the idea, no argument on this.
> 
>> Again, what are we talking about? A couple of bytes? 
> I think it will cost 22 additional bytes per check. Here's a rough example:
> 
> (Code that just assigns the address to the struct pointer -> 10 bytes)
>     p = (mx6_usb_t*)0x55667788;
>     8380:       f247 7388       movw    r3, #30600      ; 0x7788
>     8384:       f2c5 5366       movt    r3, #21862      ; 0x5566
>     8388:       60fb            str     r3, [r7, #12]
> 
> 
> (Code that checks in runtime the CPU type -> 32 bytes)
>     if (is_imx6q())
>     8380:       f7ff ffee       bl      8360 <is_imx6q>
>     8384:       4603            mov     r3, r0
>     8386:       2b00            cmp     r3, #0
>     8388:       d005            beq.n   8396 <main+0x26>
>     {
>         p = (mx6_usb_t*)0x11223344;
>     838a:       f243 3344       movw    r3, #13124      ; 0x3344
>     838e:       f2c1 1322       movt    r3, #4386       ; 0x1122
>     8392:       60fb            str     r3, [r7, #12]
>     8394:       e004            b.n     83a0 <main+0x30>
>     }
>     else
>     {
>         p = (mx6_usb_t*)0x55667788;
>     8396:       f247 7388       movw    r3, #30600      ; 0x7788
>     839a:       f2c5 5366       movt    r3, #21862      ; 0x5566
>     839e:       60fb            str     r3, [r7, #12]
>     }
> 
> If I assume that we have 20 sub-systems in U-Boot, and each needs to check the CPU type
> in 10 places, this makes 4400 bytes difference, which is roughly the size of a moderately
> small driver in U-Boot.

This looks like a very rough estimation.
It also can be improved by various ways.

Anyway, I think that even 4K addition is an acceptable compromise for a
single binary.

> I need to say that I'm in no way expert in ARM assembly, so please
> feel free to point out any mistakes in my assumptions.
> 
> Please don't get me wrong - I don't want to argue at all. I was just wondering about this topic
> and decided to check with you guys just to be sure.

No problem.


-- 
Regards,
Igor.

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Otavio Salvador]

The i.MX6SL has a different base address for the controller. This
patch adapts the driver to support the different base address for this
case.

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---

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

diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index c0a557b..5ba1c5e 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -53,6 +53,12 @@
 #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
 #define UCMD_RESET		(1 << 1) /* controller reset */
 
+#ifdef CONFIG_MX6SL
+#define USB_BASE_ADDR		USBO2H_USB_BASE_ADDR
+#else
+#define USB_BASE_ADDR		USBOH3_USB_BASE_ADDR
+#endif
+
 static const unsigned phy_bases[] = {
 	USB_PHY0_BASE_ADDR,
 	USB_PHY1_BASE_ADDR,
@@ -174,7 +180,7 @@ struct usbnc_regs {
 
 static void usb_oc_config(int index)
 {
-	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USBOH3_USB_BASE_ADDR +
+	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
 			USB_OTHERREGS_OFFSET);
 	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]);
 	u32 val;
@@ -207,7 +213,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 		struct ehci_hccr **hccr, struct ehci_hcor **hcor)
 {
 	enum usb_init_type type;
-	struct usb_ehci *ehci = (struct usb_ehci *)(USBOH3_USB_BASE_ADDR +
+	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
 		(0x200 * index));
 
 	if (index > 3)
-- 
2.0.0

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Marek Vasut]

On Monday, June 16, 2014 at 02:46:48 AM, Otavio Salvador wrote:
> The i.MX6SL has a different base address for the controller. This
> patch adapts the driver to support the different base address for this
> case.
> 
> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> ---
> 
>  drivers/usb/host/ehci-mx6.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> index c0a557b..5ba1c5e 100644
> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c
> @@ -53,6 +53,12 @@
>  #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
>  #define UCMD_RESET		(1 << 1) /* controller reset */
> 
> +#ifdef CONFIG_MX6SL
> +#define USB_BASE_ADDR		USBO2H_USB_BASE_ADDR
> +#else
> +#define USB_BASE_ADDR		USBOH3_USB_BASE_ADDR
> +#endif

Can the CPU type be detected at runtime so we don't need this ugly ifdef ?

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Otavio Salvador]

On Sun, Jun 15, 2014 at 9:49 PM, Marek Vasut <marex@denx.de> wrote:
> On Monday, June 16, 2014 at 02:46:48 AM, Otavio Salvador wrote:
>> The i.MX6SL has a different base address for the controller. This
>> patch adapts the driver to support the different base address for this
>> case.
>>
>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
...
> Can the CPU type be detected at runtime so we don't need this ugly ifdef ?

The SoloLite is not pin-compatible so it is unable to be used in same binary.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Marek Vasut]

On Monday, June 16, 2014 at 03:11:03 AM, Otavio Salvador wrote:
> On Sun, Jun 15, 2014 at 9:49 PM, Marek Vasut <marex@denx.de> wrote:
> > On Monday, June 16, 2014 at 02:46:48 AM, Otavio Salvador wrote:
> >> The i.MX6SL has a different base address for the controller. This
> >> patch adapts the driver to support the different base address for this
> >> case.
> >> 
> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> 
> ...
> 
> > Can the CPU type be detected at runtime so we don't need this ugly ifdef
> > ?
> 
> The SoloLite is not pin-compatible so it is unable to be used in same
> binary.

You didn't answer my question ;)

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Igor Grinberg]

Hi Otavio,

On 06/16/14 03:46, Otavio Salvador wrote:
> The i.MX6SL has a different base address for the controller. This
> patch adapts the driver to support the different base address for this
> case.
> 
> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> ---
> 
>  drivers/usb/host/ehci-mx6.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> index c0a557b..5ba1c5e 100644
> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c
> @@ -53,6 +53,12 @@
>  #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
>  #define UCMD_RESET		(1 << 1) /* controller reset */
>  
> +#ifdef CONFIG_MX6SL
> +#define USB_BASE_ADDR		USBO2H_USB_BASE_ADDR
> +#else
> +#define USB_BASE_ADDR		USBOH3_USB_BASE_ADDR
> +#endif
> +

Hmmm... Can't this be done dynamically?
I mean... you can check the SoC type in runtime, right?
And then substitute the correct address in a variable or so...
I would really prefer such kind of things done in runtime.

>  static const unsigned phy_bases[] = {
>  	USB_PHY0_BASE_ADDR,
>  	USB_PHY1_BASE_ADDR,
> @@ -174,7 +180,7 @@ struct usbnc_regs {
>  
>  static void usb_oc_config(int index)
>  {
> -	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USBOH3_USB_BASE_ADDR +
> +	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
>  			USB_OTHERREGS_OFFSET);
>  	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]);
>  	u32 val;
> @@ -207,7 +213,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
>  		struct ehci_hccr **hccr, struct ehci_hcor **hcor)
>  {
>  	enum usb_init_type type;
> -	struct usb_ehci *ehci = (struct usb_ehci *)(USBOH3_USB_BASE_ADDR +
> +	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
>  		(0x200 * index));
>  
>  	if (index > 3)
> 

-- 
Regards,
Igor.

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Otavio Salvador]

On Mon, Jun 16, 2014 at 4:05 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Otavio,
>
> On 06/16/14 03:46, Otavio Salvador wrote:
>> The i.MX6SL has a different base address for the controller. This
>> patch adapts the driver to support the different base address for this
>> case.
>>
>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>> ---
>>
>>  drivers/usb/host/ehci-mx6.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
>> index c0a557b..5ba1c5e 100644
>> --- a/drivers/usb/host/ehci-mx6.c
>> +++ b/drivers/usb/host/ehci-mx6.c
>> @@ -53,6 +53,12 @@
>>  #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
>>  #define UCMD_RESET           (1 << 1) /* controller reset */
>>
>> +#ifdef CONFIG_MX6SL
>> +#define USB_BASE_ADDR                USBO2H_USB_BASE_ADDR
>> +#else
>> +#define USB_BASE_ADDR                USBOH3_USB_BASE_ADDR
>> +#endif
>> +
>
> Hmmm... Can't this be done dynamically?
> I mean... you can check the SoC type in runtime, right?
> And then substitute the correct address in a variable or so...
> I would really prefer such kind of things done in runtime.

I will check about changing it for v2.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Stefano Babic]

Hi Otavio,

On 16/06/2014 02:46, Otavio Salvador wrote:
> The i.MX6SL has a different base address for the controller. This
> patch adapts the driver to support the different base address for this
> case.
> 
> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> ---
> 
>  drivers/usb/host/ehci-mx6.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> index c0a557b..5ba1c5e 100644
> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c
> @@ -53,6 +53,12 @@
>  #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
>  #define UCMD_RESET		(1 << 1) /* controller reset */
>  
> +#ifdef CONFIG_MX6SL
> +#define USB_BASE_ADDR		USBO2H_USB_BASE_ADDR
> +#else
> +#define USB_BASE_ADDR		USBOH3_USB_BASE_ADDR
> +#endif
> +

What about using the is_cpu_type() function, recently added together
with SPL support ? I think we have reached, thank to Tim's patches, the
goal to have a single image for different Freescale's mx6 variations. It
is pity to go back and decide at compile time which SOC is running.

>  static const unsigned phy_bases[] = {
>  	USB_PHY0_BASE_ADDR,
>  	USB_PHY1_BASE_ADDR,
> @@ -174,7 +180,7 @@ struct usbnc_regs {
>  
>  static void usb_oc_config(int index)
>  {
> -	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USBOH3_USB_BASE_ADDR +
> +	struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR +
>  			USB_OTHERREGS_OFFSET);
>  	void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]);
>  	u32 val;
> @@ -207,7 +213,7 @@ int ehci_hcd_init(int index, enum usb_init_type init,
>  		struct ehci_hccr **hccr, struct ehci_hcor **hcor)
>  {
>  	enum usb_init_type type;
> -	struct usb_ehci *ehci = (struct usb_ehci *)(USBOH3_USB_BASE_ADDR +
> +	struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR +
>  		(0x200 * index));
>  
>  	if (index > 3)
> 

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

[Otavio Salvador]

On Tue, Jun 17, 2014 at 11:56 AM, Stefano Babic <sbabic@denx.de> wrote:
> Hi Otavio,
>
> On 16/06/2014 02:46, Otavio Salvador wrote:
>> The i.MX6SL has a different base address for the controller. This
>> patch adapts the driver to support the different base address for this
>> case.
>>
>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>> ---
>>
>>  drivers/usb/host/ehci-mx6.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
>> index c0a557b..5ba1c5e 100644
>> --- a/drivers/usb/host/ehci-mx6.c
>> +++ b/drivers/usb/host/ehci-mx6.c
>> @@ -53,6 +53,12 @@
>>  #define UCMD_RUN_STOP           (1 << 0) /* controller run/stop */
>>  #define UCMD_RESET           (1 << 1) /* controller reset */
>>
>> +#ifdef CONFIG_MX6SL
>> +#define USB_BASE_ADDR                USBO2H_USB_BASE_ADDR
>> +#else
>> +#define USB_BASE_ADDR                USBOH3_USB_BASE_ADDR
>> +#endif
>> +
>
> What about using the is_cpu_type() function, recently added together
> with SPL support ? I think we have reached, thank to Tim's patches, the
> goal to have a single image for different Freescale's mx6 variations. It
> is pity to go back and decide at compile time which SOC is running.

Yes; I will do it for v2.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750