2016-12-05 14:04 GMT+01:00 Daniele Palmas <dnlplm@xxxxxxxxx>: > Hi, > > 2016-12-05 11:10 GMT+01:00 Bjørn Mork <bjorn@xxxxxxx>: >> Daniele Palmas <dnlplm@xxxxxxxxx> writes: >> >>> I went back to this and further checking revealed that MBIM function >>> reset is not needed and the only problem is related to commit >>> 48906f62c96cc2cd35753e59310cb70eb08cc6a5 cdc_ncm: toggle altsetting to >>> force reset before setup, that, however, is mandatory for some Huawei >>> modems. >> >> Interesting. That does sound like an odd firmware bug to me. I have a >> bit of a hard time understanding how this can be. Using data interface >> altsetting 0 to reset the function is documented in the NCM spec: >> > > Agree, this is likely a firmware bug and it is also proved by all the > other modems that accept the procedure without issues. > >> " >> 7.2 Using Alternate Settings to Reset an NCM Function >> >> To place the network aspects of a function in a known state, the host >> shall: >> • select alternate setting 0 of the NCM Data Interface (this is the >> setting with no endpoints). This can be done explicitly using >> SetInterface, or implicitly using SetConfiguration. See [USB30] for >> details. >> • select the NCM operational parameters by sending commands to the NCM >> Communication Interface, then >> • select the second alternate interface setting of the NCM Data >> Interface (this is the setting with a bulk IN endpoint and a bulk >> OUT endpoint). >> >> Whenever alternate setting 0 is selected by the host, the function >> shall: >> • flush function buffers >> • reset the packet filter to its default state >> • clear all multicast address filters >> • clear all power filters set using >> SetEthernetPowerManagementPatternFilter >> • reset statistics counters to zero >> • restore its Ethernet address to its default state >> • reset its IN NTB size to the value given by field dwNtbInMaxSize >> from the NTB Parameter Struc- ture >> • reset the NTB format to NTB-16 >> • reset the current Maximum Datagram Size to a function-specific >> default. If SetMaxDatagramSize is not supported, then the maximum >> datagram size shall be the same as the value in wMaxSegmentSize of >> the Ethernet Networking Functional Descriptor. If SetMaxDatagramSize >> is supported by the function, then the host must either query the >> function to determine the current effective maximum datagram size, >> or must explicitly set the maximum datagram size. If the host wishes >> to set the Maximum Datagram Size, it may do so prior to selecting >> the second alternate interface set- ting of the data >> interface. Doing so will ensure that the change takes effect prior >> to send or receiving data. >> • reset CRC mode so that the function will not append CRCs to >> datagrams sent on the IN pipe >> • reset NTB sequence numbers to zero >> >> When the host selects the second alternate interface setting of the >> NCM Data Interface, the function shall perform the following actions >> in the following order. >> • If connected to the network, the function shall send a >> ConnectionSpeedChange notification to the host indicating the >> current connection speed. >> • Whether connected or not, the function shall then send a >> NetworkConnection notification to the host, with wValue indicating >> the current state of network connectivity >> " >> >> The addition of the "RESET" request in MBIM did not change these >> requirements AFAICS. Supporting NCM style "altsetting 0 reset" is >> required by default inheritance. The description of dual NCM/MBIM >> functions in the MBIM spec further emphasizes this: >> >> " >> Regardless of the previous sequence of SET_INTERFACE commands targeting >> the Communication Interface, the host is able to put the function into >> a defined state by the following sequence: >> >> 1. SET_INTERFACE(Data Interface, 0) >> 2. SET_INTERFACE(Communication Interface, desired alternate setting) >> 3. Sending the required class commands for the targeted alternate >> setting >> 4. SET_INTERFACE(Data Interface, 1 if Communication Interface,0 and >> Data Interface,2 Communication Interface 1) >> " >> >> >> This, along with the lack of *any* sort of discussion of the >> relation/interfaction between "MBIM RESET" and "data interface >> altsetting 0" makes the MBIM RESET control request either ambigious or >> redundant. Or both... >> >> FWIW, I've always considered RESET a symptom of the sloppy MBIM spec >> development. It did not exactly work to their advantage that the first >> (and only?) published errata simply was an excuse to add new features >> (the "MBIM extended functional descriptor" and "MBIM loopback testmode") >> without updating the revision number. Causing even more confusion, >> since we now have two different MBIMv1.0 specs. >> >> Well, whatever. No need to get all frustrated about that. We have to >> deal with whatever the firmware developers serve us. And, >> unfortunately, it is not surprising that unclear and ambigious specs >> leads to incompatible firmware implementations. >> >> I wonder, what happens if you unload the MBIM driver and let the USB >> core reset the data interface to altsetting 0? Will the device then >> fail when the driver is loaded again? If not, then where is the >> difference? And can we possibly reorder the driver set_interface >> requests to make them similar to the USB core reset >> > > I will check this. > I verified that the behavior does not change and data connection still does not work, so I'm going to submit the new patch. Regards, Daniele > Thanks, > Daniele > >> Until now, I was under the impression that this was the >> only documented way to reset both NCM and MBIM functions (since the >> RESET is MBIM specific), >> >>> My proposal is to add something like >>> CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE quirk to avoid the toggle. >>> >>> To have a conservative approach, I would add only Telit LE922 to this >>> quirk and keep also the usleep_range delay, since I do not have a >>> clear view on all the VIDs/PIDs affected by this quirk. Then other >>> modems, once tested, can be added to the quirk if the delay is not >>> enough. >>> >>> If this approach, though ugly, has chances to be accepted I can write >>> a new patch. >> >> Yes, that sounds like the best approach at the moment. I have >> unfortunately not as much time to experiment with this as I seem to >> need. And the EM7455 workaround (the delay) is a bit hard to verify, >> since it is easily affected by small additional delays caused by the >> debugging itself. This can cause bogus test results. >> >> >> 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
