| Message ID | 20250428095945.1432389-1-heiko.thiery@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Fabio Estevam |
| Headers | show |
| Series | net: fsl_enetc: fix imdio register calculation | expand |
[+ 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);
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.
> > > 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?
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
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
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
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>
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
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.
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);