On Tue, Aug 25, 2020 at 04:44:37PM +0200, Greg Kroah-Hartman wrote:

On Tue, Aug 25, 2020 at 10:39:46AM -0400, Alan Stern wrote:

quoted

On Tue, Aug 25, 2020 at 08:51:35AM +0200, Greg Kroah-Hartman wrote:

quoted

At first glance, I think this can all be cleaned up, but it will take a
bit of tree-wide work.  I agree, we need a "read this message and error
if the whole thing is not there", as well as a "send this message and
error if the whole thing was not sent", and also a way to handle
stack-provided data, which seems to be the primary reason subsystems
wrap this call (they want to make it easier on their drivers to use it.)

Let me think about this in more detail, but maybe something like:
	usb_control_msg_read()
	usb_control_msg_send()
is a good first step (as the caller knows this) and stack provided data
would be allowed, and it would return an error if the whole message was
not read/sent properly.  That way we can start converting everything
over to a sane, and checkable, api and remove a bunch of wrapper
functions as well.

Suggestion: _read and _send are not a natural pair.  Consider instead
_read and _write.  _recv and _send don't feel right either, because it
both cases the host sends the control message -- the difference lies
in who sends the data.

Yes, naming is hard :)

	usb_control_read_msg()
	usb_control_write_msg()

feels good to me, let me try this out and see if it actually makes sense
to do this on a few in-usb-core files and various drivers...

Turns out we have a long history of using snd/rcv for USB control
messages:
	usb_rcvctrlpipe()
	usb_sndctrlpipe()

so while _recv and _send might feel a bit "odd", it is what we are used
to using, and when converting existing users, I can drop the pipe macro
from the calls, turning something like:
	usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0), ...);
into:
	usb_control_send_msg(hdev, 0, ...);

or maybe:
	usb_control_msg_send(hdev, 0, ...);
with a full noun_verb pairing, instead of noun_verb_noun.

thanks,

greg k-h