Hi Stephen,
On Tue, Dec 6, 2011 at 10:58 AM, Stephen Warren swarren@nvidia.com wrote:
Simon Glass wrote at Monday, December 05, 2011 7:03 PM:
On Mon, Dec 5, 2011 at 3:32 PM, Stephen Warren swarren@nvidia.com wrote:
On 12/02/2011 07:11 PM, Simon Glass wrote:
CONFIG_USB_EHCI_TXFIFO_THRESH enables setting of the txfilltuning field in the EHCI controller on reset.
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index 3d0ad0c..cc00ce4 100644 --- a/drivers/usb/host/ehci.h +++ b/drivers/usb/host/ehci.h @@ -80,7 +80,11 @@ struct ehci_hcor { uint32_t or_ctrldssegment; uint32_t or_periodiclistbase; uint32_t or_asynclistaddr;
- uint32_t _reserved_[9];
- uint32_t _reserved_0_;
Why not remove _reserved_0_ ...
- uint32_t or_burstsize;
- uint32_t or_txfilltuning;
+#define TXFIFO_THRESH(p) ((p & 0x3f) << 16)
- uint32_t _reserved_1_[6];
... and make _reserved_1_ 1 element bigger and keep it named _reserved_? The result would be a little simpler.
Sorry I'm a bit stuck with that one. I have:
uint32_t or_asynclistaddr; uint32_t _reserved_0_; uint32_t or_burstsize; uint32_t or_txfilltuning; uint32_t _reserved_1_[6]; uint32_t or_configflag;
How can I remove _reserved_0_? I would need to replace it with something, as need or_burstsize to stay where it is. Can you please explain a little more?
Oh right, this is a HW structure, so the layout is fixed? In which case the patch is fine. I wonder why all the fields weren't just defined from the start rather than being marked "reserved" though; perhaps it'd be best to update the code to completely flesh out the HW description at some point. Not necessarily in this patch though.
Yes that's right. Will leave this patch as is and resend series.
There are two sides to this: some people will say they want everything in there in case they want to access a particular field later. Others will say that it adds confusion and dead code. I think I'll sit on the fence.
Regards, Simon
-- nvpublic