Received-SPF: pass (google.com: domain of linux-kernel+bounces-106082-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Mon, 18 Mar 2024 12:41:31 +0100
From: Francesco Dolcini <francesco@dolcini.it>
To: David Lin <yu-hao.lin@nxp.com>
Cc: Brian Norris <briannorris@chromium.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvalo@kernel.org" <kvalo@kernel.org>,
	"francesco@dolcini.it" <francesco@dolcini.it>,
	Pete Hsieh <tsung-hsien.hsieh@nxp.com>,
	Francesco Dolcini <francesco.dolcini@toradex.com>
Subject: Re: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode
Message-ID: <20240318114131.GB9565@francesco-nb>
References: <20240306020053.18054-1-yu-hao.lin@nxp.com>
 <20240306020053.18054-2-yu-hao.lin@nxp.com>
 <ZfThCwGj-P5Owlsn@google.com>
 <PA4PR04MB9638F2586DF7E8A41330C8C9D12D2@PA4PR04MB9638.eurprd04.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <PA4PR04MB9638F2586DF7E8A41330C8C9D12D2@PA4PR04MB9638.eurprd04.prod.outlook.com>

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/