[U-Boot] [PATCH v4 0/3] AM335x: Add USB support in u-boot.

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/29/2012 08:58 PM, Marek Vasut wrote:
> Dear Harman Sohanpal,
> 
>> On Sat, Jun 30, 2012 at 6:15 AM, Marek Vasut <marex at denx.de>
>> wrote:
>>> Dear Harman Sohanpal,
>>> 
>>>> These patches add USB support in u-boot for AM335x. The
>>>> support for host or device is selected depending on the
>>>> config selected from boards.cfg file. Host mode is selected
>>>> for USB1 and device mode is selected for USB0. Base addresses
>>>> are selected accordingly.
>>>> 
>>>> Gene Zarkhin (1): AM335x : Add USB support for AM335x in
>>>> u-boot
>>>> 
>>>> Harman Sohanpal (2): AM335x : Configs to add USB host
>>>> support. musb_udc : Fix compile warning.
>>> 
>>> Dumb question ... but, can this not be made part of am35x USB
>>> ?
>> 
>> Hi Marek, Well this can always be made part of am35x.c. But there
>> would be a lot of changes required in the file.
> 
> Why? They use different IP block or something?

It's not an identical block so register maps have changed slightly.
More so if we get a later conversion of the other similar parts
(da850.c, davinci.c, omap3.c).

>> And also I believe it would not make much sense. It would require
>> ifdefs at a lot of places. Best example I can give to support
>> what i said is that the control register is at an offset of 4 in
>> am35x and 14 in am335x.
> 
> So what, define two sets of register structures and pass them
> according to CPU ID.
> 
>> I am sure adding an ifdef at that place would not seem good to
>> you to change address from 4 to 14 acc to platform.
> 
> well ... struct regs_a { uint32_t padding[?]; uint32_t reg; ... };
> 
> struct regs_b { uint32_t reg; ... };
> 
> Create IO accessors ... like ... my_usb_writel() and my_usb_readl()
> to read and write the registers. And those accessors will cover the
> differences. Or is there more?
> 
>> Is there much pain to add these 2 files?
> 
> Yes, duplication of code is always bad.
> 
>> In my opinion we must need to have a separate file for this.
> 
> Why? If it's only about the registers, won't the approach above
> work?
> 
>> This is as per my understanding. It could also cause confusions
>> to some due to name. maybe :)
> 
> I'm no omap guru, Tom is. Tom?

I think what we need to do is take a shot at converting am35x.c and
am335x.c into a 'ti_musb.[ch]' and per-family header files that give
the register layout, etc, etc.  We need to see how maintainable or not
such a setup will be.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJP8bcGAAoJENk4IS6UOR1WaSYP/0v/+GcjCUL08ZmjQANyavZf
Grl3OYjx4p7IwsBgOXCE14SKS5UKabJywoUil25O+T0ZM1ubzHyGzHuszYb7WfsZ
e/hUWwMATw7f+qnQAOYR6AGDQgbvS1aFWy01+CT4xLPmU0XO6kBrmgRuhmml9Uf1
RegM1+TrAVtHA9VPkVNvIok7598LIQhjnnq5ey9fMPYuqL4LqkBGl7ui/+28c59g
rbFEkjv52jwTPiDT8PYYSRQlWv5pgoMmm/eW8HoXUV3V4iPBU1xaW79brXNV3ZKw
FTstoAbwdGNteyBGJtmvBrwP0EL2dAsdIF+N7XstZ7UNfvRmdM+xo+1ZK0T8f/Xz
J3s5yVo4tQcwQxrUtZCaTcFNGjkcybb28oSRy5jsONPTMggWC68aqsB/RjDZUKqP
AG49caTx1aGspyHc3FLRaaSnRb0xS3JosDz235mYfwqW67tsDUm7Zqak3VlrShk0
7poY8+9WmePtdaqMGrC98vz+SMTaepgjoUO65NJGh2souVEO6nPf2+hy/3/B36Gi
9iXx+qRTKBcxxOmla/kxN0/NAHL+vTpA8nAjZeqra5dVvH2LKJUgTst4XSp7fX3E
5NljiVj9nxsxgni12XJgyYVjRIve3pFBW32P4VoDCsDvi1Q0T0SZrH4DAg7E3qLU
iX0bhFBvMYrQXxde8tsf
=TS6u
-----END PGP SIGNATURE-----