From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Felix Fietkau <nbd@nbd.name>, linux-wireless@vger.kernel.org
Subject: Re: [PATCH 2/3] mt76usb: use synchronous msg for mcu command responses
Date: Wed, 20 Feb 2019 11:38:11 +0100 [thread overview]
Message-ID: <20190220103810.GC2626@localhost.localdomain> (raw)
In-Reply-To: <1550657565-7263-2-git-send-email-sgruszka@redhat.com>
> Use usb_bulk_msg for reading MCU command responses. This simplify code
> a lot.
>
> Together with 97a3005759c ("mt76usb: allow mt76u_bulk_msg be used
> for reads") it also fix possible problems with rx data buffers
> not being aligned and contained within single page. After doing
> page_frag_alloc(1024) consecutive page_frag_alloc(PAGE_SIZE) will
> alloc PAGE_SIZE buffer at PAGE_SIZE - 1024 offset.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Hi Stanislaw,
very nice simplification. I need to post a series and this patch will help
me a lot, thx :)
Just few comments inline
Regards,
Lorenzo
> ---
> drivers/net/wireless/mediatek/mt76/mt76.h | 3 +-
> drivers/net/wireless/mediatek/mt76/mt76x0/usb.c | 11 --------
> .../net/wireless/mediatek/mt76/mt76x02_usb_mcu.c | 32 +++++++---------------
> drivers/net/wireless/mediatek/mt76/mt76x2/usb.c | 11 --------
> drivers/net/wireless/mediatek/mt76/usb.c | 1 -
> drivers/net/wireless/mediatek/mt76/usb_mcu.c | 31 +++------------------
> 6 files changed, 15 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 45e44bcaa523..ce53069461ec 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -384,8 +384,7 @@ struct mt76_usb {
>
> struct mt76u_mcu {
> struct mutex mutex;
> - struct completion cmpl;
> - struct mt76u_buf res;
> + u8 *data;
> u32 msg_seq;
>
> /* multiple reads */
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
> index da9d05f6074d..f0c33890f1a5 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
> @@ -311,13 +311,11 @@ static int __maybe_unused mt76x0_suspend(struct usb_interface *usb_intf,
> pm_message_t state)
> {
> struct mt76x02_dev *dev = usb_get_intfdata(usb_intf);
> - struct mt76_usb *usb = &dev->mt76.usb;
>
> mt76u_stop_queues(&dev->mt76);
> mt76x0u_mac_stop(dev);
> clear_bit(MT76_STATE_MCU_RUNNING, &dev->mt76.state);
> mt76x0_chip_onoff(dev, false, false);
> - usb_kill_urb(usb->mcu.res.urb);
>
> return 0;
> }
> @@ -328,15 +326,6 @@ static int __maybe_unused mt76x0_resume(struct usb_interface *usb_intf)
> struct mt76_usb *usb = &dev->mt76.usb;
> int ret;
>
> - reinit_completion(&usb->mcu.cmpl);
> - ret = mt76u_submit_buf(&dev->mt76, USB_DIR_IN,
> - MT_EP_IN_CMD_RESP,
> - &usb->mcu.res, GFP_KERNEL,
> - mt76u_mcu_complete_urb,
> - &usb->mcu.cmpl);
> - if (ret < 0)
> - goto err;
> -
> ret = mt76u_submit_rx_buffers(&dev->mt76);
> if (ret < 0)
> goto err;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
> index f497c8e4332a..0cb8751321a1 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
> @@ -61,33 +61,21 @@ mt76x02u_multiple_mcu_reads(struct mt76_dev *dev, u8 *data, int len)
> static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 seq)
> {
> struct mt76_usb *usb = &dev->usb;
> - struct mt76u_buf *buf = &usb->mcu.res;
> - struct urb *urb = buf->urb;
> - u8 *data = buf->buf;
> - int i, ret;
> + u8 *data = usb->mcu.data;
> + int i, len, ret;
> u32 rxfce;
>
> for (i = 0; i < 5; i++) {
> - if (!wait_for_completion_timeout(&usb->mcu.cmpl,
> - msecs_to_jiffies(300)))
> + ret = mt76u_bulk_msg(dev, data, MCU_RESP_URB_SIZE, &len, 300);
> + if (ret == -ETIMEDOUT)
> continue;
> -
> - if (urb->status)
> - return -EIO;
> + if (ret)
> + goto out;
>
> if (usb->mcu.rp)
> - mt76x02u_multiple_mcu_reads(dev, data + 4,
> - urb->actual_length - 8);
> + mt76x02u_multiple_mcu_reads(dev, data + 4, len - 8);
>
> rxfce = get_unaligned_le32(data);
> - ret = mt76u_submit_buf(dev, USB_DIR_IN,
> - MT_EP_IN_CMD_RESP,
> - buf, GFP_KERNEL,
> - mt76u_mcu_complete_urb,
> - &usb->mcu.cmpl);
> - if (ret)
> - return ret;
> -
> if (seq == FIELD_GET(MT_RX_FCE_INFO_CMD_SEQ, rxfce) &&
> FIELD_GET(MT_RX_FCE_INFO_EVT_TYPE, rxfce) == EVT_CMD_DONE)
> return 0;
> @@ -96,9 +84,9 @@ static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 seq)
> FIELD_GET(MT_RX_FCE_INFO_EVT_TYPE, rxfce),
> seq, FIELD_GET(MT_RX_FCE_INFO_CMD_SEQ, rxfce));
> }
> -
> - dev_err(dev->dev, "error: %s timed out\n", __func__);
> - return -ETIMEDOUT;
> +out:
> + dev_err(dev->dev, "error: %s failed with %d\n", __func__, ret);
> + return ret;
> }
>
> static int
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c
> index f81a85e96922..ddb6b2c48e01 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c
> @@ -100,11 +100,9 @@ static int __maybe_unused mt76x2u_suspend(struct usb_interface *intf,
> pm_message_t state)
> {
> struct mt76x02_dev *dev = usb_get_intfdata(intf);
> - struct mt76_usb *usb = &dev->mt76.usb;
>
> mt76u_stop_queues(&dev->mt76);
> mt76x2u_stop_hw(dev);
> - usb_kill_urb(usb->mcu.res.urb);
>
> return 0;
> }
> @@ -115,15 +113,6 @@ static int __maybe_unused mt76x2u_resume(struct usb_interface *intf)
> struct mt76_usb *usb = &dev->mt76.usb;
> int err;
>
> - reinit_completion(&usb->mcu.cmpl);
> - err = mt76u_submit_buf(&dev->mt76, USB_DIR_IN,
> - MT_EP_IN_CMD_RESP,
> - &usb->mcu.res, GFP_KERNEL,
> - mt76u_mcu_complete_urb,
> - &usb->mcu.cmpl);
> - if (err < 0)
> - goto err;
> -
> err = mt76u_submit_rx_buffers(&dev->mt76);
> if (err < 0)
> goto err;
> diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> index 78191968b4fa..5c3b7f735aae 100644
> --- a/drivers/net/wireless/mediatek/mt76/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> @@ -933,7 +933,6 @@ int mt76u_init(struct mt76_dev *dev,
> INIT_DELAYED_WORK(&usb->stat_work, mt76u_tx_status_data);
> skb_queue_head_init(&dev->rx_skb[MT_RXQ_MAIN]);
>
> - init_completion(&usb->mcu.cmpl);
> mutex_init(&usb->mcu.mutex);
>
> mutex_init(&usb->usb_ctrl_mtx);
> diff --git a/drivers/net/wireless/mediatek/mt76/usb_mcu.c b/drivers/net/wireless/mediatek/mt76/usb_mcu.c
> index 72c8607da4b4..747231edc57d 100644
> --- a/drivers/net/wireless/mediatek/mt76/usb_mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/usb_mcu.c
> @@ -16,42 +16,19 @@
>
> #include "mt76.h"
>
> -void mt76u_mcu_complete_urb(struct urb *urb)
> -{
> - struct completion *cmpl = urb->context;
> -
> - complete(cmpl);
> -}
> -EXPORT_SYMBOL_GPL(mt76u_mcu_complete_urb);
> -
> int mt76u_mcu_init_rx(struct mt76_dev *dev)
> {
> struct mt76_usb *usb = &dev->usb;
> - int err;
>
> - err = mt76u_buf_alloc(dev, &usb->mcu.res, MCU_RESP_URB_SIZE,
> - MCU_RESP_URB_SIZE, GFP_KERNEL);
> - if (err < 0)
> - return err;
> -
> - err = mt76u_submit_buf(dev, USB_DIR_IN, MT_EP_IN_CMD_RESP,
> - &usb->mcu.res, GFP_KERNEL,
> - mt76u_mcu_complete_urb,
> - &usb->mcu.cmpl);
> - if (err < 0)
> - mt76u_buf_free(&usb->mcu.res);
> -
> - return err;
> + usb->mcu.data = kmalloc(MCU_RESP_URB_SIZE, GFP_KERNEL);
if you use devm_kmalloc() you can drop mt76u_mcu_deinit
Regards,
Lorenzo
> + return usb->mcu.data ? 0 : -ENOMEM;
> }
> EXPORT_SYMBOL_GPL(mt76u_mcu_init_rx);
>
> void mt76u_mcu_deinit(struct mt76_dev *dev)
> {
> - struct mt76u_buf *buf = &dev->usb.mcu.res;
> + struct mt76_usb *usb = &dev->usb;
>
> - if (buf->urb) {
> - usb_kill_urb(buf->urb);
> - mt76u_buf_free(buf);
> - }
> + kfree(usb->mcu.data);
> }
> EXPORT_SYMBOL_GPL(mt76u_mcu_deinit);
> --
> 2.7.5
>
next prev parent reply other threads:[~2019-02-20 10:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-20 10:12 [PATCH 1/3] mt76usb: allow mt76u_bulk_msg be used for reads Stanislaw Gruszka
2019-02-20 10:12 ` [PATCH 2/3] mt76usb: use synchronous msg for mcu command responses Stanislaw Gruszka
2019-02-20 10:38 ` Lorenzo Bianconi [this message]
2019-02-20 10:12 ` [PATCH 3/3] mt76usb: remove usb_mcu.c Stanislaw Gruszka
2019-02-20 10:41 ` Lorenzo Bianconi
2019-02-20 10:31 ` [PATCH 1/3] mt76usb: allow mt76u_bulk_msg be used for reads Lorenzo Bianconi
2019-02-20 11:28 ` Lorenzo Bianconi
2019-02-20 12:01 ` Stanislaw Gruszka
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=20190220103810.GC2626@localhost.localdomain \
--to=lorenzo.bianconi@redhat.com \
--cc=linux-wireless@vger.kernel.org \
--cc=nbd@nbd.name \
--cc=sgruszka@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.