[PATCH] net: fsl_enetc: fix imdio register calculation

[PATCH] net: fsl_enetc: fix imdio register calculation

[Heiko Thiery]

From: Thomas Schaefer <thomas.schaefer@kontron.com>

With commit cc4e8af2c552, fsl_enetc register accessors have been split to
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@kontron.com>
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

---

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.

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);
-- 
2.20.1

Re: [PATCH] net: fsl_enetc: fix imdio register calculation

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 2363 bytes --]

[+ 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@kontron.com>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

With the nitpick above:

Reviewed-by: Michael Walle <mwalle@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

> 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);


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

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

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

RE: [PATCH] net: fsl_enetc: fix imdio register calculation

[Wei Fang]

> > > 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@kontron.com>
> > > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> >
> > With the nitpick above:
> >
> > Reviewed-by: Michael Walle <mwalle@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@nxp.com>
> Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com> # LS1028A
> 
> +Fang Wei to confirm/test for i.MX95.

Hi Alice,

Can you help check this patch on i.MX95?

Re: [PATCH] net: fsl_enetc: fix imdio register calculation

[Heiko Thiery]

Hi Alice, Hi Marek,



Am Di., 29. Apr. 2025 um 03:19 Uhr schrieb Wei Fang <wei.fang@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@kontron.com>
> > > > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> > >
> > > With the nitpick above:
> > >
> > > Reviewed-by: Michael Walle <mwalle@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@nxp.com>
> > Tested-by: Vladimir Oltean <vladimir.oltean@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

Re: [PATCH] net: fsl_enetc: fix imdio register calculation

[Tim Harvey]

On Tue, May 6, 2025 at 2:33 AM Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> Hi Alice, Hi Marek,
>
>
>
> Am Di., 29. Apr. 2025 um 03:19 Uhr schrieb Wei Fang <wei.fang@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@kontron.com>
>> > > > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
>> > >
>> > > With the nitpick above:
>> > >
>> > > Reviewed-by: Michael Walle <mwalle@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@nxp.com>
>> > Tested-by: Vladimir Oltean <vladimir.oltean@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@gateworks.com> # imx95_19x19_evk

Best Regards,

Tim

Re: [PATCH] net: fsl_enetc: fix imdio register calculation

[Heiko Thiery]

Hi Tim,

Am Di., 6. Mai 2025 um 17:31 Uhr schrieb Tim Harvey <tharvey@gateworks.com>:

> On Tue, May 6, 2025 at 2:33 AM Heiko Thiery <heiko.thiery@gmail.com>
> wrote:
> >
> > Hi Alice, Hi Marek,
> >
> >
> >
> > Am Di., 29. Apr. 2025 um 03:19 Uhr schrieb Wei Fang <wei.fang@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@kontron.com>
> >> > > > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> >> > >
> >> > > With the nitpick above:
> >> > >
> >> > > Reviewed-by: Michael Walle <mwalle@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@nxp.com>
> >> > Tested-by: Vladimir Oltean <vladimir.oltean@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 <tharvey@gateworks.com> # imx95_19x19_evk
>
> Best Regards,
>

> Tim
>


Thanks,
Heiko

Re: [PATCH] net: fsl_enetc: fix imdio register calculation

[Marek Vasut]

On 4/28/25 11:59 AM, Heiko Thiery wrote:
> From: Thomas Schaefer <thomas.schaefer@kontron.com>
> 
> With commit cc4e8af2c552, fsl_enetc register accessors have been split to
> 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@kontron.com>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
Reviewed-by: Marek Vasut <marex@denx.de>

回复: [PATCH] net: fsl_enetc: fix imdio register calculation

[Alice Guo (OSS)]

Reviewed-by: Alice Guo <alice.guo@nxp.com>

Best Regards,
Alice Guo

> -----邮件原件-----
> 发件人: U-Boot <u-boot-bounces@lists.denx.de> 代表 Heiko Thiery
> 发送时间: 2025年4月28日 18:00
> 收件人: u-boot@lists.denx.de
> 抄送: Joe Hershberger <joe.hershberger@ni.com>; Ramon Fried
> <rfried.dev@gmail.com>; Tom Rini <trini@konsulko.com>; Marek Vasut
> <marex@denx.de>; Alice Guo <alice.guo@nxp.com>; Ye Li <ye.li@nxp.com>;
> tharvey@gateworks.com; Michael Walle <mwalle@kernel.org>; Thomas
> Schäfer <thomas.schaefer@kontron.com>; Heiko Thiery
> <heiko.thiery@gmail.com>
> 主题: [PATCH] net: fsl_enetc: fix imdio register calculation
> 
> From: Thomas Schaefer <thomas.schaefer@kontron.com>
> 
> With commit cc4e8af2c552, fsl_enetc register accessors have been split to
> 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@kontron.com>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> 
> ---
> 
> 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.
> 
> 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);
> --
> 2.20.1

Re: [PATCH] net: fsl_enetc: fix imdio register calculation

[Fabio Estevam]

On Mon, Apr 28, 2025 at 7:00 AM Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> From: Thomas Schaefer <thomas.schaefer@kontron.com>
>
> With commit cc4e8af2c552, fsl_enetc register accessors have been split to
> 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@kontron.com>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

Applied, thanks.