Re: [PATCH net-next v6] net: dlink: add support for reporting stats via `ethtool -S` and `ip -s -s link show`

[Andrew Lunn]

On Tue, Apr 08, 2025 at 07:57:55AM +0900, Moon Yeounsu wrote:
> On Mon, Apr 07, 2025 at 10:48:01PM +0200, Andrew Lunn wrote:
> > When i see a list like this, it makes me think this should be broken
> > up into multiple patches. Ideally you want lots of simple patches
> > which are obviously correct.
> 
> Would it be appropriate to split this into a patchset, then?
> To be honest, this is my first time creating a patchset, so
> I'm not entirely sure how to divide it properly.
> 
> For now, I'm thinking of splitting it as follows:
> 	1. stat definitions and declarations
> 	2. preprocessor directives (`#ifdef`)
> 	3. `spin_[un]lock_irq()` related changes
> 	4. `get_stats()` implementation
> 	5. `ethtool_ops` implementation
> 
> Is it okay to resend the v7 patchset split as above?

You trimmed too much context, you took away the list, so it is hard
for me to reply. Trimming is good, but thing about what is needed for
the conversation.

One obvious patch is to remove the #ifdef about MMIO. That is one
logical thing.

If the spin_[un]lock_irq() is about existing code, yes that could be a
patch. Does it make sense on its own?

1, 4, and 5 probably go together, since they are one logical thing.

Since you are new, it is worth spending some time reading other
patches on the mailing list and the review comments they get. You can
learn a lot that way.

	Andrew