Re: [PATCH] cdc-wdm: fix "out-of-sync" due to missing notifications

["Bjørn Mork" <bjorn@mork.no>]

Oliver Neukum <oneukum@suse.com> writes:

> On Fri, 2016-05-13 at 18:59 +0200, Bjørn Mork wrote:
>> Bjørn Mork <bjorn@mork.no> writes:
>> 
>> > The driver enforces a strict one-to-one relationship between the
>> > received RESPONSE_AVAILABLE notifications and messages read from
>> > the device. At the same time, it will cancel the interrupt URB
>> > when there is no client holding the character device open.
>> 
>> Never mind.  Forget it.
>> 
>> This patch breaks other devices again.  The immediate and unconditional
>> reading make them barf. I guess it can be worked around by delaying the
>> flushing until at least one notification is received, but I obviously
>> have to test this theory thoroughly on all devices I have.
>
> Hi,
>
> I think the best approach would be to keep the interrupt URB always
> active. I didn't do this to conserve bandwidth, but if it makes devices
> work, it certainly would be the best option.

Yes, I considered that.  But this implies purging the device message
queue without telling userspace that we did so.  At least with the
current driver design, which is based on a single limited size
buffer. If the device queues a number of unsolictied messages between
two userspace requests, then we really want all those unsolicted
messages delivered to the userspace program on the second request.

And I do think the original bandwidth (and power) conservative approach
is worth keeping too.  There is no point in waking up these devices
unless there actually is an interested userspace application.

FWIW, my initial analysis of the problem with the patch was too quick
imprecise. The problem is simply the -EPIPE status we inevitably will
hit when the queue is empty, as I should have anticipated. It will be
returned to userspace translated to -EIO.  I am currently testing a
version taking care of that, and it seems to behave well so far. I'll
submit it as soon as I am absoltely sure that it works on all WDM, QMI
and MBIM devices I have.  Might take some time, since I am running out
of mini-PCIe and m.2 adapters..


Bjørn