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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2016-07-04 at 19:01 +0200, Bjørn Mork wrote:

> The proposed driver solution is a hack, allowing us to resyncronize
> without resetting the firmware. We have no reliable way to reset the
> firmware and doing so will have large side effects anyway.  The hack
> allows us to continue, with a possibly lost message or two as the only
> consequences.  We don't have to break any existing session etc.

OK, then I propose it is time to bring out the sledge hammer.
How about changing usb_cdc_wdm_register() to include a flag
about the necessity to drain the buffers?

> But it turns out that it is a real problem for the very same MBIM
> firmwares which caused us to need the hack in the first place: They
> stall when you try to read and there is no data.  Which means that a
> lost race can end up with an EPIPE being returned to _read(), where it
> is translated and returned to userspace.

This sucks and it tells me even more that CDC MBIM should tell us when
to apply this work around at startup.

> This is a mess.  I fully agree that it looks like this either should go
> into both open() and resume(), or none.  The real answer is 'none'.
> 
> But we need the hack for the yucky MBIM firmwares.  Putting it in open()
> is defining open() as a syncronization point for something which should
> not need syncronization in a perfect world.  I can expand the comment
> there a bit to make this clear. Making resume() a syncronization point
> as well is only making things worse.

I'd say add a flag, let CDC MBIM set it unconditionally (we can switch
to a black list if we absolutely need to) and include a big, fat
commentary. There is no good solution for this problem.

	Regards
		Oliver


--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]