[PATCH net 0/3] Regressions in Ocelot switch drivers

[PATCH net 0/3] Regressions in Ocelot switch drivers

[Vladimir Oltean]

These are 3 patches which resolve a regression in the Seville driver,
one in the Felix driver and a generic one which affects any kernel
compiled with 2 Kconfig options enabled. All of them have in common my
lack of attention during review/testing. The patches touch the DSA, MFD
and MDIO drivers for Ocelot. I think it would be preferable if all
patches went through netdev (with Lee's Ack).

Vladimir Oltean (3):
  net: dsa: seville: ignore mscc-miim read errors from Lynx PCS
  net: dsa: felix: fix internal MDIO controller resource length
  net: mscc: ocelot: fix duplicate driver name error

 drivers/mfd/ocelot-core.c                | 2 +-
 drivers/net/dsa/ocelot/felix_vsc9959.c   | 2 +-
 drivers/net/dsa/ocelot/ocelot_ext.c      | 2 +-
 drivers/net/dsa/ocelot/seville_vsc9953.c | 4 ++--
 drivers/net/mdio/mdio-mscc-miim.c        | 9 ++++++---
 include/linux/mdio/mdio-mscc-miim.h      | 2 +-
 6 files changed, 12 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH net 1/3] net: dsa: seville: ignore mscc-miim read errors from Lynx PCS

[Vladimir Oltean]

During the refactoring in the commit below, vsc9953_mdio_read() was
replaced with mscc_miim_read(), which has one extra step: it checks for
the MSCC_MIIM_DATA_ERROR bits before returning the result.

On T1040RDB, there are 8 QSGMII PCSes belonging to the switch, and they
are organized in 2 groups. First group responds to MDIO addresses 4-7
because QSGMIIACR1[MDEV_PORT] is 1, and the second group responds to
MDIO addresses 8-11 because QSGMIIBCR1[MDEV_PORT] is 2. I have double
checked that these values are correctly set in the SERDES, as well as
PCCR1[QSGMA_CFG] and PCCR1[QSGMB_CFG] are both 0b01.

mscc_miim_read: phyad 8 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 8 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 8 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 8 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 9 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 9 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 9 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 9 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 10 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 10 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 10 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 10 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 11 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 11 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 11 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 11 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 4 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 4 reg 0x5 MIIM_DATA 0x3da01, ERROR
mscc_miim_read: phyad 5 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 5 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 5 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 5 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 6 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 6 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 6 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 6 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 7 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 7 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 7 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 7 reg 0x5 MIIM_DATA 0x35801, ERROR

As can be seen, the data in MIIM_DATA is still valid despite having the
MSCC_MIIM_DATA_ERROR bits set. The driver as introduced in commit
84705fc16552 ("net: dsa: felix: introduce support for Seville VSC9953
switch") was ignoring these bits, perhaps deliberately (although
unbeknownst to me).

This is an old IP and the hardware team cannot seem to be able to help
me track down a plausible reason for these failures. I'll keep
investigating, but in the meantime, this is a direct regression which
must be restored to a working state.

The only thing I can do is keep ignoring the errors as before.

Fixes: b99658452355 ("net: dsa: ocelot: felix: utilize shared mscc-miim driver for indirect MDIO access")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/seville_vsc9953.c | 4 ++--
 drivers/net/mdio/mdio-mscc-miim.c        | 9 ++++++---
 include/linux/mdio/mdio-mscc-miim.h      | 2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index 287b64b788db..563ad338da25 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -893,8 +893,8 @@ static int vsc9953_mdio_bus_alloc(struct ocelot *ocelot)
 
 	rc = mscc_miim_setup(dev, &bus, "VSC9953 internal MDIO bus",
 			     ocelot->targets[GCB],
-			     ocelot->map[GCB][GCB_MIIM_MII_STATUS & REG_MASK]);
-
+			     ocelot->map[GCB][GCB_MIIM_MII_STATUS & REG_MASK],
+			     true);
 	if (rc) {
 		dev_err(dev, "failed to setup MDIO bus\n");
 		return rc;
diff --git a/drivers/net/mdio/mdio-mscc-miim.c b/drivers/net/mdio/mdio-mscc-miim.c
index c87e991d1a17..1a1b95ae95fa 100644
--- a/drivers/net/mdio/mdio-mscc-miim.c
+++ b/drivers/net/mdio/mdio-mscc-miim.c
@@ -52,6 +52,7 @@ struct mscc_miim_info {
 struct mscc_miim_dev {
 	struct regmap *regs;
 	int mii_status_offset;
+	bool ignore_read_errors;
 	struct regmap *phy_regs;
 	const struct mscc_miim_info *info;
 	struct clk *clk;
@@ -135,7 +136,7 @@ static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum)
 		goto out;
 	}
 
-	if (val & MSCC_MIIM_DATA_ERROR) {
+	if (!miim->ignore_read_errors && !!(val & MSCC_MIIM_DATA_ERROR)) {
 		ret = -EIO;
 		goto out;
 	}
@@ -212,7 +213,8 @@ static const struct regmap_config mscc_miim_phy_regmap_config = {
 };
 
 int mscc_miim_setup(struct device *dev, struct mii_bus **pbus, const char *name,
-		    struct regmap *mii_regmap, int status_offset)
+		    struct regmap *mii_regmap, int status_offset,
+		    bool ignore_read_errors)
 {
 	struct mscc_miim_dev *miim;
 	struct mii_bus *bus;
@@ -234,6 +236,7 @@ int mscc_miim_setup(struct device *dev, struct mii_bus **pbus, const char *name,
 
 	miim->regs = mii_regmap;
 	miim->mii_status_offset = status_offset;
+	miim->ignore_read_errors = ignore_read_errors;
 
 	*pbus = bus;
 
@@ -285,7 +288,7 @@ static int mscc_miim_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(phy_regmap),
 				     "Unable to create phy register regmap\n");
 
-	ret = mscc_miim_setup(dev, &bus, "mscc_miim", mii_regmap, 0);
+	ret = mscc_miim_setup(dev, &bus, "mscc_miim", mii_regmap, 0, false);
 	if (ret < 0) {
 		dev_err(dev, "Unable to setup the MDIO bus\n");
 		return ret;
diff --git a/include/linux/mdio/mdio-mscc-miim.h b/include/linux/mdio/mdio-mscc-miim.h
index 5b4ed2c3cbb9..1ce699740af6 100644
--- a/include/linux/mdio/mdio-mscc-miim.h
+++ b/include/linux/mdio/mdio-mscc-miim.h
@@ -14,6 +14,6 @@
 
 int mscc_miim_setup(struct device *device, struct mii_bus **bus,
 		    const char *name, struct regmap *mii_regmap,
-		    int status_offset);
+		    int status_offset, bool ignore_read_errors);
 
 #endif
-- 
2.34.1

Re: [PATCH net 1/3] net: dsa: seville: ignore mscc-miim read errors from Lynx PCS

[Florian Fainelli]

On 2/24/23 07:52, Vladimir Oltean wrote:
> During the refactoring in the commit below, vsc9953_mdio_read() was
> replaced with mscc_miim_read(), which has one extra step: it checks for
> the MSCC_MIIM_DATA_ERROR bits before returning the result.
> 
> On T1040RDB, there are 8 QSGMII PCSes belonging to the switch, and they
> are organized in 2 groups. First group responds to MDIO addresses 4-7
> because QSGMIIACR1[MDEV_PORT] is 1, and the second group responds to
> MDIO addresses 8-11 because QSGMIIBCR1[MDEV_PORT] is 2. I have double
> checked that these values are correctly set in the SERDES, as well as
> PCCR1[QSGMA_CFG] and PCCR1[QSGMB_CFG] are both 0b01.
> 
> mscc_miim_read: phyad 8 reg 0x1 MIIM_DATA 0x2d
> mscc_miim_read: phyad 8 reg 0x5 MIIM_DATA 0x5801
> mscc_miim_read: phyad 8 reg 0x1 MIIM_DATA 0x2d
> mscc_miim_read: phyad 8 reg 0x5 MIIM_DATA 0x5801
> mscc_miim_read: phyad 9 reg 0x1 MIIM_DATA 0x2d
> mscc_miim_read: phyad 9 reg 0x5 MIIM_DATA 0x5801
> mscc_miim_read: phyad 9 reg 0x1 MIIM_DATA 0x2d
> mscc_miim_read: phyad 9 reg 0x5 MIIM_DATA 0x5801
> mscc_miim_read: phyad 10 reg 0x1 MIIM_DATA 0x2d
> mscc_miim_read: phyad 10 reg 0x5 MIIM_DATA 0x5801
> mscc_miim_read: phyad 10 reg 0x1 MIIM_DATA 0x2d
> mscc_miim_read: phyad 10 reg 0x5 MIIM_DATA 0x5801
> mscc_miim_read: phyad 11 reg 0x1 MIIM_DATA 0x2d
> mscc_miim_read: phyad 11 reg 0x5 MIIM_DATA 0x5801
> mscc_miim_read: phyad 11 reg 0x1 MIIM_DATA 0x2d
> mscc_miim_read: phyad 11 reg 0x5 MIIM_DATA 0x5801
> mscc_miim_read: phyad 4 reg 0x1 MIIM_DATA 0x3002d, ERROR
> mscc_miim_read: phyad 4 reg 0x5 MIIM_DATA 0x3da01, ERROR
> mscc_miim_read: phyad 5 reg 0x1 MIIM_DATA 0x3002d, ERROR
> mscc_miim_read: phyad 5 reg 0x5 MIIM_DATA 0x35801, ERROR
> mscc_miim_read: phyad 5 reg 0x1 MIIM_DATA 0x3002d, ERROR
> mscc_miim_read: phyad 5 reg 0x5 MIIM_DATA 0x35801, ERROR
> mscc_miim_read: phyad 6 reg 0x1 MIIM_DATA 0x3002d, ERROR
> mscc_miim_read: phyad 6 reg 0x5 MIIM_DATA 0x35801, ERROR
> mscc_miim_read: phyad 6 reg 0x1 MIIM_DATA 0x3002d, ERROR
> mscc_miim_read: phyad 6 reg 0x5 MIIM_DATA 0x35801, ERROR
> mscc_miim_read: phyad 7 reg 0x1 MIIM_DATA 0x3002d, ERROR
> mscc_miim_read: phyad 7 reg 0x5 MIIM_DATA 0x35801, ERROR
> mscc_miim_read: phyad 7 reg 0x1 MIIM_DATA 0x3002d, ERROR
> mscc_miim_read: phyad 7 reg 0x5 MIIM_DATA 0x35801, ERROR
> 
> As can be seen, the data in MIIM_DATA is still valid despite having the
> MSCC_MIIM_DATA_ERROR bits set. The driver as introduced in commit
> 84705fc16552 ("net: dsa: felix: introduce support for Seville VSC9953
> switch") was ignoring these bits, perhaps deliberately (although
> unbeknownst to me).
> 
> This is an old IP and the hardware team cannot seem to be able to help
> me track down a plausible reason for these failures. I'll keep
> investigating, but in the meantime, this is a direct regression which
> must be restored to a working state.
> 
> The only thing I can do is keep ignoring the errors as before.
> 
> Fixes: b99658452355 ("net: dsa: ocelot: felix: utilize shared mscc-miim driver for indirect MDIO access")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net 2/3] net: dsa: felix: fix internal MDIO controller resource length

[Vladimir Oltean]

The blamed commit did not properly convert the resource start/end format
into the DEFINE_RES_MEM_NAMED() start/length format, resulting in a
resource for vsc9959_imdio_res which is much longer than expected:

$ cat /proc/iomem
1f8000000-1f815ffff : pcie@1f0000000
  1f8140000-1f815ffff : 0000:00:00.5
    1f8148030-1f815006f : imdio

vs (correct)

$ cat /proc/iomem
1f8000000-1f815ffff : pcie@1f0000000
  1f8140000-1f815ffff : 0000:00:00.5
    1f8148030-1f814803f : imdio

Luckily it's not big enough to exceed the size of the parent resource
(pci_resource_end(pdev, VSC9959_IMDIO_PCI_BAR)), and it doesn't overlap
with anything else that the Linux driver uses currently, so the larger
than expected size isn't a practical problem that I can see. Although it
is clearly wrong in the /proc/iomem output.

Fixes: 044d447a801f ("net: dsa: felix: use DEFINE_RES_MEM_NAMED for resources")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix_vsc9959.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 354aa3dbfde7..dddb28984bdf 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -554,7 +554,7 @@ static const char * const vsc9959_resource_names[TARGET_MAX] = {
  * SGMII/QSGMII MAC PCS can be found.
  */
 static const struct resource vsc9959_imdio_res =
-	DEFINE_RES_MEM_NAMED(0x8030, 0x8040, "imdio");
+	DEFINE_RES_MEM_NAMED(0x8030, 0x10, "imdio");
 
 static const struct reg_field vsc9959_regfields[REGFIELD_MAX] = {
 	[ANA_ADVLEARN_VLAN_CHK] = REG_FIELD(ANA_ADVLEARN, 6, 6),
-- 
2.34.1

Re: [PATCH net 2/3] net: dsa: felix: fix internal MDIO controller resource length

[Florian Fainelli]

On 2/24/23 07:52, Vladimir Oltean wrote:
> The blamed commit did not properly convert the resource start/end format
> into the DEFINE_RES_MEM_NAMED() start/length format, resulting in a
> resource for vsc9959_imdio_res which is much longer than expected:
> 
> $ cat /proc/iomem
> 1f8000000-1f815ffff : pcie@1f0000000
>    1f8140000-1f815ffff : 0000:00:00.5
>      1f8148030-1f815006f : imdio
> 
> vs (correct)
> 
> $ cat /proc/iomem
> 1f8000000-1f815ffff : pcie@1f0000000
>    1f8140000-1f815ffff : 0000:00:00.5
>      1f8148030-1f814803f : imdio
> 
> Luckily it's not big enough to exceed the size of the parent resource
> (pci_resource_end(pdev, VSC9959_IMDIO_PCI_BAR)), and it doesn't overlap
> with anything else that the Linux driver uses currently, so the larger
> than expected size isn't a practical problem that I can see. Although it
> is clearly wrong in the /proc/iomem output.
> 
> Fixes: 044d447a801f ("net: dsa: felix: use DEFINE_RES_MEM_NAMED for resources")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net 3/3] net: mscc: ocelot: fix duplicate driver name error

[Vladimir Oltean]

When compiling a kernel which has both CONFIG_NET_DSA_MSCC_OCELOT_EXT
and CONFIG_MSCC_OCELOT_SWITCH enabled, the following error message will
be printed:

[    5.266588] Error: Driver 'ocelot-switch' is already registered, aborting...

Rename the ocelot_ext.c driver to "ocelot-ext-switch" to avoid the name
duplication, and update the mfd_cell entry for its resources.

Fixes: 3d7316ac81ac ("net: dsa: ocelot: add external ocelot switch control")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/mfd/ocelot-core.c           | 2 +-
 drivers/net/dsa/ocelot/ocelot_ext.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c
index b0ff05c1759f..e1772ff00cad 100644
--- a/drivers/mfd/ocelot-core.c
+++ b/drivers/mfd/ocelot-core.c
@@ -177,7 +177,7 @@ static const struct mfd_cell vsc7512_devs[] = {
 		.num_resources = ARRAY_SIZE(vsc7512_miim1_resources),
 		.resources = vsc7512_miim1_resources,
 	}, {
-		.name = "ocelot-switch",
+		.name = "ocelot-ext-switch",
 		.of_compatible = "mscc,vsc7512-switch",
 		.num_resources = ARRAY_SIZE(vsc7512_switch_resources),
 		.resources = vsc7512_switch_resources,
diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
index 14efa6387bd7..52b41db63a28 100644
--- a/drivers/net/dsa/ocelot/ocelot_ext.c
+++ b/drivers/net/dsa/ocelot/ocelot_ext.c
@@ -149,7 +149,7 @@ MODULE_DEVICE_TABLE(of, ocelot_ext_switch_of_match);
 
 static struct platform_driver ocelot_ext_switch_driver = {
 	.driver = {
-		.name = "ocelot-switch",
+		.name = "ocelot-ext-switch",
 		.of_match_table = of_match_ptr(ocelot_ext_switch_of_match),
 	},
 	.probe = ocelot_ext_probe,
-- 
2.34.1

Re: [PATCH net 3/3] net: mscc: ocelot: fix duplicate driver name error

[Russell King (Oracle)]

On Fri, Feb 24, 2023 at 05:52:35PM +0200, Vladimir Oltean wrote:
> When compiling a kernel which has both CONFIG_NET_DSA_MSCC_OCELOT_EXT
> and CONFIG_MSCC_OCELOT_SWITCH enabled, the following error message will
> be printed:
> 
> [    5.266588] Error: Driver 'ocelot-switch' is already registered, aborting...
> 
> Rename the ocelot_ext.c driver to "ocelot-ext-switch" to avoid the name
> duplication, and update the mfd_cell entry for its resources.
> 
> Fixes: 3d7316ac81ac ("net: dsa: ocelot: add external ocelot switch control")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

I'll also send another patch to delete linux/phylink.h from
ocelot_ext.c - seems that wasn't removed when the phylink instance
was removed during review.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net 3/3] net: mscc: ocelot: fix duplicate driver name error

[Vladimir Oltean]

On Fri, Feb 24, 2023 at 03:58:15PM +0000, Russell King (Oracle) wrote:
> I'll also send another patch to delete linux/phylink.h from
> ocelot_ext.c - seems that wasn't removed when the phylink instance
> was removed during review.

Good point. I suppose that would be on net-next, after the 6th of March?
I just hope we'll remember by then.

Re: [PATCH net 3/3] net: mscc: ocelot: fix duplicate driver name error

[Russell King (Oracle)]

On Fri, Feb 24, 2023 at 06:09:20PM +0200, Vladimir Oltean wrote:
> On Fri, Feb 24, 2023 at 03:58:15PM +0000, Russell King (Oracle) wrote:
> > I'll also send another patch to delete linux/phylink.h from
> > ocelot_ext.c - seems that wasn't removed when the phylink instance
> > was removed during review.
> 
> Good point. I suppose that would be on net-next, after the 6th of March?
> I just hope we'll remember by then.

Yep - however, I'm facing challenges to build-testing it at the moment
as net-next is broken:

kernel/bpf/core.c: In function '___bpf_prog_run':
kernel/bpf/core.c:1914:3: error: implicit declaration of function 'barrier_nospec' [-Werror=implicit-function-declaration]
 1914 |   barrier_nospec();
      |   ^~~~~~~~~~~~~~
cc1: some warnings being treated as errors

... so I'm going to send the patch as untested.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net 3/3] net: mscc: ocelot: fix duplicate driver name error

[Vladimir Oltean]

On Fri, Feb 24, 2023 at 04:14:25PM +0000, Russell King (Oracle) wrote:
> On Fri, Feb 24, 2023 at 06:09:20PM +0200, Vladimir Oltean wrote:
> > On Fri, Feb 24, 2023 at 03:58:15PM +0000, Russell King (Oracle) wrote:
> > > I'll also send another patch to delete linux/phylink.h from
> > > ocelot_ext.c - seems that wasn't removed when the phylink instance
> > > was removed during review.
> > 
> > Good point. I suppose that would be on net-next, after the 6th of March?
> > I just hope we'll remember by then.
> 
> Yep - however, I'm facing challenges to build-testing it at the moment
> as net-next is broken:
> 
> kernel/bpf/core.c: In function '___bpf_prog_run':
> kernel/bpf/core.c:1914:3: error: implicit declaration of function 'barrier_nospec' [-Werror=implicit-function-declaration]
>  1914 |   barrier_nospec();
>       |   ^~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
> 
> ... so I'm going to send the patch as untested.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

https://github.com/torvalds/linux/commit/f3dd0c53370e70c0f9b7e931bbec12916f3bb8cc

Can't deny it would be great to have this on net.git ASAP :)

Re: [PATCH net 3/3] net: mscc: ocelot: fix duplicate driver name error

[Russell King (Oracle)]

On Fri, Feb 24, 2023 at 06:16:29PM +0200, Vladimir Oltean wrote:
> On Fri, Feb 24, 2023 at 04:14:25PM +0000, Russell King (Oracle) wrote:
> > On Fri, Feb 24, 2023 at 06:09:20PM +0200, Vladimir Oltean wrote:
> > > On Fri, Feb 24, 2023 at 03:58:15PM +0000, Russell King (Oracle) wrote:
> > > > I'll also send another patch to delete linux/phylink.h from
> > > > ocelot_ext.c - seems that wasn't removed when the phylink instance
> > > > was removed during review.
> > > 
> > > Good point. I suppose that would be on net-next, after the 6th of March?
> > > I just hope we'll remember by then.
> > 
> > Yep - however, I'm facing challenges to build-testing it at the moment
> > as net-next is broken:
> > 
> > kernel/bpf/core.c: In function '___bpf_prog_run':
> > kernel/bpf/core.c:1914:3: error: implicit declaration of function 'barrier_nospec' [-Werror=implicit-function-declaration]
> >  1914 |   barrier_nospec();
> >       |   ^~~~~~~~~~~~~~
> > cc1: some warnings being treated as errors
> > 
> > ... so I'm going to send the patch as untested.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> 
> https://github.com/torvalds/linux/commit/f3dd0c53370e70c0f9b7e931bbec12916f3bb8cc
> 
> Can't deny it would be great to have this on net.git ASAP :)

Agreed, and thanks for the patch, my build-test on net.git succeeded
with that.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net 3/3] net: mscc: ocelot: fix duplicate driver name error

[Florian Fainelli]

On 2/24/23 07:52, Vladimir Oltean wrote:
> When compiling a kernel which has both CONFIG_NET_DSA_MSCC_OCELOT_EXT
> and CONFIG_MSCC_OCELOT_SWITCH enabled, the following error message will
> be printed:
> 
> [    5.266588] Error: Driver 'ocelot-switch' is already registered, aborting...
> 
> Rename the ocelot_ext.c driver to "ocelot-ext-switch" to avoid the name
> duplication, and update the mfd_cell entry for its resources.
> 
> Fixes: 3d7316ac81ac ("net: dsa: ocelot: add external ocelot switch control")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net 3/3] net: mscc: ocelot: fix duplicate driver name error

[Lee Jones]

On Fri, 24 Feb 2023, Vladimir Oltean wrote:

> When compiling a kernel which has both CONFIG_NET_DSA_MSCC_OCELOT_EXT
> and CONFIG_MSCC_OCELOT_SWITCH enabled, the following error message will
> be printed:
> 
> [    5.266588] Error: Driver 'ocelot-switch' is already registered, aborting...
> 
> Rename the ocelot_ext.c driver to "ocelot-ext-switch" to avoid the name
> duplication, and update the mfd_cell entry for its resources.
> 
> Fixes: 3d7316ac81ac ("net: dsa: ocelot: add external ocelot switch control")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/mfd/ocelot-core.c           | 2 +-

Acked-by: Lee Jones <lee@kernel.org>

>  drivers/net/dsa/ocelot/ocelot_ext.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c
> index b0ff05c1759f..e1772ff00cad 100644
> --- a/drivers/mfd/ocelot-core.c
> +++ b/drivers/mfd/ocelot-core.c
> @@ -177,7 +177,7 @@ static const struct mfd_cell vsc7512_devs[] = {
>  		.num_resources = ARRAY_SIZE(vsc7512_miim1_resources),
>  		.resources = vsc7512_miim1_resources,
>  	}, {
> -		.name = "ocelot-switch",
> +		.name = "ocelot-ext-switch",
>  		.of_compatible = "mscc,vsc7512-switch",
>  		.num_resources = ARRAY_SIZE(vsc7512_switch_resources),
>  		.resources = vsc7512_switch_resources,
> diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
> index 14efa6387bd7..52b41db63a28 100644
> --- a/drivers/net/dsa/ocelot/ocelot_ext.c
> +++ b/drivers/net/dsa/ocelot/ocelot_ext.c
> @@ -149,7 +149,7 @@ MODULE_DEVICE_TABLE(of, ocelot_ext_switch_of_match);
>  
>  static struct platform_driver ocelot_ext_switch_driver = {
>  	.driver = {
> -		.name = "ocelot-switch",
> +		.name = "ocelot-ext-switch",
>  		.of_match_table = of_match_ptr(ocelot_ext_switch_of_match),
>  	},
>  	.probe = ocelot_ext_probe,
> -- 
> 2.34.1
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH net 0/3] Regressions in Ocelot switch drivers

[Jakub Kicinski]

On Fri, 24 Feb 2023 17:52:32 +0200 Vladimir Oltean wrote:
> These are 3 patches which resolve a regression in the Seville driver,
> one in the Felix driver and a generic one which affects any kernel
> compiled with 2 Kconfig options enabled. All of them have in common my
> lack of attention during review/testing. The patches touch the DSA, MFD
> and MDIO drivers for Ocelot. I think it would be preferable if all
> patches went through netdev (with Lee's Ack).

Applied, thanks!