On Wed, Jan 27, 2021 at 08:12:21PM +0530, Anant Thazhemadam wrote:
>
> On 27/01/21 7:28 pm, Johan Hovold wrote:
> > On Wed, Jan 27, 2021 at 12:03:52AM +0530, Anant Thazhemadam wrote:
> >> The newer usb_control_msg_{send|recv}() API are an improvement on the
> >> existing usb_control_msg() as it ensures that a short read/write is treated
> > As I mentioned in my comments on v2, a short write has always been
> > treated as an error so you shouldn't imply that it wasn't here (and in
> > the other commit messages).
>
> The newer API ensures that a short send/recv is an error, as they
> either complete the complete translation, or return an error, and
> remove the need for explicit return value checking that was previously
> expected to detect the short read/write (which wasn't present in a lot
> of places).
But my point is that this claim isn't factually correct; there has never
been a need to check for short *writes* (even if a few drivers have such
redundant checks).
> That's what I was trying to say. But if the commit message isn't
> representative of that, I'll try and modify it.
Just drop the bit about "short writes".
> Does this sound like a better commit message?
>
> "The newer usb_control_msg_{send|recv}() API are an improvement on the
> existing usb_control_msg().
Even this is disputable; in some situations the usb_control_msg() is
still preferred as I hope my comments have shown.
Perhaps they are better described as "convenience wrappers" or similar
as the real gain from using these helpers is to replace a pattern like:
f(data, ...) {
buf = malloc(data);
usb_control_msg(..., buf, ...);
memcpy(data, buf, ...);
kfree(buf);
}
for when data is on the stack *and* you do not expect variable-length IN
transfers.
But as soon as a driver is able to reuse a single buffer for multiple
transfers or the data buffer is already DMA-able, usb_control_msg() may
still be a better choice.
> The new API ensures either the full translation is completed,
> or an error is returned. This ensures that all short send/recv are detected as
recv only
> errors even if there is no explicit return value checking performed.
>
> The new API also allows us to use data off the stack, and don't require raw usb
> pipes to be created in the calling functions."
Johan
