diff mbox series

[v2,08/15] usb: misc: idmouse: update to use usb_control_msg_send()

Message ID 20201130013031.2580265-1-anant.thazhemadam@gmail.com
State New
Headers show
Series drivers: usb: misc: update to use usb_control_msg_{send|recv}() | expand

Commit Message

Anant Thazhemadam Nov. 30, 2020, 1:30 a.m. UTC
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 an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_send() appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/idmouse.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Johan Hovold Dec. 4, 2020, 2:46 p.m. UTC | #1
On Mon, Nov 30, 2020 at 07:00:31AM +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 an error, data can be used off the stack, and raw usb pipes need not be

> created in the calling functions.

> For this reason, the instance of usb_control_msg() has been replaced with

> usb_control_msg_send() appropriately.

> 

> Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>

> ---

>  drivers/usb/misc/idmouse.c | 5 +++--

>  1 file changed, 3 insertions(+), 2 deletions(-)


Especially for control transfers without a data stage there isn't
really any benefit of the new helper.

I'd just leave this one unchanged.

> diff --git a/drivers/usb/misc/idmouse.c b/drivers/usb/misc/idmouse.c

> index e9437a176518..52126441a633 100644

> --- a/drivers/usb/misc/idmouse.c

> +++ b/drivers/usb/misc/idmouse.c

> @@ -56,8 +56,9 @@ static const struct usb_device_id idmouse_table[] = {

>  #define FTIP_SCROLL  0x24

>  

>  #define ftip_command(dev, command, value, index) \

> -	usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0), command, \

> -	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, value, index, NULL, 0, 1000)

> +	usb_control_msg_send(dev->udev, 0, command, \

> +	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, \

> +	value, index, NULL, 0, 1000, GFP_KERNEL)

>  

>  MODULE_DEVICE_TABLE(usb, idmouse_table);


Johan
Anant Thazhemadam Jan. 7, 2021, 2:06 p.m. UTC | #2
On 04/12/20 8:16 pm, Johan Hovold wrote:
> On Mon, Nov 30, 2020 at 07:00:31AM +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 an error, data can be used off the stack, and raw usb pipes need not be

>> created in the calling functions.

>> For this reason, the instance of usb_control_msg() has been replaced with

>> usb_control_msg_send() appropriately.

>>

>> Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>

>> ---

>>  drivers/usb/misc/idmouse.c | 5 +++--

>>  1 file changed, 3 insertions(+), 2 deletions(-)

> Especially for control transfers without a data stage there isn't

> really any benefit of the new helper.

>

> I'd just leave this one unchanged.

>

>> diff --git a/drivers/usb/misc/idmouse.c b/drivers/usb/misc/idmouse.c

>> index e9437a176518..52126441a633 100644

>> --- a/drivers/usb/misc/idmouse.c

>> +++ b/drivers/usb/misc/idmouse.c

>> @@ -56,8 +56,9 @@ static const struct usb_device_id idmouse_table[] = {

>>  #define FTIP_SCROLL  0x24

>>  

>>  #define ftip_command(dev, command, value, index) \

>> -	usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0), command, \

>> -	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, value, index, NULL, 0, 1000)

>> +	usb_control_msg_send(dev->udev, 0, command, \

>> +	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, \

>> +	value, index, NULL, 0, 1000, GFP_KERNEL)

>>  

>>  MODULE_DEVICE_TABLE(usb, idmouse_table);

> Johan


Understood. I'll make sure this is left out in the v3.

Thanks,
Anant
diff mbox series

Patch

diff --git a/drivers/usb/misc/idmouse.c b/drivers/usb/misc/idmouse.c
index e9437a176518..52126441a633 100644
--- a/drivers/usb/misc/idmouse.c
+++ b/drivers/usb/misc/idmouse.c
@@ -56,8 +56,9 @@  static const struct usb_device_id idmouse_table[] = {
 #define FTIP_SCROLL  0x24
 
 #define ftip_command(dev, command, value, index) \
-	usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0), command, \
-	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, value, index, NULL, 0, 1000)
+	usb_control_msg_send(dev->udev, 0, command, \
+	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, \
+	value, index, NULL, 0, 1000, GFP_KERNEL)
 
 MODULE_DEVICE_TABLE(usb, idmouse_table);