[PATCH] net: fsl_enetc: fix imdio register calculation

[Tim Harvey]

On Tue, May 6, 2025 at 2:33 AM Heiko Thiery <heiko.thiery at gmail.com> wrote:
>
> Hi Alice, Hi Marek,
>
>
>
> Am Di., 29. Apr. 2025 um 03:19 Uhr schrieb Wei Fang <wei.fang at nxp.com>:
>>
>> > > > 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_enetc
>> > 4.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.
>>
>> Hi Alice,
>>
>> Can you help check this patch on i.MX95?
>
>
> Friendly reminder, are you able to confirm that?
>
>
> BR,
> Heiko

Hi Heiko,

I was able to test this on top of master with an imx95_19x19_evk -
enetc (1gb) interface works before and after the patch. Was there
something more specific that needs testing?

Tested-by: Tim Harvey <tharvey at gateworks.com> # imx95_19x19_evk

Best Regards,

Tim