2016-09-12 13:26:01

by Amitkumar Karwar

[permalink] [raw]
Subject: [PATCH 0/2] mwifiex: usb8997 chipset support

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


2016-09-26 08:56:42

by Kalle Valo

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

Amitkumar Karwar <[email protected]> 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

2016-09-20 13:48:29

by Amitkumar Karwar

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

Hi Kalle,

> From: Kalle Valo [mailto:[email protected]]
> Sent: Wednesday, September 14, 2016 10:29 PM
> To: Amitkumar Karwar
> Cc: [email protected]; Cathy Luo; Nishant Sarmukadam;
> Ganapathi Bhat
> Subject: Re: [PATCH 2/2] mwifiex: firmware name correction for usb8997
> chipset
>
> Amitkumar Karwar <[email protected]> writes:
>
> > From: Ganapathi Bhat <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Amitkumar Karwar <[email protected]>
> > ---
> > 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

2016-09-26 09:01:29

by Amitkumar Karwar

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

Hi Kalle,

> From: Kalle Valo [mailto:[email protected]]
> Sent: Monday, September 26, 2016 2:27 PM
> To: Amitkumar Karwar
> Cc: [email protected]; Ganapathi Bhat
> Subject: Re: [PATCH 2/2] mwifiex: firmware name correction for usb8997
> chipset
>
> Amitkumar Karwar <[email protected]> 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

2016-09-14 16:59:25

by Kalle Valo

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

Amitkumar Karwar <[email protected]> writes:

> From: Ganapathi Bhat <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> ---
> 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

2016-09-12 13:26:09

by Amitkumar Karwar

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

From: Ganapathi Bhat <[email protected]>

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 <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
---
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

2016-09-14 17:03:01

by Kalle Valo

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

Amitkumar Karwar <[email protected]> wrote:
> From: Ganapathi Bhat <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Amitkumar Karwar <[email protected]>

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/

2016-09-12 13:26:04

by Amitkumar Karwar

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

From: Ganapathi Bhat <[email protected]>

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 <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
---
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