netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Golle <daniel@makrotopia.org>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Hauke Mehrtens <hauke@hauke-m.de>, Andrew Lunn <andrew@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Russell King <linux@armlinux.org.uk>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Andreas Schirm <andreas.schirm@siemens.com>,
	Lukas Stockmann <lukas.stockmann@siemens.com>,
	Alexander Sverdlin <alexander.sverdlin@siemens.com>,
	Peter Christen <peter.christen@siemens.com>,
	Avinash Jayaraman <ajayaraman@maxlinear.com>,
	Bing tao Xu <bxu@maxlinear.com>, Liang Xu <lxu@maxlinear.com>,
	Juraj Povazanec <jpovazanec@maxlinear.com>,
	"Fanni (Fang-Yi) Chan" <fchan@maxlinear.com>,
	"Benny (Ying-Tsan) Weng" <yweng@maxlinear.com>,
	"Livia M. Rosu" <lrosu@maxlinear.com>,
	John Crispin <john@phrozen.org>
Subject: Re: [PATCH net-next v2 3/8] net: dsa: lantiq_gswip: move definitions to header
Date: Tue, 19 Aug 2025 12:11:25 +0100	[thread overview]
Message-ID: <aKRb3R1l9XLr3DHw@pidgin.makrotopia.org> (raw)
In-Reply-To: <20250819105055.tuig57u66sit2mlu@skbuf>

On Tue, Aug 19, 2025 at 01:50:55PM +0300, Vladimir Oltean wrote:
> On Tue, Aug 19, 2025 at 02:33:02AM +0100, Daniel Golle wrote:
> > +#define GSWIP_TABLE_ACTIVE_VLAN		0x01
> > +#define GSWIP_TABLE_VLAN_MAPPING	0x02
> > +#define GSWIP_TABLE_MAC_BRIDGE		0x0b
> > +#define  GSWIP_TABLE_MAC_BRIDGE_KEY3_FID	GENMASK(5, 0)	/* Filtering identifier */
> > +#define  GSWIP_TABLE_MAC_BRIDGE_VAL0_PORT	GENMASK(7, 4)	/* Port on learned entries */
> > +#define  GSWIP_TABLE_MAC_BRIDGE_VAL1_STATIC	BIT(0)		/* Static, non-aging entry */
> > +#define  GSWIP_TABLE_MAC_BRIDGE_VAL1_VALID	BIT(1)		/* Valid bit */
> 
> The VAL1_VALID bit definition sneaked in, there was no such thing in the
> code being moved.
> 
> I'm willing to let this pass (I don't think I have other review comments
> that would justify a resend), but it's not a good practice to introduce
> changes in large quantities of code as you're moving them around.

I agree that this is bad and shouldn't have happened when moving the code.
Already this makes git blame more difficult, so it should be as clean as
possible, source and destination should match byte-by-byte.
It happened because I had the fix for the gswip_port_fdb() (for which Vladimir
is working on a better solution) sitting below the series and that added this
bit.

I can resend just this single patch another time without the rest of the
series, or send it all again. Let me know your preference.


  reply	other threads:[~2025-08-19 11:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-19  1:31 [PATCH net-next v2 0/8] net: dsa: lantiq_gswip: prepare for supporting new features Daniel Golle
2025-08-19  1:31 ` [PATCH net-next v2 1/8] net: dsa: lantiq_gswip: deduplicate dsa_switch_ops Daniel Golle
2025-08-19  1:32 ` [PATCH net-next v2 2/8] net: dsa: lantiq_gswip: prepare for more CPU port options Daniel Golle
2025-08-19  1:33 ` [PATCH net-next v2 3/8] net: dsa: lantiq_gswip: move definitions to header Daniel Golle
2025-08-19 10:50   ` Vladimir Oltean
2025-08-19 11:11     ` Daniel Golle [this message]
2025-08-19 12:11       ` Vladimir Oltean
2025-08-19  1:33 ` [PATCH net-next v2 4/8] net: dsa: lantiq_gswip: introduce bitmap for MII ports Daniel Golle
2025-08-19  1:33 ` [PATCH net-next v2 5/8] net: dsa: lantiq_gswip: load model-specific microcode Daniel Golle
2025-08-19  1:33 ` [PATCH net-next v2 6/8] net: dsa: lantiq_gswip: make DSA tag protocol model-specific Daniel Golle
2025-08-19  1:34 ` [PATCH net-next v2 7/8] net: dsa: lantiq_gswip: store switch API version in priv Daniel Golle
2025-08-19 18:54   ` kernel test robot
2025-08-19  1:34 ` [PATCH net-next v2 8/8] net: dsa: lantiq_gswip: add support for SWAPI version 2.3 Daniel Golle
2025-08-19 10:53 ` [PATCH net-next v2 0/8] net: dsa: lantiq_gswip: prepare for supporting new features Vladimir Oltean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aKRb3R1l9XLr3DHw@pidgin.makrotopia.org \
    --to=daniel@makrotopia.org \
    --cc=ajayaraman@maxlinear.com \
    --cc=alexander.sverdlin@siemens.com \
    --cc=andreas.schirm@siemens.com \
    --cc=andrew@lunn.ch \
    --cc=bxu@maxlinear.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fchan@maxlinear.com \
    --cc=hauke@hauke-m.de \
    --cc=john@phrozen.org \
    --cc=jpovazanec@maxlinear.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=lrosu@maxlinear.com \
    --cc=lukas.stockmann@siemens.com \
    --cc=lxu@maxlinear.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=p.zabel@pengutronix.de \
    --cc=pabeni@redhat.com \
    --cc=peter.christen@siemens.com \
    --cc=yweng@maxlinear.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).