On 8/24/21 11:53 AM, Fabio M. De Francesco wrote:
On Tuesday, August 24, 2021 10:13:46 AM CEST Pavel Skripkin wrote:
On 8/24/21 1:37 AM, 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().
>
> Suggested-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx>
> ---
>
> Thanks to Pavel Skripkin <paskripkin@xxxxxxxxx> for his review of the
> RFC patch.
>
> drivers/staging/r8188eu/hal/usb_ops_linux.c | 25 ++++++++++-----------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> [...]
>
Hi, Fabio!
Christophe is right about semantic part.
Hi Pavel,
I haven't yet read Christophe's message (but I'm going to do it ASAP).
I hope he found out what is wrong with the code, what made Phil's tests
fail.
Also,
if (!status) {
} else {
if (status < 0) { <-
|
} else { |
|
} <-
}
Extra if-else is not needed, since status can be 0 and < 0, there is no
3rd state, like it was before.
Correct, thanks!
Now I read the following from the documentation of the new API...
"Return: If successful, 0 is returned, Otherwise, a negative error number."
I'll remove that status < 0 check and whatever else is no more necessary.
Thanks, again :)
Regards,
Btw, not related to your patch, but I start think, that this check:
if (!pIo_buf) {
DBG_88E("[%s] pIo_buf == NULL\n", __func__);
status = -ENOMEM;
goto release_mutex;
}
Should be wrapped as
if (WARN_ON(unlikely(!pIo_buf)) {
...
}
Since usb_vendor_req_buf is initialized in ->probe() and I can't see
possible calltrace, which can cause zeroing this pointer.
Something _completely_ wrong is going on if usb_vendor_req_buf is NULL,
and we should complain loud about it. What do you think?
With regards,
Pavel Skripkin
