[PATCH v3 net-next 0/9] mv88e6xxx: Add devlink regions support

[PATCH v3 net-next 0/9] mv88e6xxx: Add devlink regions support

[Andrew Lunn]

Make use of devlink regions to allow read access to some of the
internal of the switches. Currently access to global1, global2 and the
ATU is provided.

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.

v3:
Drop support for port regions
Improve the devlink API with a priv member and passing the region to
the snapshot function
Make the helper to convert from devlink to ds an inline function

Andrew Lunn (9):
  net: devlink: regions: Add a priv member to the regions ops struct
  net: devlink: region: Pass the region ops to the snapshot function
  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          | 523 +++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/devlink.h          |  21 +
 drivers/net/ethernet/intel/ice/ice_devlink.c |   2 +
 drivers/net/netdevsim/dev.c                  |   6 +-
 include/net/devlink.h                        |   6 +-
 include/net/dsa.h                            |  18 +-
 net/core/devlink.c                           |   2 +-
 net/dsa/dsa.c                                |  28 +-
 net/dsa/dsa2.c                               |  19 +-
 12 files changed, 653 insertions(+), 277 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.h

-- 
2.28.0

[PATCH v3 net-next 1/9] net: devlink: regions: Add a priv member to the regions ops struct

[Andrew Lunn]

The driver may have multiple regions which can be dumped using one
function. However, for this to work, additional information is
needed. Add a priv member to the ops structure for the driver to use
however it likes.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/devlink.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index eaec0a8cc5ef..86ce644260b3 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -542,12 +542,14 @@ struct devlink_info_req;
  *            the data variable must be updated to point to the snapshot data.
  *            The function will be called while the devlink instance lock is
  *            held.
+ * @priv: Pointer to driver private data for the region operation
  */
 struct devlink_region_ops {
 	const char *name;
 	void (*destructor)(const void *data);
 	int (*snapshot)(struct devlink *devlink, struct netlink_ext_ack *extack,
 			u8 **data);
+	void *priv;
 };
 
 struct devlink_fmsg;
-- 
2.28.0

[PATCH v3 net-next 2/9] net: devlink: region: Pass the region ops to the snapshot function

[Andrew Lunn]

Pass the region to be snapshotted to the function performing the
snapshot. This allows one function to operate on numerous regions.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/intel/ice/ice_devlink.c | 2 ++
 drivers/net/netdevsim/dev.c                  | 6 ++++--
 include/net/devlink.h                        | 4 +++-
 net/core/devlink.c                           | 2 +-
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 111d6bfe4222..eb189d2070ae 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -413,6 +413,7 @@ void ice_devlink_destroy_port(struct ice_pf *pf)
  * error code on failure.
  */
 static int ice_devlink_nvm_snapshot(struct devlink *devlink,
+				    const struct devlink_region_ops *ops,
 				    struct netlink_ext_ack *extack, u8 **data)
 {
 	struct ice_pf *pf = devlink_priv(devlink);
@@ -468,6 +469,7 @@ static int ice_devlink_nvm_snapshot(struct devlink *devlink,
  */
 static int
 ice_devlink_devcaps_snapshot(struct devlink *devlink,
+			     const struct devlink_region_ops *ops,
 			     struct netlink_ext_ack *extack, u8 **data)
 {
 	struct ice_pf *pf = devlink_priv(devlink);
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 32f339fedb21..cf763ec69bb7 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -40,7 +40,9 @@ static struct dentry *nsim_dev_ddir;
 #define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32)
 
 static int
-nsim_dev_take_snapshot(struct devlink *devlink, struct netlink_ext_ack *extack,
+nsim_dev_take_snapshot(struct devlink *devlink,
+		       const struct devlink_region_ops *ops,
+		       struct netlink_ext_ack *extack,
 		       u8 **data)
 {
 	void *dummy_data;
@@ -68,7 +70,7 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file,
 
 	devlink = priv_to_devlink(nsim_dev);
 
-	err = nsim_dev_take_snapshot(devlink, NULL, &dummy_data);
+	err = nsim_dev_take_snapshot(devlink, NULL, NULL, &dummy_data);
 	if (err)
 		return err;
 
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 86ce644260b3..e7de55223cd1 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -547,7 +547,9 @@ struct devlink_info_req;
 struct devlink_region_ops {
 	const char *name;
 	void (*destructor)(const void *data);
-	int (*snapshot)(struct devlink *devlink, struct netlink_ext_ack *extack,
+	int (*snapshot)(struct devlink *devlink,
+			const struct devlink_region_ops *ops,
+			struct netlink_ext_ack *extack,
 			u8 **data);
 	void *priv;
 };
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 91c12612f2b7..5e383a8c44d3 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4322,7 +4322,7 @@ devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info)
 		}
 	}
 
-	err = region->ops->snapshot(devlink, info->extack, &data);
+	err = region->ops->snapshot(devlink, region->ops, info->extack, &data);
 	if (err)
 		goto err_snapshot_capture;
 
-- 
2.28.0

[PATCH v3 net-next 3/9] 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 |  7 +++++++
 net/dsa/dsa.c     | 12 ++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 75c8fac82017..42ae6d4d9d43 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -664,6 +664,13 @@ struct dsa_devlink_priv {
 	struct dsa_switch *ds;
 };
 
+static inline struct dsa_switch *dsa_devlink_to_ds(struct devlink *dl)
+{
+	struct dsa_devlink_priv *dl_priv = devlink_priv(dl);
+
+	return dl_priv->ds;
+}
+
 struct dsa_switch_driver {
 	struct list_head	list;
 	const struct dsa_switch_ops *ops;
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 1ce9ba8cf545..9b7019d86165 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -330,11 +330,7 @@ EXPORT_SYMBOL_GPL(call_dsa_notifiers);
 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 +342,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 v3 net-next 4/9] 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 42ae6d4d9d43..431efb5098be 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -658,6 +658,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 9b7019d86165..5c18c0214aac 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -404,6 +404,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 v3 net-next 5/9] 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 v3 net-next 6/9] 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 v3 net-next 7/9] net: dsa: mv88e6xxx: Add devlink regions

[Andrew Lunn]

Allow the global registers, and the ATU to be snapshot via devlink
regions. It is later planned to add support for the port registers.

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

v3:
Make use of ops structure passed to snapshot function
Remove port 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 | 245 ++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/devlink.h |   2 +
 4 files changed, 272 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..e58f68bd17dc 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,247 @@ int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds)
 	return err;
 }
 
+static int mv88e6xxx_region_global_snapshot(struct devlink *dl,
+					    const struct devlink_region_ops *ops,
+					    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++) {
+		switch ((long)ops->priv) {
+		case 1:
+			err = mv88e6xxx_g1_read(chip, i, &registers[i]);
+			break;
+		case 2:
+			err = mv88e6xxx_g1_read(chip, i, &registers[i]);
+			break;
+		default:
+			err = -EOPNOTSUPP;
+		}
+
+		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,
+					 const struct devlink_region_ops *ops,
+					 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_global_snapshot,
+	.destructor = kfree,
+	.priv = (void *)1,
+};
+
+static struct devlink_region_ops mv88e6xxx_region_global2_ops = {
+	.name = "global2",
+	.snapshot = mv88e6xxx_region_global_snapshot,
+	.destructor = kfree,
+	.priv = (void *)2,
+};
+
+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_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_global(chip);
+}
+
+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;
+
+	return mv88e6xxx_setup_devlink_regions_global(ds, chip);
+}
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 v3 net-next 8/9] 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 431efb5098be..d16057c5987a 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 v3 net-next 9/9] 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 | 16 ++++++++++++++++
 drivers/net/dsa/mv88e6xxx/devlink.h |  3 +++
 3 files changed, 20 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 e58f68bd17dc..ad16e38b37c3 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -505,3 +505,19 @@ int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds)
 
 	return mv88e6xxx_setup_devlink_regions_global(ds, chip);
 }
+
+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 v3 net-next 2/9] net: devlink: region: Pass the region ops to the snapshot function

[Jakub Kicinski]

On Thu, 10 Sep 2020 01:58:20 +0200 Andrew Lunn wrote:
> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
> index 111d6bfe4222..eb189d2070ae 100644
> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> @@ -413,6 +413,7 @@ void ice_devlink_destroy_port(struct ice_pf *pf)
>   * error code on failure.
>   */
>  static int ice_devlink_nvm_snapshot(struct devlink *devlink,
> +				    const struct devlink_region_ops *ops,
>  				    struct netlink_ext_ack *extack, u8 **data)
>  {
>  	struct ice_pf *pf = devlink_priv(devlink);
> @@ -468,6 +469,7 @@ static int ice_devlink_nvm_snapshot(struct devlink *devlink,
>   */
>  static int
>  ice_devlink_devcaps_snapshot(struct devlink *devlink,
> +			     const struct devlink_region_ops *ops,
>  			     struct netlink_ext_ack *extack, u8 **data)
>  {
>  	struct ice_pf *pf = devlink_priv(devlink);


drivers/net/ethernet/intel/ice/ice_devlink.c:418: warning: Function parameter or member 'ops' not described in 'ice_devlink_nvm_snapshot'
drivers/net/ethernet/intel/ice/ice_devlink.c:474: warning: Function parameter or member 'ops' not described in 'ice_devlink_devcaps_snapshot'

Re: [PATCH v3 net-next 2/9] net: devlink: region: Pass the region ops to the snapshot function

[Andrew Lunn]

On Thu, Sep 10, 2020 at 07:38:16AM -0700, Jakub Kicinski wrote:
> On Thu, 10 Sep 2020 01:58:20 +0200 Andrew Lunn wrote:
> > diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
> > index 111d6bfe4222..eb189d2070ae 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> > @@ -413,6 +413,7 @@ void ice_devlink_destroy_port(struct ice_pf *pf)
> >   * error code on failure.
> >   */
> >  static int ice_devlink_nvm_snapshot(struct devlink *devlink,
> > +				    const struct devlink_region_ops *ops,
> >  				    struct netlink_ext_ack *extack, u8 **data)
> >  {
> >  	struct ice_pf *pf = devlink_priv(devlink);
> > @@ -468,6 +469,7 @@ static int ice_devlink_nvm_snapshot(struct devlink *devlink,
> >   */
> >  static int
> >  ice_devlink_devcaps_snapshot(struct devlink *devlink,
> > +			     const struct devlink_region_ops *ops,
> >  			     struct netlink_ext_ack *extack, u8 **data)
> >  {
> >  	struct ice_pf *pf = devlink_priv(devlink);
> 
> 
> drivers/net/ethernet/intel/ice/ice_devlink.c:418: warning: Function parameter or member 'ops' not described in 'ice_devlink_nvm_snapshot'
> drivers/net/ethernet/intel/ice/ice_devlink.c:474: warning: Function parameter or member 'ops' not described in 'ice_devlink_devcaps_snapshot'

Thanks Jakub

I did try a W=1 build, but there are so many warnings it is hard to
pick out the new ones. I assume you have some sort of automation for
this? Could you share it?

How far are we from just enabling W=1 by default under drivers/net ?
What is the best way to do this in the Makefiles? I think
drivers/net/phy is now W=1 clean, or very close. So it would be nice
to enable that checking by default.

Thanks
	Andrew

Re: [PATCH v3 net-next 2/9] net: devlink: region: Pass the region ops to the snapshot function

[Jakub Kicinski]

On Thu, 10 Sep 2020 17:09:50 +0200 Andrew Lunn wrote:
> On Thu, Sep 10, 2020 at 07:38:16AM -0700, Jakub Kicinski wrote:
> > On Thu, 10 Sep 2020 01:58:20 +0200 Andrew Lunn wrote:  
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
> > > index 111d6bfe4222..eb189d2070ae 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> > > @@ -413,6 +413,7 @@ void ice_devlink_destroy_port(struct ice_pf *pf)
> > >   * error code on failure.
> > >   */
> > >  static int ice_devlink_nvm_snapshot(struct devlink *devlink,
> > > +				    const struct devlink_region_ops *ops,
> > >  				    struct netlink_ext_ack *extack, u8 **data)
> > >  {
> > >  	struct ice_pf *pf = devlink_priv(devlink);
> > > @@ -468,6 +469,7 @@ static int ice_devlink_nvm_snapshot(struct devlink *devlink,
> > >   */
> > >  static int
> > >  ice_devlink_devcaps_snapshot(struct devlink *devlink,
> > > +			     const struct devlink_region_ops *ops,
> > >  			     struct netlink_ext_ack *extack, u8 **data)
> > >  {
> > >  	struct ice_pf *pf = devlink_priv(devlink);  
> > 
> > 
> > drivers/net/ethernet/intel/ice/ice_devlink.c:418: warning: Function parameter or member 'ops' not described in 'ice_devlink_nvm_snapshot'
> > drivers/net/ethernet/intel/ice/ice_devlink.c:474: warning: Function parameter or member 'ops' not described in 'ice_devlink_devcaps_snapshot'  
> 
> Thanks Jakub
> 
> I did try a W=1 build, but there are so many warnings it is hard to
> pick out the new ones. I assume you have some sort of automation for
> this? Could you share it?

I knew you'd ask :)

The scripts are here: https://github.com/kuba-moo/nipa
but they run off a private patchwork instance, I was planning to make
it possible for it to consume a local mdir:

https://github.com/kuba-moo/nipa/blob/master/ingest_mdir.py

But that has probably bit rotted completely by now.

I submit my own patches to netdev and check if I broke anything later.

> How far are we from just enabling W=1 by default under drivers/net ?

I think pretty far :(

> What is the best way to do this in the Makefiles? I think
> drivers/net/phy is now W=1 clean, or very close. So it would be nice
> to enable that checking by default.

No idea 🤔

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

[Jakub Kicinski]

On Thu, 10 Sep 2020 01:58:25 +0200 Andrew Lunn wrote:
> +	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));

kcalloc()

Re: [PATCH v3 net-next 1/9] net: devlink: regions: Add a priv member to the regions ops struct

[Florian Fainelli]

On 9/9/2020 4:58 PM, Andrew Lunn wrote:
> The driver may have multiple regions which can be dumped using one
> function. However, for this to work, additional information is
> needed. Add a priv member to the ops structure for the driver to use
> however it likes.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

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

Re: [PATCH v3 net-next 4/9] net: dsa: Add devlink regions support to DSA

[Florian Fainelli]

On 9/9/2020 4:58 PM, Andrew Lunn wrote:
> Allow DSA drivers to make use of devlink regions, via simple wrappers.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

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

Re: [PATCH v3 net-next 5/9] net: dsa: mv88e6xxx: Move devlink code into its own file

[Florian Fainelli]

On 9/9/2020 4:58 PM, Andrew Lunn wrote:
> 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>

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

Re: [PATCH v3 net-next 6/9] net: dsa: mv88e6xxx: Create helper for FIDs in use

[Florian Fainelli]

On 9/9/2020 4:58 PM, Andrew Lunn wrote:
> 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>

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

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

[Florian Fainelli]

On 9/9/2020 4:58 PM, Andrew Lunn wrote:
> Allow the global registers, and the ATU to be snapshot via devlink
> regions. It is later planned to add support for the port registers.
> 
> v2:
> Remove left over debug prints
> Comment ATU format is generic for mv88e6xxx, not wider
> 
> v3:
> Make use of ops structure passed to snapshot function
> Remove port regions
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---

[snip]

> +static int mv88e6xxx_region_global_snapshot(struct devlink *dl,
> +					    const struct devlink_region_ops *ops,
> +					    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++) {
> +		switch ((long)ops->priv) {
> +		case 1:
> +			err = mv88e6xxx_g1_read(chip, i, &registers[i]);
> +			break;
> +		case 2:
> +			err = mv88e6xxx_g1_read(chip, i, &registers[i]);

Should this be mv88e6xxx_g2_read() here? Can you use the region IDs you 
defined above?

> +			break;
> +		default:
> +			err = -EOPNOTSUPP;
> +		}
> +
> +		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;

Can you refactor this to use a for/while loop?


> +		(*count)++;
> +	} while (!is_broadcast_ether_addr(addr.mac));
> +
> +	return 0;
> +}
> +
> +static int mv88e6xxx_region_atu_snapshot(struct devlink *dl,
> +					 const struct devlink_region_ops *ops,
> +					 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_global_snapshot,
> +	.destructor = kfree,
> +	.priv = (void *)1,

Can you use the region IDs you defined?
-- 
Florian

Re: [PATCH v3 net-next 8/9] net: dsa: wire up devlink info get

[Florian Fainelli]

On 9/9/2020 4:58 PM, 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.
> 
> v2:
> Combine declaration and the assignment on a single line.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

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

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

[Florian Fainelli]

On 9/9/2020 4:58 PM, Andrew Lunn wrote:
> 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>

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

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

[Andrew Lunn]

> > +static int mv88e6xxx_region_global_snapshot(struct devlink *dl,
> > +					    const struct devlink_region_ops *ops,
> > +					    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++) {
> > +		switch ((long)ops->priv) {
> > +		case 1:
> > +			err = mv88e6xxx_g1_read(chip, i, &registers[i]);
> > +			break;
> > +		case 2:
> > +			err = mv88e6xxx_g1_read(chip, i, &registers[i]);
> 
> Should this be mv88e6xxx_g2_read() here?

Doh! Thanks.

> Can you use the region IDs you defined above?

Yes. That would be more readable. I probably need to make ops->priv
point to a real structure, to avoid compiler warnings about down
sizing types on casts.

       Andrew