2016-11-23 19:38 GMT+01:00 Bjørn Mork <bjorn@xxxxxxx>: > 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? Unfortunately, yes: for Telit LE922A6 altsetting toggle seems to be part of the problem. Regards, Daniele > > > 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
