Hi, On Wed, Oct 30, 2013 at 09:36:20AM -0700, David Cohen wrote: > On 10/29/2013 03:47 PM, Paul Zimmerman wrote: > >>From: David Cohen > >>Sent: Tuesday, October 29, 2013 2:53 PM > >> > >>These patches are a proposal to add gadget quirks in an immediate objective to > >>adapt f_fs when using DWC3 controller. But the quirk solution is generic and > >>can be used by other controllers to adapt gadget functions to their > >>non-standard restrictions. > >> > >>This change is necessary to make Android's adbd service to work on Intel > >>Merrifield with f_fs instead of out-of-tree android gadget. > >> > >>--- > >>David Cohen (3): > >> usb: gadget: add quirks field to struct usb_gadget > >> usb: ffs: check quirk to pad epout buf size when not aligned to > >> maxpacketsize > >> usb: dwc3: add quirk USB_GADGET_QUIRK_EP_OUT_ALIGNED_SIZE to gadget > >> driver > >> > >> drivers/usb/dwc3/gadget.c | 1 + > >> drivers/usb/gadget/f_fs.c | 17 +++++++++++++++++ > >> include/linux/usb/gadget.h | 5 +++++ > >> 3 files changed, 23 insertions(+) > > > >Wouldn't it be simpler and safer to just do this unconditionally? Sure, > >you need it for DWC3 because the controller refuses to do an OUT transfer > >at all if the transfer size is less than maxpacketsize. But it's possible > >that other controllers allow the transfer, and it works in most cases, > >but if an error occurs and the host sends too much data, they could > >overrun the buffer and crash your device. > > > >For example, the DWC2 databook says "For OUT transfers, the Transfer > >Size field in the endpoint's Transfer Size register must be a multiple > >of the maximum packet size of the endpoint". But I don't think the > >controller enforces that, it is up to the programmer to do the right > >thing. So that controller probably needs this quirk also. There could be > >more like that which we don't know about. > > Unfortunately DWC2 is a bad example... the driver couldn't even get out > of staging. If the author was reckless to ignore this restriction (s)he > should fix. > But I don't have enough data to tell it's better to waste everybody's > memory in this case in favor of DWC3. I'd still stick with the > exception. on top of that we don't what quirks other controllers might have. There could very well be a controller which quirk goes the other around: you _must_ set bulk out length to the correct size (e.g. 31 bytes for mass storage's CBW) otherwise transfer won't complete. I could see that happening on controllers with broken short packet handling. > >So unless the buffer allocation code is in a real fast path, I would > >suggest to just do the aligned buffer allocation always. > > This code would affect embedded devices which value too much memory > consumption (and performance on handling it!). IMO we'd need to be more > careful prior to take such decision. agreed. Quirk flag is the way to go here. -- balbi
Attachment:
signature.asc
Description: Digital signature
