[PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support

[PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support

[Andrew Lunn]

Make use of devlink regions to allow read access to some of the
internal of the switches. The switch itself will never trigger a
region snapshot, it is assumed it is performed from user space as
needed.

Andrew Lunn (7):
  net: dsa: Add helper to convert from devlink to ds
  net: dsa: Add devlink regions support to DSA
  net: dsa: mv88e6xxx: Move devlink code into its own file
  net: dsa: mv88e6xxx: Create helper for FIDs in use
  net: dsa: mv88e6xxx: Add devlink regions
  net: dsa: wire up devlink info get
  net: dsa: mv88e6xxx: Implement devlink info get callback

 drivers/net/dsa/mv88e6xxx/Makefile  |   1 +
 drivers/net/dsa/mv88e6xxx/chip.c    | 290 ++----------
 drivers/net/dsa/mv88e6xxx/chip.h    |  14 +
 drivers/net/dsa/mv88e6xxx/devlink.c | 690 ++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/devlink.h |  21 +
 include/net/dsa.h                   |  13 +-
 net/dsa/dsa.c                       |  36 +-
 net/dsa/dsa2.c                      |  21 +-
 8 files changed, 813 insertions(+), 273 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.h

-- 
2.28.0

[PATCH net-next 1/7] net: dsa: Add helper to convert from devlink to ds

[Andrew Lunn]

Given a devlink instance, return the dsa switch it is associated to.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/dsa.h |  2 ++
 net/dsa/dsa.c     | 20 ++++++++++----------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 75c8fac82017..63ff6f717307 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -629,6 +629,8 @@ struct dsa_switch_ops {
 	int	(*port_max_mtu)(struct dsa_switch *ds, int port);
 };
 
+struct dsa_switch *dsa_devlink_to_ds(struct devlink *dl);
+
 #define DSA_DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes)		\
 	DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes,		\
 			     dsa_devlink_param_get, dsa_devlink_param_set, NULL)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 1ce9ba8cf545..86351da4e202 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -327,14 +327,18 @@ int call_dsa_notifiers(unsigned long val, struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(call_dsa_notifiers);
 
+struct dsa_switch *dsa_devlink_to_ds(struct devlink *dl)
+{
+	struct dsa_devlink_priv *dl_priv = devlink_priv(dl);
+
+	return dl_priv->ds;
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_to_ds);
+
 int dsa_devlink_param_get(struct devlink *dl, u32 id,
 			  struct devlink_param_gset_ctx *ctx)
 {
-	struct dsa_devlink_priv *dl_priv;
-	struct dsa_switch *ds;
-
-	dl_priv = devlink_priv(dl);
-	ds = dl_priv->ds;
+	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
 
 	if (!ds->ops->devlink_param_get)
 		return -EOPNOTSUPP;
@@ -346,11 +350,7 @@ EXPORT_SYMBOL_GPL(dsa_devlink_param_get);
 int dsa_devlink_param_set(struct devlink *dl, u32 id,
 			  struct devlink_param_gset_ctx *ctx)
 {
-	struct dsa_devlink_priv *dl_priv;
-	struct dsa_switch *ds;
-
-	dl_priv = devlink_priv(dl);
-	ds = dl_priv->ds;
+	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
 
 	if (!ds->ops->devlink_param_set)
 		return -EOPNOTSUPP;
-- 
2.28.0

[PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA

[Andrew Lunn]

Allow DSA drivers to make use of devlink regions, via simple wrappers.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/dsa.h |  6 ++++++
 net/dsa/dsa.c     | 16 ++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 63ff6f717307..8963440ec7f8 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -660,6 +660,12 @@ void dsa_devlink_resource_occ_get_register(struct dsa_switch *ds,
 					   void *occ_get_priv);
 void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
 					     u64 resource_id);
+struct devlink_region *
+dsa_devlink_region_create(struct dsa_switch *ds,
+			  const struct devlink_region_ops *ops,
+			  u32 region_max_snapshots, u64 region_size);
+void dsa_devlink_region_destroy(struct devlink_region *region);
+
 struct dsa_port *dsa_port_from_netdev(struct net_device *netdev);
 
 struct dsa_devlink_priv {
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 86351da4e202..fea2efe5fe68 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -412,6 +412,22 @@ void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
 }
 EXPORT_SYMBOL_GPL(dsa_devlink_resource_occ_get_unregister);
 
+struct devlink_region *
+dsa_devlink_region_create(struct dsa_switch *ds,
+			  const struct devlink_region_ops *ops,
+			  u32 region_max_snapshots, u64 region_size)
+{
+	return devlink_region_create(ds->devlink, ops, region_max_snapshots,
+				     region_size);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_region_create);
+
+void dsa_devlink_region_destroy(struct devlink_region *region)
+{
+	devlink_region_destroy(region);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_region_destroy);
+
 struct dsa_port *dsa_port_from_netdev(struct net_device *netdev)
 {
 	if (!netdev || !dsa_slave_dev_check(netdev))
-- 
2.28.0

[PATCH net-next 3/7] net: dsa: mv88e6xxx: Move devlink code into its own file

[Andrew Lunn]

There will soon be more devlink code. Move the existing code into a
file of its own, before we start adding this new code.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/Makefile  |   1 +
 drivers/net/dsa/mv88e6xxx/chip.c    | 255 +--------------------------
 drivers/net/dsa/mv88e6xxx/devlink.c | 262 ++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/devlink.h |  16 ++
 4 files changed, 280 insertions(+), 254 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.h

diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
index aa645ff86f64..4b080b448ce7 100644
--- a/drivers/net/dsa/mv88e6xxx/Makefile
+++ b/drivers/net/dsa/mv88e6xxx/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_NET_DSA_MV88E6XXX) += mv88e6xxx.o
 mv88e6xxx-objs := chip.o
+mv88e6xxx-objs += devlink.o
 mv88e6xxx-objs += global1.o
 mv88e6xxx-objs += global1_atu.o
 mv88e6xxx-objs += global1_vtu.o
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 7a71c9902e73..940dd0c55568 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -32,6 +32,7 @@
 #include <net/dsa.h>
 
 #include "chip.h"
+#include "devlink.h"
 #include "global1.h"
 #include "global2.h"
 #include "hwtstamp.h"
@@ -1508,22 +1509,6 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
 	return mv88e6xxx_g1_atu_flush(chip, *fid, true);
 }
 
-static int mv88e6xxx_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
-{
-	if (chip->info->ops->atu_get_hash)
-		return chip->info->ops->atu_get_hash(chip, hash);
-
-	return -EOPNOTSUPP;
-}
-
-static int mv88e6xxx_atu_set_hash(struct mv88e6xxx_chip *chip, u8 hash)
-{
-	if (chip->info->ops->atu_set_hash)
-		return chip->info->ops->atu_set_hash(chip, hash);
-
-	return -EOPNOTSUPP;
-}
-
 static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 					u16 vid_begin, u16 vid_end)
 {
@@ -2837,244 +2822,6 @@ static int mv88e6390_setup_errata(struct mv88e6xxx_chip *chip)
 	return mv88e6xxx_software_reset(chip);
 }
 
-enum mv88e6xxx_devlink_param_id {
-	MV88E6XXX_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
-	MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH,
-};
-
-static int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
-				       struct devlink_param_gset_ctx *ctx)
-{
-	struct mv88e6xxx_chip *chip = ds->priv;
-	int err;
-
-	mv88e6xxx_reg_lock(chip);
-
-	switch (id) {
-	case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
-		err = mv88e6xxx_atu_get_hash(chip, &ctx->val.vu8);
-		break;
-	default:
-		err = -EOPNOTSUPP;
-		break;
-	}
-
-	mv88e6xxx_reg_unlock(chip);
-
-	return err;
-}
-
-static int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
-				       struct devlink_param_gset_ctx *ctx)
-{
-	struct mv88e6xxx_chip *chip = ds->priv;
-	int err;
-
-	mv88e6xxx_reg_lock(chip);
-
-	switch (id) {
-	case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
-		err = mv88e6xxx_atu_set_hash(chip, ctx->val.vu8);
-		break;
-	default:
-		err = -EOPNOTSUPP;
-		break;
-	}
-
-	mv88e6xxx_reg_unlock(chip);
-
-	return err;
-}
-
-static const struct devlink_param mv88e6xxx_devlink_params[] = {
-	DSA_DEVLINK_PARAM_DRIVER(MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH,
-				 "ATU_hash", DEVLINK_PARAM_TYPE_U8,
-				 BIT(DEVLINK_PARAM_CMODE_RUNTIME)),
-};
-
-static int mv88e6xxx_setup_devlink_params(struct dsa_switch *ds)
-{
-	return dsa_devlink_params_register(ds, mv88e6xxx_devlink_params,
-					   ARRAY_SIZE(mv88e6xxx_devlink_params));
-}
-
-static void mv88e6xxx_teardown_devlink_params(struct dsa_switch *ds)
-{
-	dsa_devlink_params_unregister(ds, mv88e6xxx_devlink_params,
-				      ARRAY_SIZE(mv88e6xxx_devlink_params));
-}
-
-enum mv88e6xxx_devlink_resource_id {
-	MV88E6XXX_RESOURCE_ID_ATU,
-	MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
-	MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
-	MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
-	MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
-};
-
-static u64 mv88e6xxx_devlink_atu_bin_get(struct mv88e6xxx_chip *chip,
-					 u16 bin)
-{
-	u16 occupancy = 0;
-	int err;
-
-	mv88e6xxx_reg_lock(chip);
-
-	err = mv88e6xxx_g2_atu_stats_set(chip, MV88E6XXX_G2_ATU_STATS_MODE_ALL,
-					 bin);
-	if (err) {
-		dev_err(chip->dev, "failed to set ATU stats kind/bin\n");
-		goto unlock;
-	}
-
-	err = mv88e6xxx_g1_atu_get_next(chip, 0);
-	if (err) {
-		dev_err(chip->dev, "failed to perform ATU get next\n");
-		goto unlock;
-	}
-
-	err = mv88e6xxx_g2_atu_stats_get(chip, &occupancy);
-	if (err) {
-		dev_err(chip->dev, "failed to get ATU stats\n");
-		goto unlock;
-	}
-
-	occupancy &= MV88E6XXX_G2_ATU_STATS_MASK;
-
-unlock:
-	mv88e6xxx_reg_unlock(chip);
-
-	return occupancy;
-}
-
-static u64 mv88e6xxx_devlink_atu_bin_0_get(void *priv)
-{
-	struct mv88e6xxx_chip *chip = priv;
-
-	return mv88e6xxx_devlink_atu_bin_get(chip,
-					     MV88E6XXX_G2_ATU_STATS_BIN_0);
-}
-
-static u64 mv88e6xxx_devlink_atu_bin_1_get(void *priv)
-{
-	struct mv88e6xxx_chip *chip = priv;
-
-	return mv88e6xxx_devlink_atu_bin_get(chip,
-					     MV88E6XXX_G2_ATU_STATS_BIN_1);
-}
-
-static u64 mv88e6xxx_devlink_atu_bin_2_get(void *priv)
-{
-	struct mv88e6xxx_chip *chip = priv;
-
-	return mv88e6xxx_devlink_atu_bin_get(chip,
-					     MV88E6XXX_G2_ATU_STATS_BIN_2);
-}
-
-static u64 mv88e6xxx_devlink_atu_bin_3_get(void *priv)
-{
-	struct mv88e6xxx_chip *chip = priv;
-
-	return mv88e6xxx_devlink_atu_bin_get(chip,
-					     MV88E6XXX_G2_ATU_STATS_BIN_3);
-}
-
-static u64 mv88e6xxx_devlink_atu_get(void *priv)
-{
-	return mv88e6xxx_devlink_atu_bin_0_get(priv) +
-		mv88e6xxx_devlink_atu_bin_1_get(priv) +
-		mv88e6xxx_devlink_atu_bin_2_get(priv) +
-		mv88e6xxx_devlink_atu_bin_3_get(priv);
-}
-
-static int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds)
-{
-	struct devlink_resource_size_params size_params;
-	struct mv88e6xxx_chip *chip = ds->priv;
-	int err;
-
-	devlink_resource_size_params_init(&size_params,
-					  mv88e6xxx_num_macs(chip),
-					  mv88e6xxx_num_macs(chip),
-					  1, DEVLINK_RESOURCE_UNIT_ENTRY);
-
-	err = dsa_devlink_resource_register(ds, "ATU",
-					    mv88e6xxx_num_macs(chip),
-					    MV88E6XXX_RESOURCE_ID_ATU,
-					    DEVLINK_RESOURCE_ID_PARENT_TOP,
-					    &size_params);
-	if (err)
-		goto out;
-
-	devlink_resource_size_params_init(&size_params,
-					  mv88e6xxx_num_macs(chip) / 4,
-					  mv88e6xxx_num_macs(chip) / 4,
-					  1, DEVLINK_RESOURCE_UNIT_ENTRY);
-
-	err = dsa_devlink_resource_register(ds, "ATU_bin_0",
-					    mv88e6xxx_num_macs(chip) / 4,
-					    MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
-					    MV88E6XXX_RESOURCE_ID_ATU,
-					    &size_params);
-	if (err)
-		goto out;
-
-	err = dsa_devlink_resource_register(ds, "ATU_bin_1",
-					    mv88e6xxx_num_macs(chip) / 4,
-					    MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
-					    MV88E6XXX_RESOURCE_ID_ATU,
-					    &size_params);
-	if (err)
-		goto out;
-
-	err = dsa_devlink_resource_register(ds, "ATU_bin_2",
-					    mv88e6xxx_num_macs(chip) / 4,
-					    MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
-					    MV88E6XXX_RESOURCE_ID_ATU,
-					    &size_params);
-	if (err)
-		goto out;
-
-	err = dsa_devlink_resource_register(ds, "ATU_bin_3",
-					    mv88e6xxx_num_macs(chip) / 4,
-					    MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
-					    MV88E6XXX_RESOURCE_ID_ATU,
-					    &size_params);
-	if (err)
-		goto out;
-
-	dsa_devlink_resource_occ_get_register(ds,
-					      MV88E6XXX_RESOURCE_ID_ATU,
-					      mv88e6xxx_devlink_atu_get,
-					      chip);
-
-	dsa_devlink_resource_occ_get_register(ds,
-					      MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
-					      mv88e6xxx_devlink_atu_bin_0_get,
-					      chip);
-
-	dsa_devlink_resource_occ_get_register(ds,
-					      MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
-					      mv88e6xxx_devlink_atu_bin_1_get,
-					      chip);
-
-	dsa_devlink_resource_occ_get_register(ds,
-					      MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
-					      mv88e6xxx_devlink_atu_bin_2_get,
-					      chip);
-
-	dsa_devlink_resource_occ_get_register(ds,
-					      MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
-					      mv88e6xxx_devlink_atu_bin_3_get,
-					      chip);
-
-	return 0;
-
-out:
-	dsa_devlink_resources_unregister(ds);
-	return err;
-}
-
 static void mv88e6xxx_teardown(struct dsa_switch *ds)
 {
 	mv88e6xxx_teardown_devlink_params(ds);
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
new file mode 100644
index 000000000000..91e02024c5cf
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <net/dsa.h>
+
+#include "chip.h"
+#include "devlink.h"
+#include "global1.h"
+#include "global2.h"
+
+static int mv88e6xxx_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
+{
+	if (chip->info->ops->atu_get_hash)
+		return chip->info->ops->atu_get_hash(chip, hash);
+
+	return -EOPNOTSUPP;
+}
+
+static int mv88e6xxx_atu_set_hash(struct mv88e6xxx_chip *chip, u8 hash)
+{
+	if (chip->info->ops->atu_set_hash)
+		return chip->info->ops->atu_set_hash(chip, hash);
+
+	return -EOPNOTSUPP;
+}
+
+enum mv88e6xxx_devlink_param_id {
+	MV88E6XXX_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+	MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH,
+};
+
+int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
+				struct devlink_param_gset_ctx *ctx)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	mv88e6xxx_reg_lock(chip);
+
+	switch (id) {
+	case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
+		err = mv88e6xxx_atu_get_hash(chip, &ctx->val.vu8);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	mv88e6xxx_reg_unlock(chip);
+
+	return err;
+}
+
+int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
+				struct devlink_param_gset_ctx *ctx)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	mv88e6xxx_reg_lock(chip);
+
+	switch (id) {
+	case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
+		err = mv88e6xxx_atu_set_hash(chip, ctx->val.vu8);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	mv88e6xxx_reg_unlock(chip);
+
+	return err;
+}
+
+static const struct devlink_param mv88e6xxx_devlink_params[] = {
+	DSA_DEVLINK_PARAM_DRIVER(MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH,
+				 "ATU_hash", DEVLINK_PARAM_TYPE_U8,
+				 BIT(DEVLINK_PARAM_CMODE_RUNTIME)),
+};
+
+int mv88e6xxx_setup_devlink_params(struct dsa_switch *ds)
+{
+	return dsa_devlink_params_register(ds, mv88e6xxx_devlink_params,
+					   ARRAY_SIZE(mv88e6xxx_devlink_params));
+}
+
+void mv88e6xxx_teardown_devlink_params(struct dsa_switch *ds)
+{
+	dsa_devlink_params_unregister(ds, mv88e6xxx_devlink_params,
+				      ARRAY_SIZE(mv88e6xxx_devlink_params));
+}
+
+enum mv88e6xxx_devlink_resource_id {
+	MV88E6XXX_RESOURCE_ID_ATU,
+	MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
+	MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
+	MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
+	MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
+};
+
+static u64 mv88e6xxx_devlink_atu_bin_get(struct mv88e6xxx_chip *chip,
+					 u16 bin)
+{
+	u16 occupancy = 0;
+	int err;
+
+	mv88e6xxx_reg_lock(chip);
+
+	err = mv88e6xxx_g2_atu_stats_set(chip, MV88E6XXX_G2_ATU_STATS_MODE_ALL,
+					 bin);
+	if (err) {
+		dev_err(chip->dev, "failed to set ATU stats kind/bin\n");
+		goto unlock;
+	}
+
+	err = mv88e6xxx_g1_atu_get_next(chip, 0);
+	if (err) {
+		dev_err(chip->dev, "failed to perform ATU get next\n");
+		goto unlock;
+	}
+
+	err = mv88e6xxx_g2_atu_stats_get(chip, &occupancy);
+	if (err) {
+		dev_err(chip->dev, "failed to get ATU stats\n");
+		goto unlock;
+	}
+
+	occupancy &= MV88E6XXX_G2_ATU_STATS_MASK;
+
+unlock:
+	mv88e6xxx_reg_unlock(chip);
+
+	return occupancy;
+}
+
+static u64 mv88e6xxx_devlink_atu_bin_0_get(void *priv)
+{
+	struct mv88e6xxx_chip *chip = priv;
+
+	return mv88e6xxx_devlink_atu_bin_get(chip,
+					     MV88E6XXX_G2_ATU_STATS_BIN_0);
+}
+
+static u64 mv88e6xxx_devlink_atu_bin_1_get(void *priv)
+{
+	struct mv88e6xxx_chip *chip = priv;
+
+	return mv88e6xxx_devlink_atu_bin_get(chip,
+					     MV88E6XXX_G2_ATU_STATS_BIN_1);
+}
+
+static u64 mv88e6xxx_devlink_atu_bin_2_get(void *priv)
+{
+	struct mv88e6xxx_chip *chip = priv;
+
+	return mv88e6xxx_devlink_atu_bin_get(chip,
+					     MV88E6XXX_G2_ATU_STATS_BIN_2);
+}
+
+static u64 mv88e6xxx_devlink_atu_bin_3_get(void *priv)
+{
+	struct mv88e6xxx_chip *chip = priv;
+
+	return mv88e6xxx_devlink_atu_bin_get(chip,
+					     MV88E6XXX_G2_ATU_STATS_BIN_3);
+}
+
+static u64 mv88e6xxx_devlink_atu_get(void *priv)
+{
+	return mv88e6xxx_devlink_atu_bin_0_get(priv) +
+		mv88e6xxx_devlink_atu_bin_1_get(priv) +
+		mv88e6xxx_devlink_atu_bin_2_get(priv) +
+		mv88e6xxx_devlink_atu_bin_3_get(priv);
+}
+
+int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds)
+{
+	struct devlink_resource_size_params size_params;
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	devlink_resource_size_params_init(&size_params,
+					  mv88e6xxx_num_macs(chip),
+					  mv88e6xxx_num_macs(chip),
+					  1, DEVLINK_RESOURCE_UNIT_ENTRY);
+
+	err = dsa_devlink_resource_register(ds, "ATU",
+					    mv88e6xxx_num_macs(chip),
+					    MV88E6XXX_RESOURCE_ID_ATU,
+					    DEVLINK_RESOURCE_ID_PARENT_TOP,
+					    &size_params);
+	if (err)
+		goto out;
+
+	devlink_resource_size_params_init(&size_params,
+					  mv88e6xxx_num_macs(chip) / 4,
+					  mv88e6xxx_num_macs(chip) / 4,
+					  1, DEVLINK_RESOURCE_UNIT_ENTRY);
+
+	err = dsa_devlink_resource_register(ds, "ATU_bin_0",
+					    mv88e6xxx_num_macs(chip) / 4,
+					    MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
+					    MV88E6XXX_RESOURCE_ID_ATU,
+					    &size_params);
+	if (err)
+		goto out;
+
+	err = dsa_devlink_resource_register(ds, "ATU_bin_1",
+					    mv88e6xxx_num_macs(chip) / 4,
+					    MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
+					    MV88E6XXX_RESOURCE_ID_ATU,
+					    &size_params);
+	if (err)
+		goto out;
+
+	err = dsa_devlink_resource_register(ds, "ATU_bin_2",
+					    mv88e6xxx_num_macs(chip) / 4,
+					    MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
+					    MV88E6XXX_RESOURCE_ID_ATU,
+					    &size_params);
+	if (err)
+		goto out;
+
+	err = dsa_devlink_resource_register(ds, "ATU_bin_3",
+					    mv88e6xxx_num_macs(chip) / 4,
+					    MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
+					    MV88E6XXX_RESOURCE_ID_ATU,
+					    &size_params);
+	if (err)
+		goto out;
+
+	dsa_devlink_resource_occ_get_register(ds,
+					      MV88E6XXX_RESOURCE_ID_ATU,
+					      mv88e6xxx_devlink_atu_get,
+					      chip);
+
+	dsa_devlink_resource_occ_get_register(ds,
+					      MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
+					      mv88e6xxx_devlink_atu_bin_0_get,
+					      chip);
+
+	dsa_devlink_resource_occ_get_register(ds,
+					      MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
+					      mv88e6xxx_devlink_atu_bin_1_get,
+					      chip);
+
+	dsa_devlink_resource_occ_get_register(ds,
+					      MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
+					      mv88e6xxx_devlink_atu_bin_2_get,
+					      chip);
+
+	dsa_devlink_resource_occ_get_register(ds,
+					      MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
+					      mv88e6xxx_devlink_atu_bin_3_get,
+					      chip);
+
+	return 0;
+
+out:
+	dsa_devlink_resources_unregister(ds);
+	return err;
+}
+
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.h b/drivers/net/dsa/mv88e6xxx/devlink.h
new file mode 100644
index 000000000000..f6254e049653
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/devlink.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/* Marvell 88E6xxx Switch devlink support. */
+
+#ifndef _MV88E6XXX_DEVLINK_H
+#define _MV88E6XXX_DEVLINK_H
+
+int mv88e6xxx_setup_devlink_params(struct dsa_switch *ds);
+void mv88e6xxx_teardown_devlink_params(struct dsa_switch *ds);
+int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds);
+int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
+				struct devlink_param_gset_ctx *ctx);
+int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
+				struct devlink_param_gset_ctx *ctx);
+
+#endif /* _MV88E6XXX_DEVLINK_H */
-- 
2.28.0

[PATCH net-next 4/7] net: dsa: mv88e6xxx: Create helper for FIDs in use

[Andrew Lunn]

Refactor the code in mv88e6xxx_atu_new() which builds a bitmaps of
FIDs in use into a helper function. This will be reused by the devlink
code when dumping the ATU.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 20 ++++++++++++++++----
 drivers/net/dsa/mv88e6xxx/chip.h |  2 ++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 940dd0c55568..2b3ddf2bfcea 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1466,21 +1466,21 @@ static int mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 	return chip->info->ops->vtu_loadpurge(chip, entry);
 }
 
-static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
+int mv88e6xxx_fid_map(struct mv88e6xxx_chip *chip, unsigned long *fid_bitmap)
 {
-	DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
 	struct mv88e6xxx_vtu_entry vlan;
 	int i, err;
+	u16 fid;
 
 	bitmap_zero(fid_bitmap, MV88E6XXX_N_FID);
 
 	/* Set every FID bit used by the (un)bridged ports */
 	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
-		err = mv88e6xxx_port_get_fid(chip, i, fid);
+		err = mv88e6xxx_port_get_fid(chip, i, &fid);
 		if (err)
 			return err;
 
-		set_bit(*fid, fid_bitmap);
+		set_bit(fid, fid_bitmap);
 	}
 
 	/* Set every FID bit used by the VLAN entries */
@@ -1498,6 +1498,18 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
 		set_bit(vlan.fid, fid_bitmap);
 	} while (vlan.vid < chip->info->max_vid);
 
+	return 0;
+}
+
+static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
+{
+	DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
+	int err;
+
+	err = mv88e6xxx_fid_map(chip, fid_bitmap);
+	if (err)
+		return err;
+
 	/* The reset value 0x000 is used to indicate that multiple address
 	 * databases are not needed. Return the next positive available.
 	 */
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 823ae89e5fca..77d81aa99f37 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -689,4 +689,6 @@ static inline void mv88e6xxx_reg_unlock(struct mv88e6xxx_chip *chip)
 	mutex_unlock(&chip->reg_lock);
 }
 
+int mv88e6xxx_fid_map(struct mv88e6xxx_chip *chip, unsigned long *bitmap);
+
 #endif /* _MV88E6XXX_CHIP_H */
-- 
2.28.0

[PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions

[Andrew Lunn]

Allow ports, the global registers, and the ATU to be snapshot via
devlink regions.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c    |  14 +-
 drivers/net/dsa/mv88e6xxx/chip.h    |  12 +
 drivers/net/dsa/mv88e6xxx/devlink.c | 413 ++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/devlink.h |   2 +
 4 files changed, 440 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 2b3ddf2bfcea..33e4518736a2 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2838,6 +2838,7 @@ static void mv88e6xxx_teardown(struct dsa_switch *ds)
 {
 	mv88e6xxx_teardown_devlink_params(ds);
 	dsa_devlink_resources_unregister(ds);
+	mv88e6xxx_teardown_devlink_regions(ds);
 }
 
 static int mv88e6xxx_setup(struct dsa_switch *ds)
@@ -2970,7 +2971,18 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 
 	err = mv88e6xxx_setup_devlink_params(ds);
 	if (err)
-		dsa_devlink_resources_unregister(ds);
+		goto out_resources;
+
+	err = mv88e6xxx_setup_devlink_regions(ds);
+	if (err)
+		goto out_params;
+
+	return 0;
+
+out_params:
+	mv88e6xxx_teardown_devlink_params(ds);
+out_resources:
+	dsa_devlink_resources_unregister(ds);
 
 	return err;
 }
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 77d81aa99f37..d8bd211afcec 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -238,6 +238,15 @@ struct mv88e6xxx_port {
 	bool mirror_egress;
 	unsigned int serdes_irq;
 	char serdes_irq_name[64];
+	struct devlink_region *region;
+};
+
+enum mv88e6xxx_region_id {
+	MV88E6XXX_REGION_GLOBAL1 = 0,
+	MV88E6XXX_REGION_GLOBAL2,
+	MV88E6XXX_REGION_ATU,
+
+	_MV88E6XXX_REGION_MAX,
 };
 
 struct mv88e6xxx_chip {
@@ -334,6 +343,9 @@ struct mv88e6xxx_chip {
 
 	/* Array of port structures. */
 	struct mv88e6xxx_port ports[DSA_MAX_PORTS];
+
+	/* devlink regions */
+	struct devlink_region *regions[_MV88E6XXX_REGION_MAX];
 };
 
 struct mv88e6xxx_bus_ops {
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
index 91e02024c5cf..c6ebadcfa63f 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -5,6 +5,7 @@
 #include "devlink.h"
 #include "global1.h"
 #include "global2.h"
+#include "port.h"
 
 static int mv88e6xxx_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
 {
@@ -33,6 +34,8 @@ int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
 
+	dev_info(ds->dev, "%s: enter\n", __func__);
+
 	mv88e6xxx_reg_lock(chip);
 
 	switch (id) {
@@ -55,6 +58,8 @@ int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
 
+	dev_info(ds->dev, "%s: enter\n", __func__);
+
 	mv88e6xxx_reg_lock(chip);
 
 	switch (id) {
@@ -260,3 +265,411 @@ int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds)
 	return err;
 }
 
+static int mv88e6xxx_region_port_snapshot(struct devlink *dl,
+					  struct netlink_ext_ack *extack,
+					  u8 **data,
+					  int port)
+{
+	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+	struct mv88e6xxx_chip *chip = ds->priv;
+	u16 *registers;
+	int i, err;
+
+	registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
+	if (!registers)
+		return -ENOMEM;
+
+	mv88e6xxx_reg_lock(chip);
+	for (i = 0; i < 32; i++) {
+		err = mv88e6xxx_port_read(chip, port, i, &registers[i]);
+		if (err) {
+			kfree(registers);
+			goto out;
+		}
+	}
+	*data = (u8 *)registers;
+out:
+	mv88e6xxx_reg_unlock(chip);
+
+	return err;
+}
+
+#define PORT_SNAPSHOT(_X_)						\
+static int mv88e6xxx_region_port_ ## _X_ ## _snapshot(		\
+	struct devlink *dl,						\
+	struct netlink_ext_ack *extack,					\
+	u8 **data)							\
+{									\
+	return mv88e6xxx_region_port_snapshot(dl, extack, data, _X_);	\
+}
+
+PORT_SNAPSHOT(0);
+PORT_SNAPSHOT(1);
+PORT_SNAPSHOT(2);
+PORT_SNAPSHOT(3);
+PORT_SNAPSHOT(4);
+PORT_SNAPSHOT(5);
+PORT_SNAPSHOT(6);
+PORT_SNAPSHOT(7);
+PORT_SNAPSHOT(8);
+PORT_SNAPSHOT(9);
+PORT_SNAPSHOT(10);
+PORT_SNAPSHOT(11);
+
+#define PORT_REGION_OPS(_X_)						\
+static struct devlink_region_ops mv88e6xxx_region_port_ ## _X_ ## _ops = { \
+	.name = "port" #_X_,						\
+	.snapshot = mv88e6xxx_region_port_ ## _X_ ## _snapshot,		\
+	.destructor = kfree,						\
+}
+
+PORT_REGION_OPS(0);
+PORT_REGION_OPS(1);
+PORT_REGION_OPS(2);
+PORT_REGION_OPS(3);
+PORT_REGION_OPS(4);
+PORT_REGION_OPS(5);
+PORT_REGION_OPS(6);
+PORT_REGION_OPS(7);
+PORT_REGION_OPS(8);
+PORT_REGION_OPS(9);
+PORT_REGION_OPS(10);
+PORT_REGION_OPS(11);
+
+static const struct devlink_region_ops *mv88e6xxx_region_port_ops[] = {
+	&mv88e6xxx_region_port_0_ops,
+	&mv88e6xxx_region_port_1_ops,
+	&mv88e6xxx_region_port_2_ops,
+	&mv88e6xxx_region_port_3_ops,
+	&mv88e6xxx_region_port_4_ops,
+	&mv88e6xxx_region_port_5_ops,
+	&mv88e6xxx_region_port_6_ops,
+	&mv88e6xxx_region_port_7_ops,
+	&mv88e6xxx_region_port_8_ops,
+	&mv88e6xxx_region_port_9_ops,
+	&mv88e6xxx_region_port_10_ops,
+	&mv88e6xxx_region_port_11_ops,
+};
+
+static int mv88e6xxx_region_global1_snapshot(struct devlink *dl,
+					     struct netlink_ext_ack *extack,
+					     u8 **data)
+{
+	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+	struct mv88e6xxx_chip *chip = ds->priv;
+	u16 *registers;
+	int i, err;
+
+	registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
+	if (!registers)
+		return -ENOMEM;
+
+	mv88e6xxx_reg_lock(chip);
+	for (i = 0; i < 32; i++) {
+		err = mv88e6xxx_g1_read(chip, i, &registers[i]);
+		if (err) {
+			kfree(registers);
+			goto out;
+		}
+	}
+	*data = (u8 *)registers;
+out:
+	mv88e6xxx_reg_unlock(chip);
+
+	return err;
+}
+
+static int mv88e6xxx_region_global2_snapshot(struct devlink *dl,
+					     struct netlink_ext_ack *extack,
+					     u8 **data)
+{
+	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+	struct mv88e6xxx_chip *chip = ds->priv;
+	u16 *registers;
+	int i, err;
+
+	registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
+	if (!registers)
+		return -ENOMEM;
+
+	mv88e6xxx_reg_lock(chip);
+	for (i = 0; i < 32; i++) {
+		err = mv88e6xxx_g2_read(chip, i, &registers[i]);
+		if (err) {
+			kfree(registers);
+			goto out;
+		}
+	}
+	*data = (u8 *)registers;
+out:
+	mv88e6xxx_reg_unlock(chip);
+
+	return err;
+}
+
+/* The ATU entry varies between chipset generations. Define a generic
+ * format which covers all the current and hopefully future
+ * generations
+ */
+
+struct mv88e6xxx_devlink_atu_entry {
+	/* The FID is scattered over multiple registers. */
+	u16 fid;
+	u16 atu_op;
+	u16 atu_data;
+	u16 atu_01;
+	u16 atu_23;
+	u16 atu_45;
+};
+
+static int mv88e6xxx_region_atu_snapshot_fid(struct mv88e6xxx_chip *chip,
+					     int fid,
+					     struct mv88e6xxx_devlink_atu_entry *table,
+					     int *count)
+{
+	u16 atu_op, atu_data, atu_01, atu_23, atu_45;
+	struct mv88e6xxx_atu_entry addr;
+	int err;
+
+	addr.state = 0;
+	eth_broadcast_addr(addr.mac);
+
+	do {
+		err = mv88e6xxx_g1_atu_getnext(chip, fid, &addr);
+		if (err)
+			return err;
+
+		if (!addr.state)
+			break;
+
+		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_OP, &atu_op);
+		if (err)
+			return err;
+
+		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_DATA, &atu_data);
+		if (err)
+			return err;
+
+		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC01, &atu_01);
+		if (err)
+			return err;
+
+		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC23, &atu_23);
+		if (err)
+			return err;
+
+		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC45, &atu_45);
+		if (err)
+			return err;
+
+		table[*count].fid = fid;
+		table[*count].atu_op = atu_op;
+		table[*count].atu_data = atu_data;
+		table[*count].atu_01 = atu_01;
+		table[*count].atu_23 = atu_23;
+		table[*count].atu_45 = atu_45;
+		(*count)++;
+	} while (!is_broadcast_ether_addr(addr.mac));
+
+	return 0;
+}
+
+static int mv88e6xxx_region_atu_snapshot(struct devlink *dl,
+					 struct netlink_ext_ack *extack,
+					 u8 **data)
+{
+	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+	DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
+	struct mv88e6xxx_devlink_atu_entry *table;
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int fid = -1, count, err;
+
+	table = kmalloc_array(mv88e6xxx_num_databases(chip),
+			      sizeof(struct mv88e6xxx_devlink_atu_entry),
+			      GFP_KERNEL);
+	if (!table)
+		return -ENOMEM;
+
+	memset(table, 0, mv88e6xxx_num_databases(chip) *
+	       sizeof(struct mv88e6xxx_devlink_atu_entry));
+
+	count = 0;
+
+	mv88e6xxx_reg_lock(chip);
+
+	err = mv88e6xxx_fid_map(chip, fid_bitmap);
+	if (err)
+		goto out;
+
+	while (1) {
+		fid = find_next_bit(fid_bitmap, MV88E6XXX_N_FID, fid + 1);
+		if (fid == MV88E6XXX_N_FID)
+			break;
+
+		err =  mv88e6xxx_region_atu_snapshot_fid(chip, fid, table,
+							 &count);
+		if (err) {
+			kfree(table);
+			goto out;
+		}
+	}
+	*data = (u8 *)table;
+out:
+	mv88e6xxx_reg_unlock(chip);
+
+	return err;
+}
+
+static struct devlink_region_ops mv88e6xxx_region_global1_ops = {
+	.name = "global1",
+	.snapshot = mv88e6xxx_region_global1_snapshot,
+	.destructor = kfree,
+};
+
+static struct devlink_region_ops mv88e6xxx_region_global2_ops = {
+	.name = "global2",
+	.snapshot = mv88e6xxx_region_global2_snapshot,
+	.destructor = kfree,
+};
+
+static struct devlink_region_ops mv88e6xxx_region_atu_ops = {
+	.name = "atu",
+	.snapshot = mv88e6xxx_region_atu_snapshot,
+	.destructor = kfree,
+};
+
+struct mv88e6xxx_region {
+	struct devlink_region_ops *ops;
+	u64 size;
+};
+
+static struct mv88e6xxx_region mv88e6xxx_regions[] = {
+	[MV88E6XXX_REGION_GLOBAL1] = {
+		.ops = &mv88e6xxx_region_global1_ops,
+		.size = 32 * sizeof(u16)
+	},
+	[MV88E6XXX_REGION_GLOBAL2] = {
+		.ops = &mv88e6xxx_region_global2_ops,
+		.size = 32 * sizeof(u16) },
+	[MV88E6XXX_REGION_ATU] = {
+		.ops = &mv88e6xxx_region_atu_ops
+	  /* calculated at runtime */
+	},
+};
+
+static void
+mv88e6xxx_teardown_devlink_regions_port(struct mv88e6xxx_chip *chip, int port)
+{
+	dsa_devlink_region_destroy(chip->ports[port].region);
+}
+
+static void
+mv88e6xxx_teardown_devlink_regions_ports(struct mv88e6xxx_chip *chip)
+{
+	int port;
+
+	for (port = 0; port < mv88e6xxx_num_ports(chip); port++)
+		mv88e6xxx_teardown_devlink_regions_port(chip, port);
+}
+
+static void
+mv88e6xxx_teardown_devlink_regions_global(struct mv88e6xxx_chip *chip)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mv88e6xxx_regions); i++)
+		dsa_devlink_region_destroy(chip->regions[i]);
+}
+
+void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+
+	mv88e6xxx_teardown_devlink_regions_ports(chip);
+	mv88e6xxx_teardown_devlink_regions_global(chip);
+}
+
+static int mv88e6xxx_setup_devlink_regions_port(struct dsa_switch *ds,
+						struct mv88e6xxx_chip *chip,
+						int port)
+{
+	struct devlink_region *region;
+
+	region = dsa_devlink_region_create(ds,
+					   mv88e6xxx_region_port_ops[port], 1,
+					   32 * sizeof(u16));
+	if (IS_ERR(region))
+		return PTR_ERR(region);
+
+	chip->ports[port].region = region;
+	return 0;
+}
+
+static int mv88e6xxx_setup_devlink_regions_ports(struct dsa_switch *ds,
+						 struct mv88e6xxx_chip *chip)
+{
+	int port, port_err;
+	int err;
+
+	for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
+		err = mv88e6xxx_setup_devlink_regions_port(ds, chip, port);
+		if (err)
+			goto out;
+	}
+	return 0;
+
+out:
+	for (port_err = 0; port_err < port; port_err++)
+		mv88e6xxx_teardown_devlink_regions_port(chip, port_err);
+
+	return err;
+}
+
+static int mv88e6xxx_setup_devlink_regions_global(struct dsa_switch *ds,
+						  struct mv88e6xxx_chip *chip)
+{
+	struct devlink_region_ops *ops;
+	struct devlink_region *region;
+	u64 size;
+	int i, j;
+
+	for (i = 0; i < ARRAY_SIZE(mv88e6xxx_regions); i++) {
+		ops = mv88e6xxx_regions[i].ops;
+		size = mv88e6xxx_regions[i].size;
+
+		if (i == MV88E6XXX_REGION_ATU)
+			size = mv88e6xxx_num_databases(chip) *
+				sizeof(struct mv88e6xxx_devlink_atu_entry);
+
+		region = dsa_devlink_region_create(ds, ops, 1, size);
+		if (IS_ERR(region))
+			goto out;
+		chip->regions[i] = region;
+	}
+	return 0;
+
+out:
+	for (j = 0; j < i; j++)
+		dsa_devlink_region_destroy(chip->regions[j]);
+
+	return PTR_ERR(region);
+}
+
+int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	err = mv88e6xxx_setup_devlink_regions_ports(ds, chip);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_setup_devlink_regions_global(ds, chip);
+	if (err) {
+		mv88e6xxx_teardown_devlink_regions_ports(chip);
+		return err;
+	}
+
+	return 0;
+}
+
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.h b/drivers/net/dsa/mv88e6xxx/devlink.h
index f6254e049653..da83c25d944b 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.h
+++ b/drivers/net/dsa/mv88e6xxx/devlink.h
@@ -12,5 +12,7 @@ int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
 				struct devlink_param_gset_ctx *ctx);
 int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
 				struct devlink_param_gset_ctx *ctx);
+int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds);
+void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds);
 
 #endif /* _MV88E6XXX_DEVLINK_H */
-- 
2.28.0

[PATCH net-next 6/7] net: dsa: wire up devlink info get

[Andrew Lunn]

Allow the DSA drivers to implement the devlink call to get info info,
e.g. driver name, firmware version, ASIC ID, etc.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/dsa.h |  5 ++++-
 net/dsa/dsa2.c    | 21 ++++++++++++++++++---
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8963440ec7f8..0e34193d15ba 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -612,11 +612,14 @@ struct dsa_switch_ops {
 	bool	(*port_rxtstamp)(struct dsa_switch *ds, int port,
 				 struct sk_buff *skb, unsigned int type);
 
-	/* Devlink parameters */
+	/* Devlink parameters, etc */
 	int	(*devlink_param_get)(struct dsa_switch *ds, u32 id,
 				     struct devlink_param_gset_ctx *ctx);
 	int	(*devlink_param_set)(struct dsa_switch *ds, u32 id,
 				     struct devlink_param_gset_ctx *ctx);
+	int	(*devlink_info_get)(struct dsa_switch *ds,
+				    struct devlink_info_req *req,
+				    struct netlink_ext_ack *extack);
 
 	/*
 	 * MTU change functionality. Switches can also adjust their MRU through
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index c0ffc7a2b65f..860f2fb22fe0 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -21,9 +21,6 @@
 static DEFINE_MUTEX(dsa2_mutex);
 LIST_HEAD(dsa_tree_list);
 
-static const struct devlink_ops dsa_devlink_ops = {
-};
-
 struct dsa_switch *dsa_switch_find(int tree_index, int sw_index)
 {
 	struct dsa_switch_tree *dst;
@@ -382,6 +379,24 @@ static void dsa_port_teardown(struct dsa_port *dp)
 	dp->setup = false;
 }
 
+static int dsa_devlink_info_get(struct devlink *dl,
+				struct devlink_info_req *req,
+				struct netlink_ext_ack *extack)
+{
+	struct dsa_switch *ds;
+
+	ds = dsa_devlink_to_ds(dl);
+
+	if (ds->ops->devlink_info_get)
+		return ds->ops->devlink_info_get(ds, req, extack);
+
+	return -EOPNOTSUPP;
+}
+
+static const struct devlink_ops dsa_devlink_ops = {
+	.info_get = dsa_devlink_info_get,
+};
+
 static int dsa_switch_setup(struct dsa_switch *ds)
 {
 	struct dsa_devlink_priv *dl_priv;
-- 
2.28.0

[PATCH net-next 7/7] net: dsa: mv88e6xxx: Implement devlink info get callback

[Andrew Lunn]

Return the driver name and the asic.id with the switch name.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c    |  1 +
 drivers/net/dsa/mv88e6xxx/devlink.c | 15 +++++++++++++++
 drivers/net/dsa/mv88e6xxx/devlink.h |  3 +++
 3 files changed, 19 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 33e4518736a2..ee3b1a6b1ea8 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -5385,6 +5385,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.get_ts_info		= mv88e6xxx_get_ts_info,
 	.devlink_param_get	= mv88e6xxx_devlink_param_get,
 	.devlink_param_set	= mv88e6xxx_devlink_param_set,
+	.devlink_info_get	= mv88e6xxx_devlink_info_get,
 };
 
 static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
index c6ebadcfa63f..8cb167d737e2 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -673,3 +673,18 @@ int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds)
 	return 0;
 }
 
+int mv88e6xxx_devlink_info_get(struct dsa_switch *ds,
+			       struct devlink_info_req *req,
+			       struct netlink_ext_ack *extack)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	err = devlink_info_driver_name_put(req, "mv88e6xxx");
+	if (err)
+		return err;
+
+	return devlink_info_version_fixed_put(req,
+					      DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
+					      chip->info->name);
+}
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.h b/drivers/net/dsa/mv88e6xxx/devlink.h
index da83c25d944b..3d72db3dcf95 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.h
+++ b/drivers/net/dsa/mv88e6xxx/devlink.h
@@ -15,4 +15,7 @@ int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
 int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds);
 void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds);
 
+int mv88e6xxx_devlink_info_get(struct dsa_switch *ds,
+			       struct devlink_info_req *req,
+			       struct netlink_ext_ack *extack);
 #endif /* _MV88E6XXX_DEVLINK_H */
-- 
2.28.0

Re: [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support

[Chris Healy]

I tested this on the Rev C Zii Vybrid Dev Board which has two Marvell
88E6390X switches.

Both switches worth of port, global, and atu data show up when running
the devlink show command and for each of the regions, I can dump the
contents.

I also tested on a different platform with a single 88E6352.

Full series is:

Tested-by: Chris Healy <cphealy@gmail.com>

On Sun, Aug 16, 2020 at 12:44 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> Make use of devlink regions to allow read access to some of the
> internal of the switches. The switch itself will never trigger a
> region snapshot, it is assumed it is performed from user space as
> needed.
>
> Andrew Lunn (7):
>   net: dsa: Add helper to convert from devlink to ds
>   net: dsa: Add devlink regions support to DSA
>   net: dsa: mv88e6xxx: Move devlink code into its own file
>   net: dsa: mv88e6xxx: Create helper for FIDs in use
>   net: dsa: mv88e6xxx: Add devlink regions
>   net: dsa: wire up devlink info get
>   net: dsa: mv88e6xxx: Implement devlink info get callback
>
>  drivers/net/dsa/mv88e6xxx/Makefile  |   1 +
>  drivers/net/dsa/mv88e6xxx/chip.c    | 290 ++----------
>  drivers/net/dsa/mv88e6xxx/chip.h    |  14 +
>  drivers/net/dsa/mv88e6xxx/devlink.c | 690 ++++++++++++++++++++++++++++
>  drivers/net/dsa/mv88e6xxx/devlink.h |  21 +
>  include/net/dsa.h                   |  13 +-
>  net/dsa/dsa.c                       |  36 +-
>  net/dsa/dsa2.c                      |  21 +-
>  8 files changed, 813 insertions(+), 273 deletions(-)
>  create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.c
>  create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.h
>
> --
> 2.28.0
>

Re: [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA

[Vladimir Oltean]

On Sun, Aug 16, 2020 at 09:43:11PM +0200, Andrew Lunn wrote:
> Allow DSA drivers to make use of devlink regions, via simple wrappers.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/net/dsa.h |  6 ++++++
>  net/dsa/dsa.c     | 16 ++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 63ff6f717307..8963440ec7f8 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -660,6 +660,12 @@ void dsa_devlink_resource_occ_get_register(struct dsa_switch *ds,
>  					   void *occ_get_priv);
>  void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
>  					     u64 resource_id);
> +struct devlink_region *
> +dsa_devlink_region_create(struct dsa_switch *ds,
> +			  const struct devlink_region_ops *ops,
> +			  u32 region_max_snapshots, u64 region_size);
> +void dsa_devlink_region_destroy(struct devlink_region *region);
> +
>  struct dsa_port *dsa_port_from_netdev(struct net_device *netdev);
>  
>  struct dsa_devlink_priv {
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 86351da4e202..fea2efe5fe68 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -412,6 +412,22 @@ void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
>  }
>  EXPORT_SYMBOL_GPL(dsa_devlink_resource_occ_get_unregister);
>  
> +struct devlink_region *
> +dsa_devlink_region_create(struct dsa_switch *ds,
> +			  const struct devlink_region_ops *ops,
> +			  u32 region_max_snapshots, u64 region_size)
> +{
> +	return devlink_region_create(ds->devlink, ops, region_max_snapshots,
> +				     region_size);
> +}
> +EXPORT_SYMBOL_GPL(dsa_devlink_region_create);
> +
> +void dsa_devlink_region_destroy(struct devlink_region *region)
> +{
> +	devlink_region_destroy(region);
> +}
> +EXPORT_SYMBOL_GPL(dsa_devlink_region_destroy);
> +
>  struct dsa_port *dsa_port_from_netdev(struct net_device *netdev)
>  {
>  	if (!netdev || !dsa_slave_dev_check(netdev))
> -- 
> 2.28.0
> 

Could we perhaps open-code these from the drivers themselves? There's
hardly any added value in DSA providing a "helper" for creation of
devlink resources (regions, shared buffers, etc).

Take the ocelot/felix driver for example. It is a DSA driver whose core
functionality is provided by drivers/net/ethernet/mscc/ocelot*.c, which
is non-DSA code. If it were to implement devlink regions, presumably
that code would live in drivers/net/ethernet/mscc/ and not in
drivers/net/dsa/.

Thanks,
-Vladimir

Re: [PATCH net-next 6/7] net: dsa: wire up devlink info get

[Vladimir Oltean]

On Sun, Aug 16, 2020 at 09:43:15PM +0200, Andrew Lunn wrote:
> Allow the DSA drivers to implement the devlink call to get info info,
> e.g. driver name, firmware version, ASIC ID, etc.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/net/dsa.h |  5 ++++-
>  net/dsa/dsa2.c    | 21 ++++++++++++++++++---
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 8963440ec7f8..0e34193d15ba 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -612,11 +612,14 @@ struct dsa_switch_ops {
>  	bool	(*port_rxtstamp)(struct dsa_switch *ds, int port,
>  				 struct sk_buff *skb, unsigned int type);
>  
> -	/* Devlink parameters */
> +	/* Devlink parameters, etc */
>  	int	(*devlink_param_get)(struct dsa_switch *ds, u32 id,
>  				     struct devlink_param_gset_ctx *ctx);
>  	int	(*devlink_param_set)(struct dsa_switch *ds, u32 id,
>  				     struct devlink_param_gset_ctx *ctx);
> +	int	(*devlink_info_get)(struct dsa_switch *ds,
> +				    struct devlink_info_req *req,
> +				    struct netlink_ext_ack *extack);
>  
>  	/*
>  	 * MTU change functionality. Switches can also adjust their MRU through
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index c0ffc7a2b65f..860f2fb22fe0 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -21,9 +21,6 @@
>  static DEFINE_MUTEX(dsa2_mutex);
>  LIST_HEAD(dsa_tree_list);
>  
> -static const struct devlink_ops dsa_devlink_ops = {
> -};
> -
>  struct dsa_switch *dsa_switch_find(int tree_index, int sw_index)
>  {
>  	struct dsa_switch_tree *dst;
> @@ -382,6 +379,24 @@ static void dsa_port_teardown(struct dsa_port *dp)
>  	dp->setup = false;
>  }
>  
> +static int dsa_devlink_info_get(struct devlink *dl,
> +				struct devlink_info_req *req,
> +				struct netlink_ext_ack *extack)
> +{
> +	struct dsa_switch *ds;
> +
> +	ds = dsa_devlink_to_ds(dl);
> +

Why not place the declaration and the assignment on a single line?

> +	if (ds->ops->devlink_info_get)
> +		return ds->ops->devlink_info_get(ds, req, extack);
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static const struct devlink_ops dsa_devlink_ops = {
> +	.info_get = dsa_devlink_info_get,
> +};
> +
>  static int dsa_switch_setup(struct dsa_switch *ds)
>  {
>  	struct dsa_devlink_priv *dl_priv;
> -- 
> 2.28.0
> 

Thanks,
-Vladimir

Re: [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA

[Andrew Lunn]

> Could we perhaps open-code these from the drivers themselves? There's
> hardly any added value in DSA providing a "helper" for creation of
> devlink resources (regions, shared buffers, etc).
 
It is something i considered. But we already have devlink wrappers. It
would be odd to have some parts of devlink wrapped and other parts
not.

The wrapping of phys is also causing Russell King problems. Both
phylink and devlink structures are mostly hidden away in the DSA core,
but do leak a bit into the drivers.

If we do change to open coding, would we remove the existing wrappers
as well?

> Take the ocelot/felix driver for example.

ocelot/felix is just plain odd. We have to do a balancing act for
it. We don't want to take stuff out of the core just for this one odd
switch, at the detriment for other normal DSA drivers.

	Andrew

Re: [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions

[Vladimir Oltean]

On Sun, Aug 16, 2020 at 09:43:14PM +0200, Andrew Lunn wrote:
> Allow ports, the global registers, and the ATU to be snapshot via
> devlink regions.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c    |  14 +-
>  drivers/net/dsa/mv88e6xxx/chip.h    |  12 +
>  drivers/net/dsa/mv88e6xxx/devlink.c | 413 ++++++++++++++++++++++++++++
>  drivers/net/dsa/mv88e6xxx/devlink.h |   2 +
>  4 files changed, 440 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 2b3ddf2bfcea..33e4518736a2 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2838,6 +2838,7 @@ static void mv88e6xxx_teardown(struct dsa_switch *ds)
>  {
>  	mv88e6xxx_teardown_devlink_params(ds);
>  	dsa_devlink_resources_unregister(ds);
> +	mv88e6xxx_teardown_devlink_regions(ds);
>  }
>  
>  static int mv88e6xxx_setup(struct dsa_switch *ds)
> @@ -2970,7 +2971,18 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
>  
>  	err = mv88e6xxx_setup_devlink_params(ds);
>  	if (err)
> -		dsa_devlink_resources_unregister(ds);
> +		goto out_resources;
> +
> +	err = mv88e6xxx_setup_devlink_regions(ds);
> +	if (err)
> +		goto out_params;
> +
> +	return 0;
> +
> +out_params:
> +	mv88e6xxx_teardown_devlink_params(ds);
> +out_resources:
> +	dsa_devlink_resources_unregister(ds);
>  
>  	return err;
>  }
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
> index 77d81aa99f37..d8bd211afcec 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -238,6 +238,15 @@ struct mv88e6xxx_port {
>  	bool mirror_egress;
>  	unsigned int serdes_irq;
>  	char serdes_irq_name[64];
> +	struct devlink_region *region;
> +};
> +
> +enum mv88e6xxx_region_id {
> +	MV88E6XXX_REGION_GLOBAL1 = 0,
> +	MV88E6XXX_REGION_GLOBAL2,
> +	MV88E6XXX_REGION_ATU,
> +
> +	_MV88E6XXX_REGION_MAX,
>  };
>  
>  struct mv88e6xxx_chip {
> @@ -334,6 +343,9 @@ struct mv88e6xxx_chip {
>  
>  	/* Array of port structures. */
>  	struct mv88e6xxx_port ports[DSA_MAX_PORTS];
> +
> +	/* devlink regions */
> +	struct devlink_region *regions[_MV88E6XXX_REGION_MAX];
>  };
>  
>  struct mv88e6xxx_bus_ops {
> diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
> index 91e02024c5cf..c6ebadcfa63f 100644
> --- a/drivers/net/dsa/mv88e6xxx/devlink.c
> +++ b/drivers/net/dsa/mv88e6xxx/devlink.c
> @@ -5,6 +5,7 @@
>  #include "devlink.h"
>  #include "global1.h"
>  #include "global2.h"
> +#include "port.h"
>  
>  static int mv88e6xxx_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
>  {
> @@ -33,6 +34,8 @@ int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
>  	struct mv88e6xxx_chip *chip = ds->priv;
>  	int err;
>  
> +	dev_info(ds->dev, "%s: enter\n", __func__);

Debugging leftovers (although it's curious that this patch is not about devlink
params...).

> +
>  	mv88e6xxx_reg_lock(chip);
>  
>  	switch (id) {
> @@ -55,6 +58,8 @@ int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
>  	struct mv88e6xxx_chip *chip = ds->priv;
>  	int err;
>  
> +	dev_info(ds->dev, "%s: enter\n", __func__);

Likewise.

> +
>  	mv88e6xxx_reg_lock(chip);
>  
>  	switch (id) {
> @@ -260,3 +265,411 @@ int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds)
>  	return err;
>  }
>  
> +static int mv88e6xxx_region_port_snapshot(struct devlink *dl,
> +					  struct netlink_ext_ack *extack,
> +					  u8 **data,
> +					  int port)
> +{
> +	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	u16 *registers;
> +	int i, err;
> +
> +	registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
> +	if (!registers)
> +		return -ENOMEM;
> +
> +	mv88e6xxx_reg_lock(chip);
> +	for (i = 0; i < 32; i++) {
> +		err = mv88e6xxx_port_read(chip, port, i, &registers[i]);
> +		if (err) {
> +			kfree(registers);
> +			goto out;
> +		}
> +	}
> +	*data = (u8 *)registers;
> +out:
> +	mv88e6xxx_reg_unlock(chip);
> +
> +	return err;
> +}
> +
> +#define PORT_SNAPSHOT(_X_)						\
> +static int mv88e6xxx_region_port_ ## _X_ ## _snapshot(		\
> +	struct devlink *dl,						\
> +	struct netlink_ext_ack *extack,					\
> +	u8 **data)							\
> +{									\
> +	return mv88e6xxx_region_port_snapshot(dl, extack, data, _X_);	\
> +}
> +
> +PORT_SNAPSHOT(0);
> +PORT_SNAPSHOT(1);
> +PORT_SNAPSHOT(2);
> +PORT_SNAPSHOT(3);
> +PORT_SNAPSHOT(4);
> +PORT_SNAPSHOT(5);
> +PORT_SNAPSHOT(6);
> +PORT_SNAPSHOT(7);
> +PORT_SNAPSHOT(8);
> +PORT_SNAPSHOT(9);
> +PORT_SNAPSHOT(10);
> +PORT_SNAPSHOT(11);
> +
> +#define PORT_REGION_OPS(_X_)						\
> +static struct devlink_region_ops mv88e6xxx_region_port_ ## _X_ ## _ops = { \
> +	.name = "port" #_X_,						\
> +	.snapshot = mv88e6xxx_region_port_ ## _X_ ## _snapshot,		\
> +	.destructor = kfree,						\
> +}
> +
> +PORT_REGION_OPS(0);
> +PORT_REGION_OPS(1);
> +PORT_REGION_OPS(2);
> +PORT_REGION_OPS(3);
> +PORT_REGION_OPS(4);
> +PORT_REGION_OPS(5);
> +PORT_REGION_OPS(6);
> +PORT_REGION_OPS(7);
> +PORT_REGION_OPS(8);
> +PORT_REGION_OPS(9);
> +PORT_REGION_OPS(10);
> +PORT_REGION_OPS(11);
> +
> +static const struct devlink_region_ops *mv88e6xxx_region_port_ops[] = {
> +	&mv88e6xxx_region_port_0_ops,
> +	&mv88e6xxx_region_port_1_ops,
> +	&mv88e6xxx_region_port_2_ops,
> +	&mv88e6xxx_region_port_3_ops,
> +	&mv88e6xxx_region_port_4_ops,
> +	&mv88e6xxx_region_port_5_ops,
> +	&mv88e6xxx_region_port_6_ops,
> +	&mv88e6xxx_region_port_7_ops,
> +	&mv88e6xxx_region_port_8_ops,
> +	&mv88e6xxx_region_port_9_ops,
> +	&mv88e6xxx_region_port_10_ops,
> +	&mv88e6xxx_region_port_11_ops,
> +};
> +

Sounds like there should maybe be an abstraction for 'per-port regions' in
devlink? I think your approach hardly scales if you start having
switches with more than 11 ports.

> +static int mv88e6xxx_region_global1_snapshot(struct devlink *dl,
> +					     struct netlink_ext_ack *extack,
> +					     u8 **data)
> +{
> +	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	u16 *registers;
> +	int i, err;
> +
> +	registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
> +	if (!registers)
> +		return -ENOMEM;
> +
> +	mv88e6xxx_reg_lock(chip);
> +	for (i = 0; i < 32; i++) {
> +		err = mv88e6xxx_g1_read(chip, i, &registers[i]);
> +		if (err) {
> +			kfree(registers);
> +			goto out;
> +		}
> +	}
> +	*data = (u8 *)registers;
> +out:
> +	mv88e6xxx_reg_unlock(chip);
> +
> +	return err;
> +}
> +
> +static int mv88e6xxx_region_global2_snapshot(struct devlink *dl,
> +					     struct netlink_ext_ack *extack,
> +					     u8 **data)
> +{
> +	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	u16 *registers;
> +	int i, err;
> +
> +	registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
> +	if (!registers)
> +		return -ENOMEM;
> +
> +	mv88e6xxx_reg_lock(chip);
> +	for (i = 0; i < 32; i++) {
> +		err = mv88e6xxx_g2_read(chip, i, &registers[i]);
> +		if (err) {
> +			kfree(registers);
> +			goto out;
> +		}
> +	}
> +	*data = (u8 *)registers;
> +out:
> +	mv88e6xxx_reg_unlock(chip);
> +
> +	return err;
> +}
> +
> +/* The ATU entry varies between chipset generations. Define a generic
> + * format which covers all the current and hopefully future
> + * generations
> + */

Could you please present this generic format to us? Maybe my interpretation of
the word "generic" is incorrect in this context? Is it even desirable to expose
regions like the ATU in a really generic (cross-vendor) fashion?

> +
> +struct mv88e6xxx_devlink_atu_entry {
> +	/* The FID is scattered over multiple registers. */
> +	u16 fid;
> +	u16 atu_op;
> +	u16 atu_data;
> +	u16 atu_01;
> +	u16 atu_23;
> +	u16 atu_45;
> +};
> +
> +static int mv88e6xxx_region_atu_snapshot_fid(struct mv88e6xxx_chip *chip,
> +					     int fid,
> +					     struct mv88e6xxx_devlink_atu_entry *table,
> +					     int *count)
> +{
> +	u16 atu_op, atu_data, atu_01, atu_23, atu_45;
> +	struct mv88e6xxx_atu_entry addr;
> +	int err;
> +
> +	addr.state = 0;
> +	eth_broadcast_addr(addr.mac);
> +
> +	do {
> +		err = mv88e6xxx_g1_atu_getnext(chip, fid, &addr);
> +		if (err)
> +			return err;
> +
> +		if (!addr.state)
> +			break;
> +
> +		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_OP, &atu_op);
> +		if (err)
> +			return err;
> +
> +		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_DATA, &atu_data);
> +		if (err)
> +			return err;
> +
> +		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC01, &atu_01);
> +		if (err)
> +			return err;
> +
> +		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC23, &atu_23);
> +		if (err)
> +			return err;
> +
> +		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC45, &atu_45);
> +		if (err)
> +			return err;
> +
> +		table[*count].fid = fid;
> +		table[*count].atu_op = atu_op;
> +		table[*count].atu_data = atu_data;
> +		table[*count].atu_01 = atu_01;
> +		table[*count].atu_23 = atu_23;
> +		table[*count].atu_45 = atu_45;
> +		(*count)++;
> +	} while (!is_broadcast_ether_addr(addr.mac));
> +
> +	return 0;
> +}
> +
> +static int mv88e6xxx_region_atu_snapshot(struct devlink *dl,
> +					 struct netlink_ext_ack *extack,
> +					 u8 **data)
> +{
> +	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
> +	DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
> +	struct mv88e6xxx_devlink_atu_entry *table;
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	int fid = -1, count, err;
> +
> +	table = kmalloc_array(mv88e6xxx_num_databases(chip),
> +			      sizeof(struct mv88e6xxx_devlink_atu_entry),
> +			      GFP_KERNEL);
> +	if (!table)
> +		return -ENOMEM;
> +
> +	memset(table, 0, mv88e6xxx_num_databases(chip) *
> +	       sizeof(struct mv88e6xxx_devlink_atu_entry));
> +
> +	count = 0;
> +
> +	mv88e6xxx_reg_lock(chip);
> +
> +	err = mv88e6xxx_fid_map(chip, fid_bitmap);
> +	if (err)
> +		goto out;
> +
> +	while (1) {
> +		fid = find_next_bit(fid_bitmap, MV88E6XXX_N_FID, fid + 1);
> +		if (fid == MV88E6XXX_N_FID)
> +			break;
> +
> +		err =  mv88e6xxx_region_atu_snapshot_fid(chip, fid, table,
> +							 &count);
> +		if (err) {
> +			kfree(table);
> +			goto out;
> +		}
> +	}
> +	*data = (u8 *)table;
> +out:
> +	mv88e6xxx_reg_unlock(chip);
> +
> +	return err;
> +}
> +
> +static struct devlink_region_ops mv88e6xxx_region_global1_ops = {
> +	.name = "global1",
> +	.snapshot = mv88e6xxx_region_global1_snapshot,
> +	.destructor = kfree,
> +};
> +
> +static struct devlink_region_ops mv88e6xxx_region_global2_ops = {
> +	.name = "global2",
> +	.snapshot = mv88e6xxx_region_global2_snapshot,
> +	.destructor = kfree,
> +};
> +
> +static struct devlink_region_ops mv88e6xxx_region_atu_ops = {
> +	.name = "atu",
> +	.snapshot = mv88e6xxx_region_atu_snapshot,
> +	.destructor = kfree,
> +};
> +
> +struct mv88e6xxx_region {
> +	struct devlink_region_ops *ops;
> +	u64 size;
> +};
> +
> +static struct mv88e6xxx_region mv88e6xxx_regions[] = {
> +	[MV88E6XXX_REGION_GLOBAL1] = {
> +		.ops = &mv88e6xxx_region_global1_ops,
> +		.size = 32 * sizeof(u16)
> +	},
> +	[MV88E6XXX_REGION_GLOBAL2] = {
> +		.ops = &mv88e6xxx_region_global2_ops,
> +		.size = 32 * sizeof(u16) },
> +	[MV88E6XXX_REGION_ATU] = {
> +		.ops = &mv88e6xxx_region_atu_ops
> +	  /* calculated at runtime */
> +	},
> +};
> +
> +static void
> +mv88e6xxx_teardown_devlink_regions_port(struct mv88e6xxx_chip *chip, int port)
> +{
> +	dsa_devlink_region_destroy(chip->ports[port].region);
> +}
> +
> +static void
> +mv88e6xxx_teardown_devlink_regions_ports(struct mv88e6xxx_chip *chip)
> +{
> +	int port;
> +
> +	for (port = 0; port < mv88e6xxx_num_ports(chip); port++)
> +		mv88e6xxx_teardown_devlink_regions_port(chip, port);
> +}
> +
> +static void
> +mv88e6xxx_teardown_devlink_regions_global(struct mv88e6xxx_chip *chip)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(mv88e6xxx_regions); i++)
> +		dsa_devlink_region_destroy(chip->regions[i]);
> +}
> +
> +void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds)
> +{
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +
> +	mv88e6xxx_teardown_devlink_regions_ports(chip);
> +	mv88e6xxx_teardown_devlink_regions_global(chip);
> +}
> +
> +static int mv88e6xxx_setup_devlink_regions_port(struct dsa_switch *ds,
> +						struct mv88e6xxx_chip *chip,
> +						int port)
> +{
> +	struct devlink_region *region;
> +
> +	region = dsa_devlink_region_create(ds,
> +					   mv88e6xxx_region_port_ops[port], 1,
> +					   32 * sizeof(u16));
> +	if (IS_ERR(region))
> +		return PTR_ERR(region);
> +
> +	chip->ports[port].region = region;
> +	return 0;
> +}
> +
> +static int mv88e6xxx_setup_devlink_regions_ports(struct dsa_switch *ds,
> +						 struct mv88e6xxx_chip *chip)
> +{
> +	int port, port_err;
> +	int err;
> +
> +	for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
> +		err = mv88e6xxx_setup_devlink_regions_port(ds, chip, port);
> +		if (err)
> +			goto out;
> +	}
> +	return 0;
> +
> +out:
> +	for (port_err = 0; port_err < port; port_err++)
> +		mv88e6xxx_teardown_devlink_regions_port(chip, port_err);
> +
> +	return err;
> +}
> +
> +static int mv88e6xxx_setup_devlink_regions_global(struct dsa_switch *ds,
> +						  struct mv88e6xxx_chip *chip)
> +{
> +	struct devlink_region_ops *ops;
> +	struct devlink_region *region;
> +	u64 size;
> +	int i, j;
> +
> +	for (i = 0; i < ARRAY_SIZE(mv88e6xxx_regions); i++) {
> +		ops = mv88e6xxx_regions[i].ops;
> +		size = mv88e6xxx_regions[i].size;
> +
> +		if (i == MV88E6XXX_REGION_ATU)
> +			size = mv88e6xxx_num_databases(chip) *
> +				sizeof(struct mv88e6xxx_devlink_atu_entry);
> +
> +		region = dsa_devlink_region_create(ds, ops, 1, size);
> +		if (IS_ERR(region))
> +			goto out;
> +		chip->regions[i] = region;
> +	}
> +	return 0;
> +
> +out:
> +	for (j = 0; j < i; j++)
> +		dsa_devlink_region_destroy(chip->regions[j]);
> +
> +	return PTR_ERR(region);
> +}
> +
> +int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds)
> +{
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	int err;
> +
> +	err = mv88e6xxx_setup_devlink_regions_ports(ds, chip);
> +	if (err)
> +		return err;
> +
> +	err = mv88e6xxx_setup_devlink_regions_global(ds, chip);
> +	if (err) {
> +		mv88e6xxx_teardown_devlink_regions_ports(chip);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> diff --git a/drivers/net/dsa/mv88e6xxx/devlink.h b/drivers/net/dsa/mv88e6xxx/devlink.h
> index f6254e049653..da83c25d944b 100644
> --- a/drivers/net/dsa/mv88e6xxx/devlink.h
> +++ b/drivers/net/dsa/mv88e6xxx/devlink.h
> @@ -12,5 +12,7 @@ int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
>  				struct devlink_param_gset_ctx *ctx);
>  int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
>  				struct devlink_param_gset_ctx *ctx);
> +int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds);
> +void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds);
>  
>  #endif /* _MV88E6XXX_DEVLINK_H */
> -- 
> 2.28.0
> 

Thanks,
-Vladimir

Re: [PATCH net-next 6/7] net: dsa: wire up devlink info get

[Andrew Lunn]

> > +static int dsa_devlink_info_get(struct devlink *dl,
> > +				struct devlink_info_req *req,
> > +				struct netlink_ext_ack *extack)
> > +{
> > +	struct dsa_switch *ds;
> > +
> > +	ds = dsa_devlink_to_ds(dl);
> > +
> 
> Why not place the declaration and the assignment on a single line?

This code went through a few refactors. Probably at some point i could
not keep reverse christmass tree so had to split it. And it then never
got put back together again.

I will fix it up for v2.

  Andrew

Re: [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA

[Vladimir Oltean]

On Mon, Aug 17, 2020 at 12:06:32AM +0200, Andrew Lunn wrote:
> > Could we perhaps open-code these from the drivers themselves? There's
> > hardly any added value in DSA providing a "helper" for creation of
> > devlink resources (regions, shared buffers, etc).
>
> If we do change to open coding, would we remove the existing wrappers
> as well?
> 

Maybe?
I reckon one of the main reasons why DSA hides struct net_device is to
present a unified API for the ports that don't have one.
But with devlink we don't have that problem.

> > Take the ocelot/felix driver for example.
> 
> ocelot/felix is just plain odd. We have to do a balancing act for
> it. We don't want to take stuff out of the core just for this one odd
> switch, at the detriment for other normal DSA drivers.
> 

Yes, the ocelot/felix driver _is_ odd, but in my defence it's only as
odd as the hardware was integrated.
On the other hand, the model you're proposing would be forcing me to
register devlink regions in one way for felix DSA, and in another way
for ocelot switchdev. Or could I just ignore the helper, and call
devlink directly, even if there's a helper in place?

Thanks,
-Vladimir

Re: [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions

[Andrew Lunn]

> > +static const struct devlink_region_ops *mv88e6xxx_region_port_ops[] = {
> > +	&mv88e6xxx_region_port_0_ops,
> > +	&mv88e6xxx_region_port_1_ops,
> > +	&mv88e6xxx_region_port_2_ops,
> > +	&mv88e6xxx_region_port_3_ops,
> > +	&mv88e6xxx_region_port_4_ops,
> > +	&mv88e6xxx_region_port_5_ops,
> > +	&mv88e6xxx_region_port_6_ops,
> > +	&mv88e6xxx_region_port_7_ops,
> > +	&mv88e6xxx_region_port_8_ops,
> > +	&mv88e6xxx_region_port_9_ops,
> > +	&mv88e6xxx_region_port_10_ops,
> > +	&mv88e6xxx_region_port_11_ops,
> > +};
> > +
> 
> Sounds like there should maybe be an abstraction for 'per-port regions' in
> devlink? I think your approach hardly scales if you start having
> switches with more than 11 ports.

mv88e6xxx is unlikely to have more an 11 ports. Marvell had to move
bits around in registers in non-compatible ways to support the 6390
family with this number of ports. I doubt we will ever see a 16 port
mv88e6xxx switch, the registers are just too congested.

So this scales as far as it needs to scale.

> > +/* The ATU entry varies between chipset generations. Define a generic
> > + * format which covers all the current and hopefully future
> > + * generations
> > + */
> 
> Could you please present this generic format to us? Maybe my interpretation of
> the word "generic" is incorrect in this context?

I mean generic across all mv88e6xxx switches. The fid has been slowly
getting bigger from generation to generation. If i remember correctly,
it start off as 6 bits. 2 more bits we added, in a different
register. Then it got moved into a new register and made 14 bits in
size. There are also some bits in the atu_op register which changed
meaning over time.

In order to decode any of this information in the regions, you need to
known the specific switch the dump came from. But that is the whole
point of regions.

https://www.kernel.org/doc/html/latest/networking/devlink/devlink-region.html

   As regions are likely very device or driver specific, no generic
   regions are defined. See the driver-specific documentation files
   for information on the specific regions a driver supports.

This should also make the context of 'generic' more clear.

     Andrew

Re: [PATCH net-next 7/7] net: dsa: mv88e6xxx: Implement devlink info get callback

[Jakub Kicinski]

On Sun, 16 Aug 2020 21:43:16 +0200 Andrew Lunn wrote:
> Return the driver name and the asic.id with the switch name.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support

[Florian Fainelli]

On 8/16/20 12:43 PM, Andrew Lunn wrote:
> Make use of devlink regions to allow read access to some of the
> internal of the switches. The switch itself will never trigger a
> region snapshot, it is assumed it is performed from user space as
> needed.
> 
> Andrew Lunn (7):
>   net: dsa: Add helper to convert from devlink to ds
>   net: dsa: Add devlink regions support to DSA
>   net: dsa: mv88e6xxx: Move devlink code into its own file
>   net: dsa: mv88e6xxx: Create helper for FIDs in use
>   net: dsa: mv88e6xxx: Add devlink regions
>   net: dsa: wire up devlink info get
>   net: dsa: mv88e6xxx: Implement devlink info get callback

Andrew, do you mind copying all DSA maintainers on this patch series
since it potentially affects other drivers given the standard
representation you want to see adopted?

Thanks
-- 
Florian

Re: [PATCH net-next 1/7] net: dsa: Add helper to convert from devlink to ds

[Florian Fainelli]

On 8/16/20 12:43 PM, Andrew Lunn wrote:
> Given a devlink instance, return the dsa switch it is associated to.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

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

Re: [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions

[Florian Fainelli]

On 8/16/20 3:39 PM, Andrew Lunn wrote:
>>> +static const struct devlink_region_ops *mv88e6xxx_region_port_ops[] = {
>>> +	&mv88e6xxx_region_port_0_ops,
>>> +	&mv88e6xxx_region_port_1_ops,
>>> +	&mv88e6xxx_region_port_2_ops,
>>> +	&mv88e6xxx_region_port_3_ops,
>>> +	&mv88e6xxx_region_port_4_ops,
>>> +	&mv88e6xxx_region_port_5_ops,
>>> +	&mv88e6xxx_region_port_6_ops,
>>> +	&mv88e6xxx_region_port_7_ops,
>>> +	&mv88e6xxx_region_port_8_ops,
>>> +	&mv88e6xxx_region_port_9_ops,
>>> +	&mv88e6xxx_region_port_10_ops,
>>> +	&mv88e6xxx_region_port_11_ops,
>>> +};
>>> +
>>
>> Sounds like there should maybe be an abstraction for 'per-port regions' in
>> devlink? I think your approach hardly scales if you start having
>> switches with more than 11 ports.
> 
> mv88e6xxx is unlikely to have more an 11 ports. Marvell had to move
> bits around in registers in non-compatible ways to support the 6390
> family with this number of ports. I doubt we will ever see a 16 port
> mv88e6xxx switch, the registers are just too congested.

Any number greater than 1 could justify finding a solution that scales.

> 
> So this scales as far as it needs to scale.
> 
>>> +/* The ATU entry varies between chipset generations. Define a generic
>>> + * format which covers all the current and hopefully future
>>> + * generations
>>> + */
>>
>> Could you please present this generic format to us? Maybe my interpretation of
>> the word "generic" is incorrect in this context?
> 
> I mean generic across all mv88e6xxx switches. The fid has been slowly
> getting bigger from generation to generation. If i remember correctly,
> it start off as 6 bits. 2 more bits we added, in a different
> register. Then it got moved into a new register and made 14 bits in
> size. There are also some bits in the atu_op register which changed
> meaning over time.
> 
> In order to decode any of this information in the regions, you need to
> known the specific switch the dump came from. But that is the whole
> point of regions.
> 
> https://www.kernel.org/doc/html/latest/networking/devlink/devlink-region.html
> 
>    As regions are likely very device or driver specific, no generic
>    regions are defined. See the driver-specific documentation files
>    for information on the specific regions a driver supports.
> 
> This should also make the context of 'generic' more clear.

Looking at the documentation above (assuming it is up to date), these
are raw hex dumps of the region, which is mildly useful.

If we were to pretty print those regions such that they can fully
replace the infamous debugfs interface patch from Vivien that has been
floated around before, what other information is available (besides the
driver name) for the user-space tools to do that pretty printing?

Right now, as with any single user facility it is a bit difficult to
determine whether a DSA common representation would be warranted.
-- 
Florian

Re: [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions

[Andrew Lunn]

> Looking at the documentation above (assuming it is up to date), these
> are raw hex dumps of the region, which is mildly useful.
> 
> If we were to pretty print those regions such that they can fully
> replace the infamous debugfs interface patch from Vivien that has been
> floated around before, what other information is available (besides the
> driver name) for the user-space tools to do that pretty printing?

Hi Florian

https://github.com/lunn/mv88e6xx_dump

root@rap:~# /home/andrew/mv88e6xxx_dump/mv88e6xxx_dump --atu --global1 --global2 --ports --port 1
Using device <mdio_bus/gpio-0:00>
00 Port status                            0x1e4f
      Transmit Pause Enable bit            0
      Receive Pause Enable bit             0
      802.3 PHY Detected                   1
      Link Status                          Up
      Duplex                               Full
      Speed                                1000 Mbps
      Duplex Fixed                         0
      EEE Enabled                          1
      Transmitter Paused                   0
      Flow Control                         0
      Config Mode                          0xf
01 Physical control                       0x203e
      RGMII Receive Timing Control         Default
      RGMII Transmit Timing Control        Default
      Force Speed                          1
      Alternate Speed Mode                 Normal
      MII PHY Mode                         MAC
      EEE force value                      0
      Force EEE                            0
      Link's Forced value                  Up
      Force Link                           1
      Duplex's Forced value                Full
      Force Duplex                         1
      Force Speed                          1000 Mbps
02 Flow control                           0x0100
03 Switch ID                              0x3901
04 Port control                           0x053f
      Source Address Filtering controls    Disabled
      Egress Mode                          Unmodified
      Ingress & Egress Header Mode         0
      IGMP and MLD Snooping                1
      Frame Mode                           DSA
      VLAN Tunnel                          0
      TagIfBoth                            0
      Initial Priority assignment          Tag & IP Priority
      Egress Flooding mode                 Allow unknown DA
      Port State                           Forwarding
05 Port control 1                         0x0000
      Message Port                         0
      LAG Port                             0
      VTU Page                             0
      LAG ID                               0
      FID[11:4]                            0x000
06 Port base VLAN map                     0x07fd
      FID[3:0]                             0x000
      Force Mapping                        0
      VLANTable                            0 2 3 4 5 6 7 8 9 10 
07 Def VLAN ID & Prio                     0x0000
      Default Priority                     0x0
      Force to use Default VID             0
      Default VLAN Identifier              0
08 Port control 2                         0x0080
      Force good FCS in the frame          0
      Allow bad FCS                        0
      Jumbo Mode                           1522
      802.1QMode                           Disabled
      Discard Tagged Frames                0
      Discard Untagged Frames              0
      Map using DA hits                    1
      ARP Mirror enable                    0
      Egress Monitor Source Port           0
      Ingress Monitor Source Port          0
      Allow VID of Zero                    0
      Default Queue Priority               0x0
09 Egress rate control                    0x0001
0a Egress rate control 2                  0x0000
0b Port association vec                   0x0000
0c Port ATU control                       0x0000
0d Override                               0x0000
0e Policy control                         0x0000
0f Port ether type                        0x9100
10 Reserved                               0x0000
11 Reserved                               0x0000
12 Reserved                               0x0000
13 Reserved                               0x0000
14 Reserved                               0x0000
15 Reserved                               0x0000
16 LED control                            0x0022
17 IP prio map table                      0x0000
18 IEEE prio map table                    0x3e07
19 Port control 3                         0x0000
1a Reserved                               0x0000
1b Queue counters                         0x8000
1c Queue control                          0x0000
1d Reserved                               0x0000
1e Cut through control                    0x0000
1f Debug counters                         0x003d
			   0    1    2    3    4    5    6    7    8    9   10 
00 Port status            0e07 1e4f 100f 100f de4f 100f 100f 100f 1d0f 0049 0049 
01 Physical control       0003 203e 0003 0003 0003 0003 0003 0003 0003 0003 0003 
02 Flow control           0000 0100 0100 0100 0100 0000 0000 0000 0100 0000 0000 
03 Switch ID              3901 3901 3901 3901 3901 3901 3901 3901 3901 3901 3901 
04 Port control           007c 053f 0433 0433 0433 007c 007c 007c 0433 007c 007c 
05 Port control 1         0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
06 Port base VLAN map     07fe 07fd 0002 0002 0002 07df 07bf 077f 0002 05ff 03ff 
07 Def VLAN ID & Prio     0001 0000 0000 0000 0000 0001 0001 0001 0000 0001 0001 
08 Port control 2         2080 0080 0080 0080 0080 2080 2080 2080 0080 2080 2080 
09 Egress rate control    0001 0001 0001 0001 0001 0001 0001 0001 0001 0001 0001 
0a Egress rate control 2  8000 0000 0000 0000 0000 8000 8000 8000 0000 8000 8000 
0b Port association vec   0001 0000 0004 0008 0010 0020 0040 0080 0100 0200 0400 
0c Port ATU control       0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0d Override               0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0e Policy control         0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
0f Port ether type        9100 9100 9100 9100 9100 9100 9100 9100 9100 9100 9100 
10 Reserved               0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
11 Reserved               0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
12 Reserved               0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
13 Reserved               0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
14 Reserved               0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
15 Reserved               0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
16 LED control            0000 0022 0022 0022 0022 0022 0022 0022 0022 0022 0022 
17 IP prio map table      0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
18 IEEE prio map table    0000 3e07 3e07 3e07 3e07 0000 0000 0000 3e07 0000 0000 
19 Port control 3         0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
1a Reserved               0000 0000 0000 0000 3d40 01c0 0000 0000 0000 0000 0000 
1b Queue counters         8000 8000 8000 8000 8000 8000 8000 8000 8000 8000 8000 
1c Queue control          0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
1d Reserved               0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
1e Cut through control    0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 
1f Debug counters         0000 003d 0000 0000 0077 0000 0000 0000 0000 0000 0000 
ATU:
FID  MAC	       T 0123456789A Prio State
   0 ff:ff:ff:ff:ff:ff   11111100000    0 Static
Global1:
 0 c814
 1 0000
 2 0000
 3 0000
 4 40a8
 5 4000
 6 2fff
 7 0000
 8 0000
 9 0000
10 0509
11 4000
12 7ff7
13 ffff
14 ffff
15 ffff
16 0000
17 0000
18 0000
19 0000
20 0000
21 0000
22 0000
23 0000
24 0000
25 0000
26 03ff
27 03fd
28 07c0
29 1000
30 0000
31 0000
Global2:
 0 0000
 1 811c
 2 0000
 3 0000
 4 0258
 5 0400
 6 1f1f
 7 77ff
 8 7800
 9 2a00
10 0000
11 31ff
12 0000
13 0589
14 0001
15 0f00
16 0000
17 0000
18 0000
19 0300
20 0000
21 0000
22 5e0e
23 0000
24 1885
25 c5e1
26 0000
27 110f
28 0000
29 0000
30 0000
31 0000

Still WIP. I want to add at least names for the global1 and glabel2. 

> Right now, as with any single user facility it is a bit difficult to
> determine whether a DSA common representation would be warranted.

This is all specific to the mv88e6xxx. Vivien had two debugfs
patchsets. One for generic DSA properties and a second one for
mv88e6xxx specific stuff. The regions i've added only cover the
mv88e6xxx specific stuff.

The recent devlink metric's should help with some parts of the generic
debugfs code.

Andrew

Re: [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support

[Andrew Lunn]

On Mon, Aug 17, 2020 at 10:08:07AM -0700, Florian Fainelli wrote:
> On 8/16/20 12:43 PM, Andrew Lunn wrote:
> > Make use of devlink regions to allow read access to some of the
> > internal of the switches. The switch itself will never trigger a
> > region snapshot, it is assumed it is performed from user space as
> > needed.
> > 
> > Andrew Lunn (7):
> >   net: dsa: Add helper to convert from devlink to ds
> >   net: dsa: Add devlink regions support to DSA
> >   net: dsa: mv88e6xxx: Move devlink code into its own file
> >   net: dsa: mv88e6xxx: Create helper for FIDs in use
> >   net: dsa: mv88e6xxx: Add devlink regions
> >   net: dsa: wire up devlink info get
> >   net: dsa: mv88e6xxx: Implement devlink info get callback
> 
> Andrew, do you mind copying all DSA maintainers on this patch series
> since it potentially affects other drivers given the standard
> representation you want to see adopted?

Hi Florian

I'm not proposing anything standard at all here. This is all specific
to the mv88e6xxx. As i pointed out to Vladimir, devlink regions are
supposed to be very device or driver specific.

	 Andrew