On Mon, Mar 11, 2013 at 11:28:38PM +0100, Bjørn Mork wrote: > Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> writes: > > > >> Currently, when a userspace application sees a /dev/cdc-wdmX device, it > >> will have to > >> - check which USB interface it belongs to, > >> - parse the DMM descriptor if it is CDC WDM, > >> - parse the MBIM descriptor if it is CDC MBIM > >> - check if the driver is qmi_wwan if none of the above > >> - know which value qmi_wwan has hard coded > >> > >> No application does this. > > > > Hm, I hate to ask, but what does other OSes do (OS-X, Windows, etc.) > > about this? > > I don't know, but I do believe they implement more (most?) of the > management protocols in their drivers, making this particular issue a > non-problem. The driver will take care of message formatting, > fragmentation etc. Instead they will of course have hundreds of other > kernel<->userspace API issues... > > I believe that our current approach, giving userspace full access to the > management channel using a character device, result in the simplest > possible userspace API. But the message size is an implicit part of > this API whether we want it or not. Explictly exporting it will make > this clearer and reduce the possibility of errors. > > I have no strong feelings on how it should be exported, and I do > appreciate the need to discuss all options before choosing a method. I > know we cannot change this later, so we do want it to be perfect from > the beginning. > > > And what happens today that causes us to need this size? How have we > > lived so long without it? > > That's simple. We've only just started using the /dev/cdc-wdmX devices. > > The underlying problem has always been there. The cdc-wdm driver > translates between a character device stream and a message based > protocol. The application using the character device will need to know > the maximum message size to prevent the driver from splitting messages. > > The maximum message size *is* already part of the API and always has > been. > > But until we got libqmi and qmi support in oFono a few months ago, there > wasn't a single userspace application opening these character devices. > And even after that, it was still pretty simple: There is no way for a > QMI device to tell the host what size to use, so the qmi_wwan driver has > a hard coded value. Which makes it reasonable for the applications to > hard code the same value. > > The real trouble started with cdc_mbim in v3.8. MBIM devices use wildly > different maximum message sizes, and will fail if the userspace > application doesn't get it right. I revisited this issue now because a > user had such a problem. A *real* end user with a *real* problem :) We > can of course claim that this is a bug in libmbim (certainly is), but it > is a fact that the bug is there because we haven't made it clear that > the message size is part of the API. And it doesn't help that the > application will have to reimplement a lot of lsusb just to get this > value, which the driver already has parsed. > > Yes, we can solve that by putting the MBIM functional descriptor in > sysfs, leaving QMI as it is and ignore WDM. Maybe that is best? I just > thought that I could make this a generic export on behalf of any driver > using cdc-wdm, independent of actual descriptors. And by that also > making it clearer that this single value is important to userspace. > > I'll take the full blame for introducing this problem. Writing the > above I realized that everything was fine until I started messing with > it :) Heh, I think you've done more good than harm, don't worry about it :) Anyway, this type of information should go into the patch itself (i.e. why we need to detect this information from userspace), and then I'll have no problem with the patch being accepted. Oh, also get the userspace authors to agree and sign-off on this, I want their agreement that this is something they can support. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
