Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Wei Fang" <wei.fang@nxp.com>
Cc: Frank.Li@kernel.org, imx@lists.linux.dev, krzk+dt@kernel.org,
	robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v6 net-next 11/15] net: dsa: netc: add phylink MAC operations
Date: Sun, 10 May 2026 10:48:37 +0000	[thread overview]
Message-ID: <20260510104837.B72D6C2BCB8@smtp.kernel.org> (raw)
In-Reply-To: <20260509102954.4116624-12-wei.fang@nxp.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Flawed read-modify-write logic in `netc_mac_port_rmw()` causes state desynchronization and clobbers PMAC registers.
--

commit e2073763516dc99734e9f1436d4b8e960d8ce680
Author: Wei Fang <wei.fang@nxp.com>

    net: dsa: netc: add phylink MAC operations

This commit implements phylink MAC operations for NETC switch ports, including
capabilities retrieval and link configuration callbacks.

> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index 8e3a3230226c3..8c1317b72a81d 100644
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c
[ ... ]
> @@ -43,6 +43,26 @@ static void netc_mac_port_wr(struct netc_port *np, u32 reg, u32 val)
>  		netc_port_wr(np, reg + NETC_PMAC_OFFSET, val);
>  }
>  
> +static void netc_mac_port_rmw(struct netc_port *np, u32 reg,
> +			      u32 mask, u32 val)
> +{
> +	u32 old, new;
> +
> +	if (is_netc_pseudo_port(np))
> +		return;
> +
> +	WARN_ON((mask | val) != mask);
> +
> +	old = netc_port_rd(np, reg);
> +	new = (old & ~mask) | val;
> +	if (new == old)
> +		return;
> +
> +	netc_port_wr(np, reg, new);
> +	if (np->caps.pmac)
> +		netc_port_wr(np, reg + NETC_PMAC_OFFSET, new);

Does this code lead to desynchronized state and clobbered PMAC registers?

Because old is read exclusively from the MAC 0 register, if new == old, the
function returns immediately. If the preemptable MAC 1 register had a
different state (e.g. from the bootloader), it would not be updated and could
remain permanently out of sync.

Additionally, if new != old, this writes the modified MAC 0 state to MAC 1.
Could this clobber any independent state in the MAC 1 register by overwriting
unmasked bits with MAC 0's values?

Would it be safer to perform independent read-modify-write cycles for each
MAC's register to preserve their respective unmasked bits?

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509102954.4116624-1-wei.fang@nxp.com?part=11

  reply	other threads:[~2026-05-10 10:48 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-09 10:29 [PATCH v6 net-next 00/15] Add preliminary NETC switch support for i.MX94 Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 01/15] dt-bindings: net: dsa: update the description of 'dsa,member' property Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 02/15] dt-bindings: net: dsa: add NETC switch Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-11  3:33     ` Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 03/15] net: enetc: add pre-boot initialization for i.MX94 switch Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 04/15] net: enetc: add basic operations to the FDB table Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 05/15] net: enetc: add support for the "Add" operation to VLAN filter table Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-11  2:05     ` Wei Fang
2026-05-11  2:21       ` Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 06/15] net: enetc: add support for the "Update" operation to buffer pool table Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-11  2:01     ` Wei Fang
2026-05-11  2:22       ` Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 07/15] net: enetc: add support for "Add" and "Delete" operations to IPFT Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-11  2:11     ` Wei Fang
2026-05-11  2:21       ` Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 08/15] net: enetc: add multiple command BD rings support Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 09/15] net: dsa: add NETC switch tag support Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-11  2:18     ` Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 10/15] net: dsa: netc: introduce NXP NETC switch driver for i.MX94 Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-09 10:29 ` [PATCH v6 net-next 11/15] net: dsa: netc: add phylink MAC operations Wei Fang
2026-05-10 10:48   ` sashiko-bot [this message]
2026-05-11  2:17     ` Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 12/15] net: dsa: netc: add FDB, STP, MTU, port setup and host flooding support Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-11  3:14     ` Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 13/15] net: dsa: netc: initialize buffer pool table and implement flow-control Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-11  3:16     ` Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 14/15] net: dsa: netc: add support for the standardized counters Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-11  3:24     ` Wei Fang
2026-05-09 10:29 ` [PATCH v6 net-next 15/15] net: dsa: netc: add support for ethtool private statistics Wei Fang
2026-05-10 10:48   ` sashiko-bot
2026-05-11  3:26     ` Wei Fang

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=20260510104837.B72D6C2BCB8@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=krzk+dt@kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko@lists.linux.dev \
    --cc=wei.fang@nxp.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