Re: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x

[Sascha Hauer <s.hauer@pengutronix.de>]

On Mon, Sep 02, 2024 at 02:24:53AM +0000, David Lin wrote:
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Sent: Monday, August 26, 2024 3:27 PM
> > To: Francesco Dolcini <francesco@dolcini.it>
> > Cc: Calvin Owens <calvin@wbinvd.org>; Brian Norris
> > <briannorris@chromium.org>; Kalle Valo <kvalo@kernel.org>; David Lin
> > <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org;
> > linux-kernel@vger.kernel.org; kernel@pengutronix.de; Sascha Hauer
> > <s.hauer@pengutronix.de>
> > Subject: [EXT] [RFC PATCH 0/4] mwifiex: add support for iw61x
> >
> > Caution: This is an external email. Please take care when clicking links or
> > opening attachments. When in doubt, report the message using the 'Report
> > this email' button
> >
> >
> > This series adds support for the iw61x chips to the mwifiex driver.
> > There are a few things to address, hence the RFC status. See the commit
> > messages for details. The series is based on wireless-next/main.
> >
> > I am sending this now since people requested it here [1], but as it's out now
> > feel free to leave your comments to the issues mentioned (or others I haven't
> > mentioned ;)
> >
> > [1]
> > https://lore.kern/
> > el.org%2Fall%2F20240809094533.1660-1-yu-hao.lin%40nxp.com%2F&data=05
> > %7C02%7Cyu-hao.lin%40nxp.com%7C184ab4fed58647150f8508dcc5a0789a%7
> > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638602540229716119%
> > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB
> > TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=cACBHxaQvcOqu6ri
> > BoAlZDONRlGQ4j5DcglEV9T%2BpYU%3D&reserved=0
> >
> > Sascha
> >
> >
> > Sascha Hauer (4):
> >   wifi: mwifiex: release firmware at remove time
> >   wifi: mwifiex: handle VDLL
> >   wifi: mwifiex: wait longer for SDIO card status
> >   mwifiex: add iw61x support
> >
> >  drivers/net/wireless/marvell/mwifiex/cmdevt.c | 86 +++++++++++++++++++
> >  drivers/net/wireless/marvell/mwifiex/fw.h     | 16 ++++
> >  drivers/net/wireless/marvell/mwifiex/main.c   |  9 +-
> >  drivers/net/wireless/marvell/mwifiex/main.h   |  4 +
> >  drivers/net/wireless/marvell/mwifiex/sdio.c   | 81 ++++++++++++++++-
> >  drivers/net/wireless/marvell/mwifiex/sdio.h   |  3 +
> >  .../net/wireless/marvell/mwifiex/sta_event.c  |  4
> > +  .../net/wireless/marvell/mwifiex/uap_event.c  |  4 +
> >  include/linux/mmc/sdio_ids.h                  |  3 +
> >  9 files changed, 205 insertions(+), 5 deletions(-)
> >
> > --
> > 2.39.2
> 
> I think you ported VDLL related code from nxpwifi and you also traced
> our private/downstream MXM driver.

I ported it from this repository:

https://github.com/nxp-imx/mwifiex-iw612.git

Is that the one you are referring to as MXM driver?

> If this is the case, I think you should know nxpwifi already cleaned
> up the porting VDLL code from MXM driver.
> I check your patch quickly. You ported the new SDIO data type
> (MWIFIEX_TYPE_VDLL) from nxpwifi, but you
> did not port the code to support this new data type from nxpwifi. In
> other word, you did not test your
> patch before submission (same as some of your patches).

I did test it. It works with the iw61x as well as older chips. There are
likely details I haven't tested, but it generally works. If there are
details I should test additionally please let me know.

> 
> Another thing is that this new SDIO data type should be handled
> carefully with other existed SDIO data type.
> 
> Nxpwifi only supports new SDIO mode, so the modification to support
> NXPWIFI_TYPE_VDLL can be clean and simple. If you want to port the
> code to Mwifiex, there is no one-to-one modification of the code.
> 
> Another important thing is that you should consider if your
> modifications will affect existed devices or not.
> You need to check if you should check firmware version or chip type
> before adding some code.

The VDLL code I added for the iw61x only reacts to the EVENT_VDLL_IND
event. So as long as a firmware doesn't send such an event nothing is
changed with this patch, and I haven't seen an older chip sending a VDLL
event.

> 
> BTW, no matter if you add the code with or without checking of
> firmware version or chip type, you should test
> your modifications for IW61x and at least one legacy device if the
> code is added to Mwifiex.

I did.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |