Hi Dmitry, > -----Original Message----- > From: Dmitry Torokhov [mailto:dtor@xxxxxxxxxx] > Sent: 2017年4月1日 6:50 > To: Xinming Hu > Cc: Linux Wireless; Kalle Valo; Brian Norris; Rajat Jain; Amitkumar Karwar; > Cathy Luo; Xinming Hu > Subject: [EXT] Re: [PATCH 3/3] mwifiex: pcie: avoid hardcode wifi-only > firmware name > > External Email > > ---------------------------------------------------------------------- > On Thu, Mar 30, 2017 at 2:19 AM, Xinming Hu <huxinming820@xxxxxxxxx> > wrote: > > From: Xinming Hu <huxm@xxxxxxxxxxx> > > > > Wifi-only firmware name should be chipset specific. > > > > Signed-off-by: Xinming Hu <huxm@xxxxxxxxxxx> > > Signed-off-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx> > > --- > > drivers/net/wireless/marvell/mwifiex/pcie.c | 8 +++++++- > > drivers/net/wireless/marvell/mwifiex/pcie.h | 6 ++++++ > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c > > b/drivers/net/wireless/marvell/mwifiex/pcie.c > > index 59cb01a..282aa9a 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c > > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c > > @@ -351,6 +351,12 @@ static void mwifiex_pcie_reset_notify(struct pci_dev > *pdev, bool prepare) > > struct pcie_service_card *card = pci_get_drvdata(pdev); > > struct mwifiex_adapter *adapter = card->adapter; > > > > + if (!card->pcie.flr_support) { > > + pr_err("%s: FLR disabled in current chipset\n", __func__); > > + return; > > + } > > + > > + adapter = card->adapter; > > if (!adapter) { > > dev_err(&pdev->dev, "%s: adapter structure is not > valid\n", > > __func__); > > @@ -3047,7 +3053,7 @@ static void mwifiex_pcie_up_dev(struct > mwifiex_adapter *adapter) > > * during pcie FLR, so that bluetooth part of firmware which is > > * already running doesn't get affected. > > */ > > - strcpy(adapter->fw_name, PCIE8997_DEFAULT_WIFIFW_NAME); > > + strcpy(adapter->fw_name, card->pcie.wifi_fw_name); > > > > /* tx_buf_size might be changed to 3584 by firmware during > > * data transfer, we should reset it to default size. > > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.h > > b/drivers/net/wireless/marvell/mwifiex/pcie.h > > index 00e8ee5..d6c8526 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/pcie.h > > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.h > > @@ -285,6 +285,8 @@ struct mwifiex_pcie_device { > > struct memory_type_mapping *mem_type_mapping_tbl; > > u8 num_mem_types; > > bool can_ext_scan; > > + u8 flr_support; > > This sounds like a boolean, but given that you can key off wifi_fw_name being > NULL I am not sure it is needed. > > > + char *wifi_fw_name; > > const char *. > > That said, I consider the whole wifi-only firmware business is quite fragile. Can > we have unified firmware and have driver figure out what part shoudl be > [re]loaded? > Thanks for the review, we have tried to extract wifi-only part from combo firmware. The new solution will be send in V2, in this way, we do not need separate wifi_fw_name any more. Thanks Simon > > }; > > > > static const struct mwifiex_pcie_device mwifiex_pcie8766 = { @@ > > -293,6 +295,7 @@ struct mwifiex_pcie_device { > > .tx_buf_size = MWIFIEX_TX_DATA_BUF_SIZE_2K, > > .can_dump_fw = false, > > .can_ext_scan = true, > > + .flr_support = 0, > > }; > > > > static const struct mwifiex_pcie_device mwifiex_pcie8897 = { @@ > > -303,6 +306,7 @@ struct mwifiex_pcie_device { > > .mem_type_mapping_tbl = mem_type_mapping_tbl_w8897, > > .num_mem_types = ARRAY_SIZE(mem_type_mapping_tbl_w8897), > > .can_ext_scan = true, > > + .flr_support = 0, > > }; > > > > static const struct mwifiex_pcie_device mwifiex_pcie8997 = { @@ > > -313,6 +317,8 @@ struct mwifiex_pcie_device { > > .mem_type_mapping_tbl = mem_type_mapping_tbl_w8997, > > .num_mem_types = ARRAY_SIZE(mem_type_mapping_tbl_w8997), > > .can_ext_scan = true, > > + .flr_support = 1, > > + .wifi_fw_name = "mrvl/pcie8997_wlan_v4.bin", > > }; > > > > struct mwifiex_evt_buf_desc { > > -- > > 1.8.1.4 > > > > Thanks. > > -- > Dmitry