On Thu, 2013-05-02 at 03:33 +0800, Marek Vasut 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@ti.com wrote:
On Tue, Apr 30, 2013 at 09:21:18AM -0700, Tom Warren wrote:
Marek,
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Monday, April 29, 2013 4:47 PM To: Jim Lin Cc: u-boot@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@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.
OK. I will add this into next version of patch.
btw you're also adding some kernel-doc-alike annotations to functions, why don't you follow kerneldoc style altogether?
I don't understand what you meant here. Could you give me an example? Like what I did is wrong or not good. And what is correct or better one. Thanks.
-- nvpublic