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:
"
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
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
