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

Marek Vasut
2012-03-06T08:24:34Z

Dear Mike Frysinger,

> 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
> > > >
> > > > > M
> > >
> > > 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 ?
> -mike

I believe this is OK, we're properly aligning only descriptors. Note that the
USB stack in linux and in uboot is different.

Best regards,
Marek Vasut