[review] aleksander/huawei-ndisdup-cdc-wdm-rebased-1.2

[Bjørn Mork]

[added Enrico to the Cc list]

Dan Williams <dcbw at redhat.com> writes:

> The rest looks fine; but my E3276 has some other issues that should get
> fixed too...
>
> [1402602002.666216] [mm-port-serial.c:1237] mm_port_serial_open():
> (cdc-wdm1) device open count is 3 (open)
> [1402602002.666249] [mm-port-serial.c:1296] mm_port_serial_close():
> (cdc-wdm1) device open count is 2 (close)
> [1402602002.666283] [mm-port-serial-at.c:440] debug_log(): (cdc-wdm1):
> --> 'AT^SYSCFGEX=?<CR><LF>'
> [1402602002.670020] [mm-port-serial-at.c:440] debug_log(): (cdc-wdm1):
> <-- '<CR><LF>^SYSCFGEX:
> ("00","01","02","03","99"),((400380,"GSM900/GSM1800/WCDMA2100"),(6a80000,"GSM850/GSM1900/WCDMA850/AWS/WCDMA1900"),(3fffffff,"All bands")),(0-2),(0-4),((1081b,"LTE_B1/LTE_B2/LTE_B4/LTE_B5/LTE_B12/LTE_B17"),(7fffffffffffffff,"All bands"))<CR><LF><CR><LF>OK<CR>'
> ModemManager[11313]: Error while checking ^SYSCFGEX format: Serial
> command timed out
> [1402602007.961186] [mm-port-serial-at.c:440] debug_log(): (cdc-wdm1):
> --> 'AT^SYSCFG=?<CR><LF>'
> [1402602007.962065] [mm-port-serial-at.c:440] debug_log(): (cdc-wdm1):
> <-- '<LF>'
> Error while checking ^SYSCFG format: Missing ^SYSCFG prefix
>
> Note the late-arriving <LF> that's meant for the ^SYSCFGEX; changing the
> timeout doesn't help it arrive faster, for some reason the modem just
> doesn't send the LF until later.  Is that a quirk of huawei_cdc_ncm,
> that maybe it doesn't send the last byte until later or something?
> Bjorn?

Yes, that might actually be it.  I didn't think so when I started
writing this reply, but then I checked the code :-)

The huawei_cdc_ncm driver use cdc-wdm just like qmi_wwan and cdc_mbim.
But failing to have any other guideline for minimum or maximum firmware
message size, it uses 256 bytes buffers.  Which is just an arbitrary
choice loosely based on the CDC-WMC spec, which requires wMaxCommand to
be "at least 256 decimal (0x100)".

And what do you know? Counting the bytes of that AT^SYSCFGEX=? reply I
see exactly 257 bytes including the final <LF>....

So, yes, it seems the driver will "mess up" this by splitting the read
from the modem firmware into 2 transfers.  We just didn't imagine that
would matter for AT commands, which by definition are supposed to be
a stream of single bytes.

The 256 byte choice is arbitrary as I said, so we can definitely change
it if we have reason to believe some other value is better.  But I'm not
sure this qualifies as a bug?  We will have to choose some maximum
buffer size, and there is no way for us to know what the firmware is
going to send.  So in theory we will have to cope with split messages,
no matter how big we make the buffer.


Bjørn