From: Lorenzo Bianconi <lorenzo@kernel.org>
To: Roy Luo <royluo@google.com>
Cc: nbd@nbd.name, lorenzo.bianconi@redhat.com,
linux-wireless@vger.kernel.org,
"Ryder Lee (李庚諺)" <ryder.lee@mediatek.com>
Subject: Re: [RFC 10/17] mt7615: mcu: remove skb_ret from mt7615_mcu_msg_send
Date: Thu, 2 May 2019 17:31:43 +0200 [thread overview]
Message-ID: <20190502153142.GA7459@localhost.localdomain> (raw)
In-Reply-To: <CA+zupgzQTzQyihC_UskHD=63Ag3AnQcD4_pcas7VdaehyoCE1w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 11019 bytes --]
> Hi Lorenzo,
>
Hi Roy,
> IMHO, the skb_ret parameter from mt7615_mcu_msg_send is not just used by
> patch semaphore control.
> The other cmds also got their own event response that can be interpret
> accordingly by their own caller, which can provide the status of the cmd
> (success/fail) and other information that might be helpful case by case.
Do you mean the fw does not put the status code in a 'event struct'
at the beginning of the skb data buffer? What are the cmds that do not use
that approach?
Regards,
Lorenzo
> For now, we just assume every cmd works well and do no error handling,
> which is risky.
> If we are to take FW response into consideration in the future, maybe we
> should keep this parameter.
>
>
> Regards,
> Cheng-Hao (Roy) Luo
>
>
> On Thu, May 2, 2019 at 12:08 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> > Remove skb_ret parameter from mt7615_mcu_msg_send signature since it is
> > actually used just by mt7615_mcu_patch_sem_ctrl. This is a prelimanry
> > patch to use mt76 common mcu API
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > .../net/wireless/mediatek/mt76/mt7615/mcu.c | 77 ++++++-------------
> > 1 file changed, 24 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > index b8d928e8949c..4d1d4c0bc2e2 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > @@ -116,8 +116,7 @@ static int __mt7615_mcu_msg_send(struct mt7615_dev
> > *dev, struct sk_buff *skb,
> > }
> >
> > static int
> > -mt7615_mcu_msg_send(struct mt7615_dev *dev, struct sk_buff *skb,
> > - int cmd, struct sk_buff **skb_ret)
> > +mt7615_mcu_msg_send(struct mt7615_dev *dev, struct sk_buff *skb, int cmd)
> > {
> > unsigned long expires = jiffies + 10 * HZ;
> > struct mt7615_mcu_rxd *rxd;
> > @@ -142,18 +141,11 @@ mt7615_mcu_msg_send(struct mt7615_dev *dev, struct
> > sk_buff *skb,
> > if (seq != rxd->seq)
> > continue;
> >
> > - if (skb_ret) {
> > - int hdr_len = sizeof(*rxd);
> > -
> > - if (!test_bit(MT76_STATE_MCU_RUNNING,
> > - &dev->mt76.state))
> > - hdr_len -= 4;
> > - skb_pull(skb, hdr_len);
> > - *skb_ret = skb;
> > - } else {
> > - dev_kfree_skb(skb);
> > + if (cmd == -MCU_CMD_PATCH_SEM_CONTROL) {
> > + skb_pull(skb, sizeof(*rxd) - 4);
> > + ret = *skb->data;
> > }
> > -
> > + dev_kfree_skb(skb);
> > break;
> > }
> >
> > @@ -177,8 +169,7 @@ static int mt7615_mcu_init_download(struct mt7615_dev
> > *dev, u32 addr,
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb,
> > -MCU_CMD_TARGET_ADDRESS_LEN_REQ,
> > - NULL);
> > + return mt7615_mcu_msg_send(dev, skb,
> > -MCU_CMD_TARGET_ADDRESS_LEN_REQ);
> > }
> >
> > static int mt7615_mcu_send_firmware(struct mt7615_dev *dev, const void
> > *data,
> > @@ -219,43 +210,26 @@ static int mt7615_mcu_start_firmware(struct
> > mt7615_dev *dev, u32 addr,
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_FW_START_REQ, NULL);
> > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_FW_START_REQ);
> > }
> >
> > static int mt7615_mcu_restart(struct mt7615_dev *dev)
> > {
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(NULL, 0);
> >
> > - return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_RESTART_DL_REQ,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_RESTART_DL_REQ);
> > }
> >
> > static int mt7615_mcu_patch_sem_ctrl(struct mt7615_dev *dev, bool get)
> > {
> > struct {
> > - __le32 operation;
> > + __le32 op;
> > } req = {
> > - .operation = cpu_to_le32(get ? PATCH_SEM_GET :
> > - PATCH_SEM_RELEASE),
> > + .op = cpu_to_le32(get ? PATCH_SEM_GET : PATCH_SEM_RELEASE),
> > };
> > - struct event {
> > - u8 status;
> > - u8 reserved[3];
> > - } *resp;
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > - struct sk_buff *skb_ret;
> > - int ret;
> >
> > - ret = mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_SEM_CONTROL,
> > - &skb_ret);
> > - if (ret)
> > - goto out;
> > -
> > - resp = (struct event *)(skb_ret->data);
> > - ret = resp->status;
> > - dev_kfree_skb(skb_ret);
> > -
> > -out:
> > - return ret;
> > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_SEM_CONTROL);
> > }
> >
> > static int mt7615_mcu_start_patch(struct mt7615_dev *dev)
> > @@ -268,7 +242,7 @@ static int mt7615_mcu_start_patch(struct mt7615_dev
> > *dev)
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_FINISH_REQ,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, -MCU_CMD_PATCH_FINISH_REQ);
> > }
> >
> > static int mt7615_driver_own(struct mt7615_dev *dev)
> > @@ -554,8 +528,7 @@ int mt7615_mcu_set_eeprom(struct mt7615_dev *dev)
> > for (off = MT_EE_NIC_CONF_0; off < __MT_EE_MAX; off++)
> > data[off - MT_EE_NIC_CONF_0].val = eep[off];
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_EFUSE_BUFFER_MODE,
> > - NULL);
> > + return mt7615_mcu_msg_send(dev, skb,
> > MCU_EXT_CMD_EFUSE_BUFFER_MODE);
> > }
> >
> > int mt7615_mcu_init_mac(struct mt7615_dev *dev)
> > @@ -570,7 +543,7 @@ int mt7615_mcu_init_mac(struct mt7615_dev *dev)
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_MAC_INIT_CTRL,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_MAC_INIT_CTRL);
> > }
> >
> > int mt7615_mcu_set_rts_thresh(struct mt7615_dev *dev, u32 val)
> > @@ -589,7 +562,7 @@ int mt7615_mcu_set_rts_thresh(struct mt7615_dev *dev,
> > u32 val)
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PROTECT_CTRL,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PROTECT_CTRL);
> > }
> >
> > int mt7615_mcu_set_wmm(struct mt7615_dev *dev, u8 queue,
> > @@ -627,7 +600,7 @@ int mt7615_mcu_set_wmm(struct mt7615_dev *dev, u8
> > queue,
> > }
> >
> > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_EDCA_UPDATE,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_EDCA_UPDATE);
> > }
> >
> > int mt7615_mcu_ctrl_pm_state(struct mt7615_dev *dev, int enter)
> > @@ -657,7 +630,7 @@ int mt7615_mcu_ctrl_pm_state(struct mt7615_dev *dev,
> > int enter)
> > };
> > struct sk_buff *skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PM_STATE_CTRL,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_PM_STATE_CTRL);
> > }
> >
> > static int __mt7615_mcu_set_dev_info(struct mt7615_dev *dev,
> > @@ -704,8 +677,7 @@ static int __mt7615_mcu_set_dev_info(struct mt7615_dev
> > *dev,
> >
> > memcpy(skb_push(skb, sizeof(req_hdr)), &req_hdr, sizeof(req_hdr));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_DEV_INFO_UPDATE,
> > - NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_DEV_INFO_UPDATE);
> > }
> >
> > int mt7615_mcu_set_dev_info(struct mt7615_dev *dev, struct ieee80211_vif
> > *vif,
> > @@ -830,8 +802,7 @@ static int __mt7615_mcu_set_bss_info(struct mt7615_dev
> > *dev,
> > bss_info_tag_handler[i].handler)
> > bss_info_tag_handler[i].handler(dev, bss_info,
> > skb);
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BSS_INFO_UPDATE,
> > - NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BSS_INFO_UPDATE);
> > }
> >
> > int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
> > @@ -914,7 +885,7 @@ __mt7615_mcu_set_wtbl(struct mt7615_dev *dev, int
> > wlan_idx,
> > if (buf && buf_len)
> > memcpy(skb_put(skb, buf_len), buf, buf_len);
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_WTBL_UPDATE,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_WTBL_UPDATE);
> > }
> >
> > static enum mt7615_cipher_type
> > @@ -1092,7 +1063,7 @@ __mt7615_mcu_set_sta_rec(struct mt7615_dev *dev, int
> > bss_idx,
> > if (buf && buf_len)
> > memcpy(skb_put(skb, buf_len), buf, buf_len);
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_STA_REC_UPDATE,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_STA_REC_UPDATE);
> > }
> >
> > int mt7615_mcu_set_sta_rec_bmc(struct mt7615_dev *dev,
> > @@ -1220,7 +1191,7 @@ int mt7615_mcu_set_bcn(struct mt7615_dev *dev,
> > struct ieee80211_vif *vif,
> >
> > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> >
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BCN_OFFLOAD,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BCN_OFFLOAD);
> > }
> >
> > int mt7615_mcu_set_channel(struct mt7615_dev *dev)
> > @@ -1285,12 +1256,12 @@ int mt7615_mcu_set_channel(struct mt7615_dev *dev)
> > memset(req.txpower_sku, 0x3f, 49);
> >
> > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > - ret = mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_CHANNEL_SWITCH,
> > NULL);
> > + ret = mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_CHANNEL_SWITCH);
> > if (ret)
> > return ret;
> >
> > skb = mt7615_mcu_msg_alloc(&req, sizeof(req));
> > - return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_SET_RX_PATH,
> > NULL);
> > + return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_SET_RX_PATH);
> > }
> >
> > int mt7615_mcu_set_ht_cap(struct mt7615_dev *dev, struct ieee80211_vif
> > *vif,
> > --
> > 2.20.1
> >
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2019-05-02 15:31 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-01 16:07 [RFC 00/17] use standard signature for mt7615 mcu api Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 01/17] mt7615: mcu: simplify __mt7615_mcu_set_wtbl Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 02/17] mt7615: mcu: simplify __mt7615_mcu_set_sta_rec Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 03/17] mt7615: mcu: remove bss_info_convert_vif_type routine Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 04/17] mt7615: mcu: use proper msg size in mt7615_mcu_add_wtbl_bmc Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 05/17] mt7615: mcu: use proper msg size in mt7615_mcu_add_wtbl Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 06/17] mt7615: mcu: unify mt7615_mcu_add_wtbl_bmc and mt7615_mcu_del_wtbl_bmc Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 07/17] mt7615: mcu: remove unused paramter in mt7615_mcu_del_wtbl Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 08/17] mt7615: remove query from mt7615_mcu_msg_send signature Lorenzo Bianconi
2019-05-02 9:01 ` Kalle Valo
2019-05-01 16:07 ` [RFC 09/17] mt7615: remove dest " Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 10/17] mt7615: mcu: remove skb_ret from mt7615_mcu_msg_send Lorenzo Bianconi
[not found] ` <CA+zupgzQTzQyihC_UskHD=63Ag3AnQcD4_pcas7VdaehyoCE1w@mail.gmail.com>
2019-05-02 15:31 ` Lorenzo Bianconi [this message]
[not found] ` <CA+zupgxPj_oziDiSkztE3zLxZ9BXMAbQ3Vb4eEWSJ4JrSE41=A@mail.gmail.com>
2019-05-03 9:27 ` Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 11/17] mt7615: mcu: unify __mt7615_mcu_set_dev_info and mt7615_mcu_set_dev_info Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 12/17] mt7615: mcu: do not use function pointer whenever possible Lorenzo Bianconi
2019-05-02 2:27 ` Ryder Lee
2019-05-02 8:24 ` Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 13/17] mt7615: mcu: remove unused structure in mcu.h Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 14/17] mt7615: mcu: use standard signature for mt7615_mcu_msg_send Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 15/17] mt7615: initialize mt76_mcu_ops data structure Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 16/17] mt7615: mcu: init mcu_restart function pointer Lorenzo Bianconi
2019-05-01 16:07 ` [RFC 17/17] mt7615: mcu: run __mt76_mcu_send_msg in mt7615_mcu_send_firmware 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=20190502153142.GA7459@localhost.localdomain \
--to=lorenzo@kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=lorenzo.bianconi@redhat.com \
--cc=nbd@nbd.name \
--cc=royluo@google.com \
--cc=ryder.lee@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 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).