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

[Harman Sohanpal]

On Sat, Jun 30, 2012 at 9:28 AM, Marek Vasut <marex at denx.de> 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?
>
>> 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?
>
>> Kindly give your thoughts.
>
> Oh my brain is spinning from this :-)
>
>> In case still some changes are required, we can think upon it :)
>
> I'm really glad to hear that, let's do our best to find the best possible
> solution :-)
>
>> Thanks,
>> Harman
>
>
> Best regards,
> Marek Vasut

Also we are adding the USB module to be selected dynamically
from boards.cfg as suggested by Tom.
If such is the case I believe the same should be done for am35x.
Also from the previous mail of Tom, he seemed to be fine with
separate files.
The best approach would be to wait for Tom's reply.
Thanks for the review :)
Harman