Daniele Palmas <dnlplm@xxxxxxxxx> writes: > 2016-11-23 15:49 GMT+01:00 Bjørn Mork <bjorn@xxxxxxx>: >> Daniele Palmas <dnlplm@xxxxxxxxx> writes: >>> 2016-11-21 10:49 GMT+01:00 Bjørn Mork <bjorn@xxxxxxx>: >>>> Daniele Palmas <dnlplm@xxxxxxxxx> writes: >>>> >>>>> it turned out that resetting the interface in cdc_ncm_init after >>>>> getting the NTB parameters removes the need for the sleep, making the >>>>> modem to work fine. >>>> >>>> Sounds very good, although I must admit that it isn't perfectly clear to >>>> me what kind of reset we're talking about here. But no worries, the >>>> patch will make that clear :) >>>> >>> >>> Sorry for the confusion, I meant resetting the MBIM function with >>> RESET_FUNCTION request code. >> >> Yes, the patch did make that clear, as expected :) >> >>>>> I wonder if this is an acceptable solution and can be applied also for MC7455. >>>> >>>> Quite possible. I will definitely test it. If we can avoid an >>>> arbitrary and pointless delay, then that's great. But I guess this also >>>> requires testing with a wide range of other MBIM devices to find out if >>>> we can apply it unconditionally without breaking anything. Avoiding >>>> device-specific or vendor-specific code is important in a class driver, >>>> if possible. >>>> >>> >>> Ok, unfortunately I can test only with Telit modems, so I'm not sure >>> if the change I did is harmless for all the other modems: >>> RESET_FUNCTION should not cause issues, >> >> Agreed. Unfortunately, we cannot depend on sane firmware behaviour ;) >> >> I did a semi-quick test on a Sierra Wireless EM7455 and can confirm that >> your patch makes it work without the delay. Very nice. >> > > Cool! > >> Do you happen to know if the Windows MBIM driver use RESET_FUNCTION >> during initialisation? That would make this feel much safer. >> > > Yes, Windows driver seems to send RESET_FUNCTION two times: after > getting NTB parameters and after setting NTB input size. But it seems > that only the first one is mandatory. > > Windows USB traces taken with TotalPhase Datacenter are available at > > https://drive.google.com/drive/folders/0B1kPnH2g8ISIZWJiV05qeWN5dVE > > file LE922win10_2.tdc This is comforting. It means that the risk introducing the RESET_FUNCTION call is close to zero. The only remaining issue is whether we can safely remove the altsetting toggle. >> I see that your testing also included Intel based modems. That's good. >> It would still be nice to have test results from a few more MBIM >> implementations, in particular the more "difficult" ones. But I don't > > I agree, especially Huawei ones for which the alternate setting > toggling was introduced. I'll see what I can do about that. Looks like I have to dig through my mailbox. It seems I didn't add any Reported-by tags on the relevant commits. Wonder why... Well, there you have the reason why you always should. Now I got to do the work instead of just loading it on you :) >> know how much help I can promise here. At the moment I don't even know >> which box my modems are in. Moved a month ago and am still living in a >> box. Or more like a hundred boxes ;) >> >> The easiest way to get this thoroughly tested is of course to push it >> now, and try to respond quickly in case it breaks something. Don't know >> what others think about that? >> >>> but I also removed altsetting >>> toggling for all MBIM modems and maybe this is not acceptable. >> >> I wonder about that. It was added specifically for modems which seemed >> to need it. Don't remember if we ever tried RESET_FUNCTION as an >> alternative. Removing workaorunds which have proven to be necessary at >> some point in time is a bit risky. >> > > I totally agree, but the other way would be to add another quirk at > the cdc_mbim driver level, isn't it? Yes, so I'm definitely with you here - let's try without the quirk first. I'm ready to say "commit it", if noone else objects. > If this is a preferred solution I can try to modify the patch > according to this path. No, the patch is fine as is. Just one question: If keeping the altsetting toggle proves necessary, will that break the Qualcomm devices? Yes, I could test that myself, but I'm lazy and I guess you already have tested it? Bjørn -- 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
