From: Takashi Iwai <tiwai@suse.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: himadrispandya@gmail.com, dvyukov@google.com,
linux-usb@vger.kernel.org, perex@perex.cz, tiwai@suse.com,
stern@rowland.harvard.ed, linux-kernel@vger.kernel.org,
marcel@holtmann.org, johan.hedberg@gmail.com,
linux-bluetooth@vger.kernel.org, alsa-devel@alsa-project.org,
Vasily Khoruzhick <anarsoul@gmail.com>
Subject: Re: [PATCH 08/10] sound: line6: move to use usb_control_msg_send() and usb_control_msg_recv()
Date: Wed, 02 Sep 2020 16:38:40 +0200 [thread overview]
Message-ID: <s5hd0349rbj.wl-tiwai@suse.de> (raw)
In-Reply-To: <20200902110115.1994491-11-gregkh@linuxfoundation.org>
On Wed, 02 Sep 2020 13:01:12 +0200,
Greg Kroah-Hartman wrote:
>
> The usb_control_msg_send() and usb_control_msg_recv() calls can return
> an error if a "short" write/read happens, and they can handle data off
> of the stack, so move the driver over to using those calls instead,
> saving some logic when dynamically allocating memory.
>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Vasily Khoruzhick <anarsoul@gmail.com>
> Cc: alsa-devel@alsa-project.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
thanks,
Takashi
> ---
> sound/usb/line6/driver.c | 69 +++++++++++++++-----------------------
> sound/usb/line6/podhd.c | 17 ++++------
> sound/usb/line6/toneport.c | 8 ++---
> 3 files changed, 37 insertions(+), 57 deletions(-)
>
> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
> index 60674ce4879b..601292c51491 100644
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -337,23 +337,18 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
> {
> struct usb_device *usbdev = line6->usbdev;
> int ret;
> - unsigned char *len;
> + u8 len;
> unsigned count;
>
> if (address > 0xffff || datalen > 0xff)
> return -EINVAL;
>
> - len = kmalloc(1, GFP_KERNEL);
> - if (!len)
> - return -ENOMEM;
> -
> /* query the serial number: */
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> - (datalen << 8) | 0x21, address,
> - NULL, 0, LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0) {
> + ret = usb_control_msg_send(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + (datalen << 8) | 0x21, address, NULL, 0,
> + LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
> goto exit;
> }
> @@ -362,45 +357,41 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
> for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
> mdelay(LINE6_READ_WRITE_STATUS_DELAY);
>
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE |
> - USB_DIR_IN,
> - 0x0012, 0x0000, len, 1,
> - LINE6_TIMEOUT * HZ);
> - if (ret < 0) {
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> + 0x0012, 0x0000, &len, 1,
> + LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev,
> "receive length failed (error %d)\n", ret);
> goto exit;
> }
>
> - if (*len != 0xff)
> + if (len != 0xff)
> break;
> }
>
> ret = -EIO;
> - if (*len == 0xff) {
> + if (len == 0xff) {
> dev_err(line6->ifcdev, "read failed after %d retries\n",
> count);
> goto exit;
> - } else if (*len != datalen) {
> + } else if (len != datalen) {
> /* should be equal or something went wrong */
> dev_err(line6->ifcdev,
> "length mismatch (expected %d, got %d)\n",
> - (int)datalen, (int)*len);
> + (int)datalen, len);
> goto exit;
> }
>
> /* receive the result: */
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> - 0x0013, 0x0000, data, datalen,
> - LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0)
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> + 0x0013, 0x0000, data, datalen, LINE6_TIMEOUT * HZ);
> + if (ret)
> dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
>
> exit:
> - kfree(len);
> return ret;
> }
> EXPORT_SYMBOL_GPL(line6_read_data);
> @@ -423,12 +414,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
> if (!status)
> return -ENOMEM;
>
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> - 0x0022, address, data, datalen,
> - LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0) {
> + ret = usb_control_msg_send(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + 0x0022, address, data, datalen, LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev,
> "write request failed (error %d)\n", ret);
> goto exit;
> @@ -437,14 +426,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
> for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
> mdelay(LINE6_READ_WRITE_STATUS_DELAY);
>
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
> - 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE |
> - USB_DIR_IN,
> - 0x0012, 0x0000,
> - status, 1, LINE6_TIMEOUT * HZ);
> -
> - if (ret < 0) {
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> + 0x0012, 0x0000, status, 1, LINE6_TIMEOUT * HZ);
> + if (ret) {
> dev_err(line6->ifcdev,
> "receiving status failed (error %d)\n", ret);
> goto exit;
> diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c
> index eef45f7fef0d..a1261f55d62b 100644
> --- a/sound/usb/line6/podhd.c
> +++ b/sound/usb/line6/podhd.c
> @@ -183,29 +183,25 @@ static const struct attribute_group podhd_dev_attr_group = {
> static int podhd_dev_start(struct usb_line6_podhd *pod)
> {
> int ret;
> - u8 *init_bytes;
> + u8 init_bytes[8];
> int i;
> struct usb_device *usbdev = pod->line6.usbdev;
>
> - init_bytes = kmalloc(8, GFP_KERNEL);
> - if (!init_bytes)
> - return -ENOMEM;
> -
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> + ret = usb_control_msg_send(usbdev, 0,
> 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> 0x11, 0,
> NULL, 0, LINE6_TIMEOUT * HZ);
> - if (ret < 0) {
> + if (ret) {
> dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
> goto exit;
> }
>
> /* NOTE: looks like some kind of ping message */
> - ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> + ret = usb_control_msg_recv(usbdev, 0, 0x67,
> USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> 0x11, 0x0,
> init_bytes, 3, LINE6_TIMEOUT * HZ);
> - if (ret < 0) {
> + if (ret) {
> dev_err(pod->line6.ifcdev,
> "receive length failed (error %d)\n", ret);
> goto exit;
> @@ -220,13 +216,12 @@ static int podhd_dev_start(struct usb_line6_podhd *pod)
> goto exit;
> }
>
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> + ret = usb_control_msg_send(usbdev, 0,
> USB_REQ_SET_FEATURE,
> USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
> 1, 0,
> NULL, 0, LINE6_TIMEOUT * HZ);
> exit:
> - kfree(init_bytes);
> return ret;
> }
>
> diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c
> index 94dd5e7ab2e6..a9b56085b76a 100644
> --- a/sound/usb/line6/toneport.c
> +++ b/sound/usb/line6/toneport.c
> @@ -126,11 +126,11 @@ static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2)
> {
> int ret;
>
> - ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> - cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
> + ret = usb_control_msg_send(usbdev, 0, 0x67,
> + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> + cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
>
> - if (ret < 0) {
> + if (ret) {
> dev_err(&usbdev->dev, "send failed (error %d)\n", ret);
> return ret;
> }
> --
> 2.28.0
>
next prev parent reply other threads:[~2020-09-02 14:39 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-02 11:01 [PATCH 00/10] USB: new USB control message helper functions Greg Kroah-Hartman
2020-09-02 11:01 ` [PATCH 01/10] USB: move snd_usb_pipe_sanity_check into the USB core Greg Kroah-Hartman
2020-09-02 14:35 ` Takashi Iwai
2020-09-07 14:42 ` Greg Kroah-Hartman
2020-09-03 0:45 ` Alan Stern
2020-09-03 6:40 ` Takashi Iwai
2020-09-03 7:32 ` Greg Kroah-Hartman
2020-09-07 14:16 ` Greg Kroah-Hartman
2020-09-07 14:24 ` Alan Stern
2020-09-02 11:01 ` [PATCH 02/10] USB: add usb_control_msg_send() and usb_control_msg_recv() Greg Kroah-Hartman
2020-09-02 11:01 ` [PATCH 03/10] USB: core: message.c: use usb_control_msg_send() in a few places Greg Kroah-Hartman
2020-09-02 11:23 ` Andy Shevchenko
2020-09-02 11:01 ` [PATCH 04/10] USB: core: hub.c: " Greg Kroah-Hartman
2020-09-02 14:57 ` Alan Stern
2020-09-02 15:21 ` Greg Kroah-Hartman
2020-09-02 11:01 ` [PATCH 05/10] USB: legousbtower: use usb_control_msg_recv() Greg Kroah-Hartman
2020-09-02 11:01 ` [PATCH 06/10] sound: usx2y: move to use usb_control_msg_send() Greg Kroah-Hartman
2020-09-02 14:36 ` Takashi Iwai
2020-09-02 11:01 ` [PATCH 07/10] sound: 6fire: move to use usb_control_msg_send() and usb_control_msg_recv() Greg Kroah-Hartman
2020-09-02 14:36 ` Takashi Iwai
2020-09-02 11:01 ` [PATCH 7/9] sound: line6: convert to use new usb control function Greg Kroah-Hartman
2020-09-02 14:41 ` Takashi Iwai
2020-09-02 15:22 ` Greg Kroah-Hartman
2020-09-02 11:01 ` [PATCH 8/9] sound: 6fire: move to use new usb control functions Greg Kroah-Hartman
2020-09-02 11:01 ` [PATCH 08/10] sound: line6: move to use usb_control_msg_send() and usb_control_msg_recv() Greg Kroah-Hartman
2020-09-02 14:38 ` Takashi Iwai [this message]
2020-09-02 11:01 ` [PATCH 9/9] sound: hiface: convert to use new usb_control functions Greg Kroah-Hartman
2020-09-02 11:01 ` [PATCH 09/10] sound: hiface: move to use usb_control_msg_send() Greg Kroah-Hartman
2020-09-02 14:40 ` Takashi Iwai
2020-09-02 11:01 ` [PATCH 10/10] Bluetooth: ath3k: use usb_control_msg_send() and usb_control_msg_recv() Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=s5hd0349rbj.wl-tiwai@suse.de \
--to=tiwai@suse.de \
--cc=alsa-devel@alsa-project.org \
--cc=anarsoul@gmail.com \
--cc=dvyukov@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=himadrispandya@gmail.com \
--cc=johan.hedberg@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=marcel@holtmann.org \
--cc=perex@perex.cz \
--cc=stern@rowland.harvard.ed \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).