From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To: sean.wang@mediatek.com
Cc: nbd@nbd.name, Soul.Huang@mediatek.com, YN.Chen@mediatek.com,
Leon.Yen@mediatek.com, Eric-SY.Chang@mediatek.com,
Mark-YW.Chen@mediatek.com, Deren.Wu@mediatek.com,
km.lin@mediatek.com, jenhao.yang@mediatek.com,
robin.chiu@mediatek.com, Eddie.Chen@mediatek.com,
ch.yeh@mediatek.com, posh.sun@mediatek.com,
ted.huang@mediatek.com, Eric.Liang@mediatek.com,
Stella.Chang@mediatek.com, Tom.Chou@mediatek.com,
steve.lee@mediatek.com, jsiuda@google.com, frankgor@google.com,
jemele@google.com, abhishekpandit@google.com, shawnku@google.com,
linux-wireless@vger.kernel.org,
linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 1/3] mt76: sdio: remove those unnecessary buffers in sdio.xmit_buf
Date: Thu, 13 Jan 2022 10:09:00 +0100 [thread overview]
Message-ID: <Yd/sLP8spK7S5M9U@lore-desk> (raw)
In-Reply-To: <651a010d8ff888909b5b8363fa4b243aadc62d45.1642061035.git.objelf@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 5527 bytes --]
> From: Sean Wang <sean.wang@mediatek.com>
>
> We don't have to create a separate sdio.xmit_buf buffer for each queue.
> Instead, we just need to create one, reuse it across all queues to reduce
> memory consumption further.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
> drivers/net/wireless/mediatek/mt76/mt76.h | 2 +-
> drivers/net/wireless/mediatek/mt76/mt7615/sdio.c | 15 ++++++---------
> drivers/net/wireless/mediatek/mt76/mt7921/sdio.c | 15 ++++++---------
> drivers/net/wireless/mediatek/mt76/sdio_txrx.c | 15 ++++++---------
> 4 files changed, 19 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 14f60fcb6a34..4029a2217397 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -507,7 +507,7 @@ struct mt76_sdio {
>
> struct work_struct stat_work;
>
> - u8 *xmit_buf[IEEE80211_NUM_ACS + 2];
> + u8 *xmit_buf;
>
> struct sdio_func *func;
> void *intr_data;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
> index 71162befdae8..554160b0ea9a 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
> @@ -101,7 +101,7 @@ static int mt7663s_probe(struct sdio_func *func,
> struct ieee80211_ops *ops;
> struct mt7615_dev *dev;
> struct mt76_dev *mdev;
> - int i, ret;
> + int ret;
>
> ops = devm_kmemdup(&func->dev, &mt7615_ops, sizeof(mt7615_ops),
> GFP_KERNEL);
> @@ -140,14 +140,11 @@ static int mt7663s_probe(struct sdio_func *func,
> goto error;
> }
>
> - for (i = 0; i < ARRAY_SIZE(mdev->sdio.xmit_buf); i++) {
> - mdev->sdio.xmit_buf[i] = devm_kmalloc(mdev->dev,
> - MT76S_XMIT_BUF_SZ,
> - GFP_KERNEL);
> - if (!mdev->sdio.xmit_buf[i]) {
> - ret = -ENOMEM;
> - goto error;
> - }
> + mdev->sdio.xmit_buf = devm_kmalloc(mdev->dev, MT76S_XMIT_BUF_SZ,
> + GFP_KERNEL);
> + if (!mdev->sdio.xmit_buf) {
> + ret = -ENOMEM;
> + goto error;
> }
I guess we can move buffer allocation in mt76s_init() and remove duplicated
code in mt7663/mt7921.
Reagards,
Lorenzo
>
> ret = mt76s_alloc_rx_queue(mdev, MT_RXQ_MAIN);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> index 743b63f66efa..c58c14e28430 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> @@ -121,7 +121,7 @@ static int mt7921s_probe(struct sdio_func *func,
>
> struct mt7921_dev *dev;
> struct mt76_dev *mdev;
> - int ret, i;
> + int ret;
>
> mdev = mt76_alloc_device(&func->dev, sizeof(*dev), &mt7921_ops,
> &drv_ops);
> @@ -154,14 +154,11 @@ static int mt7921s_probe(struct sdio_func *func,
> goto error;
> }
>
> - for (i = 0; i < ARRAY_SIZE(mdev->sdio.xmit_buf); i++) {
> - mdev->sdio.xmit_buf[i] = devm_kmalloc(mdev->dev,
> - MT76S_XMIT_BUF_SZ,
> - GFP_KERNEL);
> - if (!mdev->sdio.xmit_buf[i]) {
> - ret = -ENOMEM;
> - goto error;
> - }
> + mdev->sdio.xmit_buf = devm_kmalloc(mdev->dev, MT76S_XMIT_BUF_SZ,
> + GFP_KERNEL);
> + if (!mdev->sdio.xmit_buf) {
> + ret = -ENOMEM;
> + goto error;
> }
>
> ret = mt76s_alloc_rx_queue(mdev, MT_RXQ_MAIN);
> diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> index 488ad7734d85..a04cd2444247 100644
> --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> @@ -229,12 +229,11 @@ static int __mt76s_xmit_queue(struct mt76_dev *dev, u8 *data, int len)
>
> static int mt76s_tx_run_queue(struct mt76_dev *dev, struct mt76_queue *q)
> {
> - int qid, err, nframes = 0, len = 0, pse_sz = 0, ple_sz = 0;
> + int err, nframes = 0, len = 0, pse_sz = 0, ple_sz = 0;
> bool mcu = q == dev->q_mcu[MT_MCUQ_WM];
> struct mt76_sdio *sdio = &dev->sdio;
> u8 pad;
>
> - qid = mcu ? ARRAY_SIZE(sdio->xmit_buf) - 1 : q->qid;
> while (q->first != q->head) {
> struct mt76_queue_entry *e = &q->entry[q->first];
> struct sk_buff *iter;
> @@ -262,20 +261,18 @@ static int mt76s_tx_run_queue(struct mt76_dev *dev, struct mt76_queue *q)
> &ple_sz))
> break;
>
> - memcpy(sdio->xmit_buf[qid] + len, e->skb->data,
> - skb_headlen(e->skb));
> + memcpy(sdio->xmit_buf + len, e->skb->data, skb_headlen(e->skb));
> len += skb_headlen(e->skb);
> nframes++;
>
> skb_walk_frags(e->skb, iter) {
> - memcpy(sdio->xmit_buf[qid] + len, iter->data,
> - iter->len);
> + memcpy(sdio->xmit_buf + len, iter->data, iter->len);
> len += iter->len;
> nframes++;
> }
>
> if (unlikely(pad)) {
> - memset(sdio->xmit_buf[qid] + len, 0, pad);
> + memset(sdio->xmit_buf + len, 0, pad);
> len += pad;
> }
> next:
> @@ -284,8 +281,8 @@ static int mt76s_tx_run_queue(struct mt76_dev *dev, struct mt76_queue *q)
> }
>
> if (nframes) {
> - memset(sdio->xmit_buf[qid] + len, 0, 4);
> - err = __mt76s_xmit_queue(dev, sdio->xmit_buf[qid], len + 4);
> + memset(sdio->xmit_buf + len, 0, 4);
> + err = __mt76s_xmit_queue(dev, sdio->xmit_buf, len + 4);
> if (err)
> return err;
> }
> --
> 2.25.1
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
WARNING: multiple messages have this Message-ID (diff)
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To: sean.wang@mediatek.com
Cc: nbd@nbd.name, Soul.Huang@mediatek.com, YN.Chen@mediatek.com,
Leon.Yen@mediatek.com, Eric-SY.Chang@mediatek.com,
Mark-YW.Chen@mediatek.com, Deren.Wu@mediatek.com,
km.lin@mediatek.com, jenhao.yang@mediatek.com,
robin.chiu@mediatek.com, Eddie.Chen@mediatek.com,
ch.yeh@mediatek.com, posh.sun@mediatek.com,
ted.huang@mediatek.com, Eric.Liang@mediatek.com,
Stella.Chang@mediatek.com, Tom.Chou@mediatek.com,
steve.lee@mediatek.com, jsiuda@google.com, frankgor@google.com,
jemele@google.com, abhishekpandit@google.com, shawnku@google.com,
linux-wireless@vger.kernel.org,
linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 1/3] mt76: sdio: remove those unnecessary buffers in sdio.xmit_buf
Date: Thu, 13 Jan 2022 10:09:00 +0100 [thread overview]
Message-ID: <Yd/sLP8spK7S5M9U@lore-desk> (raw)
In-Reply-To: <651a010d8ff888909b5b8363fa4b243aadc62d45.1642061035.git.objelf@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5527 bytes --]
> From: Sean Wang <sean.wang@mediatek.com>
>
> We don't have to create a separate sdio.xmit_buf buffer for each queue.
> Instead, we just need to create one, reuse it across all queues to reduce
> memory consumption further.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
> drivers/net/wireless/mediatek/mt76/mt76.h | 2 +-
> drivers/net/wireless/mediatek/mt76/mt7615/sdio.c | 15 ++++++---------
> drivers/net/wireless/mediatek/mt76/mt7921/sdio.c | 15 ++++++---------
> drivers/net/wireless/mediatek/mt76/sdio_txrx.c | 15 ++++++---------
> 4 files changed, 19 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 14f60fcb6a34..4029a2217397 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -507,7 +507,7 @@ struct mt76_sdio {
>
> struct work_struct stat_work;
>
> - u8 *xmit_buf[IEEE80211_NUM_ACS + 2];
> + u8 *xmit_buf;
>
> struct sdio_func *func;
> void *intr_data;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
> index 71162befdae8..554160b0ea9a 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
> @@ -101,7 +101,7 @@ static int mt7663s_probe(struct sdio_func *func,
> struct ieee80211_ops *ops;
> struct mt7615_dev *dev;
> struct mt76_dev *mdev;
> - int i, ret;
> + int ret;
>
> ops = devm_kmemdup(&func->dev, &mt7615_ops, sizeof(mt7615_ops),
> GFP_KERNEL);
> @@ -140,14 +140,11 @@ static int mt7663s_probe(struct sdio_func *func,
> goto error;
> }
>
> - for (i = 0; i < ARRAY_SIZE(mdev->sdio.xmit_buf); i++) {
> - mdev->sdio.xmit_buf[i] = devm_kmalloc(mdev->dev,
> - MT76S_XMIT_BUF_SZ,
> - GFP_KERNEL);
> - if (!mdev->sdio.xmit_buf[i]) {
> - ret = -ENOMEM;
> - goto error;
> - }
> + mdev->sdio.xmit_buf = devm_kmalloc(mdev->dev, MT76S_XMIT_BUF_SZ,
> + GFP_KERNEL);
> + if (!mdev->sdio.xmit_buf) {
> + ret = -ENOMEM;
> + goto error;
> }
I guess we can move buffer allocation in mt76s_init() and remove duplicated
code in mt7663/mt7921.
Reagards,
Lorenzo
>
> ret = mt76s_alloc_rx_queue(mdev, MT_RXQ_MAIN);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> index 743b63f66efa..c58c14e28430 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> @@ -121,7 +121,7 @@ static int mt7921s_probe(struct sdio_func *func,
>
> struct mt7921_dev *dev;
> struct mt76_dev *mdev;
> - int ret, i;
> + int ret;
>
> mdev = mt76_alloc_device(&func->dev, sizeof(*dev), &mt7921_ops,
> &drv_ops);
> @@ -154,14 +154,11 @@ static int mt7921s_probe(struct sdio_func *func,
> goto error;
> }
>
> - for (i = 0; i < ARRAY_SIZE(mdev->sdio.xmit_buf); i++) {
> - mdev->sdio.xmit_buf[i] = devm_kmalloc(mdev->dev,
> - MT76S_XMIT_BUF_SZ,
> - GFP_KERNEL);
> - if (!mdev->sdio.xmit_buf[i]) {
> - ret = -ENOMEM;
> - goto error;
> - }
> + mdev->sdio.xmit_buf = devm_kmalloc(mdev->dev, MT76S_XMIT_BUF_SZ,
> + GFP_KERNEL);
> + if (!mdev->sdio.xmit_buf) {
> + ret = -ENOMEM;
> + goto error;
> }
>
> ret = mt76s_alloc_rx_queue(mdev, MT_RXQ_MAIN);
> diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> index 488ad7734d85..a04cd2444247 100644
> --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> @@ -229,12 +229,11 @@ static int __mt76s_xmit_queue(struct mt76_dev *dev, u8 *data, int len)
>
> static int mt76s_tx_run_queue(struct mt76_dev *dev, struct mt76_queue *q)
> {
> - int qid, err, nframes = 0, len = 0, pse_sz = 0, ple_sz = 0;
> + int err, nframes = 0, len = 0, pse_sz = 0, ple_sz = 0;
> bool mcu = q == dev->q_mcu[MT_MCUQ_WM];
> struct mt76_sdio *sdio = &dev->sdio;
> u8 pad;
>
> - qid = mcu ? ARRAY_SIZE(sdio->xmit_buf) - 1 : q->qid;
> while (q->first != q->head) {
> struct mt76_queue_entry *e = &q->entry[q->first];
> struct sk_buff *iter;
> @@ -262,20 +261,18 @@ static int mt76s_tx_run_queue(struct mt76_dev *dev, struct mt76_queue *q)
> &ple_sz))
> break;
>
> - memcpy(sdio->xmit_buf[qid] + len, e->skb->data,
> - skb_headlen(e->skb));
> + memcpy(sdio->xmit_buf + len, e->skb->data, skb_headlen(e->skb));
> len += skb_headlen(e->skb);
> nframes++;
>
> skb_walk_frags(e->skb, iter) {
> - memcpy(sdio->xmit_buf[qid] + len, iter->data,
> - iter->len);
> + memcpy(sdio->xmit_buf + len, iter->data, iter->len);
> len += iter->len;
> nframes++;
> }
>
> if (unlikely(pad)) {
> - memset(sdio->xmit_buf[qid] + len, 0, pad);
> + memset(sdio->xmit_buf + len, 0, pad);
> len += pad;
> }
> next:
> @@ -284,8 +281,8 @@ static int mt76s_tx_run_queue(struct mt76_dev *dev, struct mt76_queue *q)
> }
>
> if (nframes) {
> - memset(sdio->xmit_buf[qid] + len, 0, 4);
> - err = __mt76s_xmit_queue(dev, sdio->xmit_buf[qid], len + 4);
> + memset(sdio->xmit_buf + len, 0, 4);
> + err = __mt76s_xmit_queue(dev, sdio->xmit_buf, len + 4);
> if (err)
> return err;
> }
> --
> 2.25.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2022-01-13 9:09 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-13 8:23 [PATCH 1/3] mt76: sdio: remove those unnecessary buffers in sdio.xmit_buf sean.wang
2022-01-13 8:23 ` sean.wang
2022-01-13 8:23 ` [PATCH 2/3] mt76: sdio: honor the largest Tx buffer the hardware can support sean.wang
2022-01-13 8:23 ` sean.wang
2022-01-13 9:09 ` Lorenzo Bianconi
2022-01-13 9:09 ` Lorenzo Bianconi
2022-01-13 8:23 ` [PATCH 3/3] mt76: mt7921s: run sleep mode by default sean.wang
2022-01-13 8:23 ` sean.wang
2022-01-13 9:03 ` Lorenzo Bianconi
2022-01-13 9:03 ` Lorenzo Bianconi
2022-01-13 9:09 ` Lorenzo Bianconi [this message]
2022-01-13 9:09 ` [PATCH 1/3] mt76: sdio: remove those unnecessary buffers in sdio.xmit_buf Lorenzo Bianconi
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=Yd/sLP8spK7S5M9U@lore-desk \
--to=lorenzo.bianconi@redhat.com \
--cc=Deren.Wu@mediatek.com \
--cc=Eddie.Chen@mediatek.com \
--cc=Eric-SY.Chang@mediatek.com \
--cc=Eric.Liang@mediatek.com \
--cc=Leon.Yen@mediatek.com \
--cc=Mark-YW.Chen@mediatek.com \
--cc=Soul.Huang@mediatek.com \
--cc=Stella.Chang@mediatek.com \
--cc=Tom.Chou@mediatek.com \
--cc=YN.Chen@mediatek.com \
--cc=abhishekpandit@google.com \
--cc=ch.yeh@mediatek.com \
--cc=frankgor@google.com \
--cc=jemele@google.com \
--cc=jenhao.yang@mediatek.com \
--cc=jsiuda@google.com \
--cc=km.lin@mediatek.com \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=nbd@nbd.name \
--cc=posh.sun@mediatek.com \
--cc=robin.chiu@mediatek.com \
--cc=sean.wang@mediatek.com \
--cc=shawnku@google.com \
--cc=steve.lee@mediatek.com \
--cc=ted.huang@mediatek.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.