[PATCH net-next 0/8] Probing cleanup for the Felix DSA driver

[PATCH net-next 0/8] Probing cleanup for the Felix DSA driver

[Vladimir Oltean]

This is a follow-up to Russell King's request for code consolidation
among felix_vsc9959, seville_vsc9953 and ocelot_ext, stated here:
https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/

Details are in individual patches. Testing was done on NXP LS1028A
(felix_vsc9959).

Vladimir Oltean (8):
  net: dsa: ocelot: use devres in ocelot_ext_probe()
  net: dsa: ocelot: use devres in seville_probe()
  net: dsa: ocelot: delete open coded status = "disabled" parsing
  net: dsa: ocelot: consistently use devres in felix_pci_probe()
  net: dsa: ocelot: move devm_request_threaded_irq() to felix_setup()
  net: dsa: ocelot: use ds->num_tx_queues = OCELOT_NUM_TC for all models
  net: dsa: ocelot: common probing code
  net: dsa: ocelot: unexport felix_phylink_mac_ops and felix_switch_ops

 drivers/net/dsa/ocelot/felix.c           |  62 ++++++++++++-
 drivers/net/dsa/ocelot/felix.h           |  10 +-
 drivers/net/dsa/ocelot/felix_vsc9959.c   | 113 +++++++----------------
 drivers/net/dsa/ocelot/ocelot_ext.c      |  55 +----------
 drivers/net/dsa/ocelot/seville_vsc9953.c |  61 ++----------
 5 files changed, 106 insertions(+), 195 deletions(-)

-- 
2.34.1

[PATCH net-next 1/8] net: dsa: ocelot: use devres in ocelot_ext_probe()

[Vladimir Oltean]

Russell King suggested that felix_vsc9959, seville_vsc9953 and
ocelot_ext have a large portion of duplicated init and teardown code,
which could be made common [1]. The teardown code could even be
simplified away if we made use of devres, something which is used here
and there in the felix driver, just not very consistently.

[1] https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/

Prepare the ground in the ocelot_ext driver, by allocating the data
structures using devres and deleting the kfree() calls. This also
deletes the "Failed to allocate ..." message, since memory allocation
errors are extremely loud anyway, and it's hard to miss them.

Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/ocelot_ext.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
index a8927dc7aca4..c893f3ee238b 100644
--- a/drivers/net/dsa/ocelot/ocelot_ext.c
+++ b/drivers/net/dsa/ocelot/ocelot_ext.c
@@ -71,7 +71,7 @@ static int ocelot_ext_probe(struct platform_device *pdev)
 	struct felix *felix;
 	int err;
 
-	felix = kzalloc(sizeof(*felix), GFP_KERNEL);
+	felix = devm_kzalloc(dev, sizeof(*felix), GFP_KERNEL);
 	if (!felix)
 		return -ENOMEM;
 
@@ -84,12 +84,9 @@ static int ocelot_ext_probe(struct platform_device *pdev)
 
 	felix->info = &vsc7512_info;
 
-	ds = kzalloc(sizeof(*ds), GFP_KERNEL);
-	if (!ds) {
-		err = -ENOMEM;
-		dev_err_probe(dev, err, "Failed to allocate DSA switch\n");
-		goto err_free_felix;
-	}
+	ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
+	if (!ds)
+		return -ENOMEM;
 
 	ds->dev = dev;
 	ds->num_ports = felix->info->num_ports;
@@ -102,17 +99,9 @@ static int ocelot_ext_probe(struct platform_device *pdev)
 	felix->tag_proto = DSA_TAG_PROTO_OCELOT;
 
 	err = dsa_register_switch(ds);
-	if (err) {
+	if (err)
 		dev_err_probe(dev, err, "Failed to register DSA switch\n");
-		goto err_free_ds;
-	}
-
-	return 0;
 
-err_free_ds:
-	kfree(ds);
-err_free_felix:
-	kfree(felix);
 	return err;
 }
 
@@ -124,9 +113,6 @@ static void ocelot_ext_remove(struct platform_device *pdev)
 		return;
 
 	dsa_unregister_switch(felix->ds);
-
-	kfree(felix->ds);
-	kfree(felix);
 }
 
 static void ocelot_ext_shutdown(struct platform_device *pdev)
-- 
2.34.1

[PATCH net-next 2/8] net: dsa: ocelot: use devres in seville_probe()

[Vladimir Oltean]

Russell King suggested that felix_vsc9959, seville_vsc9953 and
ocelot_ext have a large portion of duplicated init and teardown code,
which could be made common [1]. The teardown code could even be
simplified away if we made use of devres, something which is used here
and there in the felix driver, just not very consistently.

[1] https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/

Prepare the ground in the seville_vsc9953 driver, by allocating the data
structures using devres and deleting the kfree() calls. This also
deletes the "Failed to allocate ..." message, since memory allocation
errors are extremely loud anyway, and it's hard to miss them.

Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/seville_vsc9953.c | 44 +++++++-----------------
 1 file changed, 13 insertions(+), 31 deletions(-)

diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index 5ac8897e232b..e63247d3dfdb 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -971,42 +971,36 @@ static const struct felix_info seville_info_vsc9953 = {
 
 static int seville_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct dsa_switch *ds;
 	struct ocelot *ocelot;
 	struct resource *res;
 	struct felix *felix;
 	int err;
 
-	felix = kzalloc(sizeof(struct felix), GFP_KERNEL);
-	if (!felix) {
-		err = -ENOMEM;
-		dev_err(&pdev->dev, "Failed to allocate driver memory\n");
-		goto err_alloc_felix;
-	}
+	felix = devm_kzalloc(dev, sizeof(struct felix), GFP_KERNEL);
+	if (!felix)
+		return -ENOMEM;
 
 	platform_set_drvdata(pdev, felix);
 
 	ocelot = &felix->ocelot;
-	ocelot->dev = &pdev->dev;
+	ocelot->dev = dev;
 	ocelot->num_flooding_pgids = 1;
 	felix->info = &seville_info_vsc9953;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
-		err = -EINVAL;
-		dev_err(&pdev->dev, "Invalid resource\n");
-		goto err_alloc_felix;
+		dev_err(dev, "Invalid resource\n");
+		return -EINVAL;
 	}
 	felix->switch_base = res->start;
 
-	ds = kzalloc(sizeof(struct dsa_switch), GFP_KERNEL);
-	if (!ds) {
-		err = -ENOMEM;
-		dev_err(&pdev->dev, "Failed to allocate DSA switch\n");
-		goto err_alloc_ds;
-	}
+	ds = devm_kzalloc(dev, sizeof(struct dsa_switch), GFP_KERNEL);
+	if (!ds)
+		return -ENOMEM;
 
-	ds->dev = &pdev->dev;
+	ds->dev = dev;
 	ds->num_ports = felix->info->num_ports;
 	ds->ops = &felix_switch_ops;
 	ds->phylink_mac_ops = &felix_phylink_mac_ops;
@@ -1015,18 +1009,9 @@ static int seville_probe(struct platform_device *pdev)
 	felix->tag_proto = DSA_TAG_PROTO_SEVILLE;
 
 	err = dsa_register_switch(ds);
-	if (err) {
-		dev_err(&pdev->dev, "Failed to register DSA switch: %d\n", err);
-		goto err_register_ds;
-	}
+	if (err)
+		dev_err(dev, "Failed to register DSA switch: %d\n", err);
 
-	return 0;
-
-err_register_ds:
-	kfree(ds);
-err_alloc_ds:
-err_alloc_felix:
-	kfree(felix);
 	return err;
 }
 
@@ -1038,9 +1023,6 @@ static void seville_remove(struct platform_device *pdev)
 		return;
 
 	dsa_unregister_switch(felix->ds);
-
-	kfree(felix->ds);
-	kfree(felix);
 }
 
 static void seville_shutdown(struct platform_device *pdev)
-- 
2.34.1

[PATCH net-next 3/8] net: dsa: ocelot: delete open coded status = "disabled" parsing

[Vladimir Oltean]

Since commit 6fffbc7ae137 ("PCI: Honor firmware's device disabled
status"), PCI device drivers with OF bindings no longer need this check.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix_vsc9959.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index d4799a908abc..eabb55da0982 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -2668,11 +2668,6 @@ static int felix_pci_probe(struct pci_dev *pdev,
 	struct felix *felix;
 	int err;
 
-	if (pdev->dev.of_node && !of_device_is_available(pdev->dev.of_node)) {
-		dev_info(&pdev->dev, "device is disabled, skipping\n");
-		return -ENODEV;
-	}
-
 	err = pci_enable_device(pdev);
 	if (err) {
 		dev_err(&pdev->dev, "device enable failed\n");
-- 
2.34.1

[PATCH net-next 4/8] net: dsa: ocelot: consistently use devres in felix_pci_probe()

[Vladimir Oltean]

Russell King suggested that felix_vsc9959, seville_vsc9953 and
ocelot_ext have a large portion of duplicated init and teardown code,
which could be made common [1]. The teardown code could even be
simplified away if we made use of devres, something which is used here
and there in the felix driver, just not very consistently.

[1] https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/

Prepare the ground in the felix_vsc9959 driver, by allocating the data
structures using devres and deleting the kfree() calls. This also
deletes the "Failed to allocate ..." message, since memory allocation
errors are extremely loud anyway, and it's hard to miss them.

Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix_vsc9959.c | 40 ++++++++++----------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index eabb55da0982..34155a0ffd7e 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -2663,6 +2663,7 @@ static irqreturn_t felix_irq_handler(int irq, void *data)
 static int felix_pci_probe(struct pci_dev *pdev,
 			   const struct pci_device_id *id)
 {
+	struct device *dev = &pdev->dev;
 	struct dsa_switch *ds;
 	struct ocelot *ocelot;
 	struct felix *felix;
@@ -2670,45 +2671,43 @@ static int felix_pci_probe(struct pci_dev *pdev,
 
 	err = pci_enable_device(pdev);
 	if (err) {
-		dev_err(&pdev->dev, "device enable failed\n");
-		goto err_pci_enable;
+		dev_err(dev, "device enable failed: %pe\n", ERR_PTR(err));
+		return err;
 	}
 
-	felix = kzalloc(sizeof(struct felix), GFP_KERNEL);
+	felix = devm_kzalloc(dev, sizeof(struct felix), GFP_KERNEL);
 	if (!felix) {
 		err = -ENOMEM;
-		dev_err(&pdev->dev, "Failed to allocate driver memory\n");
-		goto err_alloc_felix;
+		goto out_disable;
 	}
 
 	pci_set_drvdata(pdev, felix);
 	ocelot = &felix->ocelot;
-	ocelot->dev = &pdev->dev;
+	ocelot->dev = dev;
 	ocelot->num_flooding_pgids = OCELOT_NUM_TC;
 	felix->info = &felix_info_vsc9959;
 	felix->switch_base = pci_resource_start(pdev, VSC9959_SWITCH_PCI_BAR);
 
 	pci_set_master(pdev);
 
-	err = devm_request_threaded_irq(&pdev->dev, pdev->irq, NULL,
+	err = devm_request_threaded_irq(dev, pdev->irq, NULL,
 					&felix_irq_handler, IRQF_ONESHOT,
 					"felix-intb", ocelot);
 	if (err) {
-		dev_err(&pdev->dev, "Failed to request irq\n");
-		goto err_alloc_irq;
+		dev_err(dev, "Failed to request irq: %pe\n", ERR_PTR(err));
+		goto out_disable;
 	}
 
 	ocelot->ptp = 1;
 	ocelot->mm_supported = true;
 
-	ds = kzalloc(sizeof(struct dsa_switch), GFP_KERNEL);
+	ds = devm_kzalloc(dev, sizeof(struct dsa_switch), GFP_KERNEL);
 	if (!ds) {
 		err = -ENOMEM;
-		dev_err(&pdev->dev, "Failed to allocate DSA switch\n");
-		goto err_alloc_ds;
+		goto out_disable;
 	}
 
-	ds->dev = &pdev->dev;
+	ds->dev = dev;
 	ds->num_ports = felix->info->num_ports;
 	ds->num_tx_queues = felix->info->num_tx_queues;
 	ds->ops = &felix_switch_ops;
@@ -2719,20 +2718,14 @@ static int felix_pci_probe(struct pci_dev *pdev,
 
 	err = dsa_register_switch(ds);
 	if (err) {
-		dev_err_probe(&pdev->dev, err, "Failed to register DSA switch\n");
-		goto err_register_ds;
+		dev_err_probe(dev, err, "Failed to register DSA switch\n");
+		goto out_disable;
 	}
 
 	return 0;
 
-err_register_ds:
-	kfree(ds);
-err_alloc_ds:
-err_alloc_irq:
-	kfree(felix);
-err_alloc_felix:
+out_disable:
 	pci_disable_device(pdev);
-err_pci_enable:
 	return err;
 }
 
@@ -2745,9 +2738,6 @@ static void felix_pci_remove(struct pci_dev *pdev)
 
 	dsa_unregister_switch(felix->ds);
 
-	kfree(felix->ds);
-	kfree(felix);
-
 	pci_disable_device(pdev);
 }
 
-- 
2.34.1

[PATCH net-next 5/8] net: dsa: ocelot: move devm_request_threaded_irq() to felix_setup()

[Vladimir Oltean]

The current placement of devm_request_threaded_irq() is inconvenient.
It is between the allocation of the "felix" structure and
dsa_register_switch(), both of which we'd like to refactor into a
function that's common for all switches. But the IRQ is specific to
felix_vsc9959.

A closer inspection of the felix_irq_handler() code suggests that
it does things that depend on the data structures having been fully
initialized. For example, ocelot_get_txtstamp() takes
&port->tx_skbs.lock, which has only been initialized in
ocelot_init_port() which has not run yet.

It is not one of those IRQF_SHARED IRQs, so CONFIG_DEBUG_SHIRQ_FIXME
shouldn't apply here, and thus, it doesn't really matter, because in
practice, the IRQ will not be triggered so early. Nonetheless, it is a
good practice for the driver to be prepared for it to fire as soon as it
is requested.

Create a new felix->info method for running custom code for vsc9959 from
within felix_setup(), and move the request_irq() call there. The
ocelot_ext should have an IRQ as well, so this should be a step in the
right direction for that model (VSC7512) as well.

Some minor changes are made while moving the code. Casts from void *
aren't necessary, so drop them, and rename felix_irq_handler() to the
more specific vsc9959_irq_handler().

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c         |  9 ++++++
 drivers/net/dsa/ocelot/felix.h         |  1 +
 drivers/net/dsa/ocelot/felix_vsc9959.c | 44 ++++++++++++++------------
 3 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 3aa66bf9eafc..09c0800b18ab 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1597,6 +1597,15 @@ static int felix_setup(struct dsa_switch *ds)
 		felix_port_qos_map_init(ocelot, dp->index);
 	}
 
+	if (felix->info->request_irq) {
+		err = felix->info->request_irq(ocelot);
+		if (err) {
+			dev_err(ocelot->dev, "Failed to request IRQ: %pe\n",
+				ERR_PTR(err));
+			goto out_deinit_ports;
+		}
+	}
+
 	err = ocelot_devlink_sb_register(ocelot);
 	if (err)
 		goto out_deinit_ports;
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index 4d3489aaa659..e67a25f6f816 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -64,6 +64,7 @@ struct felix_info {
 				      const struct phylink_link_state *state);
 	int	(*configure_serdes)(struct ocelot *ocelot, int port,
 				    struct device_node *portnp);
+	int	(*request_irq)(struct ocelot *ocelot);
 };
 
 /* Methods for initializing the hardware resources specific to a tagging
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 34155a0ffd7e..20563abd617f 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -2605,6 +2605,28 @@ static void vsc9959_cut_through_fwd(struct ocelot *ocelot)
 	}
 }
 
+/* The INTB interrupt is shared between for PTP TX timestamp availability
+ * notification and MAC Merge status change on each port.
+ */
+static irqreturn_t vsc9959_irq_handler(int irq, void *data)
+{
+	struct ocelot *ocelot = data;
+
+	ocelot_get_txtstamp(ocelot);
+	ocelot_mm_irq(ocelot);
+
+	return IRQ_HANDLED;
+}
+
+static int vsc9959_request_irq(struct ocelot *ocelot)
+{
+	struct pci_dev *pdev = to_pci_dev(ocelot->dev);
+
+	return devm_request_threaded_irq(ocelot->dev, pdev->irq, NULL,
+					 &vsc9959_irq_handler, IRQF_ONESHOT,
+					 "felix-intb", ocelot);
+}
+
 static const struct ocelot_ops vsc9959_ops = {
 	.reset			= vsc9959_reset,
 	.wm_enc			= vsc9959_wm_enc,
@@ -2645,21 +2667,9 @@ static const struct felix_info felix_info_vsc9959 = {
 	.port_modes		= vsc9959_port_modes,
 	.port_setup_tc		= vsc9959_port_setup_tc,
 	.port_sched_speed_set	= vsc9959_sched_speed_set,
+	.request_irq		= vsc9959_request_irq,
 };
 
-/* The INTB interrupt is shared between for PTP TX timestamp availability
- * notification and MAC Merge status change on each port.
- */
-static irqreturn_t felix_irq_handler(int irq, void *data)
-{
-	struct ocelot *ocelot = (struct ocelot *)data;
-
-	ocelot_get_txtstamp(ocelot);
-	ocelot_mm_irq(ocelot);
-
-	return IRQ_HANDLED;
-}
-
 static int felix_pci_probe(struct pci_dev *pdev,
 			   const struct pci_device_id *id)
 {
@@ -2690,14 +2700,6 @@ static int felix_pci_probe(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	err = devm_request_threaded_irq(dev, pdev->irq, NULL,
-					&felix_irq_handler, IRQF_ONESHOT,
-					"felix-intb", ocelot);
-	if (err) {
-		dev_err(dev, "Failed to request irq: %pe\n", ERR_PTR(err));
-		goto out_disable;
-	}
-
 	ocelot->ptp = 1;
 	ocelot->mm_supported = true;
 
-- 
2.34.1

[PATCH net-next 6/8] net: dsa: ocelot: use ds->num_tx_queues = OCELOT_NUM_TC for all models

[Vladimir Oltean]

Russell King points out that seville_vsc9953 populates
felix->info->num_tx_queues = 8, but this doesn't make it all the way
into ds->num_tx_queues (which is how the user interface netdev queues
get allocated) [1].

[1]: https://lore.kernel.org/all/20240415160150.yejcazpjqvn7vhxu@skbuf/

When num_tx_queues=0 for seville, this is implicitly converted to 1 by
dsa_user_create(), and this is good enough for basic operation for a
switch port. The tc qdisc offload layer works with netdev TX queues,
so for QoS offload we need to pretend we have multiple TX queues. The
VSC9953, like ocelot_ext, doesn't export QoS offload, so it doesn't
really matter. But we can definitely set num_tx_queues=8 for all
switches.

The felix->info->num_tx_queues construct itself seems unnecessary.
It was introduced by commit de143c0e274b ("net: dsa: felix: Configure
Time-Aware Scheduler via taprio offload") at a time when vsc9959
(LS1028A) was the only switch supported by the driver.

8 traffic classes, and 1 queue per traffic class, is a common
architectural feature of all switches in the family. So they could
all just set OCELOT_NUM_TC and be fine.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.h           | 1 -
 drivers/net/dsa/ocelot/felix_vsc9959.c   | 4 ++--
 drivers/net/dsa/ocelot/ocelot_ext.c      | 3 +--
 drivers/net/dsa/ocelot/seville_vsc9953.c | 3 ++-
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index e67a25f6f816..e0bfea10ff52 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -32,7 +32,6 @@ struct felix_info {
 	const u32			*port_modes;
 	int				num_mact_rows;
 	int				num_ports;
-	int				num_tx_queues;
 	struct vcap_props		*vcap;
 	u16				vcap_pol_base;
 	u16				vcap_pol_max;
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 20563abd617f..ec8b124e8f61 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -2658,7 +2658,6 @@ static const struct felix_info felix_info_vsc9959 = {
 	.vcap_pol_max2		= 0,
 	.num_mact_rows		= 2048,
 	.num_ports		= VSC9959_NUM_PORTS,
-	.num_tx_queues		= OCELOT_NUM_TC,
 	.quirks			= FELIX_MAC_QUIRKS,
 	.quirk_no_xtr_irq	= true,
 	.ptp_caps		= &vsc9959_ptp_caps,
@@ -2711,7 +2710,8 @@ static int felix_pci_probe(struct pci_dev *pdev,
 
 	ds->dev = dev;
 	ds->num_ports = felix->info->num_ports;
-	ds->num_tx_queues = felix->info->num_tx_queues;
+	ds->num_tx_queues = OCELOT_NUM_TC;
+
 	ds->ops = &felix_switch_ops;
 	ds->phylink_mac_ops = &felix_phylink_mac_ops;
 	ds->priv = ocelot;
diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
index c893f3ee238b..9cd24f77dc49 100644
--- a/drivers/net/dsa/ocelot/ocelot_ext.c
+++ b/drivers/net/dsa/ocelot/ocelot_ext.c
@@ -57,7 +57,6 @@ static const struct felix_info vsc7512_info = {
 	.vcap				= vsc7514_vcap_props,
 	.num_mact_rows			= 1024,
 	.num_ports			= VSC7514_NUM_PORTS,
-	.num_tx_queues			= OCELOT_NUM_TC,
 	.port_modes			= vsc7512_port_modes,
 	.phylink_mac_config		= ocelot_phylink_mac_config,
 	.configure_serdes		= ocelot_port_configure_serdes,
@@ -90,7 +89,7 @@ static int ocelot_ext_probe(struct platform_device *pdev)
 
 	ds->dev = dev;
 	ds->num_ports = felix->info->num_ports;
-	ds->num_tx_queues = felix->info->num_tx_queues;
+	ds->num_tx_queues = OCELOT_NUM_TC;
 
 	ds->ops = &felix_switch_ops;
 	ds->phylink_mac_ops = &felix_phylink_mac_ops;
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index e63247d3dfdb..83bd0e1ee692 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -963,7 +963,6 @@ static const struct felix_info seville_info_vsc9953 = {
 	.quirks			= FELIX_MAC_QUIRKS,
 	.num_mact_rows		= 2048,
 	.num_ports		= VSC9953_NUM_PORTS,
-	.num_tx_queues		= OCELOT_NUM_TC,
 	.mdio_bus_alloc		= vsc9953_mdio_bus_alloc,
 	.mdio_bus_free		= vsc9953_mdio_bus_free,
 	.port_modes		= vsc9953_port_modes,
@@ -1002,6 +1001,8 @@ static int seville_probe(struct platform_device *pdev)
 
 	ds->dev = dev;
 	ds->num_ports = felix->info->num_ports;
+	ds->num_tx_queues = OCELOT_NUM_TC;
+
 	ds->ops = &felix_switch_ops;
 	ds->phylink_mac_ops = &felix_phylink_mac_ops;
 	ds->priv = ocelot;
-- 
2.34.1

[PATCH net-next 7/8] net: dsa: ocelot: common probing code

[Vladimir Oltean]

Russell King suggested that felix_vsc9959, seville_vsc9953 and
ocelot_ext have a large portion of duplicated init code, which could be
made common [1].

[1]: https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/

Here, we take the following common steps:
- "felix" and "ds" structure allocation
- "felix", "ocelot" and "ds" basic structure initialization
- dsa_register_switch() call

and we make a common function out of them.

For every driver except felix_vsc9959, this is also the entire probing
procedure. For felix_vsc9959, we also need to do some PCI-specific
stuff, which can easily be reordered to be done before, and unwound on
failure.

We also have to convert the bus-specific platform_set_drvdata() and
pci_set_drvdata() calls into dev_set_drvdata(). But this should have no
impact on the behavior.

Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c           | 47 ++++++++++++++++++++++++
 drivers/net/dsa/ocelot/felix.h           |  5 +++
 drivers/net/dsa/ocelot/felix_vsc9959.c   | 44 +++-------------------
 drivers/net/dsa/ocelot/ocelot_ext.c      | 40 +-------------------
 drivers/net/dsa/ocelot/seville_vsc9953.c | 38 ++-----------------
 5 files changed, 63 insertions(+), 111 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 09c0800b18ab..accf737f7b69 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -2195,6 +2195,53 @@ const struct dsa_switch_ops felix_switch_ops = {
 };
 EXPORT_SYMBOL_GPL(felix_switch_ops);
 
+int felix_register_switch(struct device *dev, resource_size_t switch_base,
+			  int num_flooding_pgids, bool ptp,
+			  bool mm_supported,
+			  enum dsa_tag_protocol init_tag_proto,
+			  const struct felix_info *info)
+{
+	struct dsa_switch *ds;
+	struct ocelot *ocelot;
+	struct felix *felix;
+	int err;
+
+	felix = devm_kzalloc(dev, sizeof(*felix), GFP_KERNEL);
+	if (!felix)
+		return -ENOMEM;
+
+	ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
+	if (!ds)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, felix);
+
+	ocelot = &felix->ocelot;
+	ocelot->dev = dev;
+	ocelot->num_flooding_pgids = num_flooding_pgids;
+	ocelot->ptp = ptp;
+	ocelot->mm_supported = mm_supported;
+
+	felix->info = info;
+	felix->switch_base = switch_base;
+	felix->ds = ds;
+	felix->tag_proto = init_tag_proto;
+
+	ds->dev = dev;
+	ds->num_ports = info->num_ports;
+	ds->num_tx_queues = OCELOT_NUM_TC;
+	ds->ops = &felix_switch_ops;
+	ds->phylink_mac_ops = &felix_phylink_mac_ops;
+	ds->priv = ocelot;
+
+	err = dsa_register_switch(ds);
+	if (err)
+		dev_err_probe(dev, err, "Failed to register DSA switch\n");
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(felix_register_switch);
+
 struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port)
 {
 	struct felix *felix = ocelot_to_felix(ocelot);
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index e0bfea10ff52..85b4f8616003 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -100,6 +100,11 @@ struct felix {
 	unsigned long			host_flood_mc_mask;
 };
 
+int felix_register_switch(struct device *dev, resource_size_t switch_base,
+			  int num_flooding_pgids, bool ptp,
+			  bool mm_supported,
+			  enum dsa_tag_protocol init_tag_proto,
+			  const struct felix_info *info);
 struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port);
 int felix_netdev_to_port(struct net_device *dev);
 
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index ec8b124e8f61..ba37a566da39 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -2673,9 +2673,7 @@ static int felix_pci_probe(struct pci_dev *pdev,
 			   const struct pci_device_id *id)
 {
 	struct device *dev = &pdev->dev;
-	struct dsa_switch *ds;
-	struct ocelot *ocelot;
-	struct felix *felix;
+	resource_size_t switch_base;
 	int err;
 
 	err = pci_enable_device(pdev);
@@ -2684,45 +2682,15 @@ static int felix_pci_probe(struct pci_dev *pdev,
 		return err;
 	}
 
-	felix = devm_kzalloc(dev, sizeof(struct felix), GFP_KERNEL);
-	if (!felix) {
-		err = -ENOMEM;
-		goto out_disable;
-	}
-
-	pci_set_drvdata(pdev, felix);
-	ocelot = &felix->ocelot;
-	ocelot->dev = dev;
-	ocelot->num_flooding_pgids = OCELOT_NUM_TC;
-	felix->info = &felix_info_vsc9959;
-	felix->switch_base = pci_resource_start(pdev, VSC9959_SWITCH_PCI_BAR);
-
 	pci_set_master(pdev);
 
-	ocelot->ptp = 1;
-	ocelot->mm_supported = true;
+	switch_base = pci_resource_start(pdev, VSC9959_SWITCH_PCI_BAR);
 
-	ds = devm_kzalloc(dev, sizeof(struct dsa_switch), GFP_KERNEL);
-	if (!ds) {
-		err = -ENOMEM;
+	err = felix_register_switch(dev, switch_base, OCELOT_NUM_TC,
+				    true, true, DSA_TAG_PROTO_OCELOT,
+				    &felix_info_vsc9959);
+	if (err)
 		goto out_disable;
-	}
-
-	ds->dev = dev;
-	ds->num_ports = felix->info->num_ports;
-	ds->num_tx_queues = OCELOT_NUM_TC;
-
-	ds->ops = &felix_switch_ops;
-	ds->phylink_mac_ops = &felix_phylink_mac_ops;
-	ds->priv = ocelot;
-	felix->ds = ds;
-	felix->tag_proto = DSA_TAG_PROTO_OCELOT;
-
-	err = dsa_register_switch(ds);
-	if (err) {
-		dev_err_probe(dev, err, "Failed to register DSA switch\n");
-		goto out_disable;
-	}
 
 	return 0;
 
diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
index 9cd24f77dc49..5632a7248cd4 100644
--- a/drivers/net/dsa/ocelot/ocelot_ext.c
+++ b/drivers/net/dsa/ocelot/ocelot_ext.c
@@ -64,44 +64,8 @@ static const struct felix_info vsc7512_info = {
 
 static int ocelot_ext_probe(struct platform_device *pdev)
 {
-	struct device *dev = &pdev->dev;
-	struct dsa_switch *ds;
-	struct ocelot *ocelot;
-	struct felix *felix;
-	int err;
-
-	felix = devm_kzalloc(dev, sizeof(*felix), GFP_KERNEL);
-	if (!felix)
-		return -ENOMEM;
-
-	dev_set_drvdata(dev, felix);
-
-	ocelot = &felix->ocelot;
-	ocelot->dev = dev;
-
-	ocelot->num_flooding_pgids = 1;
-
-	felix->info = &vsc7512_info;
-
-	ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
-	if (!ds)
-		return -ENOMEM;
-
-	ds->dev = dev;
-	ds->num_ports = felix->info->num_ports;
-	ds->num_tx_queues = OCELOT_NUM_TC;
-
-	ds->ops = &felix_switch_ops;
-	ds->phylink_mac_ops = &felix_phylink_mac_ops;
-	ds->priv = ocelot;
-	felix->ds = ds;
-	felix->tag_proto = DSA_TAG_PROTO_OCELOT;
-
-	err = dsa_register_switch(ds);
-	if (err)
-		dev_err_probe(dev, err, "Failed to register DSA switch\n");
-
-	return err;
+	return felix_register_switch(&pdev->dev, 0, 1, false, false,
+				     DSA_TAG_PROTO_OCELOT, &vsc7512_info);
 }
 
 static void ocelot_ext_remove(struct platform_device *pdev)
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index 83bd0e1ee692..70782649c395 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -971,49 +971,17 @@ static const struct felix_info seville_info_vsc9953 = {
 static int seville_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct dsa_switch *ds;
-	struct ocelot *ocelot;
 	struct resource *res;
-	struct felix *felix;
-	int err;
-
-	felix = devm_kzalloc(dev, sizeof(struct felix), GFP_KERNEL);
-	if (!felix)
-		return -ENOMEM;
-
-	platform_set_drvdata(pdev, felix);
-
-	ocelot = &felix->ocelot;
-	ocelot->dev = dev;
-	ocelot->num_flooding_pgids = 1;
-	felix->info = &seville_info_vsc9953;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(dev, "Invalid resource\n");
 		return -EINVAL;
 	}
-	felix->switch_base = res->start;
-
-	ds = devm_kzalloc(dev, sizeof(struct dsa_switch), GFP_KERNEL);
-	if (!ds)
-		return -ENOMEM;
-
-	ds->dev = dev;
-	ds->num_ports = felix->info->num_ports;
-	ds->num_tx_queues = OCELOT_NUM_TC;
-
-	ds->ops = &felix_switch_ops;
-	ds->phylink_mac_ops = &felix_phylink_mac_ops;
-	ds->priv = ocelot;
-	felix->ds = ds;
-	felix->tag_proto = DSA_TAG_PROTO_SEVILLE;
-
-	err = dsa_register_switch(ds);
-	if (err)
-		dev_err(dev, "Failed to register DSA switch: %d\n", err);
 
-	return err;
+	return felix_register_switch(dev, res->start, 1, false, false,
+				     DSA_TAG_PROTO_SEVILLE,
+				     &seville_info_vsc9953);
 }
 
 static void seville_remove(struct platform_device *pdev)
-- 
2.34.1

[PATCH net-next 8/8] net: dsa: ocelot: unexport felix_phylink_mac_ops and felix_switch_ops

[Vladimir Oltean]

Now that the common felix_register_switch() from the umbrella driver
is the only entity that accesses these data structures, we can remove
them from the list of the exported symbols.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c | 6 ++----
 drivers/net/dsa/ocelot/felix.h | 3 ---
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index accf737f7b69..d12c4e85baa7 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -2106,15 +2106,14 @@ static void felix_get_mm_stats(struct dsa_switch *ds, int port,
 	ocelot_port_get_mm_stats(ocelot, port, stats);
 }
 
-const struct phylink_mac_ops felix_phylink_mac_ops = {
+static const struct phylink_mac_ops felix_phylink_mac_ops = {
 	.mac_select_pcs		= felix_phylink_mac_select_pcs,
 	.mac_config		= felix_phylink_mac_config,
 	.mac_link_down		= felix_phylink_mac_link_down,
 	.mac_link_up		= felix_phylink_mac_link_up,
 };
-EXPORT_SYMBOL_GPL(felix_phylink_mac_ops);
 
-const struct dsa_switch_ops felix_switch_ops = {
+static const struct dsa_switch_ops felix_switch_ops = {
 	.get_tag_protocol		= felix_get_tag_protocol,
 	.change_tag_protocol		= felix_change_tag_protocol,
 	.connect_tag_protocol		= felix_connect_tag_protocol,
@@ -2193,7 +2192,6 @@ const struct dsa_switch_ops felix_switch_ops = {
 	.port_set_host_flood		= felix_port_set_host_flood,
 	.port_change_conduit		= felix_port_change_conduit,
 };
-EXPORT_SYMBOL_GPL(felix_switch_ops);
 
 int felix_register_switch(struct device *dev, resource_size_t switch_base,
 			  int num_flooding_pgids, bool ptp,
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index 85b4f8616003..211991f494e3 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -82,9 +82,6 @@ struct felix_tag_proto_ops {
 			      struct netlink_ext_ack *extack);
 };
 
-extern const struct phylink_mac_ops felix_phylink_mac_ops;
-extern const struct dsa_switch_ops felix_switch_ops;
-
 /* DSA glue / front-end for struct ocelot */
 struct felix {
 	struct dsa_switch		*ds;
-- 
2.34.1

Re: [PATCH net-next 1/8] net: dsa: ocelot: use devres in ocelot_ext_probe()

[Colin Foster]

Reviewed-by: Colin Foster <colin.foster@in-advantage.com>

On Thu, May 30, 2024 at 07:33:26PM +0300, Vladimir Oltean wrote:
> Russell King suggested that felix_vsc9959, seville_vsc9953 and
> ocelot_ext have a large portion of duplicated init and teardown code,
> which could be made common [1]. The teardown code could even be
> simplified away if we made use of devres, something which is used here
> and there in the felix driver, just not very consistently.
> 
> [1] https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/
> 
> Prepare the ground in the ocelot_ext driver, by allocating the data
> structures using devres and deleting the kfree() calls. This also
> deletes the "Failed to allocate ..." message, since memory allocation
> errors are extremely loud anyway, and it's hard to miss them.
> 
> Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/ocelot/ocelot_ext.c | 24 +++++-------------------
>  1 file changed, 5 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
> index a8927dc7aca4..c893f3ee238b 100644
> --- a/drivers/net/dsa/ocelot/ocelot_ext.c
> +++ b/drivers/net/dsa/ocelot/ocelot_ext.c
> @@ -71,7 +71,7 @@ static int ocelot_ext_probe(struct platform_device *pdev)
>  	struct felix *felix;
>  	int err;
>  
> -	felix = kzalloc(sizeof(*felix), GFP_KERNEL);
> +	felix = devm_kzalloc(dev, sizeof(*felix), GFP_KERNEL);
>  	if (!felix)
>  		return -ENOMEM;
>  
> @@ -84,12 +84,9 @@ static int ocelot_ext_probe(struct platform_device *pdev)
>  
>  	felix->info = &vsc7512_info;
>  
> -	ds = kzalloc(sizeof(*ds), GFP_KERNEL);
> -	if (!ds) {
> -		err = -ENOMEM;
> -		dev_err_probe(dev, err, "Failed to allocate DSA switch\n");
> -		goto err_free_felix;
> -	}
> +	ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
> +	if (!ds)
> +		return -ENOMEM;
>  
>  	ds->dev = dev;
>  	ds->num_ports = felix->info->num_ports;
> @@ -102,17 +99,9 @@ static int ocelot_ext_probe(struct platform_device *pdev)
>  	felix->tag_proto = DSA_TAG_PROTO_OCELOT;
>  
>  	err = dsa_register_switch(ds);
> -	if (err) {
> +	if (err)
>  		dev_err_probe(dev, err, "Failed to register DSA switch\n");
> -		goto err_free_ds;
> -	}
> -
> -	return 0;
>  
> -err_free_ds:
> -	kfree(ds);
> -err_free_felix:
> -	kfree(felix);
>  	return err;
>  }
>  
> @@ -124,9 +113,6 @@ static void ocelot_ext_remove(struct platform_device *pdev)
>  		return;
>  
>  	dsa_unregister_switch(felix->ds);
> -
> -	kfree(felix->ds);
> -	kfree(felix);
>  }
>  
>  static void ocelot_ext_shutdown(struct platform_device *pdev)
> -- 
> 2.34.1
> 

Re: [PATCH net-next 6/8] net: dsa: ocelot: use ds->num_tx_queues = OCELOT_NUM_TC for all models

[Colin Foster]

On Thu, May 30, 2024 at 07:33:31PM +0300, Vladimir Oltean wrote:
> Russell King points out that seville_vsc9953 populates
> felix->info->num_tx_queues = 8, but this doesn't make it all the way
> into ds->num_tx_queues (which is how the user interface netdev queues
> get allocated) [1].
> 
> [1]: https://lore.kernel.org/all/20240415160150.yejcazpjqvn7vhxu@skbuf/
> 
> When num_tx_queues=0 for seville, this is implicitly converted to 1 by
> dsa_user_create(), and this is good enough for basic operation for a
> switch port. The tc qdisc offload layer works with netdev TX queues,
> so for QoS offload we need to pretend we have multiple TX queues. The
> VSC9953, like ocelot_ext, doesn't export QoS offload, so it doesn't
> really matter. But we can definitely set num_tx_queues=8 for all
> switches.
> 
> The felix->info->num_tx_queues construct itself seems unnecessary.
> It was introduced by commit de143c0e274b ("net: dsa: felix: Configure
> Time-Aware Scheduler via taprio offload") at a time when vsc9959
> (LS1028A) was the only switch supported by the driver.
> 
> 8 traffic classes, and 1 queue per traffic class, is a common
> architectural feature of all switches in the family. So they could
> all just set OCELOT_NUM_TC and be fine.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/ocelot/felix.h           | 1 -
>  drivers/net/dsa/ocelot/felix_vsc9959.c   | 4 ++--
>  drivers/net/dsa/ocelot/ocelot_ext.c      | 3 +--
> --- a/drivers/net/dsa/ocelot/ocelot_ext.c
> +++ b/drivers/net/dsa/ocelot/ocelot_ext.c
> @@ -57,7 +57,6 @@ static const struct felix_info vsc7512_info = {
>  	.vcap				= vsc7514_vcap_props,
>  	.num_mact_rows			= 1024,
>  	.num_ports			= VSC7514_NUM_PORTS,
> -	.num_tx_queues			= OCELOT_NUM_TC,
>  	.port_modes			= vsc7512_port_modes,
>  	.phylink_mac_config		= ocelot_phylink_mac_config,
>  	.configure_serdes		= ocelot_port_configure_serdes,
> @@ -90,7 +89,7 @@ static int ocelot_ext_probe(struct platform_device *pdev)
>  
>  	ds->dev = dev;
>  	ds->num_ports = felix->info->num_ports;
> -	ds->num_tx_queues = felix->info->num_tx_queues;
> +	ds->num_tx_queues = OCELOT_NUM_TC;
>  
>  	ds->ops = &felix_switch_ops;
>  	ds->phylink_mac_ops = &felix_phylink_mac_ops;

For ocelot_ext

Reviewed-by: Colin Foster <colin.foster@in-advantage.com>

RE: [PATCH net-next 8/8] net: dsa: ocelot: unexport felix_phylink_mac_ops and felix_switch_ops

[Sai Krishna Gajula]

> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Sent: Thursday, May 30, 2024 10:04 PM
> To: netdev@vger.kernel.org
> Cc: David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Claudiu Manoil <claudiu.manoil@nxp.com>;
> Alexandre Belloni <alexandre.belloni@bootlin.com>;
> UNGLinuxDriver@microchip.com; Andrew Lunn <andrew@lunn.ch>; Florian
> Fainelli <f.fainelli@gmail.com>; Colin Foster <colin.foster@in-
> advantage.com>; Russell King <linux@armlinux.org.uk>
> Subject: [PATCH net-next 8/8] net: dsa: ocelot: unexport
> felix_phylink_mac_ops and felix_switch_ops
> 
> Now that the common felix_register_switch() from the umbrella driver is the
> only entity that accesses these data structures, we can remove them from the
> list of the exported symbols.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/ocelot/felix.c | 6 ++----  drivers/net/dsa/ocelot/felix.h | 3 ---
>  2 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c index
> accf737f7b69..d12c4e85baa7 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -2106,15 +2106,14 @@ static void felix_get_mm_stats(struct dsa_switch
> *ds, int port,
>  	ocelot_port_get_mm_stats(ocelot, port, stats);  }
> 
> -const struct phylink_mac_ops felix_phylink_mac_ops = {
> +static const struct phylink_mac_ops felix_phylink_mac_ops = {
>  	.mac_select_pcs		= felix_phylink_mac_select_pcs,
>  	.mac_config		= felix_phylink_mac_config,
>  	.mac_link_down		= felix_phylink_mac_link_down,
>  	.mac_link_up		= felix_phylink_mac_link_up,
>  };
> -EXPORT_SYMBOL_GPL(felix_phylink_mac_ops);
> 
> -const struct dsa_switch_ops felix_switch_ops = {
> +static const struct dsa_switch_ops felix_switch_ops = {
>  	.get_tag_protocol		= felix_get_tag_protocol,
>  	.change_tag_protocol		= felix_change_tag_protocol,
>  	.connect_tag_protocol		= felix_connect_tag_protocol,
> @@ -2193,7 +2192,6 @@ const struct dsa_switch_ops felix_switch_ops = {
>  	.port_set_host_flood		= felix_port_set_host_flood,
>  	.port_change_conduit		= felix_port_change_conduit,
>  };
> -EXPORT_SYMBOL_GPL(felix_switch_ops);
> 
>  int felix_register_switch(struct device *dev, resource_size_t switch_base,
>  			  int num_flooding_pgids, bool ptp,
> diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
> index 85b4f8616003..211991f494e3 100644
> --- a/drivers/net/dsa/ocelot/felix.h
> +++ b/drivers/net/dsa/ocelot/felix.h
> @@ -82,9 +82,6 @@ struct felix_tag_proto_ops {
>  			      struct netlink_ext_ack *extack);  };
> 
> -extern const struct phylink_mac_ops felix_phylink_mac_ops; -extern const
> struct dsa_switch_ops felix_switch_ops;
> -
>  /* DSA glue / front-end for struct ocelot */  struct felix {
>  	struct dsa_switch		*ds;
> --
> 2.34.1
> 
Reviewed-by: Sai Krishna <saikrishnag@marvell.com>

Re: [PATCH net-next 0/8] Probing cleanup for the Felix DSA driver

[Colin Foster]

Hi Vladimir,

On Thu, May 30, 2024 at 07:33:25PM +0300, Vladimir Oltean wrote:
> This is a follow-up to Russell King's request for code consolidation
> among felix_vsc9959, seville_vsc9953 and ocelot_ext, stated here:
> https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/
> 
> Details are in individual patches. Testing was done on NXP LS1028A
> (felix_vsc9959).
> 
> Vladimir Oltean (8):
>   net: dsa: ocelot: use devres in ocelot_ext_probe()
>   net: dsa: ocelot: use devres in seville_probe()
>   net: dsa: ocelot: delete open coded status = "disabled" parsing
>   net: dsa: ocelot: consistently use devres in felix_pci_probe()
>   net: dsa: ocelot: move devm_request_threaded_irq() to felix_setup()
>   net: dsa: ocelot: use ds->num_tx_queues = OCELOT_NUM_TC for all models
>   net: dsa: ocelot: common probing code
>   net: dsa: ocelot: unexport felix_phylink_mac_ops and felix_switch_ops
> 
>  drivers/net/dsa/ocelot/felix.c           |  62 ++++++++++++-
>  drivers/net/dsa/ocelot/felix.h           |  10 +-
>  drivers/net/dsa/ocelot/felix_vsc9959.c   | 113 +++++++----------------
>  drivers/net/dsa/ocelot/ocelot_ext.c      |  55 +----------

Just FYI I tried testing this but hit an unrelated regression in 6.10,
and a `git b4` on 6.9.3 has conflicts. So I'm still alive, but probably
won't get to testing this tonight. Looks good though.

Colin Foster

Re: [PATCH net-next 0/8] Probing cleanup for the Felix DSA driver

[Russell King (Oracle)]

On Thu, May 30, 2024 at 10:08:10PM -0500, Colin Foster wrote:
> Hi Vladimir,
> 
> On Thu, May 30, 2024 at 07:33:25PM +0300, Vladimir Oltean wrote:
> > This is a follow-up to Russell King's request for code consolidation
> > among felix_vsc9959, seville_vsc9953 and ocelot_ext, stated here:
> > https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/
> > 
> > Details are in individual patches. Testing was done on NXP LS1028A
> > (felix_vsc9959).
> > 
> > Vladimir Oltean (8):
> >   net: dsa: ocelot: use devres in ocelot_ext_probe()
> >   net: dsa: ocelot: use devres in seville_probe()
> >   net: dsa: ocelot: delete open coded status = "disabled" parsing
> >   net: dsa: ocelot: consistently use devres in felix_pci_probe()
> >   net: dsa: ocelot: move devm_request_threaded_irq() to felix_setup()
> >   net: dsa: ocelot: use ds->num_tx_queues = OCELOT_NUM_TC for all models
> >   net: dsa: ocelot: common probing code
> >   net: dsa: ocelot: unexport felix_phylink_mac_ops and felix_switch_ops
> > 
> >  drivers/net/dsa/ocelot/felix.c           |  62 ++++++++++++-
> >  drivers/net/dsa/ocelot/felix.h           |  10 +-
> >  drivers/net/dsa/ocelot/felix_vsc9959.c   | 113 +++++++----------------
> >  drivers/net/dsa/ocelot/ocelot_ext.c      |  55 +----------
> 
> Just FYI I tried testing this but hit an unrelated regression in 6.10,
> and a `git b4` on 6.9.3 has conflicts. So I'm still alive, but probably
> won't get to testing this tonight. Looks good though.

You will need... the net-next tree because it's dependent on a patch I
submitted earlier this week.

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

Re: [PATCH net-next 1/8] net: dsa: ocelot: use devres in ocelot_ext_probe()

[Colin Foster]

On Thu, May 30, 2024 at 07:33:26PM +0300, Vladimir Oltean wrote:
> Russell King suggested that felix_vsc9959, seville_vsc9953 and
> ocelot_ext have a large portion of duplicated init and teardown code,
> which could be made common [1]. The teardown code could even be
> simplified away if we made use of devres, something which is used here
> and there in the felix driver, just not very consistently.
> 
> [1] https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/
> 
> Prepare the ground in the ocelot_ext driver, by allocating the data
> structures using devres and deleting the kfree() calls. This also
> deletes the "Failed to allocate ..." message, since memory allocation
> errors are extremely loud anyway, and it's hard to miss them.
> 
> Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/ocelot/ocelot_ext.c | 24 +++++-------------------
> 1 file changed, 5 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
> index a8927dc7aca4..c893f3ee238b 100644
> --- a/drivers/net/dsa/ocelot/ocelot_ext.c
> +++ b/drivers/net/dsa/ocelot/ocelot_ext.c

I found my issue and was able to test the series.

Tested-by: Colin Foster <colin.foster@in-advantage.com>

Re: [PATCH net-next 6/8] net: dsa: ocelot: use ds->num_tx_queues = OCELOT_NUM_TC for all models

[Colin Foster]

On Thu, May 30, 2024 at 07:33:31PM +0300, Vladimir Oltean wrote:
> Russell King points out that seville_vsc9953 populates
> felix->info->num_tx_queues = 8, but this doesn't make it all the way
> into ds->num_tx_queues (which is how the user interface netdev queues
> get allocated) [1].
> 
> [1]: https://lore.kernel.org/all/20240415160150.yejcazpjqvn7vhxu@skbuf/
> 
> When num_tx_queues=0 for seville, this is implicitly converted to 1 by
> dsa_user_create(), and this is good enough for basic operation for a
> switch port. The tc qdisc offload layer works with netdev TX queues,
> so for QoS offload we need to pretend we have multiple TX queues. The
> VSC9953, like ocelot_ext, doesn't export QoS offload, so it doesn't
> really matter. But we can definitely set num_tx_queues=8 for all
> switches.
> 
> The felix->info->num_tx_queues construct itself seems unnecessary.
> It was introduced by commit de143c0e274b ("net: dsa: felix: Configure
> Time-Aware Scheduler via taprio offload") at a time when vsc9959
> (LS1028A) was the only switch supported by the driver.
> 
> 8 traffic classes, and 1 queue per traffic class, is a common
> architectural feature of all switches in the family. So they could
> all just set OCELOT_NUM_TC and be fine.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/ocelot/felix.h           | 1 -
>  drivers/net/dsa/ocelot/felix_vsc9959.c   | 4 ++--
>  drivers/net/dsa/ocelot/ocelot_ext.c      | 3 +--

Tested-by: Colin Foster <colin.foster@in-advantage.com>

Re: [PATCH net-next 7/8] net: dsa: ocelot: common probing code

[Colin Foster]

On Thu, May 30, 2024 at 07:33:32PM +0300, Vladimir Oltean wrote:
> Russell King suggested that felix_vsc9959, seville_vsc9953 and
> ocelot_ext have a large portion of duplicated init code, which could be
> made common [1].
> 
> [1]: https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/
> 
> Here, we take the following common steps:
> - "felix" and "ds" structure allocation
> - "felix", "ocelot" and "ds" basic structure initialization
> - dsa_register_switch() call
> 
> and we make a common function out of them.
> 
> For every driver except felix_vsc9959, this is also the entire probing
> procedure. For felix_vsc9959, we also need to do some PCI-specific
> stuff, which can easily be reordered to be done before, and unwound on
> failure.
> 
> We also have to convert the bus-specific platform_set_drvdata() and
> pci_set_drvdata() calls into dev_set_drvdata(). But this should have no
> impact on the behavior.
> 
> Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/ocelot/felix.c           | 47 ++++++++++++++++++++++++
>  drivers/net/dsa/ocelot/felix.h           |  5 +++
>  drivers/net/dsa/ocelot/felix_vsc9959.c   | 44 +++-------------------
>  drivers/net/dsa/ocelot/ocelot_ext.c      | 40 +-------------------
>  drivers/net/dsa/ocelot/seville_vsc9953.c | 38 ++-----------------

Tested-by: Colin Foster <colin.foster@in-advantage.com>

Re: [PATCH net-next 1/8] net: dsa: ocelot: use devres in ocelot_ext_probe()

[Vladimir Oltean]

On Sun, Jun 02, 2024 at 09:00:01PM -0500, Colin Foster wrote:
> On Thu, May 30, 2024 at 07:33:26PM +0300, Vladimir Oltean wrote:
> > Russell King suggested that felix_vsc9959, seville_vsc9953 and
> > ocelot_ext have a large portion of duplicated init and teardown code,
> > which could be made common [1]. The teardown code could even be
> > simplified away if we made use of devres, something which is used here
> > and there in the felix driver, just not very consistently.
> > 
> > [1] https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/
> > 
> > Prepare the ground in the ocelot_ext driver, by allocating the data
> > structures using devres and deleting the kfree() calls. This also
> > deletes the "Failed to allocate ..." message, since memory allocation
> > errors are extremely loud anyway, and it's hard to miss them.
> > 
> > Suggested-by: "Russell King (Oracle)" <linux@armlinux.org.uk>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> >  drivers/net/dsa/ocelot/ocelot_ext.c | 24 +++++-------------------
> > 1 file changed, 5 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
> > index a8927dc7aca4..c893f3ee238b 100644
> > --- a/drivers/net/dsa/ocelot/ocelot_ext.c
> > +++ b/drivers/net/dsa/ocelot/ocelot_ext.c
> 
> I found my issue and was able to test the series.
> 
> Tested-by: Colin Foster <colin.foster@in-advantage.com>

Thanks for testing, Colin.

Re: [PATCH net-next 0/8] Probing cleanup for the Felix DSA driver

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 30 May 2024 19:33:25 +0300 you wrote:
> This is a follow-up to Russell King's request for code consolidation
> among felix_vsc9959, seville_vsc9953 and ocelot_ext, stated here:
> https://lore.kernel.org/all/Zh1GvcOTXqb7CpQt@shell.armlinux.org.uk/
> 
> Details are in individual patches. Testing was done on NXP LS1028A
> (felix_vsc9959).
> 
> [...]

Here is the summary with links:
  - [net-next,1/8] net: dsa: ocelot: use devres in ocelot_ext_probe()
    https://git.kernel.org/netdev/net-next/c/454cfffe8dc1
  - [net-next,2/8] net: dsa: ocelot: use devres in seville_probe()
    https://git.kernel.org/netdev/net-next/c/90ee9a5b49ce
  - [net-next,3/8] net: dsa: ocelot: delete open coded status = "disabled" parsing
    https://git.kernel.org/netdev/net-next/c/cc711c523da7
  - [net-next,4/8] net: dsa: ocelot: consistently use devres in felix_pci_probe()
    https://git.kernel.org/netdev/net-next/c/4510bbd38cbe
  - [net-next,5/8] net: dsa: ocelot: move devm_request_threaded_irq() to felix_setup()
    https://git.kernel.org/netdev/net-next/c/0367a1775933
  - [net-next,6/8] net: dsa: ocelot: use ds->num_tx_queues = OCELOT_NUM_TC for all models
    https://git.kernel.org/netdev/net-next/c/4ca54dd96eca
  - [net-next,7/8] net: dsa: ocelot: common probing code
    https://git.kernel.org/netdev/net-next/c/efdbee7d0791
  - [net-next,8/8] net: dsa: ocelot: unexport felix_phylink_mac_ops and felix_switch_ops
    https://git.kernel.org/netdev/net-next/c/a4303941c6f3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html