The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* Re: [PATCH v5 net-next 04/15] net: enetc: add basic operations to the FDB table
       [not found] ` <20260430024945.3413973-5-wei.fang@nxp.com>
@ 2026-05-05  8:59   ` Paolo Abeni
  2026-05-06  6:37     ` Wei Fang
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Abeni @ 2026-05-05  8:59 UTC (permalink / raw)
  To: Wei Fang, claudiu.manoil, vladimir.oltean, xiaoning.wang,
	andrew+netdev, davem, edumazet, kuba, robh, krzk+dt, conor+dt,
	f.fainelli, frank.li, chleroy, horms, linux
  Cc: netdev, linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel,
	imx

On 4/30/26 4:49 AM, Wei Fang wrote:
> The FDB table is used for MAC learning lookups and MAC forwarding lookups.
> Each table entry includes information such as a FID and MAC address that
> may be unicast or multicast and a forwarding destination field containing
> a port bitmap identifying the associated port(s) with the MAC address.
> FDB table entries can be static or dynamic. Static entries are added from
> software whereby dynamic entries are added either by software or by the
> hardware as MAC addresses are learned in the datapath.
> 
> The FDB table can only be managed by the command BD ring using table
> management protocol version 2.0. Table management command operations Add,
> Delete, Update and Query are supported. And the FDB table supports three
> access methods: Entry ID, Exact Match Key Element and Search. This patch
> adds the following basic supports to the FDB table.
> 
> ntmp_fdbt_update_entry() - update the configuration element data of a
> specified FDB entry
> 
> ntmp_fdbt_delete_entry() - delete a specified FDB entry
> 
> ntmp_fdbt_add_entry() - add an entry into the FDB table
> 
> ntmp_fdbt_search_port_entry() - Search the FDB entry on the specified
> port based on RESUME_ENTRY_ID.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/enetc/ntmp.c   | 203 +++++++++++++++++-
>  .../ethernet/freescale/enetc/ntmp_private.h   |  61 +++++-
>  include/linux/fsl/ntmp.h                      |  44 +++-
>  3 files changed, 305 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/ntmp.c b/drivers/net/ethernet/freescale/enetc/ntmp.c
> index c94a928622fd..4ed8d783a9a2 100644
> --- a/drivers/net/ethernet/freescale/enetc/ntmp.c
> +++ b/drivers/net/ethernet/freescale/enetc/ntmp.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
>  /*
>   * NETC NTMP (NETC Table Management Protocol) 2.0 Library
> - * Copyright 2025 NXP
> + * Copyright 2025-2026 NXP
>   */
>  
>  #include <linux/dma-mapping.h>
> @@ -21,11 +21,15 @@
>  /* Define NTMP Table ID */
>  #define NTMP_MAFT_ID			1
>  #define NTMP_RSST_ID			3
> +#define NTMP_FDBT_ID			15
>  
>  /* Generic Update Actions for most tables */
>  #define NTMP_GEN_UA_CFGEU		BIT(0)
>  #define NTMP_GEN_UA_STSEU		BIT(1)
>  
> +/* Query Action: 0: Full query, 1: Only query entry ID */
> +#define NTMP_QA_ENTRY_ID		1

Sashiko noted that the above comments looks inconsistent with the update
code, where NTMP_QA_ENTRY_ID apparently uses a full query, and 0 just
the entry ID.

If you have to repost for other reasons, please fix this. Note that you
should reply on the ML to sashiko reviews ruling out invalid comments.

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 net-next 15/15] net: dsa: netc: add support for ethtool private statistics
       [not found] ` <20260430024945.3413973-16-wei.fang@nxp.com>
@ 2026-05-05  9:43   ` Paolo Abeni
  2026-05-06  7:06     ` Wei Fang
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Abeni @ 2026-05-05  9:43 UTC (permalink / raw)
  To: Wei Fang, claudiu.manoil, vladimir.oltean, xiaoning.wang,
	andrew+netdev, davem, edumazet, kuba, robh, krzk+dt, conor+dt,
	f.fainelli, frank.li, chleroy, horms, linux
  Cc: netdev, linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel,
	imx

On 4/30/26 4:49 AM, Wei Fang wrote:
> Implement the ethtool private statistics interface to expose additional
> port-level and MAC-level counters that are not covered by the standard
> IEEE 802.3 statistics. The pMAC counters are only reported when the port
> supports Frame Preemption (802.1Qbu/802.3br).
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  drivers/net/dsa/netc/netc_ethtool.c   | 107 ++++++++++++++++++++++++++
>  drivers/net/dsa/netc/netc_main.c      |   3 +
>  drivers/net/dsa/netc/netc_switch.h    |   9 +++
>  drivers/net/dsa/netc/netc_switch_hw.h |  58 ++++++++++++++
>  4 files changed, 177 insertions(+)
> 
> diff --git a/drivers/net/dsa/netc/netc_ethtool.c b/drivers/net/dsa/netc/netc_ethtool.c
> index ac8940b5a85c..8d04db534347 100644
> --- a/drivers/net/dsa/netc/netc_ethtool.c
> +++ b/drivers/net/dsa/netc/netc_ethtool.c
> @@ -19,6 +19,56 @@ static const struct ethtool_rmon_hist_range netc_rmon_ranges[] = {
>  	{ }
>  };
>  
> +static const struct netc_port_stat netc_port_counters[] = {
> +	{ NETC_PTGSLACR,	"port gate late arrival frames" },
> +	{ NETC_PSDFTCR,	"port SDF transmit frames" },
> +	{ NETC_PSDFDDCR,	"port SDF drop duplicate frames" },
> +	{ NETC_PRXDCR,		"port rx discard frames" },
> +	{ NETC_PRXDCRRR,	"port rx discard read-reset" },
> +	{ NETC_PRXDCRR0,	"port rx discard reason 0" },
> +	{ NETC_PRXDCRR1,	"port rx discard reason 1" },
> +	{ NETC_PTXDCR,		"port tx discard frames" },
> +	{ NETC_PTXDCRRR,	"port tx discard read-reset" },
> +	{ NETC_PTXDCRR0,	"port tx discard reason 0" },
> +	{ NETC_PTXDCRR1,	"port tx discard reason 1" },
> +	{ NETC_BPDCR,		"bridge port discard frames" },
> +	{ NETC_BPDCRRR,	"bridge port discard read-reset" },
> +	{ NETC_BPDCRR0,	"bridge port discard reason 0" },
> +	{ NETC_BPDCRR1,	"bridge port discard reason 1" },
> +};
> +
> +static const struct netc_port_stat netc_emac_counters[] = {
> +	{ NETC_PM_ROCT(0),	"eMAC rx octets" },
> +	{ NETC_PM_RVLAN(0),	"eMAC rx VLAN frames" },
> +	{ NETC_PM_RERR(0),	"eMAC rx frame errors" },
> +	{ NETC_PM_RUCA(0),	"eMAC rx unicast frames" },
> +	{ NETC_PM_RDRP(0),	"eMAC rx dropped packets" },
> +	{ NETC_PM_RPKT(0),	"eMAC rx packets" },
> +	{ NETC_PM_TOCT(0),	"eMAC tx octets" },
> +	{ NETC_PM_TVLAN(0),	"eMAC tx VLAN frames" },
> +	{ NETC_PM_TFCS(0),	"eMAC tx FCS errors" },
> +	{ NETC_PM_TUCA(0),	"eMAC tx unicast frames" },
> +	{ NETC_PM_TPKT(0),	"eMAC tx packets" },
> +	{ NETC_PM_TUND(0),	"eMAC tx undersized packets" },
> +	{ NETC_PM_TIOCT(0),	"eMAC tx invalid octets" },
> +};
> +
> +static const struct netc_port_stat netc_pmac_counters[] = {
> +	{ NETC_PM_ROCT(1),	"pMAC rx octets" },
> +	{ NETC_PM_RVLAN(1),	"pMAC rx VLAN frames" },
> +	{ NETC_PM_RERR(1),	"pMAC rx frame errors" },
> +	{ NETC_PM_RUCA(1),	"pMAC rx unicast frames" },
> +	{ NETC_PM_RDRP(1),	"pMAC rx dropped packets" },
> +	{ NETC_PM_RPKT(1),	"pMAC rx packets" },
> +	{ NETC_PM_TOCT(1),	"pMAC tx octets" },
> +	{ NETC_PM_TVLAN(1),	"pMAC tx VLAN frames" },
> +	{ NETC_PM_TFCS(1),	"pMAC tx FCS errors" },
> +	{ NETC_PM_TUCA(1),	"pMAC tx unicast frames" },
> +	{ NETC_PM_TPKT(1),	"pMAC tx packets" },
> +	{ NETC_PM_TUND(1),	"pMAC tx undersized packets" },
> +	{ NETC_PM_TIOCT(1),	"pMAC tx invalid octets" },
> +};

Sashiko says:

Are these standard MAC and interface statistics being duplicated in the
driver-private ethtool strings?
The networking subsystem guidelines indicate that statistics which have
a standard uAPI must not be duplicated in ethtool -S. Standard
interfaces like struct rtnl_link_stats64 or struct ethtool_eth_mac_stats
already exist for counters such as rx octets, rx packets, dropped
packets, and FCS errors.
Could these metrics be exposed through the standardized interfaces
rather than adding them to the private string sets?

Other comments are AFAICS false positives.

/P


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 net-next 04/15] net: enetc: add basic operations to the FDB table
  2026-05-05  8:59   ` [PATCH v5 net-next 04/15] net: enetc: add basic operations to the FDB table Paolo Abeni
@ 2026-05-06  6:37     ` Wei Fang
  0 siblings, 0 replies; 13+ messages in thread
From: Wei Fang @ 2026-05-06  6:37 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev,
	Claudiu Manoil, Vladimir Oltean, Clark Wang,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, f.fainelli@gmail.com, Frank Li,
	chleroy@kernel.org, horms@kernel.org, linux@armlinux.org.uk

> On 4/30/26 4:49 AM, Wei Fang wrote:
> > The FDB table is used for MAC learning lookups and MAC forwarding lookups.
> > Each table entry includes information such as a FID and MAC address that
> > may be unicast or multicast and a forwarding destination field containing
> > a port bitmap identifying the associated port(s) with the MAC address.
> > FDB table entries can be static or dynamic. Static entries are added from
> > software whereby dynamic entries are added either by software or by the
> > hardware as MAC addresses are learned in the datapath.
> >
> > The FDB table can only be managed by the command BD ring using table
> > management protocol version 2.0. Table management command operations
> Add,
> > Delete, Update and Query are supported. And the FDB table supports three
> > access methods: Entry ID, Exact Match Key Element and Search. This patch
> > adds the following basic supports to the FDB table.
> >
> > ntmp_fdbt_update_entry() - update the configuration element data of a
> > specified FDB entry
> >
> > ntmp_fdbt_delete_entry() - delete a specified FDB entry
> >
> > ntmp_fdbt_add_entry() - add an entry into the FDB table
> >
> > ntmp_fdbt_search_port_entry() - Search the FDB entry on the specified
> > port based on RESUME_ENTRY_ID.
> >
> > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > ---
> >  drivers/net/ethernet/freescale/enetc/ntmp.c   | 203
> +++++++++++++++++-
> >  .../ethernet/freescale/enetc/ntmp_private.h   |  61 +++++-
> >  include/linux/fsl/ntmp.h                      |  44 +++-
> >  3 files changed, 305 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/ntmp.c
> b/drivers/net/ethernet/freescale/enetc/ntmp.c
> > index c94a928622fd..4ed8d783a9a2 100644
> > --- a/drivers/net/ethernet/freescale/enetc/ntmp.c
> > +++ b/drivers/net/ethernet/freescale/enetc/ntmp.c
> > @@ -1,7 +1,7 @@
> >  // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> >  /*
> >   * NETC NTMP (NETC Table Management Protocol) 2.0 Library
> > - * Copyright 2025 NXP
> > + * Copyright 2025-2026 NXP
> >   */
> >
> >  #include <linux/dma-mapping.h>
> > @@ -21,11 +21,15 @@
> >  /* Define NTMP Table ID */
> >  #define NTMP_MAFT_ID			1
> >  #define NTMP_RSST_ID			3
> > +#define NTMP_FDBT_ID			15
> >
> >  /* Generic Update Actions for most tables */
> >  #define NTMP_GEN_UA_CFGEU		BIT(0)
> >  #define NTMP_GEN_UA_STSEU		BIT(1)
> >
> > +/* Query Action: 0: Full query, 1: Only query entry ID */
> > +#define NTMP_QA_ENTRY_ID		1
> 
> Sashiko noted that the above comments looks inconsistent with the update
> code, where NTMP_QA_ENTRY_ID apparently uses a full query, and 0 just
> the entry ID.
> 

The definition is correct, 0 indicates a full query, 1 indicates just query the
entry ID. It seems you misunderstood Sashiko's comment. Below is the
comment from Sashiko.

Since this command uses the NTMP_QA_ENTRY_ID ('Only query entry ID') query
action, the hardware returns only a 4-byte entry ID at offset 0. However,
in struct fdbt_resp_query, the entry_id field is located at offset 4,
following the status field.

I would say this is a false positive. Below is the response data structure of a
full query. NTMP_QA_ENTRY_ID does not mean the hardware will return
only a 4-byte entry ID at offset 0, it indicates the fields after entry_id will
not be present in the response data, such as keye, cfge, acte and resv.

struct fdbt_resp_query {
	__le32 status;
	__le32 entry_id;
	struct fdbt_keye_data keye;
	struct fdbt_cfge_data cfge;
	u8 acte;
	u8 resv[3];
};


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 net-next 15/15] net: dsa: netc: add support for ethtool private statistics
  2026-05-05  9:43   ` [PATCH v5 net-next 15/15] net: dsa: netc: add support for ethtool private statistics Paolo Abeni
@ 2026-05-06  7:06     ` Wei Fang
  0 siblings, 0 replies; 13+ messages in thread
From: Wei Fang @ 2026-05-06  7:06 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev,
	Claudiu Manoil, Vladimir Oltean, Clark Wang,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, f.fainelli@gmail.com, Frank Li,
	chleroy@kernel.org, horms@kernel.org, linux@armlinux.org.uk

> On 4/30/26 4:49 AM, Wei Fang wrote:
> > Implement the ethtool private statistics interface to expose additional
> > port-level and MAC-level counters that are not covered by the standard
> > IEEE 802.3 statistics. The pMAC counters are only reported when the port
> > supports Frame Preemption (802.1Qbu/802.3br).
> >
> > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > ---
> >  drivers/net/dsa/netc/netc_ethtool.c   | 107
> ++++++++++++++++++++++++++
> >  drivers/net/dsa/netc/netc_main.c      |   3 +
> >  drivers/net/dsa/netc/netc_switch.h    |   9 +++
> >  drivers/net/dsa/netc/netc_switch_hw.h |  58 ++++++++++++++
> >  4 files changed, 177 insertions(+)
> >
> > diff --git a/drivers/net/dsa/netc/netc_ethtool.c
> b/drivers/net/dsa/netc/netc_ethtool.c
> > index ac8940b5a85c..8d04db534347 100644
> > --- a/drivers/net/dsa/netc/netc_ethtool.c
> > +++ b/drivers/net/dsa/netc/netc_ethtool.c
> > @@ -19,6 +19,56 @@ static const struct ethtool_rmon_hist_range
> netc_rmon_ranges[] = {
> >  	{ }
> >  };
> >
> > +static const struct netc_port_stat netc_port_counters[] = {
> > +	{ NETC_PTGSLACR,	"port gate late arrival frames" },
> > +	{ NETC_PSDFTCR,	"port SDF transmit frames" },
> > +	{ NETC_PSDFDDCR,	"port SDF drop duplicate frames" },
> > +	{ NETC_PRXDCR,		"port rx discard frames" },
> > +	{ NETC_PRXDCRRR,	"port rx discard read-reset" },
> > +	{ NETC_PRXDCRR0,	"port rx discard reason 0" },
> > +	{ NETC_PRXDCRR1,	"port rx discard reason 1" },
> > +	{ NETC_PTXDCR,		"port tx discard frames" },
> > +	{ NETC_PTXDCRRR,	"port tx discard read-reset" },
> > +	{ NETC_PTXDCRR0,	"port tx discard reason 0" },
> > +	{ NETC_PTXDCRR1,	"port tx discard reason 1" },
> > +	{ NETC_BPDCR,		"bridge port discard frames" },
> > +	{ NETC_BPDCRRR,	"bridge port discard read-reset" },
> > +	{ NETC_BPDCRR0,	"bridge port discard reason 0" },
> > +	{ NETC_BPDCRR1,	"bridge port discard reason 1" },
> > +};
> > +
> > +static const struct netc_port_stat netc_emac_counters[] = {
> > +	{ NETC_PM_ROCT(0),	"eMAC rx octets" },
> > +	{ NETC_PM_RVLAN(0),	"eMAC rx VLAN frames" },
> > +	{ NETC_PM_RERR(0),	"eMAC rx frame errors" },
> > +	{ NETC_PM_RUCA(0),	"eMAC rx unicast frames" },
> > +	{ NETC_PM_RDRP(0),	"eMAC rx dropped packets" },
> > +	{ NETC_PM_RPKT(0),	"eMAC rx packets" },
> > +	{ NETC_PM_TOCT(0),	"eMAC tx octets" },
> > +	{ NETC_PM_TVLAN(0),	"eMAC tx VLAN frames" },
> > +	{ NETC_PM_TFCS(0),	"eMAC tx FCS errors" },
> > +	{ NETC_PM_TUCA(0),	"eMAC tx unicast frames" },
> > +	{ NETC_PM_TPKT(0),	"eMAC tx packets" },
> > +	{ NETC_PM_TUND(0),	"eMAC tx undersized packets" },
> > +	{ NETC_PM_TIOCT(0),	"eMAC tx invalid octets" },
> > +};
> > +
> > +static const struct netc_port_stat netc_pmac_counters[] = {
> > +	{ NETC_PM_ROCT(1),	"pMAC rx octets" },
> > +	{ NETC_PM_RVLAN(1),	"pMAC rx VLAN frames" },
> > +	{ NETC_PM_RERR(1),	"pMAC rx frame errors" },
> > +	{ NETC_PM_RUCA(1),	"pMAC rx unicast frames" },
> > +	{ NETC_PM_RDRP(1),	"pMAC rx dropped packets" },
> > +	{ NETC_PM_RPKT(1),	"pMAC rx packets" },
> > +	{ NETC_PM_TOCT(1),	"pMAC tx octets" },
> > +	{ NETC_PM_TVLAN(1),	"pMAC tx VLAN frames" },
> > +	{ NETC_PM_TFCS(1),	"pMAC tx FCS errors" },
> > +	{ NETC_PM_TUCA(1),	"pMAC tx unicast frames" },
> > +	{ NETC_PM_TPKT(1),	"pMAC tx packets" },
> > +	{ NETC_PM_TUND(1),	"pMAC tx undersized packets" },
> > +	{ NETC_PM_TIOCT(1),	"pMAC tx invalid octets" },
> > +};
> 
> Sashiko says:
> 
> Are these standard MAC and interface statistics being duplicated in the
> driver-private ethtool strings?
> The networking subsystem guidelines indicate that statistics which have
> a standard uAPI must not be duplicated in ethtool -S. Standard
> interfaces like struct rtnl_link_stats64 or struct ethtool_eth_mac_stats
> already exist for counters such as rx octets, rx packets, dropped
> packets, and FCS errors.
> Could these metrics be exposed through the standardized interfaces
> rather than adding them to the private string sets?
> 

I don't think it is an issue. For the NETC switch, its ports support preemption,
so each port has two MACs (express MAC and preemptive MAC). The driver-
private statistics can help users obtain statistics for each MAC, just like the
standard statistics of ethtool. However, rtnl_link_stats64 retrieves the combined
statistics of the two MACs.

In addition, ethtool_eth_mac_stats only has the FCS error counter for RX,
not TX. what I added to driver-private was TX's FCS error counter.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 net-next 06/15] net: enetc: add support for the "Update" operation to buffer pool table
       [not found] ` <20260430024945.3413973-7-wei.fang@nxp.com>
@ 2026-05-06  7:21   ` Wei Fang
  0 siblings, 0 replies; 13+ messages in thread
From: Wei Fang @ 2026-05-06  7:21 UTC (permalink / raw)
  To: Claudiu Manoil, Vladimir Oltean, Clark Wang,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, f.fainelli@gmail.com,
	Frank Li, chleroy@kernel.org, horms@kernel.org,
	linux@armlinux.org.uk
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev

> +int ntmp_bpt_update_entry(struct ntmp_user *user, u32 entry_id,
> +			  const struct bpt_cfge_data *cfge)
> +{
> +	struct bpt_req_update *req;
> +	struct netc_swcbd swcbd;
> +	struct netc_cbdr *cbdr;
> +	union netc_cbd cbd;
> +	int err;
> +
> +	swcbd.size = sizeof(*req);
> +	err = ntmp_alloc_data_mem(user->dev, &swcbd, (void **)&req);
> +	if (err)
> +		return err;
> +
> +	ntmp_fill_crd_eid(&req->rbe, user->tbl.bpt_ver, 0,
> +			  NTMP_GEN_UA_CFGEU | BPT_UA_BPSEU, entry_id);

Below is the comment from Sashiko.

Will setting the BPT_UA_BPSEU flag cause issues here?

The BPT_UA_BPSEU flag instructs the hardware to update the Buffer Pool Status
Entry (BPSE), but the allocated request buffer (struct bpt_req_update) only
contains the configuration data (struct bpt_cfge_data), entirely omitting the
status data (struct bpt_bpse_data):
drivers/net/ethernet/freescale/enetc/ntmp_private.h:
struct bpt_req_update {
	struct ntmp_req_by_eid rbe;
	struct bpt_cfge_data cfge;
};

Because the length passed to the hardware is only
sizeof(struct bpt_req_update), will the hardware either reject the command
due to a length mismatch, or read past the end of the cfge structure and
write garbage into the hardware buffer pool's operational status fields?

This is a false positive, BPSEU flag indicates buffer pool state element
update. The state information is maintained by the table's internal buffer,
the table will clear the information if BPSEU flag is set.

> +	req->cfge = *cfge;
> +	ntmp_fill_request_hdr(&cbd, swcbd.dma, NTMP_LEN(swcbd.size, 0),
> +			      NTMP_BPT_ID, NTMP_CMD_UPDATE,
> NTMP_AM_ENTRY_ID);
> +
> +	ntmp_select_and_lock_cbdr(user, &cbdr);
> +	err = netc_xmit_ntmp_cmd(cbdr, &cbd, &swcbd);
> +	if (err)
> +		dev_err(user->dev,
> +			"Failed to update %s entry 0x%x, err: %pe\n",
> +			ntmp_table_name(NTMP_BPT_ID), entry_id, ERR_PTR(err));
> +
> +	ntmp_unlock_cbdr(cbdr);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(ntmp_bpt_update_entry);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 net-next 09/15] net: dsa: add NETC switch tag support
       [not found] ` <20260430024945.3413973-10-wei.fang@nxp.com>
@ 2026-05-06  7:34   ` Wei Fang
  0 siblings, 0 replies; 13+ messages in thread
From: Wei Fang @ 2026-05-06  7:34 UTC (permalink / raw)
  To: Claudiu Manoil, Vladimir Oltean, Clark Wang,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, f.fainelli@gmail.com,
	Frank Li, chleroy@kernel.org, horms@kernel.org,
	linux@armlinux.org.uk
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev

> +static void netc_flow_dissect(const struct sk_buff *skb, __be16 *proto,
> +			      int *offset)
> +{
> +	struct netc_tag_cmn *tag_cmn = (struct netc_tag_cmn *)(skb->data - 2);
> +	int tag_len = netc_get_rx_tag_len(tag_cmn->type);
> +
> +	*offset = tag_len;
> +	*proto = ((__be16 *)skb->data)[(tag_len / 2) - 1];

Below is the comment from Sashiko.

Can this direct access to skb->data cause an out-of-bounds read?

When __skb_flow_dissect() invokes ops->flow_dissect() on a packet injected by
a local user with a small linear area (where skb_headlen(skb) < tag_len),
these accesses might read past skb->tail into uninitialized memory or unmapped
pages.

Since pskb_may_pull() cannot be used on a const struct sk_buff, does this
dissector need to use skb_header_pointer() to safely extract the fields?

This is a false positive, or at least it can be ignored. The RX minimum frame
length of each NETC switch port is set to 64 bytes. The frame will be received
by the ENETC driver. From the hardware perspective, each RX BD receive
buffer is at least 128 bytes, so the skb's linear buffer will definitely contain
the switch tag.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 net-next 10/15] net: dsa: netc: introduce NXP NETC switch driver for i.MX94
       [not found] ` <20260430024945.3413973-11-wei.fang@nxp.com>
@ 2026-05-06  8:03   ` Wei Fang
  0 siblings, 0 replies; 13+ messages in thread
From: Wei Fang @ 2026-05-06  8:03 UTC (permalink / raw)
  To: Claudiu Manoil, Vladimir Oltean, Clark Wang,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, f.fainelli@gmail.com,
	Frank Li, chleroy@kernel.org, horms@kernel.org,
	linux@armlinux.org.uk
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev

There are some comments from Sashiko, and see my response inline.

> +static int netc_init_all_ports(struct netc_switch *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct netc_port *np;
> +	struct dsa_port *dp;
> +	int err;
> +
> +	priv->ports = devm_kcalloc(dev, priv->info->num_ports,
> +				   sizeof(struct netc_port *),
> +				   GFP_KERNEL);
> +	if (!priv->ports)
> +		return -ENOMEM;
> +
> +	/* Some DSA interfaces may set the port even it is disabled, such
> +	 * as .port_disable(), .port_stp_state_set() and so on. To avoid
> +	 * crash caused by accessing NULL port pointer, each port is
> +	 * allocated its own memory. Otherwise, we need to check whether
> +	 * the port pointer is NULL in these interfaces. The latter is
> +	 * difficult for us to cover.
> +	 */
> +	for (int i = 0; i < priv->info->num_ports; i++) {
> +		np = devm_kzalloc(dev, sizeof(*np), GFP_KERNEL);

These devm_* allocations are tied to the PCI device (dev), but happen in a
call chain invoked by the DSA setup() callback:
netc_setup()
  netc_init_all_ports()
    devm_kcalloc(...)
    devm_kzalloc(...)
    netc_port_create_mdio_bus()
      netc_port_create_emdio_bus()
        devm_mdiobus_alloc_size(...)
        devm_of_mdiobus_register(...)
Because the DSA setup() and teardown() callbacks can execute multiple times
over the device's lifetime (e.g., when the DSA conduit interface is unbound
and rebound), these devm_* allocations won't be freed during teardown().

Will this cause memory leaks and MDIO bus registration failures due to sysfs
collisions on subsequent setup() calls?

I checked the DSA core driver with Cody help, the .teardown() is only called
when .setup() fails or the switch device is removed. So .setup() and .teardown()
are not called multiple time over the switch's lifetime, this should be a false
positive.

[ ... ]
> +static int netc_switch_pci_init(struct pci_dev *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct netc_switch_regs *regs;
> +	struct netc_switch *priv;
> +	void __iomem *base;
> +	int err;
> +
> +	pcie_flr(pdev);

Will this pcie_flr() call wipe the PCI configuration space and BARs?

Since pcie_flr() is called without first saving and later restoring the state
with pci_save_state() and pci_restore_state(), the BARs might remain zeroed.
Could subsequent calls to pcim_iomap() and MMIO accesses then fail to decode
at the hardware level, resulting in target aborts?

This is a false positive, the pcie_flr() is called when initializing the switch
to clear the configuration from U-boot or the last binding.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 net-next 11/15] net: dsa: netc: add phylink MAC operations
       [not found] ` <20260430024945.3413973-12-wei.fang@nxp.com>
@ 2026-05-06  8:20   ` Wei Fang
  2026-05-07 12:44   ` Maxime Chevallier
  1 sibling, 0 replies; 13+ messages in thread
From: Wei Fang @ 2026-05-06  8:20 UTC (permalink / raw)
  To: Claudiu Manoil, Vladimir Oltean, Clark Wang,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, f.fainelli@gmail.com,
	Frank Li, chleroy@kernel.org, horms@kernel.org,
	linux@armlinux.org.uk
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev

There are some comments from Sashiko and see my response inline.

> +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 risk corrupting the pseudo-MAC (PMAC) configuration?

By reading the original register value exclusively from the primary MAC
(MAC 0) and writing the resulting new value to both MAC 0 and the PMAC,
any independent configuration stored in the unmasked bits of the PMAC could
be blindly overwritten.

Additionally, if the new value matches the old value for MAC 0, the function
returns early. Could this leave the PMAC permanently out of sync if its state
needed updating but MAC 0 did not?

The pmac means preemption MAC not the pseudo MAC, I will add a comment
to the pmac definition. The purpose of netc_mac_port_rmw() is to make
pMAC and eMAC configurations the same. So this is a false positive.

[...]

> +static void netc_port_mac_tx_graceful_stop(struct netc_port *np) {
> +	netc_port_rmw(np, NETC_POR, POR_TXDIS, POR_TXDIS);
> +
> +	if (is_netc_pseudo_port(np))
> +		return;
> +
> +	netc_port_wait_tx_empty(np, 0);
> +	if (np->caps.pmac)
> +		netc_port_wait_tx_empty(np, 1);
> +
> +	netc_mac_port_rmw(np, NETC_PM_CMD_CFG(0), PM_CMD_CFG_TX_EN,
> 0); }

Can this sequence race with the Port actively pushing the remainder of a packet
to the MAC?

The code disables the Port TX (POR_TXDIS) and then immediately waits for the
MAC TX FIFO to empty. However, without polling the PSR_TX_BUSY bit in NETC_PSR
to ensure the port has finished its work (similar to how PSR_RX_BUSY is checked
in netc_port_mac_rx_graceful_stop), it appears the MAC TX FIFO could
momentarily empty while the port is still pushing data.

Would this lead to prematurely disabling the MAC TX (PM_CMD_CFG_TX_EN = 0) and
potentially truncating egress packets or leaving the hardware in an invalid
state?

This is also a false positive, the procedures for configuring graceful stop for TX
and RX are different, and they are all configured in accordance with the NETC
reference manual.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 net-next 02/15] dt-bindings: net: dsa: add NETC switch
       [not found] ` <20260430024945.3413973-3-wei.fang@nxp.com>
@ 2026-05-06 22:22   ` Rob Herring (Arm)
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring (Arm) @ 2026-05-06 22:22 UTC (permalink / raw)
  To: Wei Fang
  Cc: linux-kernel, chleroy, edumazet, frank.li, kuba, netdev,
	vladimir.oltean, devicetree, linux, conor+dt, horms, davem,
	andrew+netdev, xiaoning.wang, pabeni, krzk+dt, linuxppc-dev,
	claudiu.manoil, f.fainelli, linux-arm-kernel, imx


On Thu, 30 Apr 2026 10:49:32 +0800, Wei Fang wrote:
> Add bindings for NETC switch. This switch is a PCIe function of NETC IP,
> it supports advanced QoS with 8 traffic classes and 4 drop resilience
> levels, and a full range of TSN standards capabilities. The switch CPU
> port connects to an internal ENETC port, which is also a PCIe function
> of NETC IP. So these two ports use a light-weight "pseudo MAC" instead
> of a back-to-back MAC, because the "pseudo MAC" provides the delineation
> between switch and ENETC, this translates to lower power (less logic and
> memory) and lower delay (as there is no serialization delay across this
> link).
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../bindings/net/dsa/nxp,netc-switch.yaml     | 127 ++++++++++++++++++
>  1 file changed, 127 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/nxp,netc-switch.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 net-next 12/15] net: dsa: netc: add FDB, STP, MTU, port setup and host flooding support
       [not found] ` <20260430024945.3413973-13-wei.fang@nxp.com>
@ 2026-05-07  2:08   ` Wei Fang
  0 siblings, 0 replies; 13+ messages in thread
From: Wei Fang @ 2026-05-07  2:08 UTC (permalink / raw)
  To: Claudiu Manoil, Vladimir Oltean, Clark Wang,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, f.fainelli@gmail.com,
	Frank Li, chleroy@kernel.org, horms@kernel.org,
	linux@armlinux.org.uk
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev

There are some false positives reported by Sashiko, see the comments
from Sashiko and my response inline. Other comments are useful and
I will add improvements in v6.

>  static int netc_setup(struct dsa_switch *ds)
>  {
>  	struct netc_switch *priv = ds->priv;
> @@ -393,6 +627,8 @@ static int netc_setup(struct dsa_switch *ds)
>  	if (err)
>  		return err;
> 
> +	netc_get_switch_capabilities(priv);
> +
>  	err = netc_init_all_ports(priv);

Sashiko says:
Since the DSA subsystem can call setup() and teardown() multiple times
(for example, if the CPU master interface is unbound and rebound), does
calling netc_init_all_ports() here leak memory?

Looking at netc_init_all_ports(), it uses devm_kcalloc() and devm_kzalloc()
for the priv->ports array and port structures. Because devm_ allocations
are tied to the PCI device lifetime, they won't be freed during teardown(),
which could lead to a memory leak on each rebind.

Response: Analysis of the DSA core driver reveals the following call stack
for setup():

netc_switch_probe()
	└──dsa_register_switch()
		 └── dsa_switch_probe()
			   └── dsa_tree_setup()
				     └── dsa_tree_setup_switches()
						   └── dsa_switch_setup()

netc_setup() is only called when the switch driver probes, so it will not
be called multiple times during the lifetime of the DSA switch device.

[...]


> +static int netc_port_fdb_add(struct dsa_switch *ds, int port,
> +			     const unsigned char *addr, u16 vid,
> +			     struct dsa_db db)
> +{
> +	struct netc_port *np = NETC_PORT(ds, port);
> +
> +	/* Currently, only support standalone port mode, so only
> +	 * NETC_STANDALONE_PVID (= 0) is supported here.
> +	 */
> +	if (vid != NETC_STANDALONE_PVID)
> +		return -EOPNOTSUPP;
> +
> +	return netc_port_set_fdb_entry(np, addr, vid);
> +}

Sashiko says:
Does this implementation break network isolation between standalone ports?

The driver sets ds->fdb_isolation = true in netc_switch_probe(), which
means it must isolate FDB entries per standalone port or bridge. However,
the isolation context (struct dsa_db db) is ignored here and all entries
are placed in NETC_STANDALONE_PVID (0).

If the same MAC address is added to multiple standalone ports, such as an
IPv6 multicast group, the single FDB entry in VID 0 is updated to include
all these ports in its port_bitmap. A frame matching this MAC arriving on
one standalone port will then be switched in hardware to the other
standalone ports, instead of keeping the traffic properly isolated.

Response: Currently, the NETC switch only supports single port mode, so
all the FDB entries are only added to the CPU port, they will not be added
to the user ports. So this is a false positive.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 net-next 13/15] net: dsa: netc: initialize buffer pool table and implement flow-control
       [not found] ` <20260430024945.3413973-14-wei.fang@nxp.com>
@ 2026-05-07  2:27   ` Wei Fang
  0 siblings, 0 replies; 13+ messages in thread
From: Wei Fang @ 2026-05-07  2:27 UTC (permalink / raw)
  To: Claudiu Manoil, Vladimir Oltean, Clark Wang,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, f.fainelli@gmail.com,
	Frank Li, chleroy@kernel.org, horms@kernel.org,
	linux@armlinux.org.uk
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev

> +static void netc_ipv_to_buffer_pool_mapping(struct netc_switch *priv)
> +{
> +	int bp_per_port = priv->num_bp / priv->info->num_ports;
> +	int q = NETC_IPV_NUM / bp_per_port;
> +	int r = NETC_IPV_NUM % bp_per_port;
> +	int num = q + r;
> +
> +	/* IPV-to–buffer-pool mapping per port:
> +	 * Each port is allocated 'bp_per_port' buffer pools and supports 8
> +	 * IPVs, where a higher IPV indicates a higher frame priority. Each
> +	 * IPV can be mapped to only one buffer pool.
> +	 *
> +	 * The mapping rule is as follows:
> +	 * - The first 'num' IPVs share the port's first buffer pool (index
> +	 * 'base_id').
> +	 * - After that, every 'q' IPVs share one buffer pool, with pool
> +	 * indices increasing sequentially.
> +	 */
> +	for (int i = 0; i < priv->info->num_ports; i++) {
> +		u32 base_id = i * bp_per_port;
> +		u32 bp_id = base_id;
> +		u64 mapping = 0;
> +
> +		for (int ipv = 0; ipv < NETC_IPV_NUM; ipv++) {
> +			/* Update the buffer pool index */
> +			if (ipv >= num)
> +				bp_id = base_id + ((ipv - num) / q) + 1;
> +
> +			mapping |= (u64)bp_id << (ipv * 8);

Sashiko says:

If hardware ever provides more than 8 buffer pools per port (for example,
bp_per_port = 10), will this logic fail to utilize the extra pools?

With bp_per_port > 8, q evaluates to 0, r evaluates to 8, and num becomes 8.
The condition if (ipv >= num) then evaluates to if (ipv >= 8), which
is never met since the loop terminates at ipv < 8.

This would leave bp_id at base_id for all priorities, mapping them all to a
single buffer pool and leaving the rest unused. Should bp_per_port be capped
to NETC_IPV_NUM before calculating q to prevent this silent fallback?

From hardware perspective, each port has 8 IPVs, each IPV can only be
mapped to one buffer pool, it is impossible that the hardware provides
more than 8 buffer pools per port. So this is a false positive.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5 net-next 14/15] net: dsa: netc: add support for the standardized counters
       [not found] ` <20260430024945.3413973-15-wei.fang@nxp.com>
@ 2026-05-07  2:41   ` Wei Fang
  0 siblings, 0 replies; 13+ messages in thread
From: Wei Fang @ 2026-05-07  2:41 UTC (permalink / raw)
  To: Claudiu Manoil, Vladimir Oltean, Clark Wang,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, f.fainelli@gmail.com,
	Frank Li, chleroy@kernel.org, horms@kernel.org,
	linux@armlinux.org.uk
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev

> diff --git a/include/linux/fsl/netc_global.h b/include/linux/fsl/netc_global.h
> index fdecca8c90f0..5b8ff528d369 100644
> --- a/include/linux/fsl/netc_global.h
> +++ b/include/linux/fsl/netc_global.h
> @@ -5,6 +5,7 @@
>  #define __NETC_GLOBAL_H
> 
>  #include <linux/io.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> 
>  static inline u32 netc_read(void __iomem *reg)
>  {
> @@ -16,4 +17,9 @@ static inline void netc_write(void __iomem *reg, u32 val)
>  	iowrite32(val, reg);
>  }
> 
> +static inline u64 netc_read64(void __iomem *reg)
> +{
> +	return ioread64(reg);
> +}
> +

Sashiko says:

Will this cause a build failure on 32-bit architectures? I noticed
ioread64() is used here, but it is strictly gated behind #ifdef CONFIG_64BIT
in include/asm-generic/io.h. Even though linux/io-64-nonatomic-lo-hi.h is
included above, ioread64() isn't provided by that header.

If this is updated to use readq() to fix the 32-bit build, could it lead to
torn reads? On 32-bit systems, readq() falls back to lo_hi_readq(), which
reads the lower 32 bits and then the upper 32 bits sequentially. 

Since these are live, continuously incrementing hardware counters, a wrap of
the lower 32 bits between reads could result in a spurious counter spike of
~4.29 billion. Would it be safer to use a read-high, read-low, read-high
retry loop to ensure atomicity on 32-bit architectures?

First, if CONFIG_64BIT is not selected, then ioread64() is provided by
linux/io-64-nonatomic-lo-hi.h. Sashiko mistakenly believed that
linux/io-64-nonatomic-lo-hi.h did not provide ioread64().

i.MX9x and S32N7 and subsequent SoCs are all arm64 architectures,
netc_read64() is used to read 64-bit registers of NETC.

So this is a false positive.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 net-next 11/15] net: dsa: netc: add phylink MAC operations
       [not found] ` <20260430024945.3413973-12-wei.fang@nxp.com>
  2026-05-06  8:20   ` [PATCH v5 net-next 11/15] net: dsa: netc: add phylink MAC operations Wei Fang
@ 2026-05-07 12:44   ` Maxime Chevallier
  1 sibling, 0 replies; 13+ messages in thread
From: Maxime Chevallier @ 2026-05-07 12:44 UTC (permalink / raw)
  To: Wei Fang, claudiu.manoil, vladimir.oltean, xiaoning.wang,
	andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, f.fainelli, frank.li, chleroy, horms, linux
  Cc: netdev, linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel,
	imx

Hi,

On 30/04/2026 04:49, Wei Fang wrote:
> Different versions of NETC switches have different numbers of ports and
> MAC capabilities. Add .phylink_get_caps() to struct netc_switch_info,
> allowing each NETC switch version to implement its own callback for
> obtaining MAC capabilities.
> 
> Implement the phylink_mac_ops callbacks: .mac_config(), .mac_link_up(),
> and .mac_link_down(). Note that flow-control configuration is not yet
> supported in .mac_link_up(), but will be implemented in a subsequent
> patch.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>

The phylink part looks good to me,

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

> ---
>  drivers/net/dsa/netc/netc_main.c      | 243 ++++++++++++++++++++++++++
>  drivers/net/dsa/netc/netc_platform.c  |  38 ++++
>  drivers/net/dsa/netc/netc_switch.h    |   4 +
>  drivers/net/dsa/netc/netc_switch_hw.h |  26 +++
>  4 files changed, 311 insertions(+)
> 
> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index 90a2d8cfd3d2..edf50cb32cb6 100644
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c
> @@ -44,6 +44,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);
> +}
> +
>  static void netc_port_get_capability(struct netc_port *np)
>  {
>  	u32 val;
> @@ -522,10 +542,232 @@ static void netc_switch_get_ip_revision(struct netc_switch *priv)
>  	priv->revision = FIELD_GET(IPBRR0_IP_REV, val);
>  }
>  
> +static void netc_phylink_get_caps(struct dsa_switch *ds, int port,
> +				  struct phylink_config *config)
> +{
> +	struct netc_switch *priv = ds->priv;
> +
> +	priv->info->phylink_get_caps(port, config);
> +}
> +
> +static void netc_port_set_mac_mode(struct netc_port *np,
> +				   unsigned int mode,
> +				   phy_interface_t phy_mode)
> +{
> +	u32 mask = PM_IF_MODE_IFMODE | PM_IF_MODE_REVMII;
> +	u32 val = 0;
> +
> +	switch (phy_mode) {
> +	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
> +	case PHY_INTERFACE_MODE_RGMII_TXID:
> +		val |= IFMODE_RGMII;
> +		break;
> +	case PHY_INTERFACE_MODE_RMII:
> +		val |= IFMODE_RMII;
> +		break;
> +	case PHY_INTERFACE_MODE_REVMII:
> +		val |= PM_IF_MODE_REVMII;
> +		fallthrough;
> +	case PHY_INTERFACE_MODE_MII:
> +		val |= IFMODE_MII;
> +		break;
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		val |= IFMODE_SGMII;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	netc_mac_port_rmw(np, NETC_PM_IF_MODE(0), mask, val);
> +}
> +
> +static void netc_mac_config(struct phylink_config *config, unsigned int mode,
> +			    const struct phylink_link_state *state)
> +{
> +	struct dsa_port *dp = dsa_phylink_to_port(config);
> +
> +	netc_port_set_mac_mode(NETC_PORT(dp->ds, dp->index), mode,
> +			       state->interface);
> +}
> +
> +static void netc_port_set_speed(struct netc_port *np, int speed)
> +{
> +	netc_port_rmw(np, NETC_PCR, PCR_PSPEED, PSPEED_SET_VAL(speed));
> +}
> +
> +static void netc_port_set_rgmii_mac(struct netc_port *np,
> +				    int speed, int duplex)
> +{
> +	u32 mask, val;
> +
> +	mask = PM_IF_MODE_SSP | PM_IF_MODE_HD | PM_IF_MODE_M10;
> +
> +	switch (speed) {
> +	default:
> +	case SPEED_1000:
> +		val = FIELD_PREP(PM_IF_MODE_SSP, SSP_1G);
> +		break;
> +	case SPEED_100:
> +		val = FIELD_PREP(PM_IF_MODE_SSP, SSP_100M);
> +		break;
> +	case SPEED_10:
> +		val = FIELD_PREP(PM_IF_MODE_SSP, SSP_10M);
> +		break;
> +	}
> +
> +	if (duplex != DUPLEX_FULL)
> +		val |= PM_IF_MODE_HD;
> +
> +	netc_mac_port_rmw(np, NETC_PM_IF_MODE(0), mask, val);
> +}
> +
> +static void netc_port_set_rmii_mii_mac(struct netc_port *np,
> +				       int speed, int duplex)
> +{
> +	u32 mask, val = 0;
> +
> +	mask = PM_IF_MODE_SSP | PM_IF_MODE_HD | PM_IF_MODE_M10;
> +
> +	if (speed == SPEED_10)
> +		val |= PM_IF_MODE_M10;
> +
> +	if (duplex != DUPLEX_FULL)
> +		val |= PM_IF_MODE_HD;
> +
> +	netc_mac_port_rmw(np, NETC_PM_IF_MODE(0), mask, val);
> +}
> +
> +static void netc_port_mac_rx_enable(struct netc_port *np)
> +{
> +	netc_port_rmw(np, NETC_POR, POR_RXDIS, 0);
> +	netc_mac_port_rmw(np, NETC_PM_CMD_CFG(0), PM_CMD_CFG_RX_EN,
> +			  PM_CMD_CFG_RX_EN);
> +}
> +
> +static void netc_port_wait_rx_empty(struct netc_port *np, int mac)
> +{
> +	u32 val;
> +
> +	/* PM_IEVENT_RX_EMPTY is a read-only bit, it is automatically set by
> +	 * hardware if RX FIFO is empty and no RX packet receive in process.
> +	 * And it is automatically cleared if RX FIFO is not empty or RX
> +	 * packet receive in process.
> +	 */
> +	if (read_poll_timeout(netc_port_rd, val, val & PM_IEVENT_RX_EMPTY,
> +			      100, 10000, false, np, NETC_PM_IEVENT(mac)))
> +		dev_warn(np->switch_priv->dev,
> +			 "swp%d MAC%d: RX is not idle\n", np->dp->index, mac);
> +}
> +
> +static void netc_port_mac_rx_graceful_stop(struct netc_port *np)
> +{
> +	u32 val;
> +
> +	if (is_netc_pseudo_port(np))
> +		goto rx_disable;
> +
> +	if (np->caps.pmac) {
> +		netc_port_rmw(np, NETC_PM_CMD_CFG(1), PM_CMD_CFG_RX_EN, 0);
> +		netc_port_wait_rx_empty(np, 1);
> +	}
> +
> +	netc_port_rmw(np, NETC_PM_CMD_CFG(0), PM_CMD_CFG_RX_EN, 0);
> +	netc_port_wait_rx_empty(np, 0);
> +
> +	if (read_poll_timeout(netc_port_rd, val, !(val & PSR_RX_BUSY),
> +			      100, 10000, false, np, NETC_PSR))
> +		dev_warn(np->switch_priv->dev, "swp%d RX is busy\n",
> +			 np->dp->index);
> +
> +rx_disable:
> +	netc_port_rmw(np, NETC_POR, POR_RXDIS, POR_RXDIS);
> +}
> +
> +static void netc_port_mac_tx_enable(struct netc_port *np)
> +{
> +	netc_mac_port_rmw(np, NETC_PM_CMD_CFG(0), PM_CMD_CFG_TX_EN,
> +			  PM_CMD_CFG_TX_EN);
> +	netc_port_rmw(np, NETC_POR, POR_TXDIS, 0);
> +}
> +
> +static void netc_port_wait_tx_empty(struct netc_port *np, int mac)
> +{
> +	u32 val;
> +
> +	/* PM_IEVENT_TX_EMPTY is a read-only bit, it is automatically set by
> +	 * hardware if TX FIFO is empty. And it is automatically cleared if
> +	 * TX FIFO is not empty.
> +	 */
> +	if (read_poll_timeout(netc_port_rd, val, val & PM_IEVENT_TX_EMPTY,
> +			      100, 10000, false, np, NETC_PM_IEVENT(mac)))
> +		dev_warn(np->switch_priv->dev,
> +			 "swp%d MAC%d: TX FIFO is not empty\n",
> +			 np->dp->index, mac);
> +}
> +
> +static void netc_port_mac_tx_graceful_stop(struct netc_port *np)
> +{
> +	netc_port_rmw(np, NETC_POR, POR_TXDIS, POR_TXDIS);
> +
> +	if (is_netc_pseudo_port(np))
> +		return;
> +
> +	netc_port_wait_tx_empty(np, 0);
> +	if (np->caps.pmac)
> +		netc_port_wait_tx_empty(np, 1);
> +
> +	netc_mac_port_rmw(np, NETC_PM_CMD_CFG(0), PM_CMD_CFG_TX_EN, 0);
> +}
> +
> +static void netc_mac_link_up(struct phylink_config *config,
> +			     struct phy_device *phy, unsigned int mode,
> +			     phy_interface_t interface, int speed,
> +			     int duplex, bool tx_pause, bool rx_pause)
> +{
> +	struct dsa_port *dp = dsa_phylink_to_port(config);
> +	struct netc_port *np;
> +
> +	np = NETC_PORT(dp->ds, dp->index);
> +	netc_port_set_speed(np, speed);
> +
> +	if (phy_interface_mode_is_rgmii(interface))
> +		netc_port_set_rgmii_mac(np, speed, duplex);
> +
> +	if (interface == PHY_INTERFACE_MODE_RMII ||
> +	    interface == PHY_INTERFACE_MODE_REVMII ||
> +	    interface == PHY_INTERFACE_MODE_MII)
> +		netc_port_set_rmii_mii_mac(np, speed, duplex);
> +
> +	netc_port_mac_tx_enable(np);
> +	netc_port_mac_rx_enable(np);
> +}
> +
> +static void netc_mac_link_down(struct phylink_config *config,
> +			       unsigned int mode,
> +			       phy_interface_t interface)
> +{
> +	struct dsa_port *dp = dsa_phylink_to_port(config);
> +	struct netc_port *np;
> +
> +	np = NETC_PORT(dp->ds, dp->index);
> +	netc_port_mac_rx_graceful_stop(np);
> +	netc_port_mac_tx_graceful_stop(np);
> +}
> +
> +static const struct phylink_mac_ops netc_phylink_mac_ops = {
> +	.mac_config		= netc_mac_config,
> +	.mac_link_up		= netc_mac_link_up,
> +	.mac_link_down		= netc_mac_link_down,
> +};
> +
>  static const struct dsa_switch_ops netc_switch_ops = {
>  	.get_tag_protocol		= netc_get_tag_protocol,
>  	.setup				= netc_setup,
>  	.teardown			= netc_teardown,
> +	.phylink_get_caps		= netc_phylink_get_caps,
>  };
>  
>  static int netc_switch_probe(struct pci_dev *pdev,
> @@ -564,6 +806,7 @@ static int netc_switch_probe(struct pci_dev *pdev,
>  	ds->num_ports = priv->info->num_ports;
>  	ds->num_tx_queues = NETC_TC_NUM;
>  	ds->ops = &netc_switch_ops;
> +	ds->phylink_mac_ops = &netc_phylink_mac_ops;
>  	ds->priv = priv;
>  	priv->ds = ds;
>  
> diff --git a/drivers/net/dsa/netc/netc_platform.c b/drivers/net/dsa/netc/netc_platform.c
> index abd599ea9c8d..bb4f92d238cb 100644
> --- a/drivers/net/dsa/netc/netc_platform.c
> +++ b/drivers/net/dsa/netc/netc_platform.c
> @@ -11,8 +11,46 @@ struct netc_switch_platform {
>  	const struct netc_switch_info *info;
>  };
>  
> +static void imx94_switch_phylink_get_caps(int port,
> +					  struct phylink_config *config)
> +{
> +	config->mac_capabilities = MAC_1000FD;
> +
> +	switch (port) {
> +	case 0 ... 1:
> +		__set_bit(PHY_INTERFACE_MODE_SGMII,
> +			  config->supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
> +			  config->supported_interfaces);
> +		config->mac_capabilities |= MAC_2500FD;
> +		fallthrough;
> +	case 2:
> +		config->mac_capabilities |= MAC_10 | MAC_100;
> +		__set_bit(PHY_INTERFACE_MODE_MII,
> +			  config->supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_RMII,
> +			  config->supported_interfaces);
> +		/* Port 0 and 1 do not support REVMII */
> +		if (port == 2)
> +			__set_bit(PHY_INTERFACE_MODE_REVMII,
> +				  config->supported_interfaces);
> +
> +		phy_interface_set_rgmii(config->supported_interfaces);
> +		break;
> +	case 3: /* CPU port */
> +		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
> +			  config->supported_interfaces);
> +		config->mac_capabilities |= MAC_10FD | MAC_100FD |
> +					    MAC_2500FD;
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
>  static const struct netc_switch_info imx94_info = {
>  	.num_ports = 4,
> +	.phylink_get_caps = imx94_switch_phylink_get_caps,
>  };
>  
>  static const struct netc_switch_platform netc_platforms[] = {
> diff --git a/drivers/net/dsa/netc/netc_switch.h b/drivers/net/dsa/netc/netc_switch.h
> index dac19bfba02b..eb65c36ecead 100644
> --- a/drivers/net/dsa/netc/netc_switch.h
> +++ b/drivers/net/dsa/netc/netc_switch.h
> @@ -34,6 +34,7 @@ struct netc_switch;
>  
>  struct netc_switch_info {
>  	u32 num_ports;
> +	void (*phylink_get_caps)(int port, struct phylink_config *config);
>  };
>  
>  struct netc_port_caps {
> @@ -70,6 +71,9 @@ struct netc_switch {
>  	struct ntmp_user ntmp;
>  };
>  
> +#define NETC_PRIV(ds)			((struct netc_switch *)((ds)->priv))
> +#define NETC_PORT(ds, port_id)		(NETC_PRIV(ds)->ports[(port_id)])
> +
>  /* Write/Read Switch base registers */
>  #define netc_base_rd(r, o)		netc_read((r)->base + (o))
>  #define netc_base_wr(r, o, v)		netc_write((r)->base + (o), v)
> diff --git a/drivers/net/dsa/netc/netc_switch_hw.h b/drivers/net/dsa/netc/netc_switch_hw.h
> index 0419f7f9207e..7d9afb493053 100644
> --- a/drivers/net/dsa/netc/netc_switch_hw.h
> +++ b/drivers/net/dsa/netc/netc_switch_hw.h
> @@ -67,6 +67,14 @@
>  #define  PQOSMR_VQMP			GENMASK(19, 16)
>  #define  PQOSMR_QVMP			GENMASK(23, 20)
>  
> +#define NETC_POR			0x100
> +#define  POR_TXDIS			BIT(0)
> +#define  POR_RXDIS			BIT(1)
> +
> +#define NETC_PSR			0x104
> +#define  PSR_TX_BUSY			BIT(0)
> +#define  PSR_RX_BUSY			BIT(1)
> +
>  #define NETC_PTCTMSDUR(a)		(0x208 + (a) * 0x20)
>  #define  PTCTMSDUR_MAXSDU		GENMASK(15, 0)
>  #define  PTCTMSDUR_SDU_TYPE		GENMASK(17, 16)
> @@ -123,6 +131,24 @@ enum netc_mfo {
>  #define NETC_PM_MAXFRM(a)		(0x1014 + (a) * 0x400)
>  #define  PM_MAXFRAM			GENMASK(15, 0)
>  
> +#define NETC_PM_IEVENT(a)		(0x1040 + (a) * 0x400)
> +#define  PM_IEVENT_TX_EMPTY		BIT(5)
> +#define  PM_IEVENT_RX_EMPTY		BIT(6)
> +
> +#define NETC_PM_IF_MODE(a)		(0x1300 + (a) * 0x400)
> +#define  PM_IF_MODE_IFMODE		GENMASK(2, 0)
> +#define   IFMODE_MII			1
> +#define   IFMODE_RMII			3
> +#define   IFMODE_RGMII			4
> +#define   IFMODE_SGMII			5
> +#define  PM_IF_MODE_REVMII		BIT(3)
> +#define  PM_IF_MODE_M10			BIT(4)
> +#define  PM_IF_MODE_HD			BIT(6)
> +#define  PM_IF_MODE_SSP			GENMASK(14, 13)
> +#define   SSP_100M			0
> +#define   SSP_10M			1
> +#define   SSP_1G			2
> +
>  #define NETC_PEMDIOCR			0x1c00
>  #define NETC_EMDIO_BASE			NETC_PEMDIOCR
>  


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-05-07 12:44 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260430024945.3413973-1-wei.fang@nxp.com>
     [not found] ` <20260430024945.3413973-5-wei.fang@nxp.com>
2026-05-05  8:59   ` [PATCH v5 net-next 04/15] net: enetc: add basic operations to the FDB table Paolo Abeni
2026-05-06  6:37     ` Wei Fang
     [not found] ` <20260430024945.3413973-16-wei.fang@nxp.com>
2026-05-05  9:43   ` [PATCH v5 net-next 15/15] net: dsa: netc: add support for ethtool private statistics Paolo Abeni
2026-05-06  7:06     ` Wei Fang
     [not found] ` <20260430024945.3413973-7-wei.fang@nxp.com>
2026-05-06  7:21   ` [PATCH v5 net-next 06/15] net: enetc: add support for the "Update" operation to buffer pool table Wei Fang
     [not found] ` <20260430024945.3413973-10-wei.fang@nxp.com>
2026-05-06  7:34   ` [PATCH v5 net-next 09/15] net: dsa: add NETC switch tag support Wei Fang
     [not found] ` <20260430024945.3413973-11-wei.fang@nxp.com>
2026-05-06  8:03   ` [PATCH v5 net-next 10/15] net: dsa: netc: introduce NXP NETC switch driver for i.MX94 Wei Fang
     [not found] ` <20260430024945.3413973-3-wei.fang@nxp.com>
2026-05-06 22:22   ` [PATCH v5 net-next 02/15] dt-bindings: net: dsa: add NETC switch Rob Herring (Arm)
     [not found] ` <20260430024945.3413973-13-wei.fang@nxp.com>
2026-05-07  2:08   ` [PATCH v5 net-next 12/15] net: dsa: netc: add FDB, STP, MTU, port setup and host flooding support Wei Fang
     [not found] ` <20260430024945.3413973-14-wei.fang@nxp.com>
2026-05-07  2:27   ` [PATCH v5 net-next 13/15] net: dsa: netc: initialize buffer pool table and implement flow-control Wei Fang
     [not found] ` <20260430024945.3413973-15-wei.fang@nxp.com>
2026-05-07  2:41   ` [PATCH v5 net-next 14/15] net: dsa: netc: add support for the standardized counters Wei Fang
     [not found] ` <20260430024945.3413973-12-wei.fang@nxp.com>
2026-05-06  8:20   ` [PATCH v5 net-next 11/15] net: dsa: netc: add phylink MAC operations Wei Fang
2026-05-07 12:44   ` Maxime Chevallier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox