[PATCH] net: fsl_enetc: fix imdio register calculation

[Vladimir Oltean]

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.schaefer at kontron.com>
> > Signed-off-by: Heiko Thiery <heiko.thiery at gmail.com>
> 
> With the nitpick above:
> 
> Reviewed-by: Michael Walle <mwalle at 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.oltean at nxp.com>
Tested-by: Vladimir Oltean <vladimir.oltean at nxp.com> # LS1028A

+Fang Wei to confirm/test for i.MX95.