On Sun. 5 Jun. 2022 at 15:00, Oliver Neukum <oneukum@xxxxxxxx> wrote: > On 04.06.22 18:40, Alan Stern wrote: > > > > I don't see anything wrong with this, except that it would be nice to keep > > the flag values in numerical order. In other words, set URB_FREE_COHERENT > > to 0x0200 and change URB_DIR_IN to 0x0400. > Hi, > > but what sense does this patch make? You use coherent allocations > to avoid repeated DMA synchronizations. That is sort of incompatible > with the notion of discarding the buffer automatically. This is how I see things: * In the open() function, the driver will do the coherent allocation for its transfer_buffers, fill those into URBs and add all the URBs in an anchor. * During runtime, the driver will keep recycling the same URBs (no need to kill URB nor to usb_free_coherent() the transfer_buffer). * Finally, in the close() function, the driver has to kill the URBs and usb_free_coherent() the transfer_buffers. As far as I understand, no helper functions allow us to do all that, thus requiring the driver to iterate through the anchor to manually usb_free_coherent() the transfer buffer. So, the intent of this patch is to provide a method to both kill the URBs and usb_free_coherent() the transfer buffer at once. The rationale is that those two actions are done sequentially, at least in the use case exposed above, and so the driver has enough control to assert that usb_free_coherent() will occur at a good timing. For other use cases where killing the URBs and freeing the transfer_buffer should be done separately, the driver should not set URB_FREE_COHERENT and continue to manually usb_free_coherent() the transfer_buffer. Which part of this logic is incompatible with the coherent allocation? Also, if the above is incorrect, what is the canonical way to kill the URBs and free the transfer_buffers? Thank you! Yours sincerely, Vincent Mailhol
