Hi Tim,
Am Di., 6. Mai 2025 um 17:31 Uhr schrieb Tim Harvey <thar...@gateworks.com>:
> On Tue, May 6, 2025 at 2:33 AM Heiko Thiery <heiko.thi...@gmail.com>
> wrote:
> >
> > Hi Alice, Hi Marek,
> >
> >
> >
> > Am Di., 29. Apr. 2025 um 03:19 Uhr schrieb Wei Fang <wei.f...@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.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_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.olt...@nxp.com>
> >> > Tested-by: Vladimir Oltean <vladimir.olt...@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?
>
>
No, this should be ok, I think it is good to know that the fix does not
influence the behavior on the imx9.
> Tested-by: Tim Harvey <thar...@gateworks.com> # imx95_19x19_evk
>
> Best Regards,
>
> Tim
>
Thanks,
Heiko
