RE: mwifiex: PCIe8997 chip specific handling

[Amitkumar Karwar]

Hi Steve,

> From: Steve deRosier [mailto:derosier@xxxxxxxxx]
> Sent: Thursday, August 11, 2016 2:39 AM
> To: Amitkumar Karwar
> Cc: Brian Norris; linux-wireless@xxxxxxxxxxxxxxx; Cathy Luo; Nishant
> Sarmukadam; linux-kernel@xxxxxxxxxxxxxxx; Wei-Ning Huang
> Subject: Re: mwifiex: PCIe8997 chip specific handling
> 
> Hi,
> 
> On Wed, Aug 10, 2016 at 12:07 AM, Amitkumar Karwar <akarwar@xxxxxxxxxxx>
> wrote:
> > Hi Brian,
> >
> >> From: Brian Norris [mailto:briannorris@xxxxxxxxxxxx]
> >> Sent: Wednesday, August 10, 2016 12:14 AM
> >> To: Amitkumar Karwar
> >> Cc: linux-wireless@xxxxxxxxxxxxxxx; Cathy Luo; Nishant Sarmukadam;
> >> linux-kernel@xxxxxxxxxxxxxxx
> >> Subject: Re: mwifiex: PCIe8997 chip specific handling
> >>
> >> Hi,
> >>
> >> On Fri, Jul 29, 2016 at 04:08:51PM +0530, Amitkumar Karwar wrote:
> >> > The patch corrects the revision id register and uses it along with
> >> > magic value and chip version registers to download appropriate
> >> > firmware image.
> >> >
> >> > PCIe8997 Z chipset variant code has been removed, as it won't be
> >> > used in production.
> >> >
> >> > Signed-off-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx>
> >> > ---
> >> >  drivers/net/wireless/marvell/mwifiex/pcie.c | 35
> >> > ++++++++++-------------------
> >> > drivers/net/wireless/marvell/mwifiex/pcie.h | 14 +++++-------
> >> >  2 files changed, 18 insertions(+), 31 deletions(-)
> >>
> >> [...]
> >>
> >> > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.h
> >> > b/drivers/net/wireless/marvell/mwifiex/pcie.h
> >> > index f6992f0..46f99ca 100644
> >> > --- a/drivers/net/wireless/marvell/mwifiex/pcie.h
> >> > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.h
> >> > @@ -32,12 +32,9 @@
> >> >  #define PCIE8897_DEFAULT_FW_NAME "mrvl/pcie8897_uapsta.bin"
> >> >  #define PCIE8897_A0_FW_NAME "mrvl/pcie8897_uapsta_a0.bin"
> >> >  #define PCIE8897_B0_FW_NAME "mrvl/pcie8897_uapsta.bin"
> >> > -#define PCIE8997_DEFAULT_FW_NAME "mrvl/pcieusb8997_combo_v2.bin"
> >> > -#define PCIEUART8997_FW_NAME_Z "mrvl/pcieuart8997_combo.bin"
> >> > -#define PCIEUART8997_FW_NAME_V2 "mrvl/pcieuart8997_combo_v2.bin"
> >> > -#define PCIEUSB8997_FW_NAME_Z "mrvl/pcieusb8997_combo.bin"
> >> > -#define PCIEUSB8997_FW_NAME_V2 "mrvl/pcieusb8997_combo_v2.bin"
> >> > -#define PCIE8997_DEFAULT_WIFIFW_NAME "mrvl/pcie8997_wlan.bin"
> >> > +#define PCIEUART8997_FW_NAME_V4 "mrvl/pcieuart8997_combo_v4.bin"
> >> > +#define PCIEUSB8997_FW_NAME_V4 "mrvl/pcieusb8997_combo_v4.bin"
> >> > +#define PCIE8997_DEFAULT_WIFIFW_NAME "mrvl/pcie8997_wlan_v4.bin"
> >>
> >> Why do version bumps require firmware renames? Is this just to make
> >> sure you don't load the new firmware on old chip revs that you don't
> >> plan to support for production (i.e., only early revs like the _Z
> >> you're dropping)? This doesn't seems like a good long-term solution,
> >> at least once you start getting this silicon out in the wild. At some
> >> point, I'd expect to see a stable file name.
> >>
> >> Brian
> >>
> >
> > We haven't yet submitted any firmware image upstream for 8997 chipset.
> > pcieuart8997_combo_v4.bin/pcieusb8997_combo_v4.bin would be our
> firmware candidate for upstream submission. The filename would remain
> same hereafter.
> >
> > pcie*8997_combo_v2.bin had support only for A0 chipset
> > pcie*8997_combo_v3.bin was our internal development version which had
> > support for A1 chipset pcie*8997_combo_v4.bin has support for both A0
> and A1 chipsets and this is the version that shall be released to
> customers/upstream from now on.
> >
> 
> Seems to me then it should just be named pcie*8997_wlan.bin.  A version
> number shouldn't be part of the file name in this case. Having to update
> the driver for a firmware name change is silly. Most wireless drivers
> have different names for different hardware/chip revs and/or an
> incompatible API change.  Most distributions would typically only carry
> a single instance of the firmware for a particular chip.
> Speaking for the ones I work with, I usually keep the original filename
> intact (with a version number) and make a symlink to it with the name
> the driver expects.  eg:
> 
> fw-4.bin -> fw_v3.4.0.94.bin
> fw_v3.2.0.144.bin
> fw_v3.4.0.94.bin
> 
> That way I can keep track of the version in my filesystem, but I'm not
> hacking the driver every couple of weeks.  And we do issue new firmware
> every few weeks. I can't imagine asking our customers to keep updating
> the driver for each firmware enhancement.
> 
> IMHO changing the driver to rename the firmwares on new versions seems
> both inconvenient to people using it, and extra non-useful commit noise.
> 

Thanks. I agree with you. We have also maintained single instance/name for all our chipsets for last few years. We do release new firmware periodically for these chipsets, but firmware name always remains the same.

--------
root@pe-lt949:/linux-firmware/mrvl# ls
pcie8897_uapsta.bin  sd8688_helper.bin  sd8797_uapsta.bin  sd8897_uapsta.bin   usb8797_uapsta.bin
sd8688.bin           sd8787_uapsta.bin  sd8887_uapsta.bin  usb8766_uapsta.bin  usb8897_uapsta.bin
---------

Itâs just that for our new chipset 8997 for which we haven't yet submitted the firmware image upstream, we want to finalize the name as pcie*8997_combo_v4.bin

Regards,
Amitkumar Karwar