Re: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode

[Francesco Dolcini <francesco@dolcini.it>]

Hello David,

On Mon, Mar 18, 2024 at 02:00:35AM +0000, David Lin wrote:
> > From: Brian Norris <briannorris@chromium.org>
...
> > >  .../net/wireless/marvell/mwifiex/sta_ioctl.c  |   2 +-
> > >  drivers/net/wireless/marvell/mwifiex/sta_tx.c |   9 +-
> > >  drivers/net/wireless/marvell/mwifiex/util.c   |  80 +++++
> > >  15 files changed, 671 insertions(+), 14 deletions(-)
> > 
> > (Per the above, I'd normally consider whether ~671 new lines is
> > worth splitting into multiple patches. But I don't see any great
> > logical ways to do that.)
> Francesco suggested to use two patches for this host mlme new feature
> from previous many patches. I knew it is a lot of changes, but I think
> it should be the best way to add host mlme with two patches (one for
> client and one for AP).

What I explicitly asked was to not add code in a patch, and fix the
newly added code in a following patch. What you are supposed to do is to
just amend the original code when you get review feedback.

Splitting a big patch into multiple patches is welcome to easier review,
and this needs to be done breaking down in logical pieces keeping in
mind also bisect-ability.

This [1] is an example of the addition of a relatively big new driver,
and you can see that the series is broken down in smaller patches like
"Add skeleton PowerVR driver", with intermediate steps that were
non-functional, but they were building fine, they were correct and they
were enabling more effective code review.

Unfortunately, as Brian agreed here, there was no easy way to do it for
this patch.

Francesco

[1] https://lore.kernel.org/all/cover.1700668843.git.donald.robson@imgtec.com/