RE: mwifiex: PCIe8997 chip specific handling

[Amitkumar Karwar]

Hi Kalle,

> From: linux-wireless-owner@xxxxxxxxxxxxxxx [mailto:linux-wireless-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Amitkumar Karwar
> Sent: Thursday, August 11, 2016 4:11 PM
> To: Steve deRosier
> Cc: Brian Norris; linux-wireless@xxxxxxxxxxxxxxx; Cathy Luo; Nishant
> Sarmukadam; linux-kernel@xxxxxxxxxxxxxxx; Wei-Ning Huang
> Subject: RE: mwifiex: PCIe8997 chip specific handling
> 
> 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
> 

Could you please accept this patch?
I wanted to submit our first firmware for 8997 chipset.

Regards,
Amitkumar