* Re: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node
@ 2026-10-16 8:15 netdev-bot+sashiko
0 siblings, 0 replies; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-10-16 8:15 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev, s32, linux-kernel, bhelgaas, linux-pci, kuba
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 3 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] Build break when CONFIG_OF_DYNAMIC=n.
- [Medium] enetc4_link_init() uses the OF_DYNAMIC flag alone to decide
that a pseudo-MAC port's node is 'an empty node created by the PCI…
- [Medium] The pseudo-MAC fixed-link speed is the boot-time PCR[PSPEED]
rounded down to a phylink-supported value, and the existing link-up…
- [Low] enetc4_add_ppm_fixed_link() can write 20000 or 25000 into the
synthesized node's fixed-link 'speed' property.
--
net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node
This commit lets ENETCv4 pseudo-MAC ports probe when they have no static
DT description. At probe time it uses an of_changeset to add a
fixed-link child node to the port's OF node, with the speed taken from
PCR[PSPEED], so that phylink can pick it up. The changeset is reverted
and destroyed on teardown and on the probe error path.
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index 71c971618388b..70e34ca6ddf8e 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
[ ... ]
> @@ -946,23 +955,112 @@ static void enetc4_pci_remove(void *data)
[ ... ]
> +static int enetc4_add_ppm_fixed_link(struct enetc_ndev_priv *priv)
> +{
> + struct device_node *np = dev_of_node(priv->dev);
> + struct device_node *fixed_link;
> + struct of_changeset *ocs;
> + int speed, err;
> +
> + if (!np)
> + return dev_err_probe(priv->dev, -ENODEV,
> + "Missing of_node for Pseudo-MAC port\n");
> +
> + enetc4_get_pcr_speed(&priv->si->hw, &speed);
> + speed = enetc_phylink_match_pseudo_mac_speed(speed);
[Severity: Medium]
Does this rounding end up overwriting the boot-time PCR[PSPEED] value?
enetc4_get_pcr_speed() decodes PSPEED as (pspeed + 1) * 10, so the
result can be any multiple of 10 Mbps. enetc_phylink_match_pseudo_mac_speed()
then rounds down to the first table entry that is <= that value, with no
warning. For example, 4000 becomes 2500 and 40000 becomes 25000.
The rounded value goes into the fixed-link "speed" property. On link-up,
phylink passes it to enetc4_pl_mac_link_up(), which has no pseudo-MAC
check:
enetc4_pl_mac_link_up()
enetc4_set_port_speed(priv, speed);
priv->speed starts at 0, so the first call always does a read-modify-write
of ENETC4_PCR with PCR_PSPEED_VAL(rounded speed).
Suppose the switch owner programmed a PSPEED that is not in the table.
Wouldn't the driver replace it with a lower rate? priv->speed is used by
enetc_qos.c for the CBS port_transmit_rate. Wouldn't it also stop
matching the real internal link?
This seems to conflict with the commit message statement that "the fixed
link reflects the actual boot-time configuration". I could not confirm
whether firmware actually programs rates outside the table.
> +
> + ocs = kzalloc_obj(*ocs);
> + if (!ocs)
> + return -ENOMEM;
> +
> + of_changeset_init(ocs);
[Severity: Medium]
Will this build when CONFIG_OF_DYNAMIC is not set?
This function calls of_changeset_init(), of_changeset_create_node(),
of_changeset_add_prop_u32(), of_changeset_add_prop_bool() and
of_changeset_apply(). enetc4_put_ppm_fixed_link() also calls
of_changeset_revert() and of_changeset_destroy(). All of these are
declared in include/linux/of.h only under #ifdef CONFIG_OF_DYNAMIC.
The #else branch only has stubs for the of_reconfig_* helpers.
drivers/of/dynamic.c is also only built with OF_DYNAMIC=y.
config NXP_ENETC4 only depends on PTP_1588_CLOCK_OPTIONAL and PCI_MSI.
OF_DYNAMIC is a hidden symbol that is only enabled when something
selects it (OF_OVERLAY, PCI_DYNAMIC_OF_NODES, ...).
With NXP_ENETC4=y/m and OF_DYNAMIC=n, wouldn't the build fail? I'd
expect implicit declaration errors first, then undefined symbols at
modpost or link time. That includes CONFIG_OF=n builds through
COMPILE_TEST.
The commit message says:
This is intentionally not a hard Kconfig dependency, since the ENETC4
driver also serves regular ports described by static DT; when
PCI_DYNAMIC_OF_NODES is absent the PCI function has no of_node and the
pseudo-MAC probe path fails gracefully with a clear -ENODEV error.
If nothing selects OF_DYNAMIC, the driver would not build at all, so it
never gets to the -ENODEV path. Should NXP_ENETC4 select or depend on
OF_DYNAMIC, or should these calls be guarded?
> +
> + fixed_link = of_changeset_create_node(ocs, np, "fixed-link");
> + if (!fixed_link) {
> + err = -ENOMEM;
> + goto err_destroy;
> + }
> +
> + err = of_changeset_add_prop_u32(ocs, fixed_link, "speed", speed);
[Severity: Low]
This isn't a functional bug, but can this write a speed that the
fixed-link binding doesn't allow?
enetc_phylink_pseudo_mac_caps[] includes SPEED_25000 and SPEED_20000. For
PSPEED >= 20 Gbps, enetc_phylink_match_pseudo_mac_speed() returns 20000
or 25000.
For the object form of fixed-link, the "speed" property in
Documentation/devicetree/bindings/net/ethernet-controller.yaml only
allows:
enum: [10, 100, 1000, 2500, 5000, 10000]
The commit message says the node is added "exactly as if it had come
from static DT". A static DT describing this port the same way would
fail dtschema validation. phylink_parse_fixedlink() accepts these speeds
at runtime, so nothing breaks.
Should the binding be extended to 20000 and 25000, or should the driver
avoid emitting them?
[ ... ]
> static int enetc4_link_init(struct enetc_ndev_priv *priv,
> struct device_node *node)
> {
> + bool dynamic = node && of_node_check_flag(node, OF_DYNAMIC);
> struct enetc_pf *pf = enetc_si_priv(priv->si);
> struct device *dev = priv->dev;
> int err;
>
> - err = of_get_phy_mode(node, &pf->if_mode);
> - if (err) {
> - dev_err(dev, "Failed to get PHY mode\n");
> - return err;
> + /* Pseudo-MAC ENETCs are described by a runtime-synthesized fixed-link
> + * node rather than static DT. This covers both a missing OF node and a
> + * PCI-synthesized (OF_DYNAMIC) node, which is an empty node created
> + * by the PCI core and thus carries no fixed-link description.
> + */
> + if (enetc_is_pseudo_mac(priv->si) && (!node || dynamic)) {
> + pf->if_mode = PHY_INTERFACE_MODE_INTERNAL;
[Severity: Medium]
Can OF_DYNAMIC be relied on to mean the node has no link description?
OF_DYNAMIC only records that a node was allocated at runtime.
__of_node_dup() sets it on every runtime-created node. That includes
nodes created by the overlay engine (add_changeset_node()->__of_node_dup())
and PCI-synthesized nodes that a later overlay fills in. The
PCI-synthesized node is not empty either, since it has compatible and
reg.
Take an OF_DYNAMIC pseudo-MAC node that already has phy-mode and a
fixed-link child. This path skips of_get_phy_mode() and forces
PHY_INTERFACE_MODE_INTERNAL. enetc4_add_ppm_fixed_link() then adds a
second child named "fixed-link" without checking for an existing one.
__of_attach_node() inserts the new node at the head of the parent's
child list:
np->sibling = np->parent->child;
np->parent->child = np;
So phylink_parse_fixedlink()->fwnode_get_named_child_node(fwnode,
"fixed-link") would find the synthesized node first. Wouldn't the
overlay's speed, pause, asym-pause and link-gpios be silently ignored?
The tree would also end up with two sibling nodes of the same name.
Before this patch, such a node went through of_get_phy_mode() and its
own fixed-link was used. Would a check based on the node's contents (no
phy-mode, no phy-handle, and !of_phy_is_fixed_link()) match the intent
in the comment better than the flag?
I couldn't find in-tree code that applies such an overlay to an ENETC
node today, so this may be theoretical for now.
> +
> + err = enetc4_add_ppm_fixed_link(priv);
> + if (err)
> + return err;
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1791908694.git.claudiu.manoil%40nxp.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node
2026-10-15 8:43 ` kernel test robot
@ 2026-10-15 13:26 ` Claudiu Manoil
0 siblings, 0 replies; 6+ messages in thread
From: Claudiu Manoil @ 2026-10-15 13:26 UTC (permalink / raw)
To: kernel test robot, netdev
Cc: llvm, oe-kbuild-all, dl-S32, linux-kernel, Vladimir Oltean,
Wei Fang, Clark Wang, Andrew Lunn, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Saravana Kannan, Russell King, imx,
devicetree
NXP Confidential
From: kernel test robot <lkp@intel.com>
Sent: Thursday, October 15, 2026 11:44 AM
[...]
> Subject: Re: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for
> ENETCv4 Ports via a fixed-link node
>
> Hi Claudiu,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on net-next/main]
>
>
[...]
> >> drivers/net/ethernet/freescale/enetc/enetc4_pf.c:963:2: error: call
> >> to undeclared function 'of_changeset_revert'; ISO C99 and later do
> >> not support implicit function declarations
> >> [-Wimplicit-function-declaration]
> 963 | of_changeset_revert(priv->ppm_ocs);
> | ^
I'm looking into this, probably I need to guard the of_changeset_* calls with
#ifdefs inside the driver.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node
2026-10-15 2:16 ` Wei Fang
@ 2026-10-15 13:20 ` Claudiu Manoil
0 siblings, 0 replies; 6+ messages in thread
From: Claudiu Manoil @ 2026-10-15 13:20 UTC (permalink / raw)
To: Wei Fang
Cc: dl-S32, linux-kernel, Vladimir Oltean, Clark Wang, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Saravana Kannan, Russell King, imx, devicetree,
netdev
From: Wei Fang <wei.fang@nxp.com>
Sent: Thursday, October 15, 2026 5:17 AM
[...]
> Subject: RE: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for
> ENETCv4 Ports via a fixed-link node
>
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h
> > b/drivers/net/ethernet/freescale/enetc/enetc.h
> > index d9e91832a9c1..ed5402f1b73c 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc.h
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc.h
> > @@ -500,6 +500,7 @@ struct enetc_ndev_priv {
> >
> > struct clk *ref_clk; /* RGMII/RMII reference clock */
> > u64 sysclk_freq; /* NETC system clock frequency */
> > + struct of_changeset *ppm_ocs; /* pseudo-MAC fixed-link changeset
> */
>
> struct enetc_ndev_priv is shared by both PF and VF, but ppm_ocs is only
> used by PF. Would it be better to move it to struct enetc_pf?
>
Ok, will try this out.
> > };
[...]
> > +static int enetc4_add_ppm_fixed_link(struct enetc_ndev_priv *priv)
> > +{
> > + struct device_node *np = dev_of_node(priv->dev);
> > + struct device_node *fixed_link;
> > + struct of_changeset *ocs;
> > + int speed, err;
> > +
> > + if (!np)
> > + return dev_err_probe(priv->dev, -ENODEV,
> > + "Missing of_node for Pseudo-MAC port\n");
> > +
> > + enetc4_get_pcr_speed(&priv->si->hw, &speed);
> > + speed = enetc_phylink_match_pseudo_mac_speed(speed);
> > +
> > + ocs = kzalloc_obj(*ocs);
> > + if (!ocs)
> > + return -ENOMEM;
> > +
> > + of_changeset_init(ocs);
> > +
> > + fixed_link = of_changeset_create_node(ocs, np, "fixed-link");
> > + if (!fixed_link) {
> > + err = -ENOMEM;
> > + goto err_destroy;
> > + }
> > +
> > + err = of_changeset_add_prop_u32(ocs, fixed_link, "speed", speed);
> > + if (err)
> > + goto err_put_node;
> > +
> > + err = of_changeset_add_prop_bool(ocs, fixed_link, "full-duplex");
> > + if (err)
> > + goto err_put_node;
> > +
> > + err = of_changeset_apply(ocs);
> > + if (err)
> > + goto err_put_node;
> > +
> > + of_node_put(fixed_link);
> > + priv->ppm_ocs = ocs;
> > +
> > + return 0;
> > +
> > +err_put_node:
> > + of_node_put(fixed_link);
> > +err_destroy:
> > + of_changeset_destroy(ocs);
> > + kfree(ocs);
> > +
> > + return dev_err_probe(priv->dev, err,
> > + "Failed to add fixed-link node\n");
> > +}
> > +
> > static int enetc4_link_init(struct enetc_ndev_priv *priv,
> > struct device_node *node)
> > {
> > + bool dynamic = node && of_node_check_flag(node, OF_DYNAMIC);
> > struct enetc_pf *pf = enetc_si_priv(priv->si);
> > struct device *dev = priv->dev;
> > int err;
> >
> > - err = of_get_phy_mode(node, &pf->if_mode);
> > - if (err) {
> > - dev_err(dev, "Failed to get PHY mode\n");
> > - return err;
[...]
> > + if (enetc_is_pseudo_mac(priv->si) && (!node || dynamic)) {
>
> This indicates that the node pointer can be NULL, but
> enetc4_add_ppm_fixed_link() subsequently does not allow node
> to be NULL. This is puzzling.
enetc4_add_ppm_fixed_link() checks dev_of_node(priv->dev) and
if NULL, it returns error and prints following err message:
"Missing of_node for Pseudo-MAC port\n".
Which is fine for the enetc_is_pseudo_mac() case.
One alternative would be to take the common 'else' branch in case of NULL
node for the pseudo-MAC, and let of_get_phy_mode() return with error, but
in that case "Failed to get PHY mode\n" is not a good error indication.
So, nothing to change here, the 'if' branch check for the pseudo-MAC case is
correct as it is.
>
> > + pf->if_mode = PHY_INTERFACE_MODE_INTERNAL;
> > +
> > + err = enetc4_add_ppm_fixed_link(priv);
> > + if (err)
> > + return err;
> > +
> > + /* The fixed-link node was spliced onto dev's own of_node, so
> > + * phylink will find it via dev_fwnode(dev). Use that node for
> > + * the subsequent MDIO/phylink setup below.
> > + */
> > + node = dev_of_node(dev);
>
> Based on the implementation of enetc4_add_ppm_fixed_link(), the
> input parameter dev->of_node cannot be NULL. I'm not familiar
> with the of_changeset_* APIs. Will enetc4_add_ppm_fixed_link()
> update this pointer?
>
Sure, all these functions take 'np' as argument, and 'np' is
dev_of_node(priv->dev), which is the same as dev->of_node.
> > + } else {
> > + err = of_get_phy_mode(node, &pf->if_mode);
> > + if (err) {
> > + dev_err(dev, "Failed to get PHY mode\n");
> > + return err;
> > + }
> > }
> >
> > err = enetc_mdiobus_create(pf, node);
> > if (err) {
> > dev_err(dev, "Failed to create MDIO bus\n");
> > - return err;
> > + goto err_mdiobus_create;
> > }
> >
> > err = enetc_phylink_create(priv, node, &enetc_pl_mac_ops);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node
2026-10-13 17:12 ` [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node Claudiu Manoil
2026-10-15 2:16 ` Wei Fang
@ 2026-10-15 8:43 ` kernel test robot
2026-10-15 13:26 ` Claudiu Manoil
1 sibling, 1 reply; 6+ messages in thread
From: kernel test robot @ 2026-10-15 8:43 UTC (permalink / raw)
To: Claudiu Manoil, netdev
Cc: llvm, oe-kbuild-all, s32, linux-kernel, Vladimir Oltean,
Wei Fang, Clark Wang, Andrew Lunn, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Saravana Kannan, Russell King, imx,
devicetree
Hi Claudiu,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Claudiu-Manoil/net-enetc-Add-pseudo-MAC-support-for-ENETCv4-Ports-via-a-fixed-link-node/20261013-201245
base: net-next/main
patch link: https://lore.kernel.org/r/d7b51cb29758538a4c68a700dd8cd41b6e7e08b6.1791908694.git.claudiu.manoil%40nxp.com
patch subject: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node
config: x86_64-buildonly-randconfig-1004-20261015 (https://download.01.org/0day-ci/archive/20261015/202610151650.UHqL0Sb7-lkp@intel.com/config)
compiler: clang version 22.1.8 (https://github.com/llvm/llvm-project ca7933e47d3a3451d81e72ac174dcb5aa28b59d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261015/202610151650.UHqL0Sb7-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610151650.UHqL0Sb7-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/net/ethernet/freescale/enetc/enetc4_pf.c:963:2: error: call to undeclared function 'of_changeset_revert'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
963 | of_changeset_revert(priv->ppm_ocs);
| ^
>> drivers/net/ethernet/freescale/enetc/enetc4_pf.c:964:2: error: call to undeclared function 'of_changeset_destroy'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
964 | of_changeset_destroy(priv->ppm_ocs);
| ^
>> drivers/net/ethernet/freescale/enetc/enetc4_pf.c:992:2: error: call to undeclared function 'of_changeset_init'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
992 | of_changeset_init(ocs);
| ^
>> drivers/net/ethernet/freescale/enetc/enetc4_pf.c:994:15: error: call to undeclared function 'of_changeset_create_node'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
994 | fixed_link = of_changeset_create_node(ocs, np, "fixed-link");
| ^
>> drivers/net/ethernet/freescale/enetc/enetc4_pf.c:994:13: error: incompatible integer to pointer conversion assigning to 'struct device_node *' from 'int' [-Wint-conversion]
994 | fixed_link = of_changeset_create_node(ocs, np, "fixed-link");
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/freescale/enetc/enetc4_pf.c:1000:8: error: call to undeclared function 'of_changeset_add_prop_u32'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1000 | err = of_changeset_add_prop_u32(ocs, fixed_link, "speed", speed);
| ^
>> drivers/net/ethernet/freescale/enetc/enetc4_pf.c:1004:8: error: call to undeclared function 'of_changeset_add_prop_bool'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1004 | err = of_changeset_add_prop_bool(ocs, fixed_link, "full-duplex");
| ^
>> drivers/net/ethernet/freescale/enetc/enetc4_pf.c:1008:8: error: call to undeclared function 'of_changeset_apply'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1008 | err = of_changeset_apply(ocs);
| ^
drivers/net/ethernet/freescale/enetc/enetc4_pf.c:1020:2: error: call to undeclared function 'of_changeset_destroy'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1020 | of_changeset_destroy(ocs);
| ^
9 errors generated.
vim +/of_changeset_revert +963 drivers/net/ethernet/freescale/enetc/enetc4_pf.c
957
958 static void enetc4_put_ppm_fixed_link(struct enetc_ndev_priv *priv)
959 {
960 if (!priv->ppm_ocs)
961 return;
962
> 963 of_changeset_revert(priv->ppm_ocs);
> 964 of_changeset_destroy(priv->ppm_ocs);
965 kfree(priv->ppm_ocs);
966 priv->ppm_ocs = NULL;
967 }
968
969 /* Pseudo-MAC ports have no real PHY; the link is fixed, with the operating
970 * speed set at probe time from PCR[PSPEED]. Describe this with a fixed-link
971 * subnode synthesized at runtime onto the port's own OF node, so phylink
972 * picks it up through dev_fwnode() exactly as if it came from static DT.
973 */
974 static int enetc4_add_ppm_fixed_link(struct enetc_ndev_priv *priv)
975 {
976 struct device_node *np = dev_of_node(priv->dev);
977 struct device_node *fixed_link;
978 struct of_changeset *ocs;
979 int speed, err;
980
981 if (!np)
982 return dev_err_probe(priv->dev, -ENODEV,
983 "Missing of_node for Pseudo-MAC port\n");
984
985 enetc4_get_pcr_speed(&priv->si->hw, &speed);
986 speed = enetc_phylink_match_pseudo_mac_speed(speed);
987
988 ocs = kzalloc_obj(*ocs);
989 if (!ocs)
990 return -ENOMEM;
991
> 992 of_changeset_init(ocs);
993
> 994 fixed_link = of_changeset_create_node(ocs, np, "fixed-link");
995 if (!fixed_link) {
996 err = -ENOMEM;
997 goto err_destroy;
998 }
999
> 1000 err = of_changeset_add_prop_u32(ocs, fixed_link, "speed", speed);
1001 if (err)
1002 goto err_put_node;
1003
> 1004 err = of_changeset_add_prop_bool(ocs, fixed_link, "full-duplex");
1005 if (err)
1006 goto err_put_node;
1007
> 1008 err = of_changeset_apply(ocs);
1009 if (err)
1010 goto err_put_node;
1011
1012 of_node_put(fixed_link);
1013 priv->ppm_ocs = ocs;
1014
1015 return 0;
1016
1017 err_put_node:
1018 of_node_put(fixed_link);
1019 err_destroy:
1020 of_changeset_destroy(ocs);
1021 kfree(ocs);
1022
1023 return dev_err_probe(priv->dev, err,
1024 "Failed to add fixed-link node\n");
1025 }
1026
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node
2026-10-13 17:12 ` [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node Claudiu Manoil
@ 2026-10-15 2:16 ` Wei Fang
2026-10-15 13:20 ` Claudiu Manoil
2026-10-15 8:43 ` kernel test robot
1 sibling, 1 reply; 6+ messages in thread
From: Wei Fang @ 2026-10-15 2:16 UTC (permalink / raw)
To: Claudiu Manoil
Cc: dl-S32, linux-kernel, Vladimir Oltean, Clark Wang, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Saravana Kannan, Russell King, imx, devicetree,
netdev
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h
> b/drivers/net/ethernet/freescale/enetc/enetc.h
> index d9e91832a9c1..ed5402f1b73c 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.h
> @@ -500,6 +500,7 @@ struct enetc_ndev_priv {
>
> struct clk *ref_clk; /* RGMII/RMII reference clock */
> u64 sysclk_freq; /* NETC system clock frequency */
> + struct of_changeset *ppm_ocs; /* pseudo-MAC fixed-link changeset */
struct enetc_ndev_priv is shared by both PF and VF, but ppm_ocs is only
used by PF. Would it be better to move it to struct enetc_pf?
> };
>
> #define ENETC_CBD(R, i) (&(((struct enetc_cbd *)((R).bd_base))[i]))
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index 71c971618388..70e34ca6ddf8 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -1,8 +1,9 @@
> // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> -/* Copyright 2024 NXP */
> +/* Copyright 2024, 2026 NXP */
>
> #include <linux/clk.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/of_net.h>
> #include <linux/of_platform.h>
> #include <linux/unaligned.h>
> @@ -932,6 +933,14 @@ static void enetc4_pl_mac_link_down(struct
> phylink_config *config,
> enetc4_mac_tx_graceful_stop(pf);
> }
>
> +static void enetc4_get_pcr_speed(struct enetc_hw *hw, int *speed)
> +{
> + u32 val = enetc_port_rd(hw, ENETC4_PCR);
> + int pspeed = FIELD_GET(PCR_PSPEED, val);
> +
> + *speed = (pspeed + 1) * 10;
> +}
> +
> static const struct phylink_mac_ops enetc_pl_mac_ops = {
> .mac_select_pcs = enetc4_pl_mac_select_pcs,
> .mac_config = enetc4_pl_mac_config,
> @@ -946,23 +955,112 @@ static void enetc4_pci_remove(void *data)
> enetc_pci_remove(pdev);
> }
>
> +static void enetc4_put_ppm_fixed_link(struct enetc_ndev_priv *priv)
> +{
> + if (!priv->ppm_ocs)
> + return;
> +
> + of_changeset_revert(priv->ppm_ocs);
> + of_changeset_destroy(priv->ppm_ocs);
> + kfree(priv->ppm_ocs);
> + priv->ppm_ocs = NULL;
> +}
> +
> +/* Pseudo-MAC ports have no real PHY; the link is fixed, with the operating
> + * speed set at probe time from PCR[PSPEED]. Describe this with a fixed-link
> + * subnode synthesized at runtime onto the port's own OF node, so phylink
> + * picks it up through dev_fwnode() exactly as if it came from static DT.
> + */
> +static int enetc4_add_ppm_fixed_link(struct enetc_ndev_priv *priv)
> +{
> + struct device_node *np = dev_of_node(priv->dev);
> + struct device_node *fixed_link;
> + struct of_changeset *ocs;
> + int speed, err;
> +
> + if (!np)
> + return dev_err_probe(priv->dev, -ENODEV,
> + "Missing of_node for Pseudo-MAC port\n");
> +
> + enetc4_get_pcr_speed(&priv->si->hw, &speed);
> + speed = enetc_phylink_match_pseudo_mac_speed(speed);
> +
> + ocs = kzalloc_obj(*ocs);
> + if (!ocs)
> + return -ENOMEM;
> +
> + of_changeset_init(ocs);
> +
> + fixed_link = of_changeset_create_node(ocs, np, "fixed-link");
> + if (!fixed_link) {
> + err = -ENOMEM;
> + goto err_destroy;
> + }
> +
> + err = of_changeset_add_prop_u32(ocs, fixed_link, "speed", speed);
> + if (err)
> + goto err_put_node;
> +
> + err = of_changeset_add_prop_bool(ocs, fixed_link, "full-duplex");
> + if (err)
> + goto err_put_node;
> +
> + err = of_changeset_apply(ocs);
> + if (err)
> + goto err_put_node;
> +
> + of_node_put(fixed_link);
> + priv->ppm_ocs = ocs;
> +
> + return 0;
> +
> +err_put_node:
> + of_node_put(fixed_link);
> +err_destroy:
> + of_changeset_destroy(ocs);
> + kfree(ocs);
> +
> + return dev_err_probe(priv->dev, err,
> + "Failed to add fixed-link node\n");
> +}
> +
> static int enetc4_link_init(struct enetc_ndev_priv *priv,
> struct device_node *node)
> {
> + bool dynamic = node && of_node_check_flag(node, OF_DYNAMIC);
> struct enetc_pf *pf = enetc_si_priv(priv->si);
> struct device *dev = priv->dev;
> int err;
>
> - err = of_get_phy_mode(node, &pf->if_mode);
> - if (err) {
> - dev_err(dev, "Failed to get PHY mode\n");
> - return err;
> + /* Pseudo-MAC ENETCs are described by a runtime-synthesized fixed-link
> + * node rather than static DT. This covers both a missing OF node and a
> + * PCI-synthesized (OF_DYNAMIC) node, which is an empty node created
> + * by the PCI core and thus carries no fixed-link description.
> + */
> + if (enetc_is_pseudo_mac(priv->si) && (!node || dynamic)) {
This indicates that the node pointer can be NULL, but
enetc4_add_ppm_fixed_link() subsequently does not allow node
to be NULL. This is puzzling.
> + pf->if_mode = PHY_INTERFACE_MODE_INTERNAL;
> +
> + err = enetc4_add_ppm_fixed_link(priv);
> + if (err)
> + return err;
> +
> + /* The fixed-link node was spliced onto dev's own of_node, so
> + * phylink will find it via dev_fwnode(dev). Use that node for
> + * the subsequent MDIO/phylink setup below.
> + */
> + node = dev_of_node(dev);
Based on the implementation of enetc4_add_ppm_fixed_link(), the
input parameter dev->of_node cannot be NULL. I'm not familiar
with the of_changeset_* APIs. Will enetc4_add_ppm_fixed_link()
update this pointer?
> + } else {
> + err = of_get_phy_mode(node, &pf->if_mode);
> + if (err) {
> + dev_err(dev, "Failed to get PHY mode\n");
> + return err;
> + }
> }
>
> err = enetc_mdiobus_create(pf, node);
> if (err) {
> dev_err(dev, "Failed to create MDIO bus\n");
> - return err;
> + goto err_mdiobus_create;
> }
>
> err = enetc_phylink_create(priv, node, &enetc_pl_mac_ops);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node
2026-10-13 17:12 [PATCH net-next v2 0/5] net: enetc: S32N79 pseudo-MAC (PPM) networking support Claudiu Manoil
@ 2026-10-13 17:12 ` Claudiu Manoil
2026-10-15 2:16 ` Wei Fang
2026-10-15 8:43 ` kernel test robot
0 siblings, 2 replies; 6+ messages in thread
From: Claudiu Manoil @ 2026-10-13 17:12 UTC (permalink / raw)
To: netdev
Cc: s32, linux-kernel, Vladimir Oltean, Wei Fang, Clark Wang,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Saravana Kannan, Russell King, imx,
devicetree
ENETCv4 has special internal links when connected to the on-chip NETC
switch via internal switch ports, called pseudo-MAC links. These
pseudo-MACs are proprietary, they don't implement any standard IEEE
interface (like MII), and can be modeled as fixed links with the link
speed determined at boot time by the Port PCR[PSPEED] register
configuration.
We also need to be able to probe the ENETCv4 Ports featuring pseudo-MACs
as pure PCI devices, i.e. without any "ethernet" DT node representation.
The typical use case for this consists in a board with NETC connected
via PCI to another host which probes the ENETC. Note that in this
scenario, the pseudo-MAC ENETCs are connected internally to a NETC
switch that is not owned by Linux.
Since such a port has no "ethernet" DT node, its fixed link has to be
synthesized at probe time. Build the fixed-link subnode directly in C
with an of_changeset: of_changeset_create_node() adds a "fixed-link"
node onto the port's own OF node, and of_changeset_add_prop_*() populate
its "speed" and "full-duplex" properties. The changeset is applied with
of_changeset_apply(), so the fixed-link node is spliced onto the ENETC
netdev's fwnode exactly as if it had come from static DT, and phylink
picks it up through dev_fwnode().
The operating speed written into the node is read from PCR[PSPEED] at
probe time and mapped to a phylink-supported value, so the fixed link
reflects the actual boot-time configuration.
The pseudo-MAC path is selected when the port has no OF node, or when it
only has the empty PCI-synthesized node (OF_DYNAMIC), which carries no
fixed-link description. Ports described by static DT take the regular
of_get_phy_mode() path instead.
The changeset is reverted and destroyed on teardown and on the probe
error unwind. The node-less pseudo-MAC path has a runtime dependency on
CONFIG_PCI_DYNAMIC_OF_NODES: that is what makes the PCI core synthesize
the per-function OF node (of_pci_make_dev_node()) that the fixed-link
node is attached onto. This is intentionally not a hard Kconfig
dependency, since the ENETC4 driver also serves regular ports described
by static DT; when PCI_DYNAMIC_OF_NODES is absent the PCI function has no
of_node and the pseudo-MAC probe path fails gracefully with a clear
-ENODEV error.
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v2:
- Switched from a DT overlay blob (of_overlay_fdt_apply() + a .dtso
compiled to .dtbo) to of_changeset_create_node(), per review
feedback. Drops the embedded overlay externs, the Makefile .dtbo
wrapping, and the OF_OVERLAY Kconfig dependency.
- Baked the operating speed directly into the changeset-created
fixed-link node (read once from PCR[PSPEED] at probe time); the
phylink get_fixed_state callback is no longer needed and was removed.
drivers/net/ethernet/freescale/enetc/enetc.h | 1 +
.../net/ethernet/freescale/enetc/enetc4_pf.c | 113 +++++++++++++++++-
.../freescale/enetc/enetc_pf_common.c | 41 ++++++-
.../freescale/enetc/enetc_pf_common.h | 1 +
4 files changed, 147 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index d9e91832a9c1..ed5402f1b73c 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -500,6 +500,7 @@ struct enetc_ndev_priv {
struct clk *ref_clk; /* RGMII/RMII reference clock */
u64 sysclk_freq; /* NETC system clock frequency */
+ struct of_changeset *ppm_ocs; /* pseudo-MAC fixed-link changeset */
};
#define ENETC_CBD(R, i) (&(((struct enetc_cbd *)((R).bd_base))[i]))
diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
index 71c971618388..70e34ca6ddf8 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
@@ -1,8 +1,9 @@
// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
-/* Copyright 2024 NXP */
+/* Copyright 2024, 2026 NXP */
#include <linux/clk.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/of_net.h>
#include <linux/of_platform.h>
#include <linux/unaligned.h>
@@ -932,6 +933,14 @@ static void enetc4_pl_mac_link_down(struct phylink_config *config,
enetc4_mac_tx_graceful_stop(pf);
}
+static void enetc4_get_pcr_speed(struct enetc_hw *hw, int *speed)
+{
+ u32 val = enetc_port_rd(hw, ENETC4_PCR);
+ int pspeed = FIELD_GET(PCR_PSPEED, val);
+
+ *speed = (pspeed + 1) * 10;
+}
+
static const struct phylink_mac_ops enetc_pl_mac_ops = {
.mac_select_pcs = enetc4_pl_mac_select_pcs,
.mac_config = enetc4_pl_mac_config,
@@ -946,23 +955,112 @@ static void enetc4_pci_remove(void *data)
enetc_pci_remove(pdev);
}
+static void enetc4_put_ppm_fixed_link(struct enetc_ndev_priv *priv)
+{
+ if (!priv->ppm_ocs)
+ return;
+
+ of_changeset_revert(priv->ppm_ocs);
+ of_changeset_destroy(priv->ppm_ocs);
+ kfree(priv->ppm_ocs);
+ priv->ppm_ocs = NULL;
+}
+
+/* Pseudo-MAC ports have no real PHY; the link is fixed, with the operating
+ * speed set at probe time from PCR[PSPEED]. Describe this with a fixed-link
+ * subnode synthesized at runtime onto the port's own OF node, so phylink
+ * picks it up through dev_fwnode() exactly as if it came from static DT.
+ */
+static int enetc4_add_ppm_fixed_link(struct enetc_ndev_priv *priv)
+{
+ struct device_node *np = dev_of_node(priv->dev);
+ struct device_node *fixed_link;
+ struct of_changeset *ocs;
+ int speed, err;
+
+ if (!np)
+ return dev_err_probe(priv->dev, -ENODEV,
+ "Missing of_node for Pseudo-MAC port\n");
+
+ enetc4_get_pcr_speed(&priv->si->hw, &speed);
+ speed = enetc_phylink_match_pseudo_mac_speed(speed);
+
+ ocs = kzalloc_obj(*ocs);
+ if (!ocs)
+ return -ENOMEM;
+
+ of_changeset_init(ocs);
+
+ fixed_link = of_changeset_create_node(ocs, np, "fixed-link");
+ if (!fixed_link) {
+ err = -ENOMEM;
+ goto err_destroy;
+ }
+
+ err = of_changeset_add_prop_u32(ocs, fixed_link, "speed", speed);
+ if (err)
+ goto err_put_node;
+
+ err = of_changeset_add_prop_bool(ocs, fixed_link, "full-duplex");
+ if (err)
+ goto err_put_node;
+
+ err = of_changeset_apply(ocs);
+ if (err)
+ goto err_put_node;
+
+ of_node_put(fixed_link);
+ priv->ppm_ocs = ocs;
+
+ return 0;
+
+err_put_node:
+ of_node_put(fixed_link);
+err_destroy:
+ of_changeset_destroy(ocs);
+ kfree(ocs);
+
+ return dev_err_probe(priv->dev, err,
+ "Failed to add fixed-link node\n");
+}
+
static int enetc4_link_init(struct enetc_ndev_priv *priv,
struct device_node *node)
{
+ bool dynamic = node && of_node_check_flag(node, OF_DYNAMIC);
struct enetc_pf *pf = enetc_si_priv(priv->si);
struct device *dev = priv->dev;
int err;
- err = of_get_phy_mode(node, &pf->if_mode);
- if (err) {
- dev_err(dev, "Failed to get PHY mode\n");
- return err;
+ /* Pseudo-MAC ENETCs are described by a runtime-synthesized fixed-link
+ * node rather than static DT. This covers both a missing OF node and a
+ * PCI-synthesized (OF_DYNAMIC) node, which is an empty node created
+ * by the PCI core and thus carries no fixed-link description.
+ */
+ if (enetc_is_pseudo_mac(priv->si) && (!node || dynamic)) {
+ pf->if_mode = PHY_INTERFACE_MODE_INTERNAL;
+
+ err = enetc4_add_ppm_fixed_link(priv);
+ if (err)
+ return err;
+
+ /* The fixed-link node was spliced onto dev's own of_node, so
+ * phylink will find it via dev_fwnode(dev). Use that node for
+ * the subsequent MDIO/phylink setup below.
+ */
+ node = dev_of_node(dev);
+ } else {
+ err = of_get_phy_mode(node, &pf->if_mode);
+ if (err) {
+ dev_err(dev, "Failed to get PHY mode\n");
+ return err;
+ }
}
err = enetc_mdiobus_create(pf, node);
if (err) {
dev_err(dev, "Failed to create MDIO bus\n");
- return err;
+ goto err_mdiobus_create;
}
err = enetc_phylink_create(priv, node, &enetc_pl_mac_ops);
@@ -975,6 +1073,8 @@ static int enetc4_link_init(struct enetc_ndev_priv *priv,
err_phylink_create:
enetc_mdiobus_destroy(pf);
+err_mdiobus_create:
+ enetc4_put_ppm_fixed_link(priv);
return err;
}
@@ -985,6 +1085,7 @@ static void enetc4_link_deinit(struct enetc_ndev_priv *priv)
enetc_phylink_destroy(priv);
enetc_mdiobus_destroy(pf);
+ enetc4_put_ppm_fixed_link(priv);
}
static void enetc4_pf_link_status_task(struct work_struct *work)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
index 8206884294a4..3ba46c3a7670 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
@@ -436,6 +436,43 @@ void enetc_mdiobus_destroy(struct enetc_pf *pf)
}
EXPORT_SYMBOL_GPL(enetc_mdiobus_destroy);
+static struct {
+ unsigned long mac_cap;
+ int speed; /* descending order sorted */
+} enetc_phylink_pseudo_mac_caps[] = {
+ { MAC_25000FD, SPEED_25000 },
+ { MAC_20000FD, SPEED_20000 },
+ { MAC_10000FD, SPEED_10000 },
+ { MAC_5000FD, SPEED_5000 },
+ { MAC_2500FD, SPEED_2500 },
+ { MAC_1000FD, SPEED_1000 },
+ { MAC_100FD, SPEED_100 },
+ { MAC_10FD, SPEED_10 },
+};
+
+static unsigned long enetc_phylink_get_pseudo_mac_caps(void)
+{
+ unsigned long mac_caps = 0;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(enetc_phylink_pseudo_mac_caps); i++)
+ mac_caps |= enetc_phylink_pseudo_mac_caps[i].mac_cap;
+
+ return mac_caps;
+}
+
+int enetc_phylink_match_pseudo_mac_speed(int speed)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(enetc_phylink_pseudo_mac_caps); i++)
+ if (enetc_phylink_pseudo_mac_caps[i].speed <= speed)
+ return enetc_phylink_pseudo_mac_caps[i].speed;
+
+ return SPEED_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(enetc_phylink_match_pseudo_mac_speed);
+
int enetc_phylink_create(struct enetc_ndev_priv *priv, struct device_node *node,
const struct phylink_mac_ops *ops)
{
@@ -471,9 +508,7 @@ int enetc_phylink_create(struct enetc_ndev_priv *priv, struct device_node *node,
phy_interface_set_rgmii(pf->phylink_config.supported_interfaces);
} else {
- mac_caps |= MAC_10FD | MAC_100FD | MAC_1000FD | MAC_2500FD |
- MAC_5000FD | MAC_10000FD | MAC_20000FD |
- MAC_25000FD;
+ mac_caps |= enetc_phylink_get_pseudo_mac_caps();
}
pf->phylink_config.mac_capabilities = mac_caps;
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.h b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.h
index c9eed879d5a3..aede1aad6362 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.h
@@ -13,6 +13,7 @@ void enetc_mdiobus_destroy(struct enetc_pf *pf);
int enetc_phylink_create(struct enetc_ndev_priv *priv, struct device_node *node,
const struct phylink_mac_ops *ops);
void enetc_phylink_destroy(struct enetc_ndev_priv *priv);
+int enetc_phylink_match_pseudo_mac_speed(int speed);
void enetc_set_default_rss_key(struct enetc_pf *pf);
int enetc_vlan_rx_add_vid(struct net_device *ndev, __be16 prot, u16 vid);
int enetc_vlan_rx_del_vid(struct net_device *ndev, __be16 prot, u16 vid);
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-16 8:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-16 8:15 [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node netdev-bot+sashiko
-- strict thread matches above, loose matches on Subject: below --
2026-10-13 17:12 [PATCH net-next v2 0/5] net: enetc: S32N79 pseudo-MAC (PPM) networking support Claudiu Manoil
2026-10-13 17:12 ` [PATCH net-next v2 1/5] net: enetc: Add pseudo-MAC support for ENETCv4 Ports via a fixed-link node Claudiu Manoil
2026-10-15 2:16 ` Wei Fang
2026-10-15 13:20 ` Claudiu Manoil
2026-10-15 8:43 ` kernel test robot
2026-10-15 13:26 ` Claudiu Manoil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®