* [PATCH 1/2] mwifiex: make addba request command clean @ 2017-07-31 13:07 Xinming Hu 2017-07-31 13:07 ` [PATCH 2/2] mwifiex: pcie: compatible with wifi-only image while extract wifi-part fw Xinming Hu 0 siblings, 1 reply; 4+ messages in thread From: Xinming Hu @ 2017-07-31 13:07 UTC (permalink / raw) To: Linux Wireless Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, rajatja, Zhiyuan Yang, Tim Song, Cathy Luo, Ganapathi Bhat, Xinming Hu From: Xinming Hu <huxm@marvell.com> uninitilized variable, such as .add_req_result might be magic stack value. Initialize the structure to make it clean. Signed-off-by: Xinming Hu <huxm@marvell.com> Signed-off-by: Cathy Luo <cluo@marvell.com> --- drivers/net/wireless/marvell/mwifiex/11n.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/wireless/marvell/mwifiex/11n.c b/drivers/net/wireless/marvell/mwifiex/11n.c index 16c77c2..7252069 100644 --- a/drivers/net/wireless/marvell/mwifiex/11n.c +++ b/drivers/net/wireless/marvell/mwifiex/11n.c @@ -572,6 +572,8 @@ int mwifiex_send_addba(struct mwifiex_private *priv, int tid, u8 *peer_mac) mwifiex_dbg(priv->adapter, CMD, "cmd: %s: tid %d\n", __func__, tid); + memset(&add_ba_req, 0, sizeof(add_ba_req)); + if ((GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA) && ISSUPP_TDLS_ENABLED(priv->adapter->fw_cap_info) && priv->adapter->is_hw_11ac_capable && -- 1.9.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] mwifiex: pcie: compatible with wifi-only image while extract wifi-part fw 2017-07-31 13:07 [PATCH 1/2] mwifiex: make addba request command clean Xinming Hu @ 2017-07-31 13:07 ` Xinming Hu 2017-07-31 16:58 ` Brian Norris 0 siblings, 1 reply; 4+ messages in thread From: Xinming Hu @ 2017-07-31 13:07 UTC (permalink / raw) To: Linux Wireless Cc: Kalle Valo, Brian Norris, Dmitry Torokhov, rajatja, Zhiyuan Yang, Tim Song, Cathy Luo, Ganapathi Bhat, Xinming Hu From: Xinming Hu <huxm@marvell.com> Sometimes, we might using wifi-only firmware with a combo firmware name, in this case, do not need to filter bluetooth part from header. Signed-off-by: Xinming Hu <huxm@marvell.com> Signed-off-by: Cathy Luo <cluo@marvell.com> --- drivers/net/wireless/marvell/mwifiex/pcie.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c index 3da1eeb..dc4e054 100644 --- a/drivers/net/wireless/marvell/mwifiex/pcie.c +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c @@ -1985,7 +1985,8 @@ static int mwifiex_pcie_event_complete(struct mwifiex_adapter *adapter, * (3) wifi image. * * This function bypass the header and bluetooth part, return - * the offset of tail wifi-only part. + * the offset of tail wifi-only part. if the image is already wifi-only, + * that is start with CMD1, return 0. */ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, @@ -1993,7 +1994,7 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, const struct mwifiex_fw_data *fwdata; u32 offset = 0, data_len, dnld_cmd; int ret = 0; - bool cmd7_before = false; + bool cmd7_before = false, first_cmd = false; while (1) { /* Check for integer and buffer overflow */ @@ -2014,20 +2015,29 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, switch (dnld_cmd) { case MWIFIEX_FW_DNLD_CMD_1: - if (!cmd7_before) { - mwifiex_dbg(adapter, ERROR, - "no cmd7 before cmd1!\n"); + if (offset + data_len < data_len) { + mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); ret = -1; goto done; } - if (offset + data_len < data_len) { - mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); + + /* Image start with cmd1, already wifi-only firmware*/ + if (!first_cmd) { + mwifiex_dbg(adapter, MSG, + "input wifi-only firmware\n"); + return 0; + } + + if (!cmd7_before) { + mwifiex_dbg(adapter, ERROR, + "no cmd7 before cmd1!\n"); ret = -1; goto done; } offset += data_len; break; case MWIFIEX_FW_DNLD_CMD_5: + first_cmd = true; /* Check for integer overflow */ if (offset + data_len < data_len) { mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); @@ -2037,6 +2047,7 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, offset += data_len; break; case MWIFIEX_FW_DNLD_CMD_6: + first_cmd = true; /* Check for integer overflow */ if (offset + data_len < data_len) { mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); @@ -2053,6 +2064,7 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, } goto done; case MWIFIEX_FW_DNLD_CMD_7: + first_cmd = true; cmd7_before = true; break; default: -- 1.9.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] mwifiex: pcie: compatible with wifi-only image while extract wifi-part fw 2017-07-31 13:07 ` [PATCH 2/2] mwifiex: pcie: compatible with wifi-only image while extract wifi-part fw Xinming Hu @ 2017-07-31 16:58 ` Brian Norris 2017-08-01 1:40 ` Xinming Hu 0 siblings, 1 reply; 4+ messages in thread From: Brian Norris @ 2017-07-31 16:58 UTC (permalink / raw) To: Xinming Hu Cc: Linux Wireless, Kalle Valo, Dmitry Torokhov, rajatja, Zhiyuan Yang, Tim Song, Cathy Luo, Ganapathi Bhat, Xinming Hu Hi, Two nitpicks below: On Mon, Jul 31, 2017 at 01:07:27PM +0000, Xinming Hu wrote: > From: Xinming Hu <huxm@marvell.com> > > Sometimes, we might using wifi-only firmware with a combo firmware name, > in this case, do not need to filter bluetooth part from header. > > Signed-off-by: Xinming Hu <huxm@marvell.com> > Signed-off-by: Cathy Luo <cluo@marvell.com> > --- > drivers/net/wireless/marvell/mwifiex/pcie.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c > index 3da1eeb..dc4e054 100644 > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c > @@ -1985,7 +1985,8 @@ static int mwifiex_pcie_event_complete(struct mwifiex_adapter *adapter, > * (3) wifi image. > * > * This function bypass the header and bluetooth part, return > - * the offset of tail wifi-only part. > + * the offset of tail wifi-only part. if the image is already wifi-only, Sentences start with capital letters (i.e., "If the image ..."). > + * that is start with CMD1, return 0. > */ > > static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > @@ -1993,7 +1994,7 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > const struct mwifiex_fw_data *fwdata; > u32 offset = 0, data_len, dnld_cmd; > int ret = 0; > - bool cmd7_before = false; > + bool cmd7_before = false, first_cmd = false; > > while (1) { > /* Check for integer and buffer overflow */ > @@ -2014,20 +2015,29 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > > switch (dnld_cmd) { > case MWIFIEX_FW_DNLD_CMD_1: > - if (!cmd7_before) { > - mwifiex_dbg(adapter, ERROR, > - "no cmd7 before cmd1!\n"); > + if (offset + data_len < data_len) { > + mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); > ret = -1; > goto done; > } > - if (offset + data_len < data_len) { > - mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); > + > + /* Image start with cmd1, already wifi-only firmware*/ You need a space before closing the comment. i.e.: /* Image starts with cmd1; already wifi-only firmware */ Otherwise, I think both of these look fine: Reviewed-by: Brian Norris <briannorris@chromium.org> > + if (!first_cmd) { > + mwifiex_dbg(adapter, MSG, > + "input wifi-only firmware\n"); > + return 0; > + } > + > + if (!cmd7_before) { > + mwifiex_dbg(adapter, ERROR, > + "no cmd7 before cmd1!\n"); > ret = -1; > goto done; > } > offset += data_len; > break; > case MWIFIEX_FW_DNLD_CMD_5: > + first_cmd = true; > /* Check for integer overflow */ > if (offset + data_len < data_len) { > mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); > @@ -2037,6 +2047,7 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > offset += data_len; > break; > case MWIFIEX_FW_DNLD_CMD_6: > + first_cmd = true; > /* Check for integer overflow */ > if (offset + data_len < data_len) { > mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); > @@ -2053,6 +2064,7 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > } > goto done; > case MWIFIEX_FW_DNLD_CMD_7: > + first_cmd = true; > cmd7_before = true; > break; > default: > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH 2/2] mwifiex: pcie: compatible with wifi-only image while extract wifi-part fw 2017-07-31 16:58 ` Brian Norris @ 2017-08-01 1:40 ` Xinming Hu 0 siblings, 0 replies; 4+ messages in thread From: Xinming Hu @ 2017-08-01 1:40 UTC (permalink / raw) To: Brian Norris, Xinming Hu Cc: Linux Wireless, Kalle Valo, Dmitry Torokhov, rajatja@google.com, Zhiyuan Yang, Tim Song, Cathy Luo, Ganapathi Bhat Hi Brian, Thanks for the review, fix below comment format in V2. Regards, Simon ________________________________________ From: Brian Norris <briannorris@chromium.org> Sent: Tuesday, August 1, 2017 0:58 To: Xinming Hu Cc: Linux Wireless; Kalle Valo; Dmitry Torokhov; rajatja@google.com; Zhiyuan Yang; Tim Song; Cathy Luo; Ganapathi Bhat; Xinming Hu Subject: [EXT] Re: [PATCH 2/2] mwifiex: pcie: compatible with wifi-only image while extract wifi-part fw External Email ---------------------------------------------------------------------- Hi, Two nitpicks below: On Mon, Jul 31, 2017 at 01:07:27PM +0000, Xinming Hu wrote: > From: Xinming Hu <huxm@marvell.com> > > Sometimes, we might using wifi-only firmware with a combo firmware name, > in this case, do not need to filter bluetooth part from header. > > Signed-off-by: Xinming Hu <huxm@marvell.com> > Signed-off-by: Cathy Luo <cluo@marvell.com> > --- > drivers/net/wireless/marvell/mwifiex/pcie.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c > index 3da1eeb..dc4e054 100644 > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c > @@ -1985,7 +1985,8 @@ static int mwifiex_pcie_event_complete(struct mwifiex_adapter *adapter, > * (3) wifi image. > * > * This function bypass the header and bluetooth part, return > - * the offset of tail wifi-only part. > + * the offset of tail wifi-only part. if the image is already wifi-only, Sentences start with capital letters (i.e., "If the image ..."). > + * that is start with CMD1, return 0. > */ > > static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > @@ -1993,7 +1994,7 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > const struct mwifiex_fw_data *fwdata; > u32 offset = 0, data_len, dnld_cmd; > int ret = 0; > - bool cmd7_before = false; > + bool cmd7_before = false, first_cmd = false; > > while (1) { > /* Check for integer and buffer overflow */ > @@ -2014,20 +2015,29 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > > switch (dnld_cmd) { > case MWIFIEX_FW_DNLD_CMD_1: > - if (!cmd7_before) { > - mwifiex_dbg(adapter, ERROR, > - "no cmd7 before cmd1!\n"); > + if (offset + data_len < data_len) { > + mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); > ret = -1; > goto done; > } > - if (offset + data_len < data_len) { > - mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); > + > + /* Image start with cmd1, already wifi-only firmware*/ You need a space before closing the comment. i.e.: /* Image starts with cmd1; already wifi-only firmware */ Otherwise, I think both of these look fine: Reviewed-by: Brian Norris <briannorris@chromium.org> > + if (!first_cmd) { > + mwifiex_dbg(adapter, MSG, > + "input wifi-only firmware\n"); > + return 0; > + } > + > + if (!cmd7_before) { > + mwifiex_dbg(adapter, ERROR, > + "no cmd7 before cmd1!\n"); > ret = -1; > goto done; > } > offset += data_len; > break; > case MWIFIEX_FW_DNLD_CMD_5: > + first_cmd = true; > /* Check for integer overflow */ > if (offset + data_len < data_len) { > mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); > @@ -2037,6 +2047,7 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > offset += data_len; > break; > case MWIFIEX_FW_DNLD_CMD_6: > + first_cmd = true; > /* Check for integer overflow */ > if (offset + data_len < data_len) { > mwifiex_dbg(adapter, ERROR, "bad FW parse\n"); > @@ -2053,6 +2064,7 @@ static int mwifiex_extract_wifi_fw(struct mwifiex_adapter *adapter, > } > goto done; > case MWIFIEX_FW_DNLD_CMD_7: > + first_cmd = true; > cmd7_before = true; > break; > default: > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-01 1:41 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-07-31 13:07 [PATCH 1/2] mwifiex: make addba request command clean Xinming Hu 2017-07-31 13:07 ` [PATCH 2/2] mwifiex: pcie: compatible with wifi-only image while extract wifi-part fw Xinming Hu 2017-07-31 16:58 ` Brian Norris 2017-08-01 1:40 ` Xinming Hu
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).