Re: [PATCH v3 1/2] staging: r8188eu: Use usb_control_msg_recv/send() in usbctrl_vendorreq()

[Greg Kroah-Hartman]

On Wed, Aug 25, 2021 at 05:53:10AM +0200, Fabio M. De Francesco wrote:
> Replace usb_control_msg() with the new usb_control_msg_recv() and
> usb_control_msg_send() API of USB Core in usbctrl_vendorreq().
> Remove no more needed variables. Move out of an if-else block
> some code that it is no more dependent on status < 0. Remove
> redundant code depending on status > 0 or status == len.
> 
> Suggested-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx>
> ---
> 
> v2->v3: Restore the test for success of usb_control_message_recv/send
> that was inadvertently removed. Issue reported by Pavel Skripkin.
> 
> v1->v2: According to suggestions by Christophe JAILLET 
> <christophe.jaillet@xxxxxxxxxx>, remove 'pipe' and pass an explicit 0
> to the new API. According to suggestions by Pavel Skripkin 
> <paskripkin@xxxxxxxxx>, remove an extra if-else that is no more needed, 
> since status can be 0 and < 0 and there is no 3rd state, like it was before.
> Many thanks to them and also to Phillip Potter <phil@xxxxxxxxxxxxxxxx>
> who kindly offered his time for the purpose of testing v1.
> 
>  drivers/staging/r8188eu/hal/usb_ops_linux.c | 45 ++++++++-------------
>  1 file changed, 17 insertions(+), 28 deletions(-)

This doesn't apply to my tree at all.  Please rebase and resend.

But first, are you sure you want to use these new functions here?  This
is a "common" function that is called from different places for
different things.  How about unwinding the callers of this function
first, to see if they really need all of the complexity in this function
at all, and if not, then call the real USB function in those locations
instead.

It's only used in this single file, so it shouldn't be that hard to
unwind (after seeing where those calls are made from, and if they even
need to be present at all.  Hint, look at the mess of where _write16 and
friends are set to realize that structure is not needed at all, right?
It's a long chain, the more you pull on it, the messier you realize it
is...)

thanks,

greg k-h