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

[PATCH net-next v2 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.

v2:
Remove left of debug print
Comment ATU format is generic to mv88e6xxx
Combine declaration and the assignment on a single line.

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 | 686 ++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/devlink.h |  21 +
 include/net/dsa.h                   |  13 +-
 net/dsa/dsa.c                       |  36 +-
 net/dsa/dsa2.c                      |  19 +-
 8 files changed, 807 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 v2 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.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
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 v2 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 v2 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 15b97a4f8d93..984bdcaff1ea 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 v2 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 984bdcaff1ea..d8bb5e5e8583 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 v2 5/7] net: dsa: mv88e6xxx: Add devlink regions

[Andrew Lunn]

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

v2:
Remove left over debug prints
Comment ATU format is generic for mv88e6xxx, not wider

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 | 409 ++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/devlink.h |   2 +
 4 files changed, 436 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d8bb5e5e8583..8d1710c896ae 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..d93a2b33e355 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)
 {
@@ -260,3 +261,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 mv88e6xxx chipset generations. Define
+ * a generic format which covers all the current and hopefully future
+ * mv88e6xxx 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 v2 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.

v2:
Combine declaration and the assignment on a single line.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/dsa.h |  5 ++++-
 net/dsa/dsa2.c    | 19 ++++++++++++++++---
 2 files changed, 20 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..3cf67f5fe54a 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,22 @@ 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 = 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 v2 7/7] net: dsa: mv88e6xxx: Implement devlink info get callback

[Andrew Lunn]

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

Reviewed-by: Jakub Kicinski <kuba@kernel.org>
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 8d1710c896ae..9417412e5fce 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -5378,6 +5378,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 d93a2b33e355..5accd34a9862 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -669,3 +669,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 v2 0/7] net: dsa: mv88e6xxx: Add devlink regions support

[Florian Fainelli]

On 9/7/2020 5:51 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.
> 
> v2:
> Remove left of debug print
> Comment ATU format is generic to mv88e6xxx
> Combine declaration and the assignment on a single line.

Andrew, can you run scripts/get_maintainters.pl for your patch 
submissions and copy the various DSA maintainers as Vladimir who gives 
valuable feedback? Thanks

> 
> 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 | 686 ++++++++++++++++++++++++++++
>   drivers/net/dsa/mv88e6xxx/devlink.h |  21 +
>   include/net/dsa.h                   |  13 +-
>   net/dsa/dsa.c                       |  36 +-
>   net/dsa/dsa2.c                      |  19 +-
>   8 files changed, 807 insertions(+), 273 deletions(-)
>   create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.c
>   create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.h
> 

-- 
Florian

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

[Marek Behún]

On Tue,  8 Sep 2020 02:51:49 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> +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);

Hi Andrew,
why not  make this a static inline function?
Marek

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

[Jakub Kicinski]

On Tue,  8 Sep 2020 02:51:53 +0200 Andrew Lunn wrote:
> Allow ports, the global registers, and the ATU to be snapshot via
> devlink regions.
> 
> v2:
> Remove left over debug prints
> Comment ATU format is generic for mv88e6xxx, not wider
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Probably best CCing devlink maintainers on devlink patches.

Also - it's always useful to include show command outputs in the commit
message for devlink patches.

> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index d8bb5e5e8583..8d1710c896ae 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..d93a2b33e355 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)
>  {
> @@ -260,3 +261,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,						\
> +}

This is a little awkward, can we make devlink pass the region pointer
back to the callback instead? Plus perhaps an ability to allocate "priv"
data inside the region would also h

> +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,
> +};

Ahh, seems like regions will get a per-port incarnation as some point as
well..

> +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);

FWIW I'd use

	while (port--)

but some consider it error prone.

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

[Andrew Lunn]

On Tue, Sep 08, 2020 at 12:01:06PM -0700, Jakub Kicinski wrote:
> On Tue,  8 Sep 2020 02:51:53 +0200 Andrew Lunn wrote:
> > Allow ports, the global registers, and the ATU to be snapshot via
> > devlink regions.
> > 
> > v2:
> > Remove left over debug prints
> > Comment ATU format is generic for mv88e6xxx, not wider
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> 
> Probably best CCing devlink maintainers on devlink patches.
> 
> Also - it's always useful to include show command outputs in the commit
> message for devlink patches.

Hi Jakub

root@rap:~# devlink region dump mdio_bus/gpio-0:00/port5 snapshot 42
0000000000000000 0f 10 03 00 00 00 01 39 7c 00 00 00 df 07 01 00 
0000000000000010 80 20 01 00 00 80 20 00 00 00 00 00 00 00 00 91 
0000000000000020 00 00 00 00 00 00 00 00 00 00 00 00 22 00 00 00 
0000000000000030 00 00 00 00 c0 01 00 80 00 00 00 00 00 00 00 00 

Not very informative. The whole point of devlink regions is that they
are suppose to be specific to a device, and you need intimate
knowledge of the device to decode it.

> > +#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,						\
> > +}
> 
> This is a little awkward, can we make devlink pass the region pointer
> back to the callback instead? Plus perhaps an ability to allocate "priv"
> data inside the region would also h

Yes, this API is not easy to use. I suspect it is because it was
developed to support 'core dump' of the firmware, and you only have
one core to dump.

> > +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,
> > +};
> 
> Ahh, seems like regions will get a per-port incarnation as some point as
> well..

Again, i think this is back to the history of dumping firmware core.
I guess the existing users don't have per port CPUs which could dump a
core.

	Andrew

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

[Jakub Kicinski]

On Tue, 8 Sep 2020 21:22:31 +0200 Andrew Lunn wrote:
> On Tue, Sep 08, 2020 at 12:01:06PM -0700, Jakub Kicinski wrote:
> > On Tue,  8 Sep 2020 02:51:53 +0200 Andrew Lunn wrote:  
> > > Allow ports, the global registers, and the ATU to be snapshot via
> > > devlink regions.
> > > 
> > > v2:
> > > Remove left over debug prints
> > > Comment ATU format is generic for mv88e6xxx, not wider
> > > 
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>  
> > 
> > Probably best CCing devlink maintainers on devlink patches.
> > 
> > Also - it's always useful to include show command outputs in the commit
> > message for devlink patches.  
> 
> Hi Jakub
> 
> root@rap:~# devlink region dump mdio_bus/gpio-0:00/port5 snapshot 42
> 0000000000000000 0f 10 03 00 00 00 01 39 7c 00 00 00 df 07 01 00 
> 0000000000000010 80 20 01 00 00 80 20 00 00 00 00 00 00 00 00 91 
> 0000000000000020 00 00 00 00 00 00 00 00 00 00 00 00 22 00 00 00 
> 0000000000000030 00 00 00 00 c0 01 00 80 00 00 00 00 00 00 00 00 
> 
> Not very informative. The whole point of devlink regions is that they
> are suppose to be specific to a device, and you need intimate
> knowledge of the device to decode it.

I meant the list of regions the device would create.

> > > +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,
> > > +};  
> > 
> > Ahh, seems like regions will get a per-port incarnation as some point as
> > well..  
> 
> Again, i think this is back to the history of dumping firmware core.
> I guess the existing users don't have per port CPUs which could dump a
> core.

Ack, I'm referring to the fact that we recently converted health
reporters to per-port and now traps are getting the same treatment.