[U-Boot] [PATCH 2/3] ARM: Tegra: USB: Add driver support for Tegra30/Tegra114

[Tom Warren]

Adding Jim back into the conversation since he got dropped a couple of
emails back.


On Wed, May 1, 2013 at 4:20 PM, Tom Warren <twarren.nvidia at gmail.com> wrote:

>
>
>
> On Wed, May 1, 2013 at 12:33 PM, Marek Vasut <marex at denx.de> wrote:
>
>> Dear Tom Rini,
>>
>> > On Wed, May 01, 2013 at 09:16:45AM -0700, Tom Warren wrote:
>> > > Tom,
>> > >
>> > > On Tue, Apr 30, 2013 at 10:20 AM, Tom Rini <trini at ti.com> wrote:
>> > > > On Tue, Apr 30, 2013 at 09:21:18AM -0700, Tom Warren wrote:
>> > > > > Marek,
>> > > > >
>> > > > > > -----Original Message-----
>> > > > > > From: Marek Vasut [mailto:marex at denx.de]
>> > > > > > Sent: Monday, April 29, 2013 4:47 PM
>> > > > > > To: Jim Lin
>> > > > > > Cc: u-boot at lists.denx.de; Tom Warren; Stephen Warren
>> > > > > > Subject: Re: [PATCH 2/3] ARM: Tegra: USB: Add driver support for
>> > > > > > Tegra30/Tegra114
>> > > > > >
>> > > > > > Dear Jim Lin,
>> > > > > >
>> > > > > > > Tegra30 and Tegra114 are compatible except 1. T30 takes 55 ms
>> to
>> > > > > > > finish Port Reset. T114 takes
>> > > > > > >
>> > > > > > >    50 ms.
>> > > > > > >
>> > > > > > > 2. PLL parameters
>> > > > > > >
>> > > > > > > Tested on Tegra20 Harmony/Seaboard, Tegra30 Cardhu, and
>> Tegra114
>> > > > > > > Dalmore platforms. All works well.
>> > > > > > >
>> > > > > > > Signed-off-by: Jim Lin <jilin at nvidia.com>
>> > > > > > > ---
>> > > > > > >
>> > > > > > >  arch/arm/include/asm/arch-tegra/clk_rst.h  |   10 +
>> > > > > > >  arch/arm/include/asm/arch-tegra/usb.h      |  249
>> > > > > > >  ------------------
>> arch/arm/include/asm/arch-tegra114/tegra.h |
>> > > > > > >     1 +
>> > > > > > >  arch/arm/include/asm/arch-tegra114/usb.h   |  287
>> > > > > >
>> > > > > > +++++++++++++++++++++
>> > > > > >
>> > > > > > >  arch/arm/include/asm/arch-tegra20/usb.h    |  279
>> > > > > >
>> > > > > > +++++++++++++++++++++
>> > > > > >
>> > > > > > >  arch/arm/include/asm/arch-tegra30/usb.h    |  294
>> > > > > >
>> > > > > > ++++++++++++++++++++++
>> > > > > >
>> > > > > > Do we now have three copies of the same stuff ?
>> > > > >
>> > > > > When only T20 was supported (for USB), there was a common
>> > > > > (arch-tegra/usb.h) header. That's been moved to
>> arch-tegra20/usb.h,
>> > > > > and (unfortunately) there are 2 new usb.h files due to the HW
>> > > > > differences in the registers between T20 and T30/T114.  I don't
>> see
>> > > > > any easy way to have a common usb.h file for Tegra w/o adding ugly
>> > > > > #ifdefs to the USB register space struct(s).
>> > > >
>> > > > Just how different are they?  Are all of the related defines and
>> masks
>> > > > different too?  Do we have conflicts? Moved registers?  Just
>> > > > conflicting values?  A quick peek shows '30' and '114' are pretty
>> > > > similar, except for masks.  Maybe splitting the struct up so you can
>> > > > discard some of the reserved gaps, run-time checking to see if we
>> can
>> > > > or cannot use a particular part of the struct?
>> > >
>> > > This is really Jim's patchset (and his specialty), but here's what I
>> know
>> > > about Tegra USB regs:
>> > >
>> > > T20 had a gap in the registers @ offset 0x130. T30 (and T114) moved
>> the
>> > > offset of the command/status/interrupt regs down to fill in this gap,
>> > > which dragged all the subsequent registers back 16 bytes. The two SoCs
>> > > 'families' sync up again at offset 0x400 and are pretty much equal
>> from
>> > > there on out to 0x840.
>> > >
>> > > The defines are probably 90% the same, with some weirdness for the
>> first
>> > > USB controller (USB1) and its PTS/STS bits that differs in offset from
>> > > the other 2 controllers (again, no clue why the HW guys would do
>> this).
>> > >
>> > > So we could have the 3 different USB headers in the arch-tegraXX area
>> > > contain the register structs, and have a common arch-tegra/usb.h that
>> has
>> > > the #defines that are the same, and is included in the
>> arch-tegraxx/usb.h
>> > > files. That would reduce this down somewhat, without the ugliness of
>> > > #ifdefs in the structs.
>> > >
>> > > What do you think?
>> >
>> > Sounds like the best we can do then.  It's probable that trying to
>> > define USB_REGMAP_GAPSIZE1/2 or whatever to do it on the fly would just
>> > be uglier still.  Thanks!
>>
>> This is a problem with the struct-based access indeed. I agree with Tom
>> it'd be
>> worth to at least try distilling the common part into header shared
>> between
>> those three CPUs.
>>
>> btw you're also adding some kernel-doc-alike annotations to functions,
>> why don't
>> you follow kerneldoc style altogether?
>>
>
> I'll let Jim answer that.
>
>
>>
>> Best regards,
>> Marek Vasut
>>
>
>