[PATCH 0/2] mwifiex: usb8997 chipset support

[PATCH 0/2] mwifiex: usb8997 chipset support

[Amitkumar Karwar]

This patch series includes changes needed to support Marvell usb8997
chipset.

Ganapathi Bhat (2):
  mwifiex: Command 7 handling for USB chipsets
  mwifiex: firmware name correction for usb8997 chipset

 drivers/net/wireless/marvell/mwifiex/usb.c | 4 ++++
 drivers/net/wireless/marvell/mwifiex/usb.h | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
1.9.1

[PATCH 1/2] mwifiex: Command 7 handling for USB chipsets

[Amitkumar Karwar]

From: Ganapathi Bhat <gbhat@marvell.com>

Firmware image for newer USB chipsets starts with a command 7 block
(special command). It doesn't contain data length field. This patch adds
necessary handling.

Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 drivers/net/wireless/marvell/mwifiex/usb.c | 4 ++++
 drivers/net/wireless/marvell/mwifiex/usb.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 9213516..8a20620 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -1026,6 +1026,10 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
 			dnld_cmd = le32_to_cpu(fwdata->fw_hdr.dnld_cmd);
 			tlen += sizeof(struct fw_header);
 
+			/* Command 7 doesn't have data length field */
+			if (dnld_cmd == FW_CMD_7)
+				dlen = 0;
+
 			memcpy(fwdata->data, &firmware[tlen], dlen);
 
 			fwdata->seq_num = cpu_to_le32(fw_seqnum);
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.h b/drivers/net/wireless/marvell/mwifiex/usb.h
index b4e9246..1b49c52 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.h
+++ b/drivers/net/wireless/marvell/mwifiex/usb.h
@@ -51,6 +51,7 @@
 #define FW_DNLD_TX_BUF_SIZE	620
 #define FW_DNLD_RX_BUF_SIZE	2048
 #define FW_HAS_LAST_BLOCK	0x00000004
+#define FW_CMD_7		0x00000007
 
 #define FW_DATA_XMIT_SIZE \
 	(sizeof(struct fw_header) + dlen + sizeof(u32))
-- 
1.9.1

Re: [1/2] mwifiex: Command 7 handling for USB chipsets

[Kalle Valo]

Amitkumar Karwar <akarwar@marvell.com> wrote:
> From: Ganapathi Bhat <gbhat@marvell.com>
> 
> Firmware image for newer USB chipsets starts with a command 7 block
> (special command). It doesn't contain data length field. This patch adds
> necessary handling.
> 
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>

Thanks, 2 patches applied to wireless-drivers-next.git:

787764676f94 mwifiex: Command 7 handling for USB chipsets
b7450e248d71 mwifiex: firmware name correction for usb8997 chipset

-- 
Sent by pwcli
https://patchwork.kernel.org/patch/9326515/

[PATCH 2/2] mwifiex: firmware name correction for usb8997 chipset

[Amitkumar Karwar]

From: Ganapathi Bhat <gbhat@marvell.com>

Similar to pcie8997 chipset, first firmware submitted for usb8997
chipset will be usbusb8997_combo_v4.bin. This patch corrects the
name used in driver.

Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 drivers/net/wireless/marvell/mwifiex/usb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/usb.h b/drivers/net/wireless/marvell/mwifiex/usb.h
index 1b49c52..30e8eb8 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.h
+++ b/drivers/net/wireless/marvell/mwifiex/usb.h
@@ -46,7 +46,7 @@
 #define USB8766_DEFAULT_FW_NAME	"mrvl/usb8766_uapsta.bin"
 #define USB8797_DEFAULT_FW_NAME	"mrvl/usb8797_uapsta.bin"
 #define USB8801_DEFAULT_FW_NAME	"mrvl/usb8801_uapsta.bin"
-#define USB8997_DEFAULT_FW_NAME	"mrvl/usb8997_uapsta.bin"
+#define USB8997_DEFAULT_FW_NAME	"mrvl/usbusb8997_combo_v4.bin"
 
 #define FW_DNLD_TX_BUF_SIZE	620
 #define FW_DNLD_RX_BUF_SIZE	2048
-- 
1.9.1

Re: [PATCH 2/2] mwifiex: firmware name correction for usb8997 chipset

[Kalle Valo]

Amitkumar Karwar <akarwar@marvell.com> writes:

> From: Ganapathi Bhat <gbhat@marvell.com>
>
> Similar to pcie8997 chipset, first firmware submitted for usb8997
> chipset will be usbusb8997_combo_v4.bin. This patch corrects the
> name used in driver.
>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/usb.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/usb.h b/drivers/net/wireless/marvell/mwifiex/usb.h
> index 1b49c52..30e8eb8 100644
> --- a/drivers/net/wireless/marvell/mwifiex/usb.h
> +++ b/drivers/net/wireless/marvell/mwifiex/usb.h
> @@ -46,7 +46,7 @@
>  #define USB8766_DEFAULT_FW_NAME	"mrvl/usb8766_uapsta.bin"
>  #define USB8797_DEFAULT_FW_NAME	"mrvl/usb8797_uapsta.bin"
>  #define USB8801_DEFAULT_FW_NAME	"mrvl/usb8801_uapsta.bin"
> -#define USB8997_DEFAULT_FW_NAME	"mrvl/usb8997_uapsta.bin"
> +#define USB8997_DEFAULT_FW_NAME	"mrvl/usbusb8997_combo_v4.bin"

Like discussed earlier, the firmware names are supposed to be stable. I
consider them to be an interface between kernel and user space. Instead
of changing the driver you should actually rename the firmware file. I'm
planning to take this anyway but in the future please pay extra
attention to do this properly.

My recommendation is to keep the firmware name simple as possible and
get rid of any extra cruft, for example in this case a good name would
be "mrvl/usb8997.bin". That extra "usb" and "combo_v4" don't bring any
benefit.

-- 
Kalle Valo

RE: [PATCH 2/2] mwifiex: firmware name correction for usb8997 chipset

[Amitkumar Karwar]

Hi Kalle,

> From: Kalle Valo [mailto:kvalo@codeaurora.org]
> Sent: Wednesday, September 14, 2016 10:29 PM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> Ganapathi Bhat
> Subject: Re: [PATCH 2/2] mwifiex: firmware name correction for usb8997
> chipset
> 
> Amitkumar Karwar <akarwar@marvell.com> writes:
> 
> > From: Ganapathi Bhat <gbhat@marvell.com>
> >
> > Similar to pcie8997 chipset, first firmware submitted for usb8997
> > chipset will be usbusb8997_combo_v4.bin. This patch corrects the name
> > used in driver.
> >
> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> >  drivers/net/wireless/marvell/mwifiex/usb.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/usb.h
> > b/drivers/net/wireless/marvell/mwifiex/usb.h
> > index 1b49c52..30e8eb8 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/usb.h
> > +++ b/drivers/net/wireless/marvell/mwifiex/usb.h
> > @@ -46,7 +46,7 @@
> >  #define USB8766_DEFAULT_FW_NAME	"mrvl/usb8766_uapsta.bin"
> >  #define USB8797_DEFAULT_FW_NAME	"mrvl/usb8797_uapsta.bin"
> >  #define USB8801_DEFAULT_FW_NAME	"mrvl/usb8801_uapsta.bin"
> > -#define USB8997_DEFAULT_FW_NAME	"mrvl/usb8997_uapsta.bin"
> > +#define USB8997_DEFAULT_FW_NAME	"mrvl/usbusb8997_combo_v4.bin"
> 
> Like discussed earlier, the firmware names are supposed to be stable. I
> consider them to be an interface between kernel and user space. Instead
> of changing the driver you should actually rename the firmware file. I'm
> planning to take this anyway but in the future please pay extra
> attention to do this properly.

Thanks for accepting the patch. We will stick to the same name for 8997 and ensure v3/v4 etc won't be used for future chipsets/firmwares.
 
> My recommendation is to keep the firmware name simple as possible and
> get rid of any extra cruft, for example in this case a good name would
> be "mrvl/usb8997.bin". That extra "usb" and "combo_v4" don't bring any
> benefit.
> 

'combo' here indicates it's a combine firmware image for bluetooth and wifi. Firmware images with bluetooth only OR WiFi only functionality are also possible.
Example use case: We support PCIe function level reset feature as a recovery mechanism. With this mechanism, WiFi recovery can happen by re-downloading WiFi only firmware without affecting bluetooth functionality for PCIe-USB8997 chipset.

'usbusb' here indicates this firmware is for a USB-USB8997 chipset where WiFi and bluetooth are over USB bus. There would be a different firmware if chipset is USB-UART8997 where bluetooth is via UART.

I agree that we should have avoided v4.

Regards,
Amitkumar Karwar

Re: [PATCH 2/2] mwifiex: firmware name correction for usb8997 chipset

[Kalle Valo]

Amitkumar Karwar <akarwar@marvell.com> writes:

>> Like discussed earlier, the firmware names are supposed to be stable. I
>> consider them to be an interface between kernel and user space. Instead
>> of changing the driver you should actually rename the firmware file. I'm
>> planning to take this anyway but in the future please pay extra
>> attention to do this properly.
>
> Thanks for accepting the patch. We will stick to the same name for
> 8997 and ensure v3/v4 etc won't be used for future chipsets/firmwares.

Good, thanks.

>> My recommendation is to keep the firmware name simple as possible and
>> get rid of any extra cruft, for example in this case a good name would
>> be "mrvl/usb8997.bin". That extra "usb" and "combo_v4" don't bring any
>> benefit.
>> 
>
> 'combo' here indicates it's a combine firmware image for bluetooth and
> wifi. Firmware images with bluetooth only OR WiFi only functionality
> are also possible. Example use case: We support PCIe function level
> reset feature as a recovery mechanism. With this mechanism, WiFi
> recovery can happen by re-downloading WiFi only firmware without
> affecting bluetooth functionality for PCIe-USB8997 chipset.

Ok, makes sense.

> 'usbusb' here indicates this firmware is for a USB-USB8997 chipset
> where WiFi and bluetooth are over USB bus. There would be a different
> firmware if chipset is USB-UART8997 where bluetooth is via UART.

So the bluetooth via UART chip would be 'usbuart'? Sounds reasonable
then.

-- 
Kalle Valo

RE: [PATCH 2/2] mwifiex: firmware name correction for usb8997 chipset

[Amitkumar Karwar]

Hi Kalle,

> From: Kalle Valo [mailto:kvalo@codeaurora.org]
> Sent: Monday, September 26, 2016 2:27 PM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Ganapathi Bhat
> Subject: Re: [PATCH 2/2] mwifiex: firmware name correction for usb8997
> chipset
> 
> Amitkumar Karwar <akarwar@marvell.com> writes:
> 
> >> Like discussed earlier, the firmware names are supposed to be stable.
> >> I consider them to be an interface between kernel and user space.
> >> Instead of changing the driver you should actually rename the
> >> firmware file. I'm planning to take this anyway but in the future
> >> please pay extra attention to do this properly.
> >
> > Thanks for accepting the patch. We will stick to the same name for
> > 8997 and ensure v3/v4 etc won't be used for future chipsets/firmwares.
> 
> Good, thanks.
> 
> >> My recommendation is to keep the firmware name simple as possible and
> >> get rid of any extra cruft, for example in this case a good name
> >> would be "mrvl/usb8997.bin". That extra "usb" and "combo_v4" don't
> >> bring any benefit.
> >>
> >
> > 'combo' here indicates it's a combine firmware image for bluetooth and
> > wifi. Firmware images with bluetooth only OR WiFi only functionality
> > are also possible. Example use case: We support PCIe function level
> > reset feature as a recovery mechanism. With this mechanism, WiFi
> > recovery can happen by re-downloading WiFi only firmware without
> > affecting bluetooth functionality for PCIe-USB8997 chipset.
> 
> Ok, makes sense.
> 
> > 'usbusb' here indicates this firmware is for a USB-USB8997 chipset
> > where WiFi and bluetooth are over USB bus. There would be a different
> > firmware if chipset is USB-UART8997 where bluetooth is via UART.
> 
> So the bluetooth via UART chip would be 'usbuart'? Sounds reasonable
> then.
> 

Right. 'usbuart' string would be used in firmware name for USB-UART8997 where bluetooth is via uart.

Regards,
Amitkumar Karwar