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. 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
