Hi Johan, On Tue, Jan 14, 2025 at 4:39 PM Johan Hovold <johan@xxxxxxxxxx> wrote: > > Hi Martin (and Corentin), > > and sorry about the late reply here. Sometimes there's more important things in life than USB to serial adapters. [...] > > > You could try increasing the buffer size to 2k and see how much is > > > received on the other end if you submit one URB (e.g. does the hardware > > > just drop the last 1k of data when the device fifo is full). > > > I have not tried this yet but if still relevant (after the info about > > the THRE interrupt) then I can try it and share the results. > > It would only be to really confirm that this is how the vendor protocol > and device works. Your call. I can give that a try in the next few days and will report back. [...] > > > I understand, but the generic implementation is not a good fit here as > > > it actively tries to make sure the device buffers are always full (e.g. > > > by using two URBs back-to-back). > > > > > > If you can't find a way to make the hardware behave properly then a > > > custom implementation using a single URBs is preferable over trying > > > to limit the generic implementation like you did here. Perhaps bits can > > > be reused anyway (e.g. chars_in_buffer if you use the write fifo). > > > I cannot find any other usb-serial driver which uses this pattern. > > Most devices seem to be happy to take more data once they trigger the > > write_bulk_callback but not ch348. > > Right. I think the io_edgeport driver maintains some kind of TxCredits. > I guess that's related, but not sure how relevant that is here (and you > probably shouldn't based anything on that old driver directly anyway). > > > If there's any other (even if it's not a usb-serial) driver that I can > > use as a reference implementation for your suggestion? > > I'm not sure whether to use a dedicated kthread, single threaded workqueue, ... > > Not sure what Corentin has been preparing, but yeah, you need some kind > of deferred mechanism to make write() non-blocking and hold off sending > more data to the device until you're sure there's room in its buffers. I > guess a workqueue should do fine. We're currently testing an updated driver based on a workqueue (work_struct) and it's working fine. The issue that Corentin reported is unrelated to the workqueue part. At this point we're thinking it may be a regression in linux-next, but we're running more tests to verify this. Best regards, Martin
