[U-Boot] [PATCH v4] usb: align buffers at cacheline

Mike Frysinger
2012-03-06T16:42:08Z

On Tuesday 06 March 2012 03:24:34 Marek Vasut wrote:
> > On Friday 02 March 2012 11:45:15 Wolfgang Denk wrote:
> > > > > That's what I did in original patch where I am aligning it by
> > > > > adding the line
> > > > >
> > > > > + /* Device Descriptor */
> > > > > +#ifdef ARCH_DMA_MINALIGN
> > > > > + struct usb_device_descriptor descriptor
> > > > > + __attribute__((aligned(ARCH_DMA_MINALIGN)));
> > > > > +#else
> > > > > + struct usb_device_descriptor descriptor;
> > > > > +#endif
> > > > >
> > > > > in usb.h Line:112
> > > >
> > > > I see ...and I told you it's wrong? I must have misunderstood, I'm
> > > > sorry about that. But if you actually do this, you can avoid memcpy,
> > > > right?
> > >
> > > And eventually wd can also avoid the #ifdef ? I guess the
> > > __attribute__((aligned...)) would not hurt anything?
> >
> > the reason i disliked that was because it adds padding to the structure.
> > on my system, seems to go from 144 bytes to 160, and the other goes from
> > 1352 to 1376. the scsi structure isn't specific to usb either. i can't
> > tell if this is a structure that represents data on the wire ... the fact
> > it's written all using char types makes me suspicious. if it is, then
> > obviously we can't change the padding in the struct.
> >
> > further, it doesn't seem like Linux imposes this restriction at the
> > structure level (does it do memcopies instead ?), and imposing it on
> > arbitrary members in there w/out documentation easily leads to rot. if
> > the code changes and no longer needs this alignment, how do we tell ? if
> > the code starts transferring another structure, do we end up aligning
> > every member in there until there's padding everywhere ?
>
> I believe this is OK, we're properly aligning only descriptors.

you're looking at one change (the one quoted above). i was referring to the
scsi structure change (which is not quoted above).

> Note that the USB stack in linux and in uboot is different.

i know the stacks are different, but they do share. my point was that if Linux
is managing this, then why can't we ? or maybe we're fighting over an
insignificant memcpy ... the scsi buffer is all of 64 bytes. on some systems,
that's like 1 cache line :P.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120306/086be190/attachment.pgp>