Subject: Re: [PATCH net-next] net: ethernet: Switch back to struct platform_driver::remove()

From: Simon Horman <[email-masked]>
Date: Tue, 24 Sep 2024 13:53:47 +0100

On Tue, Sep 24, 2024 at 09:48:53AM +0200, Uwe Kleine-König wrote:
> Hello Simon,
> 
> On Tue, Sep 24, 2024 at 08:29:37AM +0100, Simon Horman wrote:
> > On Mon, Sep 23, 2024 at 06:22:01PM +0200, Uwe Kleine-König wrote:
> > > I converted all drivers below drivers/net/ethernet in a single patch. If
> > > you want it split, just tell me (per vendor? per driver?). Also note I
> > > didn't add all the maintainers of the individual drivers to Cc: to not
> > > trigger sending restrictions and spam filters.
> > 
> > I think that given that the changes to each file are very simple,
> > and the number of files changed, a single, or small number of patches
> > make sense. Because the overhead of managing per-driver patches,
> > which I would ordinarily prefer, seems too large.
> 
> full ack.
> 
> > However, touching so many files does lead to a substantial risk of
> > conflicts. And indeed, the patch does not currently apply cleanly
> > to net-next (although it can trivially be made to do so). Perhaps
> > the maintainers can handle that, but I would suggest reposting in
> > a form that does apply cleanly so that automations can run.
> 
> I based it on plain next in the expectation that this matches the
> network tree well enough. I agree that the conflicts are not hard to
> resolve, but it's totally ok for me if only the parts of the patch are
> taken that apply without problems. I expect that I'll have to go through
> more than one subsystem a second time anyhow because new drivers pop up
> using the old idioms.
> 
> Also note that git can handle the changes just fine if you use
> 3-way merging:
> 
> 	uwe@taurus:~/gsrc/linux$ git checkout net-next/main 
> 	HEAD is now at 151ac45348af net: sparx5: Fix invalid timestamps
> 
> 	uwe@taurus:~/gsrc/linux$ b4 am -3 https://lore.kernel.org/all/[email-masked]/
> 	Grabbing thread from lore.kernel.org/all/[email-masked]/t.mbox.gz
> 	Analyzing 3 messages in the thread
> 	Analyzing 0 code-review messages
> 	Checking attestation on all messages, may take a moment...
> 	---
> 	  ✓ [PATCH] net: ethernet: Switch back to struct platform_driver::remove()
> 	    + Reviewed-by: Florian Fainelli <[email-masked]> (✓ DKIM/gmail.com)
> 	  ---
> 	  ✓ Signed: openpgp/[email-masked]
> 	  ✓ Signed: DKIM/baylibre-com.20230601.gappssmtp.com (From: [email-masked])
> 	---
> 	Total patches: 1
> 	Preared a fake commit range for 3-way merge (77e0c079ace8..198dd8fb7661)
> 	---
> 	 Link: https://lore.kernel.org/r/[email-masked]
> 	 Base: using specified base-commit ef545bc03a65438cabe87beb1b9a15b0ffcb6ace
> 	       git checkout -b 20240923_u_kleine_koenig_baylibre_com ef545bc03a65438cabe87beb1b9a15b0ffcb6ace
> 	       git am -3 ./20240923_u_kleine_koenig_net_ethernet_switch_back_to_struct_platform_driver_remove.mbx
> 
> 	uwe@taurus:~/gsrc/linux$ git am -3 ./20240923_u_kleine_koenig_net_ethernet_switch_back_to_struct_platform_driver_remove.mbx
> 	Applying: net: ethernet: Switch back to struct platform_driver::remove()
> 	Using index info to reconstruct a base tree...
> 	M	drivers/net/ethernet/cirrus/ep93xx_eth.c
> 	M	drivers/net/ethernet/marvell/mvmdio.c
> 	M	drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> 	Falling back to patching base and 3-way merge...
> 	Auto-merging drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> 	Auto-merging drivers/net/ethernet/marvell/mvmdio.c
> 	Auto-merging drivers/net/ethernet/cirrus/ep93xx_eth.c

Understood, I agree the conflicts can trivially be resolved.
But as things stand the CI stopped when it couldn't apply
the patchset. And, IMHO, that is not the best.

> 
> > Which brings me to to a separate, process issue: net-next is currently
> > closed for the v6.12 merge window. It should reopen once v6.12-rc1 has
> > been released. And patches for net-next should be posted after it
> > has reopened, with the caveat that RFC patches may be posted any time.
> 
> This was a concious choice. Because of the big amount of drivers touched
> I thought to post early to have a chance to get the patch applied before
> the gates are opened for other patches was a reasonable (but I admit
> selfish) idea.

Understood, I hesitated with my response assuming that was the case.

> Anyhow, I can repost once the merge window closes.

I think that would be best.