[PATCH net-next 00/15] net: sparx5: prepare for lan969x switch driver

[PATCH net-next 00/15] net: sparx5: prepare for lan969x switch driver

[Daniel Machon]

== Description:

This series is the first of a multi-part series, that prepares and adds
support for the new lan969x switch driver.

The upstreaming efforts is split into multiple series (might change a
bit as we go along):

    1) Prepare the Sparx5 driver for lan969x (this series)
    2) Add support lan969x (same basic features as Sparx5 provides +
       RGMII, excl.  FDMA and VCAP)
    3) Add support for lan969x FDMA
    4) Add support for lan969x VCAP

== Lan969x in short:

The lan969x Ethernet switch family [1] provides a rich set of
switching features and port configurations (up to 30 ports) from 10Mbps
to 10Gbps, with support for RGMII, SGMII, QSGMII, USGMII, and USXGMII,
ideal for industrial & process automation infrastructure applications,
transport, grid automation, power substation automation, and ring &
intra-ring topologies. The LAN969x family is hardware and software
compatible and scalable supporting 46Gbps to 102Gbps switch bandwidths.

== Preparing Sparx5 for lan969x:

The lan969x switch chip reuses many of the IP's of the Sparx5 switch
chip, therefore it has been decided to add support through the existing
Sparx5 driver, in order to avoid a bunch of duplicate code. However, in
order to reuse the Sparx5 switch driver, we have to introduce some
mechanisms to handle the chip differences that are there.  These
mechanisms are:

    - Platform match data to contain all the differences that needs to
      be handled (constants, ops etc.)

    - Register macro indirection layer so that we can reuse the existing
      register macros.

    - Function for branching out on platform type where required.

In some places we ops out functions and in other places we branch on the
chip type. Exactly when we choose one over the other, is an estimate in
each case.

After this series is applied, the Sparx5 driver will be prepared for
lan969x and still function exactly as before.

== Patch breakdown:

Patch #1     adds private match data

Patch #2     adds register macro indirection layer

Patch #3-#5  does some preparation work

Patch #6-#8  adds chip constants and updates the code to use them

Patch #9-#14 adds and uses ops for handling functions differently on the
             two platforms.

Patch #15    adds and uses a macro for branching out on the chip type

[1] https://www.microchip.com/en-us/product/lan9698

To: David S. Miller <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Lars Povlsen <lars.povlsen@microchip.com>
To: Steen Hegelund <Steen.Hegelund@microchip.com>
To: horatiu.vultur@microchip.com
To: jensemil.schulzostergaard@microchip.com
To: UNGLinuxDriver@microchip.com
To: Richard Cochran <richardcochran@gmail.com>
To: horms@kernel.org
To: justinstitt@google.com
To: gal@nvidia.com
To: aakash.r.menon@gmail.com
To: jacob.e.keller@intel.com
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
Daniel Machon (15):
      net: sparx5: add support for private match data
      net: sparx5: add indirection layer to register macros
      net: sparx5: rename *spx5 to *sparx5 in a few places
      net: sparx5: modify SPX5_PORTS_ALL macro
      net: sparx5: add *sparx5 argument to a few functions
      net: sparx5: add constants to match data
      net: sparx5: use SPX5_CONST for constants which already have a symbol
      net: sparx5: use SPX5_CONST for constants which do not have a symbol
      net: sparx5: add ops to match data
      net: sparx5: ops out chip port to device index/bit functions
      net: sparx5: ops out functions for getting certain array values
      net: sparx5: ops out function for setting the port mux
      net: sparx5: ops out PTP IRQ handler
      net: sparx5: ops out function for DSM calendar calculation
      net: sparx5: add is_sparx5 macro and use it throughout

 drivers/net/ethernet/microchip/sparx5/Makefile     |    2 +-
 .../ethernet/microchip/sparx5/sparx5_calendar.c    |   41 +-
 drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c |    5 +-
 .../net/ethernet/microchip/sparx5/sparx5_ethtool.c |   33 +-
 .../net/ethernet/microchip/sparx5/sparx5_fdma.c    |    6 +-
 .../ethernet/microchip/sparx5/sparx5_mactable.c    |    6 +-
 .../net/ethernet/microchip/sparx5/sparx5_main.c    |  198 +-
 .../net/ethernet/microchip/sparx5/sparx5_main.h    |  111 +-
 .../ethernet/microchip/sparx5/sparx5_main_regs.h   | 4128 +++++++++++---------
 .../net/ethernet/microchip/sparx5/sparx5_netdev.c  |    8 +-
 .../net/ethernet/microchip/sparx5/sparx5_packet.c  |    4 +-
 .../net/ethernet/microchip/sparx5/sparx5_pgid.c    |   24 +-
 .../net/ethernet/microchip/sparx5/sparx5_police.c  |    3 +-
 .../net/ethernet/microchip/sparx5/sparx5_port.c    |   71 +-
 .../net/ethernet/microchip/sparx5/sparx5_port.h    |   23 +-
 .../net/ethernet/microchip/sparx5/sparx5_psfp.c    |   45 +-
 drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c |    6 +-
 drivers/net/ethernet/microchip/sparx5/sparx5_qos.c |    8 +-
 drivers/net/ethernet/microchip/sparx5/sparx5_qos.h |    4 +-
 .../net/ethernet/microchip/sparx5/sparx5_regs.c    |  219 ++
 .../net/ethernet/microchip/sparx5/sparx5_regs.h    |  244 ++
 .../net/ethernet/microchip/sparx5/sparx5_sdlb.c    |   15 +-
 .../ethernet/microchip/sparx5/sparx5_switchdev.c   |   53 +-
 drivers/net/ethernet/microchip/sparx5/sparx5_tc.c  |    8 +-
 .../net/ethernet/microchip/sparx5/sparx5_vlan.c    |   44 +-
 25 files changed, 3191 insertions(+), 2118 deletions(-)
---
base-commit: 3a39d672e7f48b8d6b91a09afa4b55352773b4b5
change-id: 20240927-b4-sparx5-lan969x-switch-driver-dfcd5277fa70

Best regards,
-- 
Daniel Machon <daniel.machon@microchip.com>

[PATCH net-next 01/15] net: sparx5: add support for private match data

[Daniel Machon]

In preparation for lan969x, add support for private match data. This
will be needed for abstracting away differences between the Sparx5 and
lan969x platforms. We initially add values for: iomap, iomap size and
ioranges. Update the use of these throughout.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 .../net/ethernet/microchip/sparx5/sparx5_main.c    | 36 +++++++++++++---------
 .../net/ethernet/microchip/sparx5/sparx5_main.h    | 13 ++++++++
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index b64c814eac11..179a1dc0d8f6 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -45,12 +45,6 @@ struct sparx5_ram_config {
 	u32 init_val;
 };
 
-struct sparx5_main_io_resource {
-	enum sparx5_target id;
-	phys_addr_t offset;
-	int range;
-};
-
 static const struct sparx5_main_io_resource sparx5_main_iomap[] =  {
 	{ TARGET_CPU,                         0, 0 }, /* 0x600000000 */
 	{ TARGET_FDMA,                  0x80000, 0 }, /* 0x600080000 */
@@ -216,21 +210,24 @@ static const struct sparx5_main_io_resource sparx5_main_iomap[] =  {
 
 static int sparx5_create_targets(struct sparx5 *sparx5)
 {
+	const struct sparx5_main_io_resource *iomap = sparx5->data->iomap;
+	int iomap_size = sparx5->data->iomap_size;
+	int ioranges = sparx5->data->ioranges;
 	struct resource *iores[IO_RANGES];
 	void __iomem *iomem[IO_RANGES];
 	void __iomem *begin[IO_RANGES];
 	int range_id[IO_RANGES];
 	int idx, jdx;
 
-	for (idx = 0, jdx = 0; jdx < ARRAY_SIZE(sparx5_main_iomap); jdx++) {
-		const struct sparx5_main_io_resource *iomap = &sparx5_main_iomap[jdx];
+	for (idx = 0, jdx = 0; jdx < iomap_size; jdx++) {
+		const struct sparx5_main_io_resource *io = &iomap[jdx];
 
-		if (idx == iomap->range) {
+		if (idx == io->range) {
 			range_id[idx] = jdx;
 			idx++;
 		}
 	}
-	for (idx = 0; idx < IO_RANGES; idx++) {
+	for (idx = 0; idx < ioranges; idx++) {
 		iores[idx] = platform_get_resource(sparx5->pdev, IORESOURCE_MEM,
 						   idx);
 		if (!iores[idx]) {
@@ -245,12 +242,12 @@ static int sparx5_create_targets(struct sparx5 *sparx5)
 				iores[idx]->name);
 			return -ENOMEM;
 		}
-		begin[idx] = iomem[idx] - sparx5_main_iomap[range_id[idx]].offset;
+		begin[idx] = iomem[idx] - iomap[range_id[idx]].offset;
 	}
-	for (jdx = 0; jdx < ARRAY_SIZE(sparx5_main_iomap); jdx++) {
-		const struct sparx5_main_io_resource *iomap = &sparx5_main_iomap[jdx];
+	for (jdx = 0; jdx < iomap_size; jdx++) {
+		const struct sparx5_main_io_resource *io = &iomap[jdx];
 
-		sparx5->regs[iomap->id] = begin[iomap->range] + iomap->offset;
+		sparx5->regs[io->id] = begin[io->range] + io->offset;
 	}
 	return 0;
 }
@@ -759,6 +756,9 @@ static int mchp_sparx5_probe(struct platform_device *pdev)
 	sparx5->dev = &pdev->dev;
 	spin_lock_init(&sparx5->tx_lock);
 
+	sparx5->data = device_get_match_data(sparx5->dev);
+	if (!sparx5->data)
+		return -EINVAL;
 	/* Do switch core reset if available */
 	reset = devm_reset_control_get_optional_shared(&pdev->dev, "switch");
 	if (IS_ERR(reset))
@@ -937,8 +937,14 @@ static void mchp_sparx5_remove(struct platform_device *pdev)
 	destroy_workqueue(sparx5->mact_queue);
 }
 
+static const struct sparx5_match_data sparx5_desc = {
+	.iomap = sparx5_main_iomap,
+	.iomap_size = ARRAY_SIZE(sparx5_main_iomap),
+	.ioranges = 3,
+};
+
 static const struct of_device_id mchp_sparx5_match[] = {
-	{ .compatible = "microchip,sparx5-switch" },
+	{ .compatible = "microchip,sparx5-switch", .data = &sparx5_desc },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mchp_sparx5_match);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 3309060b1e4c..845f918aaf5e 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -226,6 +226,18 @@ struct sparx5_mall_entry {
 #define SPARX5_SKB_CB(skb) \
 	((struct sparx5_skb_cb *)((skb)->cb))
 
+struct sparx5_main_io_resource {
+	enum sparx5_target id;
+	phys_addr_t offset;
+	int range;
+};
+
+struct sparx5_match_data {
+	const struct sparx5_main_io_resource *iomap;
+	int ioranges;
+	int iomap_size;
+};
+
 struct sparx5 {
 	struct platform_device *pdev;
 	struct device *dev;
@@ -293,6 +305,7 @@ struct sparx5 {
 	struct list_head mall_entries;
 	/* Common root for debugfs */
 	struct dentry *debugfs_root;
+	const struct sparx5_match_data *data;
 };
 
 /* sparx5_switchdev.c */

-- 
2.34.1

Re: [PATCH net-next 01/15] net: sparx5: add support for private match data

[Jacob Keller]

On 10/1/2024 6:50 AM, Daniel Machon wrote:
> In preparation for lan969x, add support for private match data. This
> will be needed for abstracting away differences between the Sparx5 and
> lan969x platforms. We initially add values for: iomap, iomap size and
> ioranges. Update the use of these throughout.
> 
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
> ---

Nice use of the match data here.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

[PATCH net-next 03/15] net: sparx5: rename *spx5 to *sparx5 in a few places

[Daniel Machon]

In preparation for lan969x, we need to handle platform specific
constants (which will be added in a subsequent patch). These constants
will be accessed through a macro that requires the *sparx5 context
pointer to be called exactly that.

Therefore rename *spx5 to *sparx5 in a few places.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 .../net/ethernet/microchip/sparx5/sparx5_main.h    |  6 +--
 .../net/ethernet/microchip/sparx5/sparx5_pgid.c    | 18 ++++----
 .../ethernet/microchip/sparx5/sparx5_switchdev.c   | 48 +++++++++++-----------
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index e3f22b730d80..fdff83537418 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -438,9 +438,9 @@ enum sparx5_pgid_type {
 	SPX5_PGID_MULTICAST,
 };
 
-void sparx5_pgid_init(struct sparx5 *spx5);
-int sparx5_pgid_alloc_mcast(struct sparx5 *spx5, u16 *idx);
-int sparx5_pgid_free(struct sparx5 *spx5, u16 idx);
+void sparx5_pgid_init(struct sparx5 *sparx5);
+int sparx5_pgid_alloc_mcast(struct sparx5 *sparx5, u16 *idx);
+int sparx5_pgid_free(struct sparx5 *sparx5, u16 idx);
 
 /* sparx5_pool.c */
 struct sparx5_pool_entry {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c b/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
index af8b435009f4..97adccea5352 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
@@ -1,21 +1,21 @@
 // SPDX-License-Identifier: GPL-2.0+
 #include "sparx5_main.h"
 
-void sparx5_pgid_init(struct sparx5 *spx5)
+void sparx5_pgid_init(struct sparx5 *sparx5)
 {
 	int i;
 
 	for (i = 0; i < PGID_TABLE_SIZE; i++)
-		spx5->pgid_map[i] = SPX5_PGID_FREE;
+		sparx5->pgid_map[i] = SPX5_PGID_FREE;
 
 	/* Reserved for unicast, flood control, broadcast, and CPU.
 	 * These cannot be freed.
 	 */
 	for (i = 0; i <= PGID_CPU; i++)
-		spx5->pgid_map[i] = SPX5_PGID_RESERVED;
+		sparx5->pgid_map[i] = SPX5_PGID_RESERVED;
 }
 
-int sparx5_pgid_alloc_mcast(struct sparx5 *spx5, u16 *idx)
+int sparx5_pgid_alloc_mcast(struct sparx5 *sparx5, u16 *idx)
 {
 	int i;
 
@@ -23,8 +23,8 @@ int sparx5_pgid_alloc_mcast(struct sparx5 *spx5, u16 *idx)
 	 * are reserved for flood masks and CPU. Start alloc after that.
 	 */
 	for (i = PGID_MCAST_START; i < PGID_TABLE_SIZE; i++) {
-		if (spx5->pgid_map[i] == SPX5_PGID_FREE) {
-			spx5->pgid_map[i] = SPX5_PGID_MULTICAST;
+		if (sparx5->pgid_map[i] == SPX5_PGID_FREE) {
+			sparx5->pgid_map[i] = SPX5_PGID_MULTICAST;
 			*idx = i;
 			return 0;
 		}
@@ -33,14 +33,14 @@ int sparx5_pgid_alloc_mcast(struct sparx5 *spx5, u16 *idx)
 	return -EBUSY;
 }
 
-int sparx5_pgid_free(struct sparx5 *spx5, u16 idx)
+int sparx5_pgid_free(struct sparx5 *sparx5, u16 idx)
 {
 	if (idx <= PGID_CPU || idx >= PGID_TABLE_SIZE)
 		return -EINVAL;
 
-	if (spx5->pgid_map[idx] == SPX5_PGID_FREE)
+	if (sparx5->pgid_map[idx] == SPX5_PGID_FREE)
 		return -EINVAL;
 
-	spx5->pgid_map[idx] = SPX5_PGID_FREE;
+	sparx5->pgid_map[idx] = SPX5_PGID_FREE;
 	return 0;
 }
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
index 0b4abc3eb53d..bcee9adcfbdb 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
@@ -359,10 +359,10 @@ static int sparx5_switchdev_event(struct notifier_block *nb,
 	struct sparx5_switchdev_event_work *switchdev_work;
 	struct switchdev_notifier_fdb_info *fdb_info;
 	struct switchdev_notifier_info *info = ptr;
-	struct sparx5 *spx5;
+	struct sparx5 *sparx5;
 	int err;
 
-	spx5 = container_of(nb, struct sparx5, switchdev_nb);
+	sparx5 = container_of(nb, struct sparx5, switchdev_nb);
 
 	switch (event) {
 	case SWITCHDEV_PORT_ATTR_SET:
@@ -379,7 +379,7 @@ static int sparx5_switchdev_event(struct notifier_block *nb,
 
 		switchdev_work->dev = dev;
 		switchdev_work->event = event;
-		switchdev_work->sparx5 = spx5;
+		switchdev_work->sparx5 = sparx5;
 
 		fdb_info = container_of(info,
 					struct switchdev_notifier_fdb_info,
@@ -503,10 +503,10 @@ static struct sparx5_mdb_entry *sparx5_mdb_get_entry(struct sparx5 *sparx5,
 	return found;
 }
 
-static void sparx5_cpu_copy_ena(struct sparx5 *spx5, u16 pgid, bool enable)
+static void sparx5_cpu_copy_ena(struct sparx5 *sparx5, u16 pgid, bool enable)
 {
 	spx5_rmw(ANA_AC_PGID_MISC_CFG_PGID_CPU_COPY_ENA_SET(enable),
-		 ANA_AC_PGID_MISC_CFG_PGID_CPU_COPY_ENA, spx5,
+		 ANA_AC_PGID_MISC_CFG_PGID_CPU_COPY_ENA, sparx5,
 		 ANA_AC_PGID_MISC_CFG(pgid));
 }
 
@@ -515,7 +515,7 @@ static int sparx5_handle_port_mdb_add(struct net_device *dev,
 				      const struct switchdev_obj_port_mdb *v)
 {
 	struct sparx5_port *port = netdev_priv(dev);
-	struct sparx5 *spx5 = port->sparx5;
+	struct sparx5 *sparx5 = port->sparx5;
 	struct sparx5_mdb_entry *entry;
 	bool is_host, is_new;
 	int err, i;
@@ -529,40 +529,40 @@ static int sparx5_handle_port_mdb_add(struct net_device *dev,
 	/* When VLAN unaware the vlan value is not parsed and we receive vid 0.
 	 * Fall back to bridge vid 1.
 	 */
-	if (!br_vlan_enabled(spx5->hw_bridge_dev))
+	if (!br_vlan_enabled(sparx5->hw_bridge_dev))
 		vid = 1;
 	else
 		vid = v->vid;
 
 	is_new = false;
-	entry = sparx5_mdb_get_entry(spx5, v->addr, vid);
+	entry = sparx5_mdb_get_entry(sparx5, v->addr, vid);
 	if (!entry) {
-		err = sparx5_alloc_mdb_entry(spx5, v->addr, vid, &entry);
+		err = sparx5_alloc_mdb_entry(sparx5, v->addr, vid, &entry);
 		is_new = true;
 		if (err)
 			return err;
 	}
 
-	mutex_lock(&spx5->mdb_lock);
+	mutex_lock(&sparx5->mdb_lock);
 
 	/* Add any mrouter ports to the new entry */
 	if (is_new && ether_addr_is_ip_mcast(v->addr))
 		for (i = 0; i < SPX5_PORTS; i++)
-			if (spx5->ports[i] && spx5->ports[i]->is_mrouter)
-				sparx5_pgid_update_mask(spx5->ports[i],
+			if (sparx5->ports[i] && sparx5->ports[i]->is_mrouter)
+				sparx5_pgid_update_mask(sparx5->ports[i],
 							entry->pgid_idx,
 							true);
 
 	if (is_host && !entry->cpu_copy) {
-		sparx5_cpu_copy_ena(spx5, entry->pgid_idx, true);
+		sparx5_cpu_copy_ena(sparx5, entry->pgid_idx, true);
 		entry->cpu_copy = true;
 	} else if (!is_host) {
 		sparx5_pgid_update_mask(port, entry->pgid_idx, true);
 		set_bit(port->portno, entry->port_mask);
 	}
-	mutex_unlock(&spx5->mdb_lock);
+	mutex_unlock(&sparx5->mdb_lock);
 
-	sparx5_mact_learn(spx5, entry->pgid_idx, entry->addr, entry->vid);
+	sparx5_mact_learn(sparx5, entry->pgid_idx, entry->addr, entry->vid);
 
 	return 0;
 }
@@ -572,7 +572,7 @@ static int sparx5_handle_port_mdb_del(struct net_device *dev,
 				      const struct switchdev_obj_port_mdb *v)
 {
 	struct sparx5_port *port = netdev_priv(dev);
-	struct sparx5 *spx5 = port->sparx5;
+	struct sparx5 *sparx5 = port->sparx5;
 	struct sparx5_mdb_entry *entry;
 	bool is_host;
 	u16 vid;
@@ -582,18 +582,18 @@ static int sparx5_handle_port_mdb_del(struct net_device *dev,
 
 	is_host = netif_is_bridge_master(v->obj.orig_dev);
 
-	if (!br_vlan_enabled(spx5->hw_bridge_dev))
+	if (!br_vlan_enabled(sparx5->hw_bridge_dev))
 		vid = 1;
 	else
 		vid = v->vid;
 
-	entry = sparx5_mdb_get_entry(spx5, v->addr, vid);
+	entry = sparx5_mdb_get_entry(sparx5, v->addr, vid);
 	if (!entry)
 		return 0;
 
-	mutex_lock(&spx5->mdb_lock);
+	mutex_lock(&sparx5->mdb_lock);
 	if (is_host && entry->cpu_copy) {
-		sparx5_cpu_copy_ena(spx5, entry->pgid_idx, false);
+		sparx5_cpu_copy_ena(sparx5, entry->pgid_idx, false);
 		entry->cpu_copy = false;
 	} else if (!is_host) {
 		clear_bit(port->portno, entry->port_mask);
@@ -602,15 +602,15 @@ static int sparx5_handle_port_mdb_del(struct net_device *dev,
 		if (!port->is_mrouter || !ether_addr_is_ip_mcast(v->addr))
 			sparx5_pgid_update_mask(port, entry->pgid_idx, false);
 	}
-	mutex_unlock(&spx5->mdb_lock);
+	mutex_unlock(&sparx5->mdb_lock);
 
 	if (bitmap_empty(entry->port_mask, SPX5_PORTS) && !entry->cpu_copy) {
 		 /* Clear pgid in case mrouter ports exists
 		  * that are not part of the group.
 		  */
-		sparx5_pgid_clear(spx5, entry->pgid_idx);
-		sparx5_mact_forget(spx5, entry->addr, entry->vid);
-		sparx5_free_mdb_entry(spx5, entry->addr, entry->vid);
+		sparx5_pgid_clear(sparx5, entry->pgid_idx);
+		sparx5_mact_forget(sparx5, entry->addr, entry->vid);
+		sparx5_free_mdb_entry(sparx5, entry->addr, entry->vid);
 	}
 	return 0;
 }

-- 
2.34.1

Re: [PATCH net-next 03/15] net: sparx5: rename *spx5 to *sparx5 in a few places

[Jacob Keller]

On 10/1/2024 6:50 AM, Daniel Machon wrote:
> In preparation for lan969x, we need to handle platform specific
> constants (which will be added in a subsequent patch). These constants
> will be accessed through a macro that requires the *sparx5 context
> pointer to be called exactly that.

Another place where macros implicitly assume some local variable? Not a
big fan of that, though I suppose it does leave shorter code.

No issue with the renames though.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

[PATCH net-next 04/15] net: sparx5: modify SPX5_PORTS_ALL macro

[Daniel Machon]

In preparation for lan969x, we need to define the SPX5_PORTS_ALL macro
as 70 (65 front ports + 5 internal ports). This is required as the
SPX5_PORT_CPU will be redefined as a non-constant in a subsequent patch.
And as SPX5_PORTS_ALL is used as an array size troughout the code, we
have to make sure that it stays a constant.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_main.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index fdff83537418..824849869f61 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -52,13 +52,14 @@ enum sparx5_vlan_port_type {
 };
 
 #define SPX5_PORTS             65
+#define SPX5_PORTS_ALL         70 /* Total number of ports */
+
 #define SPX5_PORT_CPU          (SPX5_PORTS)  /* Next port is CPU port */
 #define SPX5_PORT_CPU_0        (SPX5_PORT_CPU + 0) /* CPU Port 65 */
 #define SPX5_PORT_CPU_1        (SPX5_PORT_CPU + 1) /* CPU Port 66 */
 #define SPX5_PORT_VD0          (SPX5_PORT_CPU + 2) /* VD0/Port 67 used for IPMC */
 #define SPX5_PORT_VD1          (SPX5_PORT_CPU + 3) /* VD1/Port 68 used for AFI/OAM */
 #define SPX5_PORT_VD2          (SPX5_PORT_CPU + 4) /* VD2/Port 69 used for IPinIP*/
-#define SPX5_PORTS_ALL         (SPX5_PORT_CPU + 5) /* Total number of ports */
 
 #define PGID_BASE              SPX5_PORTS /* Starts after port PGIDs */
 #define PGID_UC_FLOOD          (PGID_BASE + 0)

-- 
2.34.1

Re: [PATCH net-next 04/15] net: sparx5: modify SPX5_PORTS_ALL macro

[Jacob Keller]

On 10/1/2024 6:50 AM, Daniel Machon wrote:
> In preparation for lan969x, we need to define the SPX5_PORTS_ALL macro
> as 70 (65 front ports + 5 internal ports). This is required as the
> SPX5_PORT_CPU will be redefined as a non-constant in a subsequent patch.
> And as SPX5_PORTS_ALL is used as an array size troughout the code, we
> have to make sure that it stays a constant.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

[PATCH net-next 05/15] net: sparx5: add *sparx5 argument to a few functions

[Daniel Machon]

The *sparx5 context pointer is required in functions that need to use
platform specific data using the SPX5_CONST macro (which will be added
in a subsequent patch). Prepare for this by updating the prototype and
use of such functions.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 .../net/ethernet/microchip/sparx5/sparx5_ethtool.c | 24 +++++++++----------
 .../net/ethernet/microchip/sparx5/sparx5_main.h    |  2 +-
 .../net/ethernet/microchip/sparx5/sparx5_netdev.c  |  2 +-
 .../net/ethernet/microchip/sparx5/sparx5_packet.c  |  2 +-
 .../net/ethernet/microchip/sparx5/sparx5_port.c    | 28 +++++++++++-----------
 .../net/ethernet/microchip/sparx5/sparx5_port.h    |  6 ++---
 .../net/ethernet/microchip/sparx5/sparx5_psfp.c    | 24 +++++++++----------
 drivers/net/ethernet/microchip/sparx5/sparx5_tc.c  |  8 +++----
 8 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c b/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c
index d898a7238b48..ca97d51e6a8d 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c
@@ -505,8 +505,8 @@ static void sparx5_get_dev_misc_stats(u64 *portstats, void __iomem *inst, u32
 static void sparx5_get_device_stats(struct sparx5 *sparx5, int portno)
 {
 	u64 *portstats = &sparx5->stats[portno * sparx5->num_stats];
-	u32 tinst = sparx5_port_dev_index(portno);
-	u32 dev = sparx5_to_high_dev(portno);
+	u32 tinst = sparx5_port_dev_index(sparx5, portno);
+	u32 dev = sparx5_to_high_dev(sparx5, portno);
 	void __iomem *inst;
 
 	inst = spx5_inst_get(sparx5, dev, tinst);
@@ -819,8 +819,8 @@ static void sparx5_get_eth_phy_stats(struct net_device *ndev,
 
 	portstats = &sparx5->stats[portno * sparx5->num_stats];
 	if (sparx5_is_baser(port->conf.portmode)) {
-		u32 tinst = sparx5_port_dev_index(portno);
-		u32 dev = sparx5_to_high_dev(portno);
+		u32 tinst = sparx5_port_dev_index(sparx5, portno);
+		u32 dev = sparx5_to_high_dev(sparx5, portno);
 
 		inst = spx5_inst_get(sparx5, dev, tinst);
 		sparx5_get_dev_phy_stats(portstats, inst, tinst);
@@ -844,8 +844,8 @@ static void sparx5_get_eth_mac_stats(struct net_device *ndev,
 
 	portstats = &sparx5->stats[portno * sparx5->num_stats];
 	if (sparx5_is_baser(port->conf.portmode)) {
-		u32 tinst = sparx5_port_dev_index(portno);
-		u32 dev = sparx5_to_high_dev(portno);
+		u32 tinst = sparx5_port_dev_index(sparx5, portno);
+		u32 dev = sparx5_to_high_dev(sparx5, portno);
 
 		inst = spx5_inst_get(sparx5, dev, tinst);
 		sparx5_get_dev_mac_stats(portstats, inst, tinst);
@@ -912,8 +912,8 @@ static void sparx5_get_eth_mac_ctrl_stats(struct net_device *ndev,
 
 	portstats = &sparx5->stats[portno * sparx5->num_stats];
 	if (sparx5_is_baser(port->conf.portmode)) {
-		u32 tinst = sparx5_port_dev_index(portno);
-		u32 dev = sparx5_to_high_dev(portno);
+		u32 tinst = sparx5_port_dev_index(sparx5, portno);
+		u32 dev = sparx5_to_high_dev(sparx5, portno);
 
 		inst = spx5_inst_get(sparx5, dev, tinst);
 		sparx5_get_dev_mac_ctrl_stats(portstats, inst, tinst);
@@ -944,8 +944,8 @@ static void sparx5_get_eth_rmon_stats(struct net_device *ndev,
 
 	portstats = &sparx5->stats[portno * sparx5->num_stats];
 	if (sparx5_is_baser(port->conf.portmode)) {
-		u32 tinst = sparx5_port_dev_index(portno);
-		u32 dev = sparx5_to_high_dev(portno);
+		u32 tinst = sparx5_port_dev_index(sparx5, portno);
+		u32 dev = sparx5_to_high_dev(sparx5, portno);
 
 		inst = spx5_inst_get(sparx5, dev, tinst);
 		sparx5_get_dev_rmon_stats(portstats, inst, tinst);
@@ -1027,8 +1027,8 @@ static void sparx5_get_sset_data(struct net_device *ndev,
 
 	portstats = &sparx5->stats[portno * sparx5->num_stats];
 	if (sparx5_is_baser(port->conf.portmode)) {
-		u32 tinst = sparx5_port_dev_index(portno);
-		u32 dev = sparx5_to_high_dev(portno);
+		u32 tinst = sparx5_port_dev_index(sparx5, portno);
+		u32 dev = sparx5_to_high_dev(sparx5, portno);
 
 		inst = spx5_inst_get(sparx5, dev, tinst);
 		sparx5_get_dev_misc_stats(portstats, inst, tinst);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 824849869f61..738b86999fd8 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -404,7 +404,7 @@ void sparx5_set_port_ifh_timestamp(void *ifh_hdr, u64 timestamp);
 void sparx5_set_port_ifh_rew_op(void *ifh_hdr, u32 rew_op);
 void sparx5_set_port_ifh_pdu_type(void *ifh_hdr, u32 pdu_type);
 void sparx5_set_port_ifh_pdu_w16_offset(void *ifh_hdr, u32 pdu_w16_offset);
-void sparx5_set_port_ifh(void *ifh_hdr, u16 portno);
+void sparx5_set_port_ifh(struct sparx5 *sparx5, void *ifh_hdr, u16 portno);
 bool sparx5_netdevice_check(const struct net_device *dev);
 struct net_device *sparx5_create_netdev(struct sparx5 *sparx5, u32 portno);
 int sparx5_register_netdevs(struct sparx5 *sparx5);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
index 705a004b324f..3ae6bad3bbb3 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
@@ -55,7 +55,7 @@ static void __ifh_encode_bitfield(void *ifh, u64 value, u32 pos, u32 width)
 		ifh_hdr[byte - 5] |= (u8)((encode & 0xFF0000000000) >> 40);
 }
 
-void sparx5_set_port_ifh(void *ifh_hdr, u16 portno)
+void sparx5_set_port_ifh(struct sparx5 *sparx5, void *ifh_hdr, u16 portno)
 {
 	/* VSTAX.RSV = 1. MSBit must be 1 */
 	ifh_encode_bitfield(ifh_hdr, 1, VSTAX + 79,  1);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
index 70427643f777..e637834b56df 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
@@ -235,7 +235,7 @@ netdev_tx_t sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev)
 	netdev_tx_t ret;
 
 	memset(ifh, 0, IFH_LEN * 4);
-	sparx5_set_port_ifh(ifh, port->portno);
+	sparx5_set_port_ifh(sparx5, ifh, port->portno);
 
 	if (sparx5->ptp && skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {
 		if (sparx5_ptp_txtstamp_request(port, skb) < 0)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index 062e486c002c..ea3454342665 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -132,8 +132,8 @@ static int sparx5_get_sfi_status(struct sparx5 *sparx5,
 		return -EINVAL;
 	}
 
-	dev = sparx5_to_high_dev(portno);
-	tinst = sparx5_port_dev_index(portno);
+	dev = sparx5_to_high_dev(sparx5, portno);
+	tinst = sparx5_port_dev_index(sparx5, portno);
 	inst = spx5_inst_get(sparx5, dev, tinst);
 
 	value = spx5_inst_rd(inst, DEV10G_MAC_TX_MONITOR_STICKY(0));
@@ -316,9 +316,9 @@ static int sparx5_port_flush_poll(struct sparx5 *sparx5, u32 portno)
 static int sparx5_port_disable(struct sparx5 *sparx5, struct sparx5_port *port, bool high_spd_dev)
 {
 	u32 tinst = high_spd_dev ?
-		    sparx5_port_dev_index(port->portno) : port->portno;
+		    sparx5_port_dev_index(sparx5, port->portno) : port->portno;
 	u32 dev = high_spd_dev ?
-		  sparx5_to_high_dev(port->portno) : TARGET_DEV2G5;
+		  sparx5_to_high_dev(sparx5, port->portno) : TARGET_DEV2G5;
 	void __iomem *devinst = spx5_inst_get(sparx5, dev, tinst);
 	u32 spd = port->conf.speed;
 	u32 spd_prm;
@@ -427,7 +427,7 @@ static int sparx5_port_disable(struct sparx5 *sparx5, struct sparx5_port *port,
 		 HSCH_FLUSH_CTRL);
 
 	if (high_spd_dev) {
-		u32 pcs = sparx5_to_pcs_dev(port->portno);
+		u32 pcs = sparx5_to_pcs_dev(sparx5, port->portno);
 		void __iomem *pcsinst = spx5_inst_get(sparx5, pcs, tinst);
 
 		/* 12: Disable 5G/10G/25 BaseR PCS */
@@ -558,8 +558,8 @@ static int sparx5_port_max_tags_set(struct sparx5 *sparx5,
 	bool dtag           = max_tags == SPX5_PORT_MAX_TAGS_TWO;
 	enum sparx5_vlan_port_type vlan_type  = port->vlan_type;
 	bool dotag          = max_tags != SPX5_PORT_MAX_TAGS_NONE;
-	u32 dev             = sparx5_to_high_dev(port->portno);
-	u32 tinst           = sparx5_port_dev_index(port->portno);
+	u32 dev             = sparx5_to_high_dev(sparx5, port->portno);
+	u32 tinst           = sparx5_port_dev_index(sparx5, port->portno);
 	void __iomem *inst  = spx5_inst_get(sparx5, dev, tinst);
 	u32 etype;
 
@@ -789,9 +789,9 @@ static int sparx5_port_pcs_high_set(struct sparx5 *sparx5,
 				    struct sparx5_port_config *conf)
 {
 	u32 clk_spd = conf->portmode == PHY_INTERFACE_MODE_5GBASER ? 1 : 0;
-	u32 pix = sparx5_port_dev_index(port->portno);
-	u32 dev = sparx5_to_high_dev(port->portno);
-	u32 pcs = sparx5_to_pcs_dev(port->portno);
+	u32 pix = sparx5_port_dev_index(sparx5, port->portno);
+	u32 dev = sparx5_to_high_dev(sparx5, port->portno);
+	u32 pcs = sparx5_to_pcs_dev(sparx5, port->portno);
 	void __iomem *devinst;
 	void __iomem *pcsinst;
 	int err;
@@ -843,7 +843,7 @@ static int sparx5_port_pcs_high_set(struct sparx5 *sparx5,
 /* Switch between 1G/2500 and 5G/10G/25G devices */
 static void sparx5_dev_switch(struct sparx5 *sparx5, int port, bool hsd)
 {
-	int bt_indx = BIT(sparx5_port_dev_index(port));
+	int bt_indx = BIT(sparx5_port_dev_index(sparx5, port));
 
 	if (sparx5_port_is_5g(port)) {
 		spx5_rmw(hsd ? 0 : bt_indx,
@@ -1016,9 +1016,9 @@ int sparx5_port_init(struct sparx5 *sparx5,
 {
 	u32 pause_start = sparx5_wm_enc(6  * (ETH_MAXLEN / SPX5_BUFFER_CELL_SZ));
 	u32 atop = sparx5_wm_enc(20 * (ETH_MAXLEN / SPX5_BUFFER_CELL_SZ));
-	u32 devhigh = sparx5_to_high_dev(port->portno);
-	u32 pix = sparx5_port_dev_index(port->portno);
-	u32 pcs = sparx5_to_pcs_dev(port->portno);
+	u32 devhigh = sparx5_to_high_dev(sparx5, port->portno);
+	u32 pix = sparx5_port_dev_index(sparx5, port->portno);
+	u32 pcs = sparx5_to_pcs_dev(sparx5, port->portno);
 	bool sd_pol = port->signd_active_high;
 	bool sd_sel = !port->signd_internal;
 	bool sd_ena = port->signd_enable;
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
index 607c4ff1df6b..468e3d34d6e1 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
@@ -40,7 +40,7 @@ static inline bool sparx5_port_is_25g(int portno)
 	return portno >= 56 && portno <= 63;
 }
 
-static inline u32 sparx5_to_high_dev(int port)
+static inline u32 sparx5_to_high_dev(struct sparx5 *sparx5, int port)
 {
 	if (sparx5_port_is_5g(port))
 		return TARGET_DEV5G;
@@ -49,7 +49,7 @@ static inline u32 sparx5_to_high_dev(int port)
 	return TARGET_DEV25G;
 }
 
-static inline u32 sparx5_to_pcs_dev(int port)
+static inline u32 sparx5_to_pcs_dev(struct sparx5 *sparx5, int port)
 {
 	if (sparx5_port_is_5g(port))
 		return TARGET_PCS5G_BR;
@@ -58,7 +58,7 @@ static inline u32 sparx5_to_pcs_dev(int port)
 	return TARGET_PCS25G_BR;
 }
 
-static inline int sparx5_port_dev_index(int port)
+static inline int sparx5_port_dev_index(struct sparx5 *sparx5, int port)
 {
 	if (sparx5_port_is_2g5(port))
 		return port;
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c b/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c
index 8dee1ab1fa75..5d9c7b782352 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c
@@ -20,34 +20,34 @@ static struct sparx5_pool_entry sparx5_psfp_sg_pool[SPX5_PSFP_SG_CNT];
 /* Pool of available stream filters */
 static struct sparx5_pool_entry sparx5_psfp_sf_pool[SPX5_PSFP_SF_CNT];
 
-static int sparx5_psfp_sf_get(u32 *id)
+static int sparx5_psfp_sf_get(struct sparx5 *sparx5, u32 *id)
 {
 	return sparx5_pool_get(sparx5_psfp_sf_pool, SPX5_PSFP_SF_CNT, id);
 }
 
-static int sparx5_psfp_sf_put(u32 id)
+static int sparx5_psfp_sf_put(struct sparx5 *sparx5, u32 id)
 {
 	return sparx5_pool_put(sparx5_psfp_sf_pool, SPX5_PSFP_SF_CNT, id);
 }
 
-static int sparx5_psfp_sg_get(u32 idx, u32 *id)
+static int sparx5_psfp_sg_get(struct sparx5 *sparx5, u32 idx, u32 *id)
 {
 	return sparx5_pool_get_with_idx(sparx5_psfp_sg_pool, SPX5_PSFP_SG_CNT,
 					idx, id);
 }
 
-static int sparx5_psfp_sg_put(u32 id)
+static int sparx5_psfp_sg_put(struct sparx5 *sparx5, u32 id)
 {
 	return sparx5_pool_put(sparx5_psfp_sg_pool, SPX5_PSFP_SG_CNT, id);
 }
 
-static int sparx5_psfp_fm_get(u32 idx, u32 *id)
+static int sparx5_psfp_fm_get(struct sparx5 *sparx5, u32 idx, u32 *id)
 {
 	return sparx5_pool_get_with_idx(sparx5_psfp_fm_pool, SPX5_SDLB_CNT, idx,
 					id);
 }
 
-static int sparx5_psfp_fm_put(u32 id)
+static int sparx5_psfp_fm_put(struct sparx5 *sparx5, u32 id)
 {
 	return sparx5_pool_put(sparx5_psfp_fm_pool, SPX5_SDLB_CNT, id);
 }
@@ -205,7 +205,7 @@ int sparx5_psfp_sf_add(struct sparx5 *sparx5, const struct sparx5_psfp_sf *sf,
 {
 	int ret;
 
-	ret = sparx5_psfp_sf_get(id);
+	ret = sparx5_psfp_sf_get(sparx5, id);
 	if (ret < 0)
 		return ret;
 
@@ -220,7 +220,7 @@ int sparx5_psfp_sf_del(struct sparx5 *sparx5, u32 id)
 
 	sparx5_psfp_sf_set(sparx5, id, &sf);
 
-	return sparx5_psfp_sf_put(id);
+	return sparx5_psfp_sf_put(sparx5, id);
 }
 
 int sparx5_psfp_sg_add(struct sparx5 *sparx5, u32 uidx,
@@ -229,7 +229,7 @@ int sparx5_psfp_sg_add(struct sparx5 *sparx5, u32 uidx,
 	ktime_t basetime;
 	int ret;
 
-	ret = sparx5_psfp_sg_get(uidx, id);
+	ret = sparx5_psfp_sg_get(sparx5, uidx, id);
 	if (ret < 0)
 		return ret;
 	/* Was already in use, no need to reconfigure */
@@ -253,7 +253,7 @@ int sparx5_psfp_sg_del(struct sparx5 *sparx5, u32 id)
 	const struct sparx5_psfp_sg sg = { 0 };
 	int ret;
 
-	ret = sparx5_psfp_sg_put(id);
+	ret = sparx5_psfp_sg_put(sparx5, id);
 	if (ret < 0)
 		return ret;
 	/* Stream gate still in use ? */
@@ -270,7 +270,7 @@ int sparx5_psfp_fm_add(struct sparx5 *sparx5, u32 uidx,
 	int ret;
 
 	/* Get flow meter */
-	ret = sparx5_psfp_fm_get(uidx, &fm->pol.idx);
+	ret = sparx5_psfp_fm_get(sparx5, uidx, &fm->pol.idx);
 	if (ret < 0)
 		return ret;
 	/* Was already in use, no need to reconfigure */
@@ -303,7 +303,7 @@ int sparx5_psfp_fm_del(struct sparx5 *sparx5, u32 id)
 	if (ret < 0)
 		return ret;
 
-	ret = sparx5_psfp_fm_put(id);
+	ret = sparx5_psfp_fm_put(sparx5, id);
 	if (ret < 0)
 		return ret;
 	/* Do not reset flow-meter if still in use. */
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_tc.c b/drivers/net/ethernet/microchip/sparx5/sparx5_tc.c
index e80f3166db7d..28b2514c8330 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_tc.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_tc.c
@@ -60,8 +60,8 @@ static int sparx5_tc_setup_block(struct net_device *ndev,
 					  cb, ndev, ndev, false);
 }
 
-static void sparx5_tc_get_layer_and_idx(u32 parent, u32 portno, u32 *layer,
-					u32 *idx)
+static void sparx5_tc_get_layer_and_idx(struct sparx5 *sparx5, u32 parent,
+					u32 portno, u32 *layer, u32 *idx)
 {
 	if (parent == TC_H_ROOT) {
 		*layer = 2;
@@ -90,8 +90,8 @@ static int sparx5_tc_setup_qdisc_tbf(struct net_device *ndev,
 	struct sparx5_port *port = netdev_priv(ndev);
 	u32 layer, se_idx;
 
-	sparx5_tc_get_layer_and_idx(qopt->parent, port->portno, &layer,
-				    &se_idx);
+	sparx5_tc_get_layer_and_idx(port->sparx5, qopt->parent, port->portno,
+				    &layer, &se_idx);
 
 	switch (qopt->command) {
 	case TC_TBF_REPLACE:

-- 
2.34.1

[PATCH net-next 06/15] net: sparx5: add constants to match data

[Daniel Machon]

Add new struct sparx5_consts, containing all the chip constants that are
known to be different for Sparx5 and lan969x. Also add a macro to access
the constants.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 .../net/ethernet/microchip/sparx5/sparx5_main.c    | 21 ++++++++++++++++++++
 .../net/ethernet/microchip/sparx5/sparx5_main.h    | 23 ++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index 9a8d2e8c02a5..5f3690a59ac1 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -953,11 +953,32 @@ static const struct sparx5_regs sparx5_regs = {
 	.fsize = sparx5_fsize,
 };
 
+static const struct sparx5_consts sparx5_consts = {
+	.n_ports             = 65,
+	.n_ports_all         = 70,
+	.n_hsch_l1_elems     = 64,
+	.n_hsch_queues       = 8,
+	.n_lb_groups         = 10,
+	.n_pgids             = 2113, /* (2048 + n_ports) */
+	.n_sio_clks          = 3,
+	.n_own_upsids        = 3,
+	.n_auto_cals         = 7,
+	.n_filters           = 1024,
+	.n_gates             = 1024,
+	.n_sdlbs             = 4096,
+	.n_dsm_cal_taxis     = 8,
+	.buf_size            = 4194280,
+	.qres_max_prio_idx   = 630,
+	.qres_max_colour_idx = 638,
+	.tod_pin             = 4,
+};
+
 static const struct sparx5_match_data sparx5_desc = {
 	.iomap = sparx5_main_iomap,
 	.iomap_size = ARRAY_SIZE(sparx5_main_iomap),
 	.ioranges = 3,
 	.regs = &sparx5_regs,
+	.consts = &sparx5_consts,
 };
 
 static const struct of_device_id mchp_sparx5_match[] = {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 738b86999fd8..91f5a3be829e 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -51,6 +51,8 @@ enum sparx5_vlan_port_type {
 	SPX5_VLAN_PORT_TYPE_S_CUSTOM /* S-port using custom type */
 };
 
+#define SPX5_CONST(const) sparx5->data->consts->const
+
 #define SPX5_PORTS             65
 #define SPX5_PORTS_ALL         70 /* Total number of ports */
 
@@ -238,6 +240,26 @@ struct sparx5_regs {
 	const unsigned int *fsize;
 };
 
+struct sparx5_consts {
+	u32 n_ports;             /* Number of front ports */
+	u32 n_ports_all;         /* Number of front ports + internal ports */
+	u32 n_hsch_l1_elems;     /* Number of HSCH layer 1 elements */
+	u32 n_hsch_queues;       /* Number of HSCH queues */
+	u32 n_lb_groups;         /* Number of leacky bucket groupd */
+	u32 n_pgids;             /* Number of PGID's */
+	u32 n_sio_clks;          /* Number of serial IO clocks */
+	u32 n_own_upsids;        /* Number of own UPSID's */
+	u32 n_auto_cals;         /* Number of auto calendars */
+	u32 n_filters;           /* Number of PSFP filters */
+	u32 n_gates;             /* Number of PSFP gates */
+	u32 n_sdlbs;             /* Number of service dual leaky buckets */
+	u32 n_dsm_cal_taxis;     /* Number of DSM calendar taxis */
+	u32 buf_size;            /* Amount of QLIM watermark memory */
+	u32 qres_max_prio_idx;   /* Maximum QRES prio index */
+	u32 qres_max_colour_idx; /* Maximum QRES colour index */
+	u32 tod_pin;             /* PTP TOD pin */
+};
+
 struct sparx5_main_io_resource {
 	enum sparx5_target id;
 	phys_addr_t offset;
@@ -246,6 +268,7 @@ struct sparx5_main_io_resource {
 
 struct sparx5_match_data {
 	const struct sparx5_regs *regs;
+	const struct sparx5_consts *consts;
 	const struct sparx5_main_io_resource *iomap;
 	int ioranges;
 	int iomap_size;

-- 
2.34.1

Re: [PATCH net-next 06/15] net: sparx5: add constants to match data

[Jacob Keller]

On 10/1/2024 6:50 AM, Daniel Machon wrote:
> Add new struct sparx5_consts, containing all the chip constants that are
> known to be different for Sparx5 and lan969x. Also add a macro to access
> the constants.
> 
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
> ---
>  .../net/ethernet/microchip/sparx5/sparx5_main.c    | 21 ++++++++++++++++++++
>  .../net/ethernet/microchip/sparx5/sparx5_main.h    | 23 ++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> index 9a8d2e8c02a5..5f3690a59ac1 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> @@ -953,11 +953,32 @@ static const struct sparx5_regs sparx5_regs = {
>  	.fsize = sparx5_fsize,
>  };
>  
> +static const struct sparx5_consts sparx5_consts = {
> +	.n_ports             = 65,
> +	.n_ports_all         = 70,
> +	.n_hsch_l1_elems     = 64,
> +	.n_hsch_queues       = 8,
> +	.n_lb_groups         = 10,
> +	.n_pgids             = 2113, /* (2048 + n_ports) */
> +	.n_sio_clks          = 3,
> +	.n_own_upsids        = 3,
> +	.n_auto_cals         = 7,
> +	.n_filters           = 1024,
> +	.n_gates             = 1024,
> +	.n_sdlbs             = 4096,
> +	.n_dsm_cal_taxis     = 8,
> +	.buf_size            = 4194280,
> +	.qres_max_prio_idx   = 630,
> +	.qres_max_colour_idx = 638,
> +	.tod_pin             = 4,
> +};
> +
>  static const struct sparx5_match_data sparx5_desc = {
>  	.iomap = sparx5_main_iomap,
>  	.iomap_size = ARRAY_SIZE(sparx5_main_iomap),
>  	.ioranges = 3,
>  	.regs = &sparx5_regs,
> +	.consts = &sparx5_consts,
>  };
>  
>  static const struct of_device_id mchp_sparx5_match[] = {
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> index 738b86999fd8..91f5a3be829e 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> @@ -51,6 +51,8 @@ enum sparx5_vlan_port_type {
>  	SPX5_VLAN_PORT_TYPE_S_CUSTOM /* S-port using custom type */
>  };
>  
> +#define SPX5_CONST(const) sparx5->data->consts->const
> +

I'm not a fan of implicit dependency here. Whats the reason for having
it done this way vs passing something in as a parameter here?

Re: [PATCH net-next 06/15] net: sparx5: add constants to match data

[Jakub Kicinski]

On Tue, 1 Oct 2024 15:50:36 +0200 Daniel Machon wrote:
> +#define SPX5_CONST(const) sparx5->data->consts->const

This is way too ugly too live.
Please type the code out, there's no prize for having low LoC count.

Re: [PATCH net-next 06/15] net: sparx5: add constants to match data

[Daniel Machon]

> > +#define SPX5_CONST(const) sparx5->data->consts->const
> 
> This is way too ugly too live.
> Please type the code out, there's no prize for having low LoC count.
>

Hi Jakub,

By "type the code out" - are you saying that we should not be using a macro
for accessing the const at all? and rather just:

    struct sparx5_consts *consts = sparx5->data->consts;
    consts->some_var

or pass in the sparx5 pointer to the macro too, which was the concert that
Jacob raised.

Thanks.

/Daniel

Re: [PATCH net-next 06/15] net: sparx5: add constants to match data

[Jakub Kicinski]

On Wed, 2 Oct 2024 13:31:32 +0000 Daniel Machon wrote:
> By "type the code out" - are you saying that we should not be using a macro
> for accessing the const at all? and rather just:
> 
>     struct sparx5_consts *consts = sparx5->data->consts;
>     consts->some_var

This.

> or pass in the sparx5 pointer to the macro too, which was the concert that
> Jacob raised.

The implicit arguments are part of the ugliness, and should also go
away. But in this case the entire macro should go.

Re: [PATCH net-next 06/15] net: sparx5: add constants to match data

[Daniel Machon]

> > By "type the code out" - are you saying that we should not be using a macro
> > for accessing the const at all? and rather just:
> >
> >     struct sparx5_consts *consts = sparx5->data->consts;
> >     consts->some_var
> 
> This.
> 
> > or pass in the sparx5 pointer to the macro too, which was the concert that
> > Jacob raised.
> 
> The implicit arguments are part of the ugliness, and should also go
> away. But in this case the entire macro should go.

Ack.

Will get rid of the macro in v2.

/Daniel

[PATCH net-next 07/15] net: sparx5: use SPX5_CONST for constants which already have a symbol

[Daniel Machon]

Now that we have indentified all the chip constants, update the use of
them where a symbol is already defined for the constant.

Constants are accessed using the SPX5_CONSTS macro. Note that this macro
might hide the use of the *sparx5 context pointer. In such case, a
comment is added.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 .../net/ethernet/microchip/sparx5/sparx5_calendar.c    | 12 ++++++------
 drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c     |  5 +++--
 drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c |  7 ++++---
 drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c    |  6 ++++--
 .../net/ethernet/microchip/sparx5/sparx5_mactable.c    |  6 +++---
 drivers/net/ethernet/microchip/sparx5/sparx5_main.c    |  6 +++---
 drivers/net/ethernet/microchip/sparx5/sparx5_main.h    |  6 +++---
 drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c  |  6 +++---
 drivers/net/ethernet/microchip/sparx5/sparx5_packet.c  |  2 +-
 drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c    |  6 +++---
 drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c    | 18 +++++++++---------
 drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c     |  6 +++---
 drivers/net/ethernet/microchip/sparx5/sparx5_qos.h     |  2 +-
 drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c    |  4 ++--
 .../net/ethernet/microchip/sparx5/sparx5_switchdev.c   |  5 ++++-
 drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c    |  2 +-
 16 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c b/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
index 76a8bb596aec..b7b611b1ad34 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
@@ -131,7 +131,7 @@ static enum sparx5_cal_bw sparx5_get_port_cal_speed(struct sparx5 *sparx5,
 {
 	struct sparx5_port *port;
 
-	if (portno >= SPX5_PORTS) {
+	if (portno >= SPX5_CONST(n_ports)) {
 		/* Internal ports */
 		if (portno == SPX5_PORT_CPU_0 || portno == SPX5_PORT_CPU_1) {
 			/* Equals 1.25G */
@@ -174,7 +174,7 @@ int sparx5_config_auto_calendar(struct sparx5 *sparx5)
 	}
 
 	/* Setup the calendar with the bandwidth to each port */
-	for (portno = 0; portno < SPX5_PORTS_ALL; portno++) {
+	for (portno = 0; portno < SPX5_CONST(n_ports_all); portno++) {
 		u64 reg, offset, this_bw;
 
 		spd = sparx5_get_port_cal_speed(sparx5, portno);
@@ -182,7 +182,7 @@ int sparx5_config_auto_calendar(struct sparx5 *sparx5)
 			continue;
 
 		this_bw = sparx5_cal_speed_to_value(spd);
-		if (portno < SPX5_PORTS)
+		if (portno < SPX5_CONST(n_ports))
 			used_port_bw += this_bw;
 		else
 			/* Internal ports are granted half the value */
@@ -213,7 +213,7 @@ int sparx5_config_auto_calendar(struct sparx5 *sparx5)
 		 sparx5, QSYS_CAL_CTRL);
 
 	/* Assign port bandwidth to auto calendar */
-	for (idx = 0; idx < ARRAY_SIZE(cal); idx++)
+	for (idx = 0; idx < SPX5_CONST(n_auto_cals); idx++)
 		spx5_wr(cal[idx], sparx5, QSYS_CAL_AUTO(idx));
 
 	/* Increase grant rate of all ports to account for
@@ -304,7 +304,7 @@ static int sparx5_dsm_calendar_calc(struct sparx5 *sparx5, u32 taxi,
 	for (idx = 0; idx < SPX5_DSM_CAL_MAX_DEVS_PER_TAXI; idx++) {
 		u32 portno = data->taxi_ports[idx];
 
-		if (portno < SPX5_TAXI_PORT_MAX) {
+		if (portno < SPX5_CONST(n_ports_all)) {
 			data->taxi_speeds[idx] = sparx5_cal_speed_to_value
 				(sparx5_get_port_cal_speed(sparx5, portno));
 		} else {
@@ -573,7 +573,7 @@ int sparx5_config_dsm_calendar(struct sparx5 *sparx5)
 	if (!data)
 		return -ENOMEM;
 
-	for (taxi = 0; taxi < SPX5_DSM_CAL_TAXIS; ++taxi) {
+	for (taxi = 0; taxi < SPX5_CONST(n_dsm_cal_taxis); ++taxi) {
 		err = sparx5_dsm_calendar_calc(sparx5, taxi, data);
 		if (err) {
 			dev_err(sparx5->dev, "DSM calendar calculation failed\n");
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
index 2d763664dcda..9378d4d82480 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
@@ -234,10 +234,11 @@ static int sparx5_dcb_ieee_dscp_setdel(struct net_device *dev,
 						     struct dcb_app *))
 {
 	struct sparx5_port *port = netdev_priv(dev);
+	struct sparx5 *sparx5 = port->sparx5;
 	struct sparx5_port *port_itr;
 	int err, i;
 
-	for (i = 0; i < SPX5_PORTS; i++) {
+	for (i = 0; i < SPX5_CONST(n_ports); i++) {
 		port_itr = port->sparx5->ports[i];
 		if (!port_itr)
 			continue;
@@ -386,7 +387,7 @@ int sparx5_dcb_init(struct sparx5 *sparx5)
 	struct sparx5_port *port;
 	int i;
 
-	for (i = 0; i < SPX5_PORTS; i++) {
+	for (i = 0; i < SPX5_CONST(n_ports); i++) {
 		port = sparx5->ports[i];
 		if (!port)
 			continue;
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c b/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c
index ca97d51e6a8d..4176733179db 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c
@@ -1122,7 +1122,7 @@ static void sparx5_update_stats(struct sparx5 *sparx5)
 {
 	int idx;
 
-	for (idx = 0; idx < SPX5_PORTS; idx++)
+	for (idx = 0; idx < SPX5_CONST(n_ports); idx++)
 		if (sparx5->ports[idx])
 			sparx5_update_port_stats(sparx5, idx);
 }
@@ -1235,14 +1235,15 @@ int sparx_stats_init(struct sparx5 *sparx5)
 	sparx5->num_stats = spx5_stats_count;
 	sparx5->num_ethtool_stats = ARRAY_SIZE(sparx5_stats_layout);
 	sparx5->stats = devm_kcalloc(sparx5->dev,
-				     SPX5_PORTS_ALL * sparx5->num_stats,
+				     SPX5_CONST(n_ports_all) *
+				     sparx5->num_stats,
 				     sizeof(u64), GFP_KERNEL);
 	if (!sparx5->stats)
 		return -ENOMEM;
 
 	mutex_init(&sparx5->queue_stats_lock);
 	sparx5_config_stats(sparx5);
-	for (portno = 0; portno < SPX5_PORTS; portno++)
+	for (portno = 0; portno < SPX5_CONST(n_ports); portno++)
 		if (sparx5->ports[portno])
 			sparx5_config_port_stats(sparx5, portno);
 
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c b/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c
index 61df874b7623..a3ad82cd29aa 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c
@@ -156,7 +156,9 @@ static bool sparx5_fdma_rx_get_frame(struct sparx5 *sparx5, struct sparx5_rx *rx
 	/* Now do the normal processing of the skb */
 	sparx5_ifh_parse((u32 *)skb->data, &fi);
 	/* Map to port netdev */
-	port = fi.src_port < SPX5_PORTS ?  sparx5->ports[fi.src_port] : NULL;
+	port = fi.src_port < SPX5_CONST(n_ports) ?
+		       sparx5->ports[fi.src_port] :
+		       NULL;
 	if (!port || !port->ndev) {
 		dev_err(sparx5->dev, "Data on inactive port %d\n", fi.src_port);
 		sparx5_xtr_flush(sparx5, XTR_QUEUE);
@@ -296,7 +298,7 @@ static void sparx5_fdma_rx_init(struct sparx5 *sparx5,
 	fdma->ops.dataptr_cb = &sparx5_fdma_rx_dataptr_cb;
 	fdma->ops.nextptr_cb = &fdma_nextptr_cb;
 	/* Fetch a netdev for SKB and NAPI use, any will do */
-	for (idx = 0; idx < SPX5_PORTS; ++idx) {
+	for (idx = 0; idx < SPX5_CONST(n_ports); ++idx) {
 		struct sparx5_port *port = sparx5->ports[idx];
 
 		if (port && port->ndev) {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
index 75868b3f548e..2bebca3460ae 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
@@ -82,13 +82,13 @@ int sparx5_mact_learn(struct sparx5 *sparx5, int pgid,
 {
 	int addr, type, ret;
 
-	if (pgid < SPX5_PORTS) {
+	if (pgid < SPX5_CONST(n_ports)) {
 		type = MAC_ENTRY_ADDR_TYPE_UPSID_PN;
 		addr = pgid % 32;
 		addr += (pgid / 32) << 5; /* Add upsid */
 	} else {
 		type = MAC_ENTRY_ADDR_TYPE_MC_IDX;
-		addr = pgid - SPX5_PORTS;
+		addr = pgid - SPX5_CONST(n_ports);
 	}
 
 	mutex_lock(&sparx5->lock);
@@ -371,7 +371,7 @@ static void sparx5_mact_handle_entry(struct sparx5 *sparx5,
 		return;
 
 	port = LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_ADDR_GET(cfg2);
-	if (port >= SPX5_PORTS)
+	if (port >= SPX5_CONST(n_ports))
 		return;
 
 	if (!test_bit(port, sparx5->bridge_mask))
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index 5f3690a59ac1..063f02fd36c3 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -32,7 +32,7 @@
 const struct sparx5_regs *regs;
 
 #define QLIM_WM(fraction) \
-	((SPX5_BUFFER_MEMORY / SPX5_BUFFER_CELL_SZ - 100) * (fraction) / 100)
+	((SPX5_CONST(buf_size) / SPX5_BUFFER_CELL_SZ - 100) * (fraction) / 100)
 #define IO_RANGES 3
 
 struct initial_port_config {
@@ -584,7 +584,7 @@ static void sparx5_board_init(struct sparx5 *sparx5)
 		 GCB_HW_SGPIO_SD_CFG);
 
 	/* Refer to LOS SGPIO */
-	for (idx = 0; idx < SPX5_PORTS; idx++)
+	for (idx = 0; idx < SPX5_CONST(n_ports); idx++)
 		if (sparx5->ports[idx])
 			if (sparx5->ports[idx]->conf.sd_sgpio != ~0)
 				spx5_wr(sparx5->ports[idx]->conf.sd_sgpio,
@@ -608,7 +608,7 @@ static int sparx5_start(struct sparx5 *sparx5)
 	}
 
 	/* Enable CPU ports */
-	for (idx = SPX5_PORTS; idx < SPX5_PORTS_ALL; idx++)
+	for (idx = SPX5_CONST(n_ports); idx < SPX5_CONST(n_ports_all); idx++)
 		spx5_rmw(QFWD_SWITCH_PORT_MODE_PORT_ENA_SET(1),
 			 QFWD_SWITCH_PORT_MODE_PORT_ENA,
 			 sparx5,
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 91f5a3be829e..8398f32ffaad 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -56,14 +56,14 @@ enum sparx5_vlan_port_type {
 #define SPX5_PORTS             65
 #define SPX5_PORTS_ALL         70 /* Total number of ports */
 
-#define SPX5_PORT_CPU          (SPX5_PORTS)  /* Next port is CPU port */
+#define SPX5_PORT_CPU          SPX5_CONST(n_ports)  /* Next port is CPU port */
 #define SPX5_PORT_CPU_0        (SPX5_PORT_CPU + 0) /* CPU Port 65 */
 #define SPX5_PORT_CPU_1        (SPX5_PORT_CPU + 1) /* CPU Port 66 */
 #define SPX5_PORT_VD0          (SPX5_PORT_CPU + 2) /* VD0/Port 67 used for IPMC */
 #define SPX5_PORT_VD1          (SPX5_PORT_CPU + 3) /* VD1/Port 68 used for AFI/OAM */
 #define SPX5_PORT_VD2          (SPX5_PORT_CPU + 4) /* VD2/Port 69 used for IPinIP*/
 
-#define PGID_BASE              SPX5_PORTS /* Starts after port PGIDs */
+#define PGID_BASE              SPX5_CONST(n_ports) /* Starts after port PGIDs */
 #define PGID_UC_FLOOD          (PGID_BASE + 0)
 #define PGID_MC_FLOOD          (PGID_BASE + 1)
 #define PGID_IPV4_MC_DATA      (PGID_BASE + 2)
@@ -532,7 +532,7 @@ int sparx5_policer_conf_set(struct sparx5 *sparx5, struct sparx5_policer *pol);
 #define SPX5_PSFP_SG_MIN_CYCLE_TIME_NS (1 * NSEC_PER_USEC)
 #define SPX5_PSFP_SG_MAX_CYCLE_TIME_NS ((1 * NSEC_PER_SEC) - 1)
 #define SPX5_PSFP_SG_MAX_IPV (SPX5_PRIOS - 1)
-#define SPX5_PSFP_SG_OPEN (SPX5_PSFP_SG_CNT - 1)
+#define SPX5_PSFP_SG_OPEN (SPX5_CONST(n_gates) - 1)
 #define SPX5_PSFP_SG_CYCLE_TIME_DEFAULT 1000000
 #define SPX5_PSFP_SF_MAX_SDU 16383
 
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
index 3ae6bad3bbb3..321ca9d51100 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
@@ -290,7 +290,7 @@ int sparx5_register_netdevs(struct sparx5 *sparx5)
 	int portno;
 	int err;
 
-	for (portno = 0; portno < SPX5_PORTS; portno++)
+	for (portno = 0; portno < SPX5_CONST(n_ports); portno++)
 		if (sparx5->ports[portno]) {
 			err = register_netdev(sparx5->ports[portno]->ndev);
 			if (err) {
@@ -309,7 +309,7 @@ void sparx5_destroy_netdevs(struct sparx5 *sparx5)
 	struct sparx5_port *port;
 	int portno;
 
-	for (portno = 0; portno < SPX5_PORTS; portno++) {
+	for (portno = 0; portno < SPX5_CONST(n_ports); portno++) {
 		port = sparx5->ports[portno];
 		if (port && port->phylink) {
 			/* Disconnect the phy */
@@ -327,7 +327,7 @@ void sparx5_unregister_netdevs(struct sparx5 *sparx5)
 {
 	int portno;
 
-	for (portno = 0; portno < SPX5_PORTS; portno++)
+	for (portno = 0; portno < SPX5_CONST(n_ports); portno++)
 		if (sparx5->ports[portno])
 			unregister_netdev(sparx5->ports[portno]->ndev);
 }
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
index e637834b56df..22f6d753f5d1 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
@@ -75,7 +75,7 @@ static void sparx5_xtr_grp(struct sparx5 *sparx5, u8 grp, bool byte_swap)
 	sparx5_ifh_parse(ifh, &fi);
 
 	/* Map to port netdev */
-	port = fi.src_port < SPX5_PORTS ?
+	port = fi.src_port < SPX5_CONST(n_ports) ?
 		sparx5->ports[fi.src_port] : NULL;
 	if (!port || !port->ndev) {
 		dev_err(sparx5->dev, "Data on inactive port %d\n", fi.src_port);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c b/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
index 97adccea5352..56e96555b134 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
@@ -5,7 +5,7 @@ void sparx5_pgid_init(struct sparx5 *sparx5)
 {
 	int i;
 
-	for (i = 0; i < PGID_TABLE_SIZE; i++)
+	for (i = 0; i < SPX5_CONST(n_pgids); i++)
 		sparx5->pgid_map[i] = SPX5_PGID_FREE;
 
 	/* Reserved for unicast, flood control, broadcast, and CPU.
@@ -22,7 +22,7 @@ int sparx5_pgid_alloc_mcast(struct sparx5 *sparx5, u16 *idx)
 	/* The multicast area starts at index 65, but the first 7
 	 * are reserved for flood masks and CPU. Start alloc after that.
 	 */
-	for (i = PGID_MCAST_START; i < PGID_TABLE_SIZE; i++) {
+	for (i = PGID_MCAST_START; i < SPX5_CONST(n_pgids); i++) {
 		if (sparx5->pgid_map[i] == SPX5_PGID_FREE) {
 			sparx5->pgid_map[i] = SPX5_PGID_MULTICAST;
 			*idx = i;
@@ -35,7 +35,7 @@ int sparx5_pgid_alloc_mcast(struct sparx5 *sparx5, u16 *idx)
 
 int sparx5_pgid_free(struct sparx5 *sparx5, u16 idx)
 {
-	if (idx <= PGID_CPU || idx >= PGID_TABLE_SIZE)
+	if (idx <= PGID_CPU || idx >= SPX5_CONST(n_pgids))
 		return -EINVAL;
 
 	if (sparx5->pgid_map[idx] == SPX5_PGID_FREE)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c b/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c
index 5d9c7b782352..58bc4eba996b 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c
@@ -22,34 +22,34 @@ static struct sparx5_pool_entry sparx5_psfp_sf_pool[SPX5_PSFP_SF_CNT];
 
 static int sparx5_psfp_sf_get(struct sparx5 *sparx5, u32 *id)
 {
-	return sparx5_pool_get(sparx5_psfp_sf_pool, SPX5_PSFP_SF_CNT, id);
+	return sparx5_pool_get(sparx5_psfp_sf_pool, SPX5_CONST(n_filters), id);
 }
 
 static int sparx5_psfp_sf_put(struct sparx5 *sparx5, u32 id)
 {
-	return sparx5_pool_put(sparx5_psfp_sf_pool, SPX5_PSFP_SF_CNT, id);
+	return sparx5_pool_put(sparx5_psfp_sf_pool, SPX5_CONST(n_filters), id);
 }
 
 static int sparx5_psfp_sg_get(struct sparx5 *sparx5, u32 idx, u32 *id)
 {
-	return sparx5_pool_get_with_idx(sparx5_psfp_sg_pool, SPX5_PSFP_SG_CNT,
-					idx, id);
+	return sparx5_pool_get_with_idx(sparx5_psfp_sg_pool,
+					SPX5_CONST(n_gates), idx, id);
 }
 
 static int sparx5_psfp_sg_put(struct sparx5 *sparx5, u32 id)
 {
-	return sparx5_pool_put(sparx5_psfp_sg_pool, SPX5_PSFP_SG_CNT, id);
+	return sparx5_pool_put(sparx5_psfp_sg_pool, SPX5_CONST(n_gates), id);
 }
 
 static int sparx5_psfp_fm_get(struct sparx5 *sparx5, u32 idx, u32 *id)
 {
-	return sparx5_pool_get_with_idx(sparx5_psfp_fm_pool, SPX5_SDLB_CNT, idx,
-					id);
+	return sparx5_pool_get_with_idx(sparx5_psfp_fm_pool,
+					SPX5_CONST(n_sdlbs), idx, id);
 }
 
 static int sparx5_psfp_fm_put(struct sparx5 *sparx5, u32 id)
 {
-	return sparx5_pool_put(sparx5_psfp_fm_pool, SPX5_SDLB_CNT, id);
+	return sparx5_pool_put(sparx5_psfp_fm_pool, SPX5_CONST(n_sdlbs), id);
 }
 
 u32 sparx5_psfp_isdx_get_sf(struct sparx5 *sparx5, u32 isdx)
@@ -318,7 +318,7 @@ void sparx5_psfp_init(struct sparx5 *sparx5)
 	const struct sparx5_sdlb_group *group;
 	int i;
 
-	for (i = 0; i < SPX5_SDLB_GROUP_CNT; i++) {
+	for (i = 0; i < SPX5_CONST(n_lb_groups); i++) {
 		group = &sdlb_groups[i];
 		sparx5_sdlb_group_init(sparx5, group->max_rate,
 				       group->min_burst, group->frame_size, i);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c b/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c
index 5a932460db58..d2626fade52c 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c
@@ -13,7 +13,7 @@
 
 #define SPARX5_MAX_PTP_ID	512
 
-#define TOD_ACC_PIN		0x4
+#define TOD_ACC_PIN            SPX5_CONST(tod_pin)
 
 enum {
 	PTP_PIN_ACTION_IDLE = 0,
@@ -630,7 +630,7 @@ int sparx5_ptp_init(struct sparx5 *sparx5)
 	/* Enable master counters */
 	spx5_wr(PTP_PTP_DOM_CFG_PTP_ENA_SET(0x7), sparx5, PTP_PTP_DOM_CFG);
 
-	for (i = 0; i < SPX5_PORTS; i++) {
+	for (i = 0; i < SPX5_CONST(n_ports); i++) {
 		port = sparx5->ports[i];
 		if (!port)
 			continue;
@@ -646,7 +646,7 @@ void sparx5_ptp_deinit(struct sparx5 *sparx5)
 	struct sparx5_port *port;
 	int i;
 
-	for (i = 0; i < SPX5_PORTS; i++) {
+	for (i = 0; i < SPX5_CONST(n_ports); i++) {
 		port = sparx5->ports[i];
 		if (!port)
 			continue;
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h
index ced35033a6c5..8419577cfda0 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h
@@ -14,7 +14,7 @@
 
 /* Scheduling elements per layer */
 #define SPX5_HSCH_L0_SE_CNT 5040
-#define SPX5_HSCH_L1_SE_CNT 64
+#define SPX5_HSCH_L1_SE_CNT SPX5_CONST(n_hsch_l1_elems)
 #define SPX5_HSCH_L2_SE_CNT 64
 
 /* Calculate Layer 0 Scheduler Element when using normal hierarchy */
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c
index f5267218caeb..77fc2a14450d 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c
@@ -184,7 +184,7 @@ int sparx5_sdlb_group_get_by_rate(struct sparx5 *sparx5, u32 rate, u32 burst)
 
 	rate_bps = rate * 1000;
 
-	for (i = SPX5_SDLB_GROUP_CNT - 1; i >= 0; i--) {
+	for (i = SPX5_CONST(n_lb_groups) - 1; i >= 0; i--) {
 		group = &sdlb_groups[i];
 
 		count = sparx5_sdlb_group_get_count(sparx5, i);
@@ -208,7 +208,7 @@ int sparx5_sdlb_group_get_by_index(struct sparx5 *sparx5, u32 idx, u32 *group)
 	u32 itr, next;
 	int i;
 
-	for (i = 0; i < SPX5_SDLB_GROUP_CNT; i++) {
+	for (i = 0; i < SPX5_CONST(n_lb_groups); i++) {
 		if (sparx5_sdlb_group_is_empty(sparx5, i))
 			continue;
 
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
index bcee9adcfbdb..c583479126fa 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
@@ -32,6 +32,7 @@ static int sparx5_port_attr_pre_bridge_flags(struct sparx5_port *port,
 static void sparx5_port_update_mcast_ip_flood(struct sparx5_port *port, bool flood_flag)
 {
 	bool should_flood = flood_flag || port->is_mrouter;
+	struct sparx5 *sparx5 = port->sparx5; /* Used by PGID_* macros */
 	int pgid;
 
 	for (pgid = PGID_IPV4_MC_DATA; pgid <= PGID_IPV6_MC_CTRL; pgid++)
@@ -41,6 +42,8 @@ static void sparx5_port_update_mcast_ip_flood(struct sparx5_port *port, bool flo
 static void sparx5_port_attr_bridge_flags(struct sparx5_port *port,
 					  struct switchdev_brport_flags flags)
 {
+	struct sparx5 *sparx5 = port->sparx5; /* Used by PGID_* macros */
+
 	if (flags.mask & BR_MCAST_FLOOD) {
 		sparx5_pgid_update_mask(port, PGID_MC_FLOOD, !!(flags.val & BR_MCAST_FLOOD));
 		sparx5_port_update_mcast_ip_flood(port, !!(flags.val & BR_MCAST_FLOOD));
@@ -547,7 +550,7 @@ static int sparx5_handle_port_mdb_add(struct net_device *dev,
 
 	/* Add any mrouter ports to the new entry */
 	if (is_new && ether_addr_is_ip_mcast(v->addr))
-		for (i = 0; i < SPX5_PORTS; i++)
+		for (i = 0; i < SPX5_CONST(n_ports); i++)
 			if (sparx5->ports[i] && sparx5->ports[i]->is_mrouter)
 				sparx5_pgid_update_mask(sparx5->ports[i],
 							entry->pgid_idx,
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c b/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
index ac001ae59a38..5d5e5c2c05c5 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
@@ -169,7 +169,7 @@ void sparx5_update_fwd(struct sparx5 *sparx5)
 	}
 
 	/* Update SRC masks */
-	for (port = 0; port < SPX5_PORTS; port++) {
+	for (port = 0; port < SPX5_CONST(n_ports); port++) {
 		if (test_bit(port, sparx5->bridge_fwd_mask)) {
 			/* Allow to send to all bridged but self */
 			bitmap_copy(workmask, sparx5->bridge_fwd_mask, SPX5_PORTS);

-- 
2.34.1

Re: [PATCH net-next 07/15] net: sparx5: use SPX5_CONST for constants which already have a symbol

[Jacob Keller]

On 10/1/2024 6:50 AM, Daniel Machon wrote:
> Now that we have indentified all the chip constants, update the use of
> them where a symbol is already defined for the constant.
> 
> Constants are accessed using the SPX5_CONSTS macro. Note that this macro
> might hide the use of the *sparx5 context pointer. In such case, a
> comment is added.
> 

I guess its shorter than doing something like 'SPX5_CONST(sparx5,
<constant>)'? Is that really worth it over the additional burden of
tracking that this macro accesses the struct?

> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>

[PATCH net-next 08/15] net: sparx5: use SPX5_CONST for constants which do not have a symbol

[Daniel Machon]

Now that we have indentified all the chip constants, update the use of
them where a symbol is not defined for the constant.

Constants are accessed using the SPX5_CONSTS macro. Note that this macro
might hide the use of the *sparx5 context pointer. In such case, a
comment is added.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index 063f02fd36c3..4b3e6986af55 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -524,7 +524,7 @@ static int sparx5_init_coreclock(struct sparx5 *sparx5)
 		 sparx5,
 		 LRN_AUTOAGE_CFG_1);
 
-	for (idx = 0; idx < 3; idx++)
+	for (idx = 0; idx < SPX5_CONST(n_sio_clks); idx++)
 		spx5_rmw(GCB_SIO_CLOCK_SYS_CLK_PERIOD_SET(clk_period / 100),
 			 GCB_SIO_CLOCK_SYS_CLK_PERIOD,
 			 sparx5,
@@ -551,11 +551,15 @@ static int sparx5_qlim_set(struct sparx5 *sparx5)
 	for (res = 0; res < 2; res++) {
 		for (prio = 0; prio < 8; prio++)
 			spx5_wr(0xFFF, sparx5,
-				QRES_RES_CFG(prio + 630 + res * 1024));
+				QRES_RES_CFG(prio +
+					     SPX5_CONST(qres_max_prio_idx) +
+					     res * 1024));
 
 		for (dp = 0; dp < 4; dp++)
 			spx5_wr(0xFFF, sparx5,
-				QRES_RES_CFG(dp + 638 + res * 1024));
+				QRES_RES_CFG(dp +
+					     SPX5_CONST(qres_max_colour_idx) +
+					     res * 1024));
 	}
 
 	/* Set 80,90,95,100% of memory size for top watermarks */
@@ -600,7 +604,7 @@ static int sparx5_start(struct sparx5 *sparx5)
 	int err;
 
 	/* Setup own UPSIDs */
-	for (idx = 0; idx < 3; idx++) {
+	for (idx = 0; idx < SPX5_CONST(n_own_upsids); idx++) {
 		spx5_wr(idx, sparx5, ANA_AC_OWN_UPSID(idx));
 		spx5_wr(idx, sparx5, ANA_CL_OWN_UPSID(idx));
 		spx5_wr(idx, sparx5, ANA_L2_OWN_UPSID(idx));

-- 
2.34.1

[PATCH net-next 09/15] net: sparx5: add ops to match data

[Daniel Machon]

Add new struct sparx5_ops, containing functions that needs to be
different as the implementation differs on Sparx5 and lan969x. Initially
we add functions for checking the port type (2g5, 5g, 10g or 25g) based
on the port number. Update the code to use the ops instead of the
platform specific functions.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 .../net/ethernet/microchip/sparx5/sparx5_main.c    |  8 +++++
 .../net/ethernet/microchip/sparx5/sparx5_main.h    |  8 +++++
 .../net/ethernet/microchip/sparx5/sparx5_port.c    | 34 +++++++++++++---------
 .../net/ethernet/microchip/sparx5/sparx5_port.h    | 12 +++++---
 4 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index 4b3e6986af55..8b1033c49cfe 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -977,12 +977,20 @@ static const struct sparx5_consts sparx5_consts = {
 	.tod_pin             = 4,
 };
 
+static const struct sparx5_ops sparx5_ops = {
+	.is_port_2g5             = &sparx5_port_is_2g5,
+	.is_port_5g              = &sparx5_port_is_5g,
+	.is_port_10g             = &sparx5_port_is_10g,
+	.is_port_25g             = &sparx5_port_is_25g,
+};
+
 static const struct sparx5_match_data sparx5_desc = {
 	.iomap = sparx5_main_iomap,
 	.iomap_size = ARRAY_SIZE(sparx5_main_iomap),
 	.ioranges = 3,
 	.regs = &sparx5_regs,
 	.consts = &sparx5_consts,
+	.ops = &sparx5_ops,
 };
 
 static const struct of_device_id mchp_sparx5_match[] = {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 8398f32ffaad..5a8a37681312 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -260,6 +260,13 @@ struct sparx5_consts {
 	u32 tod_pin;             /* PTP TOD pin */
 };
 
+struct sparx5_ops {
+	bool (*is_port_2g5)(int portno);
+	bool (*is_port_5g)(int portno);
+	bool (*is_port_10g)(int portno);
+	bool (*is_port_25g)(int portno);
+};
+
 struct sparx5_main_io_resource {
 	enum sparx5_target id;
 	phys_addr_t offset;
@@ -269,6 +276,7 @@ struct sparx5_main_io_resource {
 struct sparx5_match_data {
 	const struct sparx5_regs *regs;
 	const struct sparx5_consts *consts;
+	const struct sparx5_ops *ops;
 	const struct sparx5_main_io_resource *iomap;
 	int ioranges;
 	int iomap_size;
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index ea3454342665..c5dfe0fa847b 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -213,11 +213,13 @@ static int sparx5_port_verify_speed(struct sparx5 *sparx5,
 				    struct sparx5_port *port,
 				    struct sparx5_port_config *conf)
 {
-	if ((sparx5_port_is_2g5(port->portno) &&
+	const struct sparx5_ops *ops = sparx5->data->ops;
+
+	if ((ops->is_port_2g5(port->portno) &&
 	     conf->speed > SPEED_2500) ||
-	    (sparx5_port_is_5g(port->portno)  &&
+	    (ops->is_port_5g(port->portno)  &&
 	     conf->speed > SPEED_5000) ||
-	    (sparx5_port_is_10g(port->portno) &&
+	    (ops->is_port_10g(port->portno) &&
 	     conf->speed > SPEED_10000))
 		return sparx5_port_error(port, conf, SPX5_PERR_SPEED);
 
@@ -226,14 +228,14 @@ static int sparx5_port_verify_speed(struct sparx5 *sparx5,
 		return -EINVAL;
 	case PHY_INTERFACE_MODE_1000BASEX:
 		if (conf->speed != SPEED_1000 ||
-		    sparx5_port_is_2g5(port->portno))
+		    ops->is_port_2g5(port->portno))
 			return sparx5_port_error(port, conf, SPX5_PERR_SPEED);
-		if (sparx5_port_is_2g5(port->portno))
+		if (ops->is_port_2g5(port->portno))
 			return sparx5_port_error(port, conf, SPX5_PERR_IFTYPE);
 		break;
 	case PHY_INTERFACE_MODE_2500BASEX:
 		if (conf->speed != SPEED_2500 ||
-		    sparx5_port_is_2g5(port->portno))
+		    ops->is_port_2g5(port->portno))
 			return sparx5_port_error(port, conf, SPX5_PERR_SPEED);
 		break;
 	case PHY_INTERFACE_MODE_QSGMII:
@@ -320,6 +322,7 @@ static int sparx5_port_disable(struct sparx5 *sparx5, struct sparx5_port *port,
 	u32 dev = high_spd_dev ?
 		  sparx5_to_high_dev(sparx5, port->portno) : TARGET_DEV2G5;
 	void __iomem *devinst = spx5_inst_get(sparx5, dev, tinst);
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	u32 spd = port->conf.speed;
 	u32 spd_prm;
 	int err;
@@ -436,7 +439,7 @@ static int sparx5_port_disable(struct sparx5 *sparx5, struct sparx5_port *port,
 			      pcsinst,
 			      PCS10G_BR_PCS_CFG(0));
 
-		if (sparx5_port_is_25g(port->portno))
+		if (ops->is_port_25g(port->portno))
 			/* Disable 25G PCS */
 			spx5_rmw(DEV25G_PCS25G_CFG_PCS25G_ENA_SET(0),
 				 DEV25G_PCS25G_CFG_PCS25G_ENA,
@@ -561,6 +564,7 @@ static int sparx5_port_max_tags_set(struct sparx5 *sparx5,
 	u32 dev             = sparx5_to_high_dev(sparx5, port->portno);
 	u32 tinst           = sparx5_port_dev_index(sparx5, port->portno);
 	void __iomem *inst  = spx5_inst_get(sparx5, dev, tinst);
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	u32 etype;
 
 	etype = (vlan_type == SPX5_VLAN_PORT_TYPE_S_CUSTOM ?
@@ -575,7 +579,7 @@ static int sparx5_port_max_tags_set(struct sparx5 *sparx5,
 		sparx5,
 		DEV2G5_MAC_TAGS_CFG(port->portno));
 
-	if (sparx5_port_is_2g5(port->portno))
+	if (ops->is_port_2g5(port->portno))
 		return 0;
 
 	spx5_inst_rmw(DEV10G_MAC_TAGS_CFG_TAG_ID_SET(etype) |
@@ -844,18 +848,19 @@ static int sparx5_port_pcs_high_set(struct sparx5 *sparx5,
 static void sparx5_dev_switch(struct sparx5 *sparx5, int port, bool hsd)
 {
 	int bt_indx = BIT(sparx5_port_dev_index(sparx5, port));
+	const struct sparx5_ops *ops = sparx5->data->ops;
 
-	if (sparx5_port_is_5g(port)) {
+	if (ops->is_port_5g(port)) {
 		spx5_rmw(hsd ? 0 : bt_indx,
 			 bt_indx,
 			 sparx5,
 			 PORT_CONF_DEV5G_MODES);
-	} else if (sparx5_port_is_10g(port)) {
+	} else if (ops->is_port_10g(port)) {
 		spx5_rmw(hsd ? 0 : bt_indx,
 			 bt_indx,
 			 sparx5,
 			 PORT_CONF_DEV10G_MODES);
-	} else if (sparx5_port_is_25g(port)) {
+	} else if (ops->is_port_25g(port)) {
 		spx5_rmw(hsd ? 0 : bt_indx,
 			 bt_indx,
 			 sparx5,
@@ -1016,6 +1021,7 @@ int sparx5_port_init(struct sparx5 *sparx5,
 {
 	u32 pause_start = sparx5_wm_enc(6  * (ETH_MAXLEN / SPX5_BUFFER_CELL_SZ));
 	u32 atop = sparx5_wm_enc(20 * (ETH_MAXLEN / SPX5_BUFFER_CELL_SZ));
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	u32 devhigh = sparx5_to_high_dev(sparx5, port->portno);
 	u32 pix = sparx5_port_dev_index(sparx5, port->portno);
 	u32 pcs = sparx5_to_pcs_dev(sparx5, port->portno);
@@ -1082,7 +1088,7 @@ int sparx5_port_init(struct sparx5 *sparx5,
 		if (err)
 			return err;
 
-		if (!sparx5_port_is_2g5(port->portno))
+		if (!ops->is_port_2g5(port->portno))
 			/* Enable shadow device */
 			spx5_rmw(DSM_DEV_TX_STOP_WM_CFG_DEV10G_SHADOW_ENA_SET(1),
 				 DSM_DEV_TX_STOP_WM_CFG_DEV10G_SHADOW_ENA,
@@ -1105,7 +1111,7 @@ int sparx5_port_init(struct sparx5 *sparx5,
 		sparx5,
 		DEV2G5_MAC_IFG_CFG(port->portno));
 
-	if (sparx5_port_is_2g5(port->portno))
+	if (ops->is_port_2g5(port->portno))
 		return 0; /* Low speed device only - return */
 
 	/* Now setup the high speed device */
@@ -1128,7 +1134,7 @@ int sparx5_port_init(struct sparx5 *sparx5,
 		     pcsinst,
 		     PCS10G_BR_PCS_SD_CFG(0));
 
-	if (sparx5_port_is_25g(port->portno)) {
+	if (ops->is_port_25g(port->portno)) {
 		/* Handle Signal Detect in 25G PCS */
 		spx5_wr(DEV25G_PCS25G_SD_CFG_SD_POL_SET(sd_pol) |
 			DEV25G_PCS25G_SD_CFG_SD_SEL_SET(sd_sel) |
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
index 468e3d34d6e1..934e2d3dedbb 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
@@ -42,18 +42,22 @@ static inline bool sparx5_port_is_25g(int portno)
 
 static inline u32 sparx5_to_high_dev(struct sparx5 *sparx5, int port)
 {
-	if (sparx5_port_is_5g(port))
+	const struct sparx5_ops *ops = sparx5->data->ops;
+
+	if (ops->is_port_5g(port))
 		return TARGET_DEV5G;
-	if (sparx5_port_is_10g(port))
+	if (ops->is_port_10g(port))
 		return TARGET_DEV10G;
 	return TARGET_DEV25G;
 }
 
 static inline u32 sparx5_to_pcs_dev(struct sparx5 *sparx5, int port)
 {
-	if (sparx5_port_is_5g(port))
+	const struct sparx5_ops *ops = sparx5->data->ops;
+
+	if (ops->is_port_5g(port))
 		return TARGET_PCS5G_BR;
-	if (sparx5_port_is_10g(port))
+	if (ops->is_port_10g(port))
 		return TARGET_PCS10G_BR;
 	return TARGET_PCS25G_BR;
 }

-- 
2.34.1

[PATCH net-next 10/15] net: sparx5: ops out chip port to device index/bit functions

[Daniel Machon]

The chip port device index and mode bit can be obtained using the port
number.  However the mapping of port number to chip device index and
mode bit differs on Sparx5 and lan969x. Therefore ops out the function.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 2 ++
 drivers/net/ethernet/microchip/sparx5/sparx5_main.h | 2 ++
 drivers/net/ethernet/microchip/sparx5/sparx5_port.c | 4 +++-
 drivers/net/ethernet/microchip/sparx5/sparx5_port.h | 7 ++++++-
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index 8b1033c49cfe..8617fc3983cc 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -982,6 +982,8 @@ static const struct sparx5_ops sparx5_ops = {
 	.is_port_5g              = &sparx5_port_is_5g,
 	.is_port_10g             = &sparx5_port_is_10g,
 	.is_port_25g             = &sparx5_port_is_25g,
+	.get_port_dev_index      = &sparx5_port_dev_mapping,
+	.get_port_dev_bit        = &sparx5_port_dev_mapping,
 };
 
 static const struct sparx5_match_data sparx5_desc = {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 5a8a37681312..68d5a14603dc 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -265,6 +265,8 @@ struct sparx5_ops {
 	bool (*is_port_5g)(int portno);
 	bool (*is_port_10g)(int portno);
 	bool (*is_port_25g)(int portno);
+	u32  (*get_port_dev_index)(struct sparx5 *sparx5, int port);
+	u32  (*get_port_dev_bit)(struct sparx5 *sparx5, int port);
 };
 
 struct sparx5_main_io_resource {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index c5dfe0fa847b..49ff94db0e63 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -847,8 +847,10 @@ static int sparx5_port_pcs_high_set(struct sparx5 *sparx5,
 /* Switch between 1G/2500 and 5G/10G/25G devices */
 static void sparx5_dev_switch(struct sparx5 *sparx5, int port, bool hsd)
 {
-	int bt_indx = BIT(sparx5_port_dev_index(sparx5, port));
 	const struct sparx5_ops *ops = sparx5->data->ops;
+	int bt_indx;
+
+	bt_indx = BIT(ops->get_port_dev_bit(sparx5, port));
 
 	if (ops->is_port_5g(port)) {
 		spx5_rmw(hsd ? 0 : bt_indx,
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
index 934e2d3dedbb..9b9bcc6834bc 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
@@ -62,7 +62,7 @@ static inline u32 sparx5_to_pcs_dev(struct sparx5 *sparx5, int port)
 	return TARGET_PCS25G_BR;
 }
 
-static inline int sparx5_port_dev_index(struct sparx5 *sparx5, int port)
+static inline u32 sparx5_port_dev_mapping(struct sparx5 *sparx5, int port)
 {
 	if (sparx5_port_is_2g5(port))
 		return port;
@@ -74,6 +74,11 @@ static inline int sparx5_port_dev_index(struct sparx5 *sparx5, int port)
 	return (port - 56);
 }
 
+static inline u32 sparx5_port_dev_index(struct sparx5 *sparx5, int port)
+{
+	return sparx5->data->ops->get_port_dev_index(sparx5, port);
+}
+
 int sparx5_port_init(struct sparx5 *sparx5,
 		     struct sparx5_port *spx5_port,
 		     struct sparx5_port_config *conf);

-- 
2.34.1

Re: [PATCH net-next 10/15] net: sparx5: ops out chip port to device index/bit functions

[Jacob Keller]

On 10/1/2024 6:50 AM, Daniel Machon wrote:
> The chip port device index and mode bit can be obtained using the port
> number.  However the mapping of port number to chip device index and
> mode bit differs on Sparx5 and lan969x. Therefore ops out the function.
> 
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
> ---
>  drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 2 ++
>  drivers/net/ethernet/microchip/sparx5/sparx5_main.h | 2 ++
>  drivers/net/ethernet/microchip/sparx5/sparx5_port.c | 4 +++-
>  drivers/net/ethernet/microchip/sparx5/sparx5_port.h | 7 ++++++-
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> index 8b1033c49cfe..8617fc3983cc 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> @@ -982,6 +982,8 @@ static const struct sparx5_ops sparx5_ops = {
>  	.is_port_5g              = &sparx5_port_is_5g,
>  	.is_port_10g             = &sparx5_port_is_10g,
>  	.is_port_25g             = &sparx5_port_is_25g,
> +	.get_port_dev_index      = &sparx5_port_dev_mapping,
> +	.get_port_dev_bit        = &sparx5_port_dev_mapping,

So for sparx5, these are identical operations, but for lan969x these
will be different? Ok.

Re: [PATCH net-next 10/15] net: sparx5: ops out chip port to device index/bit functions

[Daniel Machon]

> > The chip port device index and mode bit can be obtained using the port
> > number.  However the mapping of port number to chip device index and
> > mode bit differs on Sparx5 and lan969x. Therefore ops out the function.
> >
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
> > ---
> >  drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 2 ++
> >  drivers/net/ethernet/microchip/sparx5/sparx5_main.h | 2 ++
> >  drivers/net/ethernet/microchip/sparx5/sparx5_port.c | 4 +++-
> >  drivers/net/ethernet/microchip/sparx5/sparx5_port.h | 7 ++++++-
> >  4 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> > index 8b1033c49cfe..8617fc3983cc 100644
> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> > @@ -982,6 +982,8 @@ static const struct sparx5_ops sparx5_ops = {
> >       .is_port_5g              = &sparx5_port_is_5g,
> >       .is_port_10g             = &sparx5_port_is_10g,
> >       .is_port_25g             = &sparx5_port_is_25g,
> > +     .get_port_dev_index      = &sparx5_port_dev_mapping,
> > +     .get_port_dev_bit        = &sparx5_port_dev_mapping,
> 
> So for sparx5, these are identical operations, but for lan969x these
> will be different? Ok.
>

Exactly :-)

/Daniel

[PATCH net-next 11/15] net: sparx5: ops out functions for getting certain array values

[Daniel Machon]

Add getters for getting values in arrays: sdlb_groups and
sparx5_hsch_max_group_rate and ops out the getters, as these arrays will
differ on lan969x.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_main.c   |  2 ++
 drivers/net/ethernet/microchip/sparx5/sparx5_main.h   |  3 +++
 drivers/net/ethernet/microchip/sparx5/sparx5_police.c |  3 ++-
 drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c   |  3 ++-
 drivers/net/ethernet/microchip/sparx5/sparx5_qos.c    |  8 +++++++-
 drivers/net/ethernet/microchip/sparx5/sparx5_qos.h    |  2 ++
 drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c   | 11 +++++++++--
 7 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index 8617fc3983cc..0d8cb9a3ed1f 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -984,6 +984,8 @@ static const struct sparx5_ops sparx5_ops = {
 	.is_port_25g             = &sparx5_port_is_25g,
 	.get_port_dev_index      = &sparx5_port_dev_mapping,
 	.get_port_dev_bit        = &sparx5_port_dev_mapping,
+	.get_hsch_max_group_rate = &sparx5_get_hsch_max_group_rate,
+	.get_sdlb_group          = &sparx5_get_sdlb_group,
 };
 
 static const struct sparx5_match_data sparx5_desc = {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 68d5a14603dc..99174aef88f8 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -267,6 +267,8 @@ struct sparx5_ops {
 	bool (*is_port_25g)(int portno);
 	u32  (*get_port_dev_index)(struct sparx5 *sparx5, int port);
 	u32  (*get_port_dev_bit)(struct sparx5 *sparx5, int port);
+	u32  (*get_hsch_max_group_rate)(int grp);
+	struct sparx5_sdlb_group *(*get_sdlb_group)(int idx);
 };
 
 struct sparx5_main_io_resource {
@@ -506,6 +508,7 @@ struct sparx5_sdlb_group {
 };
 
 extern struct sparx5_sdlb_group sdlb_groups[SPX5_SDLB_GROUP_CNT];
+struct sparx5_sdlb_group *sparx5_get_sdlb_group(int idx);
 int sparx5_sdlb_pup_token_get(struct sparx5 *sparx5, u32 pup_interval,
 			      u64 rate);
 
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_police.c b/drivers/net/ethernet/microchip/sparx5/sparx5_police.c
index 8ada5cee1342..c88820e83812 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_police.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_police.c
@@ -11,10 +11,11 @@ static int sparx5_policer_service_conf_set(struct sparx5 *sparx5,
 					   struct sparx5_policer *pol)
 {
 	u32 idx, pup_tokens, max_pup_tokens, burst, thres;
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	struct sparx5_sdlb_group *g;
 	u64 rate;
 
-	g = &sdlb_groups[pol->group];
+	g = ops->get_sdlb_group(pol->group);
 	idx = pol->idx;
 
 	rate = pol->rate * 1000;
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c b/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c
index 58bc4eba996b..e8d6f580676d 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_psfp.c
@@ -315,11 +315,12 @@ int sparx5_psfp_fm_del(struct sparx5 *sparx5, u32 id)
 
 void sparx5_psfp_init(struct sparx5 *sparx5)
 {
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	const struct sparx5_sdlb_group *group;
 	int i;
 
 	for (i = 0; i < SPX5_CONST(n_lb_groups); i++) {
-		group = &sdlb_groups[i];
+		group = ops->get_sdlb_group(i);
 		sparx5_sdlb_group_init(sparx5, group->max_rate,
 				       group->min_burst, group->frame_size, i);
 	}
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
index 5f34febaee6b..d065f8c40d37 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
@@ -74,6 +74,11 @@ static const u32 spx5_hsch_max_group_rate[SPX5_HSCH_LEAK_GRP_CNT] = {
 	26214200 /* 26.214 Gbps */
 };
 
+u32 sparx5_get_hsch_max_group_rate(int grp)
+{
+	return spx5_hsch_max_group_rate[grp];
+}
+
 static struct sparx5_layer layers[SPX5_HSCH_LAYER_CNT];
 
 static u32 sparx5_lg_get_leak_time(struct sparx5 *sparx5, u32 layer, u32 group)
@@ -385,6 +390,7 @@ static int sparx5_dwrr_conf_set(struct sparx5_port *port,
 
 static int sparx5_leak_groups_init(struct sparx5 *sparx5)
 {
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	struct sparx5_layer *layer;
 	u32 sys_clk_per_100ps;
 	struct sparx5_lg *lg;
@@ -397,7 +403,7 @@ static int sparx5_leak_groups_init(struct sparx5 *sparx5)
 		layer = &layers[i];
 		for (ii = 0; ii < SPX5_HSCH_LEAK_GRP_CNT; ii++) {
 			lg = &layer->leak_groups[ii];
-			lg->max_rate = spx5_hsch_max_group_rate[ii];
+			lg->max_rate = ops->get_hsch_max_group_rate(i);
 
 			/* Calculate the leak time in us, to serve a maximum
 			 * rate of 'max_rate' for this group
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h
index 8419577cfda0..dbd2e7fff275 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h
@@ -79,4 +79,6 @@ int sparx5_tc_ets_add(struct sparx5_port *port,
 
 int sparx5_tc_ets_del(struct sparx5_port *port);
 
+u32 sparx5_get_hsch_max_group_rate(int grp);
+
 #endif	/* __SPARX5_QOS_H__ */
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c
index 77fc2a14450d..28ba35c889b9 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_sdlb.c
@@ -20,6 +20,11 @@ struct sparx5_sdlb_group sdlb_groups[SPX5_SDLB_GROUP_CNT] = {
 	{     5000000ULL,              8192 / 8, 64 }  /*   5 M */
 };
 
+struct sparx5_sdlb_group *sparx5_get_sdlb_group(int idx)
+{
+	return &sdlb_groups[idx];
+}
+
 int sparx5_sdlb_clk_hz_get(struct sparx5 *sparx5)
 {
 	u32 clk_per_100ps;
@@ -178,6 +183,7 @@ static int sparx5_sdlb_group_get_count(struct sparx5 *sparx5, u32 group)
 
 int sparx5_sdlb_group_get_by_rate(struct sparx5 *sparx5, u32 rate, u32 burst)
 {
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	const struct sparx5_sdlb_group *group;
 	u64 rate_bps;
 	int i, count;
@@ -185,7 +191,7 @@ int sparx5_sdlb_group_get_by_rate(struct sparx5 *sparx5, u32 rate, u32 burst)
 	rate_bps = rate * 1000;
 
 	for (i = SPX5_CONST(n_lb_groups) - 1; i >= 0; i--) {
-		group = &sdlb_groups[i];
+		group = ops->get_sdlb_group(i);
 
 		count = sparx5_sdlb_group_get_count(sparx5, i);
 
@@ -303,11 +309,12 @@ int sparx5_sdlb_group_del(struct sparx5 *sparx5, u32 group, u32 idx)
 void sparx5_sdlb_group_init(struct sparx5 *sparx5, u64 max_rate, u32 min_burst,
 			    u32 frame_size, u32 idx)
 {
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	u32 thres_shift, mask = 0x01, power = 0;
 	struct sparx5_sdlb_group *group;
 	u64 max_token;
 
-	group = &sdlb_groups[idx];
+	group = ops->get_sdlb_group(idx);
 
 	/* Number of positions to right-shift LB's threshold value. */
 	while ((min_burst & mask) == 0) {

-- 
2.34.1

[PATCH net-next 12/15] net: sparx5: ops out function for setting the port mux

[Daniel Machon]

Port muxing is configured based on the supported port modes. As these
modes can differ on Sparx5 and lan969x we ops out the port muxing
function.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 1 +
 drivers/net/ethernet/microchip/sparx5/sparx5_main.h | 6 ++++++
 drivers/net/ethernet/microchip/sparx5/sparx5_port.c | 7 +++----
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index 0d8cb9a3ed1f..bcdce23b735f 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -986,6 +986,7 @@ static const struct sparx5_ops sparx5_ops = {
 	.get_port_dev_bit        = &sparx5_port_dev_mapping,
 	.get_hsch_max_group_rate = &sparx5_get_hsch_max_group_rate,
 	.get_sdlb_group          = &sparx5_get_sdlb_group,
+	.set_port_mux            = &sparx5_port_mux_set,
 };
 
 static const struct sparx5_match_data sparx5_desc = {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 99174aef88f8..6fe840dbaf98 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -269,6 +269,8 @@ struct sparx5_ops {
 	u32  (*get_port_dev_bit)(struct sparx5 *sparx5, int port);
 	u32  (*get_hsch_max_group_rate)(int grp);
 	struct sparx5_sdlb_group *(*get_sdlb_group)(int idx);
+	int (*set_port_mux)(struct sparx5 *sparx5, struct sparx5_port *port,
+			    struct sparx5_port_config *conf);
 };
 
 struct sparx5_main_io_resource {
@@ -490,6 +492,10 @@ int sparx5_pool_get(struct sparx5_pool_entry *pool, int size, u32 *id);
 int sparx5_pool_get_with_idx(struct sparx5_pool_entry *pool, int size, u32 idx,
 			     u32 *id);
 
+/* sparx5_port.c */
+int sparx5_port_mux_set(struct sparx5 *sparx5, struct sparx5_port *port,
+			struct sparx5_port_config *conf);
+
 /* sparx5_sdlb.c */
 #define SPX5_SDLB_PUP_TOKEN_DISABLE 0x1FFF
 #define SPX5_SDLB_PUP_TOKEN_MAX (SPX5_SDLB_PUP_TOKEN_DISABLE - 1)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index 49ff94db0e63..0dc2201fe653 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -516,9 +516,8 @@ static int sparx5_port_fifo_sz(struct sparx5 *sparx5,
 /* Configure port muxing:
  * QSGMII:     4x2G5 devices
  */
-static int sparx5_port_mux_set(struct sparx5 *sparx5,
-			       struct sparx5_port *port,
-			       struct sparx5_port_config *conf)
+int sparx5_port_mux_set(struct sparx5 *sparx5, struct sparx5_port *port,
+			struct sparx5_port_config *conf)
 {
 	u32 portno = port->portno;
 	u32 inst;
@@ -1039,7 +1038,7 @@ int sparx5_port_init(struct sparx5 *sparx5,
 	pcsinst = spx5_inst_get(sparx5, pcs, pix);
 
 	/* Set the mux port mode  */
-	err = sparx5_port_mux_set(sparx5, port, conf);
+	err = ops->set_port_mux(sparx5, port, conf);
 	if (err)
 		return err;
 

-- 
2.34.1

[PATCH net-next 13/15] net: sparx5: ops out PTP IRQ handler

[Daniel Machon]

The PTP registers are located in two different register targets on
Sparx5 and lan969x. We can't handle this with the register macros, so
ops out the handler.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 4 +++-
 drivers/net/ethernet/microchip/sparx5/sparx5_main.h | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index bcdce23b735f..c5239e547c35 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -599,6 +599,7 @@ static void sparx5_board_init(struct sparx5 *sparx5)
 static int sparx5_start(struct sparx5 *sparx5)
 {
 	u8 broadcast[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	char queue_name[32];
 	u32 idx;
 	int err;
@@ -723,7 +724,7 @@ static int sparx5_start(struct sparx5 *sparx5)
 
 	if (sparx5->ptp_irq >= 0) {
 		err = devm_request_threaded_irq(sparx5->dev, sparx5->ptp_irq,
-						NULL, sparx5_ptp_irq_handler,
+						NULL, ops->ptp_irq_handler,
 						IRQF_ONESHOT, "sparx5-ptp",
 						sparx5);
 		if (err)
@@ -987,6 +988,7 @@ static const struct sparx5_ops sparx5_ops = {
 	.get_hsch_max_group_rate = &sparx5_get_hsch_max_group_rate,
 	.get_sdlb_group          = &sparx5_get_sdlb_group,
 	.set_port_mux            = &sparx5_port_mux_set,
+	.ptp_irq_handler         = &sparx5_ptp_irq_handler,
 };
 
 static const struct sparx5_match_data sparx5_desc = {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 6fe840dbaf98..eb71bba02a77 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -271,6 +271,8 @@ struct sparx5_ops {
 	struct sparx5_sdlb_group *(*get_sdlb_group)(int idx);
 	int (*set_port_mux)(struct sparx5 *sparx5, struct sparx5_port *port,
 			    struct sparx5_port_config *conf);
+
+	irqreturn_t (*ptp_irq_handler)(int irq, void *args);
 };
 
 struct sparx5_main_io_resource {

-- 
2.34.1

[PATCH net-next 14/15] net: sparx5: ops out function for DSM calendar calculation

[Daniel Machon]

The DSM (Disassembler) calendar grants each port access to internal
busses. The configuration of the calendar is done differently on Sparx5
and lan969x. Therefore ops out the function that calculates the
calendar.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 .../ethernet/microchip/sparx5/sparx5_calendar.c    | 22 ++++------------------
 .../net/ethernet/microchip/sparx5/sparx5_main.c    |  1 +
 .../net/ethernet/microchip/sparx5/sparx5_main.h    | 21 +++++++++++++++++++++
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c b/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
index b7b611b1ad34..35456cd35a40 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
@@ -15,9 +15,7 @@
 #define SPX5_CALBITS_PER_PORT          3   /* Bit per port in calendar register */
 
 /* DSM calendar information */
-#define SPX5_DSM_CAL_LEN               64
 #define SPX5_DSM_CAL_EMPTY             0xFFFF
-#define SPX5_DSM_CAL_MAX_DEVS_PER_TAXI 13
 #define SPX5_DSM_CAL_TAXIS             8
 #define SPX5_DSM_CAL_BW_LOSS           553
 
@@ -37,19 +35,6 @@ static u32 sparx5_taxi_ports[SPX5_DSM_CAL_TAXIS][SPX5_DSM_CAL_MAX_DEVS_PER_TAXI]
 	{64, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99},
 };
 
-struct sparx5_calendar_data {
-	u32 schedule[SPX5_DSM_CAL_LEN];
-	u32 avg_dist[SPX5_DSM_CAL_MAX_DEVS_PER_TAXI];
-	u32 taxi_ports[SPX5_DSM_CAL_MAX_DEVS_PER_TAXI];
-	u32 taxi_speeds[SPX5_DSM_CAL_MAX_DEVS_PER_TAXI];
-	u32 dev_slots[SPX5_DSM_CAL_MAX_DEVS_PER_TAXI];
-	u32 new_slots[SPX5_DSM_CAL_LEN];
-	u32 temp_sched[SPX5_DSM_CAL_LEN];
-	u32 indices[SPX5_DSM_CAL_LEN];
-	u32 short_list[SPX5_DSM_CAL_LEN];
-	u32 long_list[SPX5_DSM_CAL_LEN];
-};
-
 static u32 sparx5_target_bandwidth(struct sparx5 *sparx5)
 {
 	switch (sparx5->target_ct) {
@@ -278,8 +263,8 @@ static u32 sparx5_dsm_cp_cal(u32 *sched)
 	return SPX5_DSM_CAL_EMPTY;
 }
 
-static int sparx5_dsm_calendar_calc(struct sparx5 *sparx5, u32 taxi,
-				    struct sparx5_calendar_data *data)
+int sparx5_dsm_calendar_calc(struct sparx5 *sparx5, u32 taxi,
+			     struct sparx5_calendar_data *data)
 {
 	bool slow_mode;
 	u32 gcd, idx, sum, min, factor;
@@ -565,6 +550,7 @@ static int sparx5_dsm_calendar_update(struct sparx5 *sparx5, u32 taxi,
 /* Configure the DSM calendar based on port configuration */
 int sparx5_config_dsm_calendar(struct sparx5 *sparx5)
 {
+	const struct sparx5_ops *ops = sparx5->data->ops;
 	int taxi;
 	struct sparx5_calendar_data *data;
 	int err = 0;
@@ -574,7 +560,7 @@ int sparx5_config_dsm_calendar(struct sparx5 *sparx5)
 		return -ENOMEM;
 
 	for (taxi = 0; taxi < SPX5_CONST(n_dsm_cal_taxis); ++taxi) {
-		err = sparx5_dsm_calendar_calc(sparx5, taxi, data);
+		err = ops->dsm_calendar_calc(sparx5, taxi, data);
 		if (err) {
 			dev_err(sparx5->dev, "DSM calendar calculation failed\n");
 			goto cal_out;
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index c5239e547c35..67e8d2d70816 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -989,6 +989,7 @@ static const struct sparx5_ops sparx5_ops = {
 	.get_sdlb_group          = &sparx5_get_sdlb_group,
 	.set_port_mux            = &sparx5_port_mux_set,
 	.ptp_irq_handler         = &sparx5_ptp_irq_handler,
+	.dsm_calendar_calc       = &sparx5_dsm_calendar_calc,
 };
 
 static const struct sparx5_match_data sparx5_desc = {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index eb71bba02a77..55fc21fbf63d 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -103,8 +103,24 @@ enum sparx5_vlan_port_type {
 #define IFH_PDU_TYPE_IPV4_UDP_PTP	0x6
 #define IFH_PDU_TYPE_IPV6_UDP_PTP	0x7
 
+#define SPX5_DSM_CAL_LEN               64
+#define SPX5_DSM_CAL_MAX_DEVS_PER_TAXI 13
+
 struct sparx5;
 
+struct sparx5_calendar_data {
+	u32 schedule[SPX5_DSM_CAL_LEN];
+	u32 avg_dist[SPX5_DSM_CAL_MAX_DEVS_PER_TAXI];
+	u32 taxi_ports[SPX5_DSM_CAL_MAX_DEVS_PER_TAXI];
+	u32 taxi_speeds[SPX5_DSM_CAL_MAX_DEVS_PER_TAXI];
+	u32 dev_slots[SPX5_DSM_CAL_MAX_DEVS_PER_TAXI];
+	u32 new_slots[SPX5_DSM_CAL_LEN];
+	u32 temp_sched[SPX5_DSM_CAL_LEN];
+	u32 indices[SPX5_DSM_CAL_LEN];
+	u32 short_list[SPX5_DSM_CAL_LEN];
+	u32 long_list[SPX5_DSM_CAL_LEN];
+};
+
 /* Frame DMA receive state:
  * For each DB, there is a SKB, and the skb data pointer is mapped in
  * the DB. Once a frame is received the skb is given to the upper layers
@@ -273,6 +289,8 @@ struct sparx5_ops {
 			    struct sparx5_port_config *conf);
 
 	irqreturn_t (*ptp_irq_handler)(int irq, void *args);
+	int (*dsm_calendar_calc)(struct sparx5 *sparx5, u32 taxi,
+				 struct sparx5_calendar_data *data);
 };
 
 struct sparx5_main_io_resource {
@@ -423,6 +441,9 @@ void sparx5_vlan_port_apply(struct sparx5 *sparx5, struct sparx5_port *port);
 /* sparx5_calendar.c */
 int sparx5_config_auto_calendar(struct sparx5 *sparx5);
 int sparx5_config_dsm_calendar(struct sparx5 *sparx5);
+int sparx5_dsm_calendar_calc(struct sparx5 *sparx5, u32 taxi,
+			     struct sparx5_calendar_data *data);
+
 
 /* sparx5_ethtool.c */
 void sparx5_get_stats64(struct net_device *ndev, struct rtnl_link_stats64 *stats);

-- 
2.34.1

[PATCH net-next 15/15] net: sparx5: add is_sparx5 macro and use it throughout

[Daniel Machon]

We dont want to ops out each time a function needs to do some platform
specifics. In particular we have a few places, where it would be
convenient to just branch out on the platform type. Add the function
is_sparx5() and, initially, use it for:

    - register writes that should only be done on Sparx5 (QSYS_CAL_CTRL,
      CLKGEN_LCPLL1_CORE_CLK).

    - function calls that should only be done on Sparx5
      (ethtool_op_get_ts_info())

    - register writes that are chip-exclusive (MASK_CFG1/2, PGID_CFG1/2,
      these are replicated for n_ports >32 on Sparx5).

The is_sparx5() function simply checks the target chip type, to
determine if this is a Sparx5 SKU or not.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
---
 .../ethernet/microchip/sparx5/sparx5_calendar.c    |  7 +-
 .../net/ethernet/microchip/sparx5/sparx5_ethtool.c |  2 +-
 .../net/ethernet/microchip/sparx5/sparx5_main.c    | 88 +++++++++++++---------
 .../net/ethernet/microchip/sparx5/sparx5_main.h    |  1 +
 .../net/ethernet/microchip/sparx5/sparx5_vlan.c    | 42 +++++++----
 5 files changed, 88 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c b/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
index 35456cd35a40..78600b6aeaf2 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c
@@ -193,9 +193,10 @@ int sparx5_config_auto_calendar(struct sparx5 *sparx5)
 	}
 
 	/* Halt the calendar while changing it */
-	spx5_rmw(QSYS_CAL_CTRL_CAL_MODE_SET(10),
-		 QSYS_CAL_CTRL_CAL_MODE,
-		 sparx5, QSYS_CAL_CTRL);
+	if (is_sparx5(sparx5))
+		spx5_rmw(QSYS_CAL_CTRL_CAL_MODE_SET(10),
+			 QSYS_CAL_CTRL_CAL_MODE,
+			 sparx5, QSYS_CAL_CTRL);
 
 	/* Assign port bandwidth to auto calendar */
 	for (idx = 0; idx < SPX5_CONST(n_auto_cals); idx++)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c b/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c
index 4176733179db..516eb107040f 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_ethtool.c
@@ -1189,7 +1189,7 @@ static int sparx5_get_ts_info(struct net_device *dev,
 	struct sparx5 *sparx5 = port->sparx5;
 	struct sparx5_phc *phc;
 
-	if (!sparx5->ptp)
+	if (!sparx5->ptp && is_sparx5(sparx5))
 		return ethtool_op_get_ts_info(dev, info);
 
 	phc = &sparx5->phc[SPARX5_PHC_PORT];
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index 67e8d2d70816..04ccfb448c2c 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -210,6 +210,25 @@ static const struct sparx5_main_io_resource sparx5_main_iomap[] =  {
 	{ TARGET_VOP,                0x11a00000, 2 }, /* 0x611a00000 */
 };
 
+bool is_sparx5(struct sparx5 *sparx5)
+{
+	switch (sparx5->target_ct) {
+	case SPX5_TARGET_CT_7546:
+	case SPX5_TARGET_CT_7549:
+	case SPX5_TARGET_CT_7552:
+	case SPX5_TARGET_CT_7556:
+	case SPX5_TARGET_CT_7558:
+	case SPX5_TARGET_CT_7546TSN:
+	case SPX5_TARGET_CT_7549TSN:
+	case SPX5_TARGET_CT_7552TSN:
+	case SPX5_TARGET_CT_7556TSN:
+	case SPX5_TARGET_CT_7558TSN:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int sparx5_create_targets(struct sparx5 *sparx5)
 {
 	const struct sparx5_main_io_resource *iomap = sparx5->data->iomap;
@@ -464,44 +483,45 @@ static int sparx5_init_coreclock(struct sparx5 *sparx5)
 		return -ENODEV;
 	}
 
-	switch (freq) {
-	case SPX5_CORE_CLOCK_250MHZ:
-		clk_div = 10;
-		pol_upd_int = 312;
-		break;
-	case SPX5_CORE_CLOCK_500MHZ:
-		clk_div = 5;
-		pol_upd_int = 624;
-		break;
-	case SPX5_CORE_CLOCK_625MHZ:
-		clk_div = 4;
-		pol_upd_int = 780;
-		break;
-	default:
-		dev_err(sparx5->dev, "%d coreclock not supported on (%#04x)\n",
-			sparx5->coreclock, sparx5->target_ct);
-		return -EINVAL;
+	if (is_sparx5(sparx5)) {
+		switch (freq) {
+		case SPX5_CORE_CLOCK_250MHZ:
+			clk_div = 10;
+			pol_upd_int = 312;
+			break;
+		case SPX5_CORE_CLOCK_500MHZ:
+			clk_div = 5;
+			pol_upd_int = 624;
+			break;
+		case SPX5_CORE_CLOCK_625MHZ:
+			clk_div = 4;
+			pol_upd_int = 780;
+			break;
+		default:
+			dev_err(sparx5->dev,
+				"%d coreclock not supported on (%#04x)\n",
+				sparx5->coreclock, sparx5->target_ct);
+			return -EINVAL;
+		}
+
+		/* Configure the LCPLL */
+		spx5_rmw(CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_CLK_DIV_SET(clk_div) |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_PRE_DIV_SET(0) |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_DIR_SET(0) |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_SEL_SET(0) |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_ENA_SET(0) |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_CLK_ENA_SET(1),
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_CLK_DIV |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_PRE_DIV |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_DIR |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_SEL |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_ENA |
+			 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_CLK_ENA,
+			 sparx5, CLKGEN_LCPLL1_CORE_CLK_CFG);
 	}
 
 	/* Update state with chosen frequency */
 	sparx5->coreclock = freq;
-
-	/* Configure the LCPLL */
-	spx5_rmw(CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_CLK_DIV_SET(clk_div) |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_PRE_DIV_SET(0) |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_DIR_SET(0) |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_SEL_SET(0) |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_ENA_SET(0) |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_CLK_ENA_SET(1),
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_CLK_DIV |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_PRE_DIV |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_DIR |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_SEL |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_ROT_ENA |
-		 CLKGEN_LCPLL1_CORE_CLK_CFG_CORE_CLK_ENA,
-		 sparx5,
-		 CLKGEN_LCPLL1_CORE_CLK_CFG);
-
 	clk_period = sparx5_clk_period(freq);
 
 	spx5_rmw(HSCH_SYS_CLK_PER_100PS_SET(clk_period / 100),
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 55fc21fbf63d..6aa7b58556e9 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -380,6 +380,7 @@ struct sparx5 {
 
 /* sparx5_main.c */
 extern const struct sparx5_regs *regs;
+bool is_sparx5(struct sparx5 *sparx5);
 
 /* sparx5_switchdev.c */
 int sparx5_register_notifier_blocks(struct sparx5 *sparx5);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c b/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
index 5d5e5c2c05c5..36a5b3c09469 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
@@ -16,8 +16,10 @@ static int sparx5_vlant_set_mask(struct sparx5 *sparx5, u16 vid)
 
 	/* Output mask to respective registers */
 	spx5_wr(mask[0], sparx5, ANA_L3_VLAN_MASK_CFG(vid));
-	spx5_wr(mask[1], sparx5, ANA_L3_VLAN_MASK_CFG1(vid));
-	spx5_wr(mask[2], sparx5, ANA_L3_VLAN_MASK_CFG2(vid));
+	if (is_sparx5(sparx5)) {
+		spx5_wr(mask[1], sparx5, ANA_L3_VLAN_MASK_CFG1(vid));
+		spx5_wr(mask[2], sparx5, ANA_L3_VLAN_MASK_CFG2(vid));
+	}
 
 	return 0;
 }
@@ -141,15 +143,19 @@ void sparx5_pgid_update_mask(struct sparx5_port *port, int pgid, bool enable)
 void sparx5_pgid_clear(struct sparx5 *spx5, int pgid)
 {
 	spx5_wr(0, spx5, ANA_AC_PGID_CFG(pgid));
-	spx5_wr(0, spx5, ANA_AC_PGID_CFG1(pgid));
-	spx5_wr(0, spx5, ANA_AC_PGID_CFG2(pgid));
+	if (is_sparx5(spx5)) {
+		spx5_wr(0, spx5, ANA_AC_PGID_CFG1(pgid));
+		spx5_wr(0, spx5, ANA_AC_PGID_CFG2(pgid));
+	}
 }
 
 void sparx5_pgid_read_mask(struct sparx5 *spx5, int pgid, u32 portmask[3])
 {
 	portmask[0] = spx5_rd(spx5, ANA_AC_PGID_CFG(pgid));
-	portmask[1] = spx5_rd(spx5, ANA_AC_PGID_CFG1(pgid));
-	portmask[2] = spx5_rd(spx5, ANA_AC_PGID_CFG2(pgid));
+	if (is_sparx5(spx5)) {
+		portmask[1] = spx5_rd(spx5, ANA_AC_PGID_CFG1(pgid));
+		portmask[2] = spx5_rd(spx5, ANA_AC_PGID_CFG2(pgid));
+	}
 }
 
 void sparx5_update_fwd(struct sparx5 *sparx5)
@@ -164,8 +170,10 @@ void sparx5_update_fwd(struct sparx5 *sparx5)
 	/* Update flood masks */
 	for (port = PGID_UC_FLOOD; port <= PGID_BCAST; port++) {
 		spx5_wr(mask[0], sparx5, ANA_AC_PGID_CFG(port));
-		spx5_wr(mask[1], sparx5, ANA_AC_PGID_CFG1(port));
-		spx5_wr(mask[2], sparx5, ANA_AC_PGID_CFG2(port));
+		if (is_sparx5(sparx5)) {
+			spx5_wr(mask[1], sparx5, ANA_AC_PGID_CFG1(port));
+			spx5_wr(mask[2], sparx5, ANA_AC_PGID_CFG2(port));
+		}
 	}
 
 	/* Update SRC masks */
@@ -176,12 +184,16 @@ void sparx5_update_fwd(struct sparx5 *sparx5)
 			clear_bit(port, workmask);
 			bitmap_to_arr32(mask, workmask, SPX5_PORTS);
 			spx5_wr(mask[0], sparx5, ANA_AC_SRC_CFG(port));
-			spx5_wr(mask[1], sparx5, ANA_AC_SRC_CFG1(port));
-			spx5_wr(mask[2], sparx5, ANA_AC_SRC_CFG2(port));
+			if (is_sparx5(sparx5)) {
+				spx5_wr(mask[1], sparx5, ANA_AC_SRC_CFG1(port));
+				spx5_wr(mask[2], sparx5, ANA_AC_SRC_CFG2(port));
+			}
 		} else {
 			spx5_wr(0, sparx5, ANA_AC_SRC_CFG(port));
-			spx5_wr(0, sparx5, ANA_AC_SRC_CFG1(port));
-			spx5_wr(0, sparx5, ANA_AC_SRC_CFG2(port));
+			if (is_sparx5(sparx5)) {
+				spx5_wr(0, sparx5, ANA_AC_SRC_CFG1(port));
+				spx5_wr(0, sparx5, ANA_AC_SRC_CFG2(port));
+			}
 		}
 	}
 
@@ -192,8 +204,10 @@ void sparx5_update_fwd(struct sparx5 *sparx5)
 
 	/* Apply learning mask */
 	spx5_wr(mask[0], sparx5, ANA_L2_AUTO_LRN_CFG);
-	spx5_wr(mask[1], sparx5, ANA_L2_AUTO_LRN_CFG1);
-	spx5_wr(mask[2], sparx5, ANA_L2_AUTO_LRN_CFG2);
+	if (is_sparx5(sparx5)) {
+		spx5_wr(mask[1], sparx5, ANA_L2_AUTO_LRN_CFG1);
+		spx5_wr(mask[2], sparx5, ANA_L2_AUTO_LRN_CFG2);
+	}
 }
 
 void sparx5_vlan_port_apply(struct sparx5 *sparx5,

-- 
2.34.1

Re: [PATCH net-next 02/15] net: sparx5: add indirection layer to register macros

[Jacob Keller]

On 10/1/2024 6:50 AM, Daniel Machon wrote:
> The register macros are used to read and write to the switch registers.
> The registers are largely the same on Sparx5 and lan969x, however in some
> cases they differ. The differences can be one or more of the following:
> target size, register address, register count, group address, group
> count, group size, field position, field size.
> 
> In order to handle these differences, we introduce a new indirection
> layer, that defines and maps them to corresponding values, based on the
> platform. As the register macro arguments can now be non-constants, we
> also add non-constant variants of FIELD_GET and FIELD_PREP.
> 
> Since the indirection layer contributes to longer macros, we have
> changed the formatting of them slightly, to adhere to a 80 character
> limit, and added a comment if a macro is platform-specific.
> 
> With these additions, we can reuse all the existing macros for
> lan969x.
> 
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
> ---
>  drivers/net/ethernet/microchip/sparx5/Makefile     |    2 +-
>  .../net/ethernet/microchip/sparx5/sparx5_main.c    |   17 +
>  .../net/ethernet/microchip/sparx5/sparx5_main.h    |   15 +
>  .../ethernet/microchip/sparx5/sparx5_main_regs.h   | 4128 +++++++++++---------
>  .../net/ethernet/microchip/sparx5/sparx5_regs.c    |  219 ++
>  .../net/ethernet/microchip/sparx5/sparx5_regs.h    |  244 ++
>  6 files changed, 2755 insertions(+), 1870 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/sparx5/Makefile b/drivers/net/ethernet/microchip/sparx5/Makefile
> index 288de95add18..3435ca86dd70 100644
> --- a/drivers/net/ethernet/microchip/sparx5/Makefile
> +++ b/drivers/net/ethernet/microchip/sparx5/Makefile
> @@ -11,7 +11,7 @@ sparx5-switch-y  := sparx5_main.o sparx5_packet.o \
>   sparx5_ptp.o sparx5_pgid.o sparx5_tc.o sparx5_qos.o \
>   sparx5_vcap_impl.o sparx5_vcap_ag_api.o sparx5_tc_flower.o \
>   sparx5_tc_matchall.o sparx5_pool.o sparx5_sdlb.o sparx5_police.o \
> - sparx5_psfp.o sparx5_mirror.o
> + sparx5_psfp.o sparx5_mirror.o sparx5_regs.o
>  
>  sparx5-switch-$(CONFIG_SPARX5_DCB) += sparx5_dcb.o
>  sparx5-switch-$(CONFIG_DEBUG_FS) += sparx5_vcap_debugfs.o
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> index 179a1dc0d8f6..9a8d2e8c02a5 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> @@ -29,6 +29,8 @@
>  #include "sparx5_port.h"
>  #include "sparx5_qos.h"
>  
> +const struct sparx5_regs *regs;
> +
>  #define QLIM_WM(fraction) \
>  	((SPX5_BUFFER_MEMORY / SPX5_BUFFER_CELL_SZ - 100) * (fraction) / 100)
>  #define IO_RANGES 3
> @@ -759,6 +761,9 @@ static int mchp_sparx5_probe(struct platform_device *pdev)
>  	sparx5->data = device_get_match_data(sparx5->dev);
>  	if (!sparx5->data)
>  		return -EINVAL;
> +
> +	regs = sparx5->data->regs;
> +
>  	/* Do switch core reset if available */
>  	reset = devm_reset_control_get_optional_shared(&pdev->dev, "switch");
>  	if (IS_ERR(reset))
> @@ -937,10 +942,22 @@ static void mchp_sparx5_remove(struct platform_device *pdev)
>  	destroy_workqueue(sparx5->mact_queue);
>  }
>  
> +static const struct sparx5_regs sparx5_regs = {
> +	.tsize = sparx5_tsize,
> +	.gaddr = sparx5_gaddr,
> +	.gcnt = sparx5_gcnt,
> +	.gsize = sparx5_gsize,
> +	.raddr = sparx5_raddr,
> +	.rcnt = sparx5_rcnt,
> +	.fpos = sparx5_fpos,
> +	.fsize = sparx5_fsize,
> +};
> +
>  static const struct sparx5_match_data sparx5_desc = {
>  	.iomap = sparx5_main_iomap,
>  	.iomap_size = ARRAY_SIZE(sparx5_main_iomap),
>  	.ioranges = 3,
> +	.regs = &sparx5_regs,
>  };
>  
>  static const struct of_device_id mchp_sparx5_match[] = {
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> index 845f918aaf5e..e3f22b730d80 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> @@ -226,6 +226,17 @@ struct sparx5_mall_entry {
>  #define SPARX5_SKB_CB(skb) \
>  	((struct sparx5_skb_cb *)((skb)->cb))
>  
> +struct sparx5_regs {
> +	const unsigned int *tsize;
> +	const unsigned int *gaddr;
> +	const unsigned int *gcnt;
> +	const unsigned int *gsize;
> +	const unsigned int *raddr;
> +	const unsigned int *rcnt;
> +	const unsigned int *fpos;
> +	const unsigned int *fsize;
> +};
> +
>  struct sparx5_main_io_resource {
>  	enum sparx5_target id;
>  	phys_addr_t offset;
> @@ -233,6 +244,7 @@ struct sparx5_main_io_resource {
>  };
>  
>  struct sparx5_match_data {
> +	const struct sparx5_regs *regs;
>  	const struct sparx5_main_io_resource *iomap;
>  	int ioranges;
>  	int iomap_size;
> @@ -308,6 +320,9 @@ struct sparx5 {
>  	const struct sparx5_match_data *data;
>  };
>  
> +/* sparx5_main.c */
> +extern const struct sparx5_regs *regs;
> +
>  /* sparx5_switchdev.c */
>  int sparx5_register_notifier_blocks(struct sparx5 *sparx5);
>  void sparx5_unregister_notifier_blocks(struct sparx5 *sparx5);
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h
> index 22acc1f3380c..3783cfd1d855 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h
> @@ -1,11 +1,11 @@
>  /* SPDX-License-Identifier: GPL-2.0+
>   * Microchip Sparx5 Switch driver
>   *
> - * Copyright (c) 2021 Microchip Technology Inc.
> + * Copyright (c) 2024 Microchip Technology Inc.
>   */
>  
> -/* This file is autogenerated by cml-utils 2023-02-10 11:18:53 +0100.
> - * Commit ID: c30fb4bf0281cd4a7133bdab6682f9e43c872ada
> +/* This file is autogenerated by cml-utils 2024-09-24 14:13:28 +0200.
> + * Commit ID: 9d07b8d19363f3cd3590ddb3f7a2e2768e16524b
>   */
>  
>  #ifndef _SPARX5_MAIN_REGS_H_
> @@ -15,6 +15,8 @@
>  #include <linux/types.h>
>  #include <linux/bug.h>
>  
> +#include "sparx5_regs.h"
> +
>  enum sparx5_target {
>  	TARGET_ANA_AC = 1,
>  	TARGET_ANA_ACL = 2,
> @@ -52,14 +54,28 @@ enum sparx5_target {
>  	TARGET_VCAP_SUPER = 326,
>  	TARGET_VOP = 327,
>  	TARGET_XQS = 331,
> -	NUM_TARGETS = 332
> +	NUM_TARGETS = 517
>  };
>  
> +/* Non-constant mask variant of FIELD_GET() and FIELD_PREP() */
> +#define spx5_field_get(_mask, _reg) (((_reg) & (_mask)) >> (ffs(_mask) - 1))
> +#define spx5_field_prep(_mask, _val) (((_val) << (ffs(_mask) - 1)) & (_mask))
> +

FIELD_GET and FIELD_SET have restrictions in place to enforce constant
mask, which enables strict checks to ensure things fit and determine the
bit shifts at compile time without ffs.

Would it make sense for these to exist in <linux/bitfields.h>? I'm not
sure how common it is to have non-const masks..

> +#define GADDR(o)  regs->gaddr[o]
> +#define GCNT(o)   regs->gcnt[o]
> +#define GSIZE(o)  regs->gsize[o]
> +#define RADDR(o)  regs->raddr[o]
> +#define RCNT(o)   regs->rcnt[o]
> +#define FPOS(o)   regs->fpos[o]
> +#define FSIZE(o)  regs->fsize[o]
> +#define TSIZE(o)  regs->tsize[o]

This implementation requires 'regs' to be in scope without being passed
as a parameter. I guess thats just assumed here?

Re: [PATCH net-next 02/15] net: sparx5: add indirection layer to register macros

[Daniel Machon]

> > +/* Non-constant mask variant of FIELD_GET() and FIELD_PREP() */
> > +#define spx5_field_get(_mask, _reg) (((_reg) & (_mask)) >> (ffs(_mask) - 1))
> > +#define spx5_field_prep(_mask, _val) (((_val) << (ffs(_mask) - 1)) & (_mask))
> > +
> 
> FIELD_GET and FIELD_SET have restrictions in place to enforce constant
> mask, which enables strict checks to ensure things fit and determine the
> bit shifts at compile time without ffs.
> 
> Would it make sense for these to exist in <linux/bitfields.h>? I'm not
> sure how common it is to have non-const masks..

There was a patch for this some time ago [1], it got some push-back and
never got in, AFAICS. 

[1] https://patchwork.kernel.org/project/linux-pm/patch/3a54a6703879d10f08cf0275a2a69297ebd2b1d4.1637592133.git.geert+renesas@glider.be/#24611465

/Daniel

Re: [PATCH net-next 00/15] net: sparx5: prepare for lan969x switch driver

[Jacob Keller]

On 10/1/2024 6:50 AM, Daniel Machon wrote:
> == Description:
> 
> This series is the first of a multi-part series, that prepares and adds
> support for the new lan969x switch driver.
> 
> The upstreaming efforts is split into multiple series (might change a
> bit as we go along):
> 
>     1) Prepare the Sparx5 driver for lan969x (this series)
>     2) Add support lan969x (same basic features as Sparx5 provides +
>        RGMII, excl.  FDMA and VCAP)
>     3) Add support for lan969x FDMA
>     4) Add support for lan969x VCAP
> 
> == Lan969x in short:
> 
> The lan969x Ethernet switch family [1] provides a rich set of
> switching features and port configurations (up to 30 ports) from 10Mbps
> to 10Gbps, with support for RGMII, SGMII, QSGMII, USGMII, and USXGMII,
> ideal for industrial & process automation infrastructure applications,
> transport, grid automation, power substation automation, and ring &
> intra-ring topologies. The LAN969x family is hardware and software
> compatible and scalable supporting 46Gbps to 102Gbps switch bandwidths.
> 
> == Preparing Sparx5 for lan969x:
> 
> The lan969x switch chip reuses many of the IP's of the Sparx5 switch
> chip, therefore it has been decided to add support through the existing
> Sparx5 driver, in order to avoid a bunch of duplicate code. However, in
> order to reuse the Sparx5 switch driver, we have to introduce some
> mechanisms to handle the chip differences that are there.  These
> mechanisms are:
> 
>     - Platform match data to contain all the differences that needs to
>       be handled (constants, ops etc.)
> 
>     - Register macro indirection layer so that we can reuse the existing
>       register macros.
> 
>     - Function for branching out on platform type where required.
> 
> In some places we ops out functions and in other places we branch on the
> chip type. Exactly when we choose one over the other, is an estimate in
> each case.
> 
> After this series is applied, the Sparx5 driver will be prepared for
> lan969x and still function exactly as before.
> 
> == Patch breakdown:
> 
> Patch #1     adds private match data
> 
> Patch #2     adds register macro indirection layer
> 
> Patch #3-#5  does some preparation work
> 
> Patch #6-#8  adds chip constants and updates the code to use them
> 
> Patch #9-#14 adds and uses ops for handling functions differently on the
>              two platforms.
> 
> Patch #15    adds and uses a macro for branching out on the chip type
> 
> [1] https://www.microchip.com/en-us/product/lan9698
> 

The series seems ok to me. I'm not personally a fan of the implicit
local variables used by macros. I do not know how common that is, or
what others on the list feel about this.

For everything else:

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH net-next 00/15] net: sparx5: prepare for lan969x switch driver

[Daniel Machon]

> > == Description:
> >
> > This series is the first of a multi-part series, that prepares and adds
> > support for the new lan969x switch driver.
> >
> > The upstreaming efforts is split into multiple series (might change a
> > bit as we go along):
> >
> >     1) Prepare the Sparx5 driver for lan969x (this series)
> >     2) Add support lan969x (same basic features as Sparx5 provides +
> >        RGMII, excl.  FDMA and VCAP)
> >     3) Add support for lan969x FDMA
> >     4) Add support for lan969x VCAP
> >
> > == Lan969x in short:
> >
> > The lan969x Ethernet switch family [1] provides a rich set of
> > switching features and port configurations (up to 30 ports) from 10Mbps
> > to 10Gbps, with support for RGMII, SGMII, QSGMII, USGMII, and USXGMII,
> > ideal for industrial & process automation infrastructure applications,
> > transport, grid automation, power substation automation, and ring &
> > intra-ring topologies. The LAN969x family is hardware and software
> > compatible and scalable supporting 46Gbps to 102Gbps switch bandwidths.
> >
> > == Preparing Sparx5 for lan969x:
> >
> > The lan969x switch chip reuses many of the IP's of the Sparx5 switch
> > chip, therefore it has been decided to add support through the existing
> > Sparx5 driver, in order to avoid a bunch of duplicate code. However, in
> > order to reuse the Sparx5 switch driver, we have to introduce some
> > mechanisms to handle the chip differences that are there.  These
> > mechanisms are:
> >
> >     - Platform match data to contain all the differences that needs to
> >       be handled (constants, ops etc.)
> >
> >     - Register macro indirection layer so that we can reuse the existing
> >       register macros.
> >
> >     - Function for branching out on platform type where required.
> >
> > In some places we ops out functions and in other places we branch on the
> > chip type. Exactly when we choose one over the other, is an estimate in
> > each case.
> >
> > After this series is applied, the Sparx5 driver will be prepared for
> > lan969x and still function exactly as before.
> >
> > == Patch breakdown:
> >
> > Patch #1     adds private match data
> >
> > Patch #2     adds register macro indirection layer
> >
> > Patch #3-#5  does some preparation work
> >
> > Patch #6-#8  adds chip constants and updates the code to use them
> >
> > Patch #9-#14 adds and uses ops for handling functions differently on the
> >              two platforms.
> >
> > Patch #15    adds and uses a macro for branching out on the chip type
> >
> > [1] https://www.microchip.com/en-us/product/lan9698
> >
> 
> The series seems ok to me. I'm not personally a fan of the implicit
> local variables used by macros. I do not know how common that is, or
> what others on the list feel about this.
> 
> For everything else:
> 
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Hi Jakob,

First off, thank you for your reviews - I really appreciate it.

Let me address your "variable scope" conerns:

I had the feeling that this could pontentially be somewhat contentious.

Basically, this comes down to making the least invasive changes to the
existing driver code. With this approach:

    For the SPX5_CONST macro this means shorter lines, and less 80 char
    wrapping.

    For the "*regs" variable this means not having to pass in the sparx5
    pointer to *all* register macros, which requires changes *all* over
    the code.

I thought the solution with an in-scope implicit variable was less
invasive (and maybe even more readable?). Just my opinion, given the
alternative.

Obviously I did a bit of research on this upfront, and I can point to
*many* places where drivers do the exact same (not justifying the use,
just pointing that out). Here is an intel driver that does the same [1]
(look at the *hw variable)

I am willing to come up with something different, if you really think
this is a no-go. Let me hear your thoughts. I think this applies to your
comments on #2, #3 and #6 as well.

/Daniel

[1] https://elixir.bootlin.com/linux/v6.12-rc1/source/drivers/net/ethernet/intel/igb/igb_main.c#L4746

Re: [PATCH net-next 00/15] net: sparx5: prepare for lan969x switch driver

[Jacob Keller]

On 10/2/2024 12:47 AM, Daniel Machon wrote:
> 
> Hi Jakob,
> 
> First off, thank you for your reviews - I really appreciate it.
> 
> Let me address your "variable scope" conerns:
> 
> I had the feeling that this could pontentially be somewhat contentious.
> 
> Basically, this comes down to making the least invasive changes to the
> existing driver code. With this approach:
> 
>     For the SPX5_CONST macro this means shorter lines, and less 80 char
>     wrapping.
> 
>     For the "*regs" variable this means not having to pass in the sparx5
>     pointer to *all* register macros, which requires changes *all* over
>     the code.
> 
> I thought the solution with an in-scope implicit variable was less
> invasive (and maybe even more readable?). Just my opinion, given the
> alternative.
>

Obviously there is style preference here, and someone working
day-in/day-out on the code is likely to know which macros have which
variable dependencies. As an external reviewer, I would not know that,
so I would find it surprising when looking at some code which passes a
parameter which is never directly used.

> Obviously I did a bit of research on this upfront, and I can point to
> *many* places where drivers do the exact same (not justifying the use,
> just pointing that out). Here is an intel driver that does the same [1]
> (look at the *hw variable)

Yea, I'm sure a lot of the old Intel drivers have bad examples :D I've
spent a career trying to improve this.

> 
> I am willing to come up with something different, if you really think
> this is a no-go. Let me hear your thoughts. I think this applies to your
> comments on #2, #3 and #6 as well.
> 

It seems that Jakub Kicinski wants the entire macro removed, and his
opinion as maintainer matters more than mine.

Personally, I'm not opposed to a macro itself especially if the direct
access starts to get very long. However, I think the parameter being
accessed should be, well, a parameter of the macro.

> /Daniel
> 
> [1] https://elixir.bootlin.com/linux/v6.12-rc1/source/drivers/net/ethernet/intel/igb/igb_main.c#L4746
> 
> 

As an example of *why* I don't like this practice: It took me a while to
realize the hw variable was implicit to wr32. And I worked on this driver.

Thanks,
Jake