On Mon, Apr 28, 2025 at 12:06:28PM +0200, Michael Walle wrote: > [+ Vladimir] > > Hi, > > nice catch! > > > With commit cc4e8af2c552, fsl_enetc register accessors have been split to > > With 'commit cc4e8af2c552 ("net: fsl_enetc: Split register accessors")' > > > handle different register offsets on different SoCs. However, for > > internal MDIO register calculation, only ENETC_PM_IMDIO_BASE was fixed > > without adding the SoC specific MAC register offset. > > > > As a result, the network support for the Kontron SMARC-sAL28 and > > probably other boards based on the LS1028A CPU is broken. > > > > Add the SoC specific MAC register offset to calculation of imdio.priv to > > fix this. > > > > Fixes: cc4e8af2c552 ("net: fsl_enetc: Split register accessors") > > Signed-off-by: Thomas Schaefer <thomas.schae...@kontron.com> > > Signed-off-by: Heiko Thiery <heiko.thi...@gmail.com> > > With the nitpick above: > > Reviewed-by: Michael Walle <mwa...@kernel.org> > > > --- > > > > But the question that now arises is, does this code path work on the imx > > SoCs as well. Now the imx specific offset 0x5000 is added here and was not > > used before. > > Maybe the imx9 doesn't use the internal MDIO controller or the board > which was this tested on doesn't use the PCS? > > -michael
The imx specific offset 0x5000 should be correct. In downstream, the ENETC_PM_IMDIO_BASE macro is used in common code, but defined twice - there is a conditional include of either fsl_enetc.h, or fsl_enetc4.h, depending on whether CONFIG_ARCH_IMX9 is defined. Thus, ENETC_PM_IMDIO_BASE is replaced either with 0x8030 or 0x5030 in downstream. https://github.com/nxp-qoriq/u-boot/blob/lf-6.12.3-1.0.0/drivers/net/fsl_enetc.c#L25 https://github.com/nxp-qoriq/u-boot/blob/lf-6.12.3-1.0.0/drivers/net/fsl_enetc.h#L76 https://github.com/nxp-qoriq/u-boot/blob/lf-6.12.3-1.0.0/drivers/net/fsl_enetc4.h#L126 I guess Marek fell into the pitfall when upstreaming. > > Can someone please test this on an imx9 and confirm? > > > > drivers/net/fsl_enetc.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c > > index 52fa820f51..97cccda451 100644 > > --- a/drivers/net/fsl_enetc.c > > +++ b/drivers/net/fsl_enetc.c > > @@ -473,13 +473,15 @@ static int enetc_init_sxgmii(struct udevice *dev) > > /* Apply protocol specific configuration to MAC, serdes as needed */ > > static void enetc_start_pcs(struct udevice *dev) > > { > > + struct enetc_data *data = (struct enetc_data *)dev_get_driver_data(dev); > > struct enetc_priv *priv = dev_get_priv(dev); > > > > /* register internal MDIO for debug purposes */ > > if (enetc_read_pcapr_mdio(dev)) { > > priv->imdio.read = enetc_mdio_read; > > priv->imdio.write = enetc_mdio_write; > > - priv->imdio.priv = priv->port_regs + ENETC_PM_IMDIO_BASE; > > + priv->imdio.priv = priv->port_regs + data->reg_offset_mac + > > + ENETC_PM_IMDIO_BASE; > > strlcpy(priv->imdio.name, dev->name, MDIO_NAME_LEN); > > if (!miiphy_get_dev_by_name(priv->imdio.name)) > > mdio_register(&priv->imdio); > Reviewed-by: Vladimir Oltean <vladimir.olt...@nxp.com> Tested-by: Vladimir Oltean <vladimir.olt...@nxp.com> # LS1028A +Fang Wei to confirm/test for i.MX95.