netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource
@ 2019-11-05  0:12 Andrew Lunn
  2019-11-05  0:12 ` [PATCH net-next 1/5] net: dsa: Add support for devlink resources Andrew Lunn
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Andrew Lunn @ 2019-11-05  0:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, jiri, Andrew Lunn

This patchset add generic support to DSA for devlink resources. The
Marvell switch Address Translation Unit occupancy is then exported as
a resource. In order to do this, the number of ATU entries is added to
the per switch info structure. Helpers are added, and then the
resource itself is then added.

Andrew Lunn (5):
  net: dsa: Add support for devlink resources
  net: dsa: mv88e6xxx: Add number of MACs in the ATU
  net: dsa: mv88e6xxx: global2: Expose ATU stats register
  net: dsa: mv88e6xxx: global1_atu: Add helper for get next
  net: dsa: mv88e6xxx: Add ATU occupancy via devlink resources

 drivers/net/dsa/mv88e6xxx/chip.c        | 215 +++++++++++++++++++++++-
 drivers/net/dsa/mv88e6xxx/chip.h        |   6 +
 drivers/net/dsa/mv88e6xxx/global1.h     |   1 +
 drivers/net/dsa/mv88e6xxx/global1_atu.c |   5 +
 drivers/net/dsa/mv88e6xxx/global2.c     |  13 ++
 drivers/net/dsa/mv88e6xxx/global2.h     |  24 ++-
 include/net/dsa.h                       |  16 ++
 net/dsa/dsa.c                           |  37 ++++
 8 files changed, 312 insertions(+), 5 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next 1/5] net: dsa: Add support for devlink resources
  2019-11-05  0:12 [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource Andrew Lunn
@ 2019-11-05  0:12 ` Andrew Lunn
  2019-11-05  8:47   ` Jiri Pirko
  2019-11-05  0:12 ` [PATCH net-next 2/5] net: dsa: mv88e6xxx: Add number of MACs in the ATU Andrew Lunn
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2019-11-05  0:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, jiri, Andrew Lunn

Add wrappers around the devlink resource API, so that DSA drivers can
register and unregister devlink resources.

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

diff --git a/include/net/dsa.h b/include/net/dsa.h
index e4c697b95c70..9507611a41f0 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -586,6 +586,22 @@ int dsa_devlink_params_register(struct dsa_switch *ds,
 void dsa_devlink_params_unregister(struct dsa_switch *ds,
 				   const struct devlink_param *params,
 				   size_t params_count);
+int dsa_devlink_resource_register(struct dsa_switch *ds,
+				  const char *resource_name,
+				  u64 resource_size,
+				  u64 resource_id,
+				  u64 parent_resource_id,
+				  const struct devlink_resource_size_params *size_params);
+
+void dsa_devlink_resources_unregister(struct dsa_switch *ds);
+
+void dsa_devlink_resource_occ_get_register(struct dsa_switch *ds,
+					   u64 resource_id,
+					   devlink_resource_occ_get_t *occ_get,
+					   void *occ_get_priv);
+void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
+					     u64 resource_id);
+
 struct dsa_devlink_priv {
 	struct dsa_switch *ds;
 };
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index db1c1c7e40e9..17281fec710c 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -379,6 +379,43 @@ void dsa_devlink_params_unregister(struct dsa_switch *ds,
 }
 EXPORT_SYMBOL_GPL(dsa_devlink_params_unregister);
 
+int dsa_devlink_resource_register(struct dsa_switch *ds,
+				  const char *resource_name,
+				  u64 resource_size,
+				  u64 resource_id,
+				  u64 parent_resource_id,
+				  const struct devlink_resource_size_params *size_params)
+{
+	return devlink_resource_register(ds->devlink, resource_name,
+					 resource_size, resource_id,
+					 parent_resource_id,
+					 size_params);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_resource_register);
+
+void dsa_devlink_resources_unregister(struct dsa_switch *ds)
+{
+	devlink_resources_unregister(ds->devlink, NULL);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_resources_unregister);
+
+void dsa_devlink_resource_occ_get_register(struct dsa_switch *ds,
+					   u64 resource_id,
+					   devlink_resource_occ_get_t *occ_get,
+					   void *occ_get_priv)
+{
+	return devlink_resource_occ_get_register(ds->devlink, resource_id,
+						 occ_get, occ_get_priv);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_resource_occ_get_register);
+
+void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
+					     u64 resource_id)
+{
+	devlink_resource_occ_get_unregister(ds->devlink, resource_id);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_resource_occ_get_unregister);
+
 static int __init dsa_init_module(void)
 {
 	int rc;
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next 2/5] net: dsa: mv88e6xxx: Add number of MACs in the ATU
  2019-11-05  0:12 [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource Andrew Lunn
  2019-11-05  0:12 ` [PATCH net-next 1/5] net: dsa: Add support for devlink resources Andrew Lunn
@ 2019-11-05  0:12 ` Andrew Lunn
  2019-11-05  0:12 ` [PATCH net-next 3/5] net: dsa: mv88e6xxx: global2: Expose ATU stats register Andrew Lunn
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2019-11-05  0:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, jiri, Andrew Lunn

For each supported switch, add an entry to the info structure for the
number of MACs which can be stored in the ATU. This will later be used
to export the ATU as a devlink resource, and indicate its occupancy,
how full the ATU is.

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

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 66de492117ad..3540ca2e3b6f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4299,6 +4299,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6097,
 		.name = "Marvell 88E6085",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 10,
 		.num_internal_phys = 5,
 		.max_vid = 4095,
@@ -4321,6 +4322,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6095,
 		.name = "Marvell 88E6095/88E6095F",
 		.num_databases = 256,
+		.num_macs = 8192,
 		.num_ports = 11,
 		.num_internal_phys = 0,
 		.max_vid = 4095,
@@ -4341,6 +4343,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6097,
 		.name = "Marvell 88E6097/88E6097F",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 11,
 		.num_internal_phys = 8,
 		.max_vid = 4095,
@@ -4363,6 +4366,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6165,
 		.name = "Marvell 88E6123",
 		.num_databases = 4096,
+		.num_macs = 1024,
 		.num_ports = 3,
 		.num_internal_phys = 5,
 		.max_vid = 4095,
@@ -4385,6 +4389,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6185,
 		.name = "Marvell 88E6131",
 		.num_databases = 256,
+		.num_macs = 8192,
 		.num_ports = 8,
 		.num_internal_phys = 0,
 		.max_vid = 4095,
@@ -4405,6 +4410,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6341,
 		.name = "Marvell 88E6141",
 		.num_databases = 4096,
+		.num_macs = 2048,
 		.num_ports = 6,
 		.num_internal_phys = 5,
 		.num_gpio = 11,
@@ -4428,6 +4434,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6165,
 		.name = "Marvell 88E6161",
 		.num_databases = 4096,
+		.num_macs = 1024,
 		.num_ports = 6,
 		.num_internal_phys = 5,
 		.max_vid = 4095,
@@ -4451,6 +4458,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6165,
 		.name = "Marvell 88E6165",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 6,
 		.num_internal_phys = 0,
 		.max_vid = 4095,
@@ -4474,6 +4482,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6351,
 		.name = "Marvell 88E6171",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.max_vid = 4095,
@@ -4496,6 +4505,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6352,
 		.name = "Marvell 88E6172",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.num_gpio = 15,
@@ -4519,6 +4529,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6351,
 		.name = "Marvell 88E6175",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.max_vid = 4095,
@@ -4541,6 +4552,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6352,
 		.name = "Marvell 88E6176",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.num_gpio = 15,
@@ -4564,6 +4576,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6185,
 		.name = "Marvell 88E6185",
 		.num_databases = 256,
+		.num_macs = 8192,
 		.num_ports = 10,
 		.num_internal_phys = 0,
 		.max_vid = 4095,
@@ -4584,6 +4597,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6390,
 		.name = "Marvell 88E6190",
 		.num_databases = 4096,
+		.num_macs = 16384,
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.num_gpio = 16,
@@ -4607,6 +4621,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6390,
 		.name = "Marvell 88E6190X",
 		.num_databases = 4096,
+		.num_macs = 16384,
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.num_gpio = 16,
@@ -4630,6 +4645,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6390,
 		.name = "Marvell 88E6191",
 		.num_databases = 4096,
+		.num_macs = 16384,
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.max_vid = 8191,
@@ -4680,6 +4696,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6352,
 		.name = "Marvell 88E6240",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.num_gpio = 15,
@@ -4750,6 +4767,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6320,
 		.name = "Marvell 88E6320",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.num_gpio = 15,
@@ -4774,6 +4792,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6320,
 		.name = "Marvell 88E6321",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.num_gpio = 15,
@@ -4797,6 +4816,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6341,
 		.name = "Marvell 88E6341",
 		.num_databases = 4096,
+		.num_macs = 2048,
 		.num_internal_phys = 5,
 		.num_ports = 6,
 		.num_gpio = 11,
@@ -4821,6 +4841,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6351,
 		.name = "Marvell 88E6350",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.max_vid = 4095,
@@ -4843,6 +4864,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6351,
 		.name = "Marvell 88E6351",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.max_vid = 4095,
@@ -4865,6 +4887,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6352,
 		.name = "Marvell 88E6352",
 		.num_databases = 4096,
+		.num_macs = 8192,
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.num_gpio = 15,
@@ -4888,6 +4911,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6390,
 		.name = "Marvell 88E6390",
 		.num_databases = 4096,
+		.num_macs = 16384,
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.num_gpio = 16,
@@ -4911,6 +4935,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.family = MV88E6XXX_FAMILY_6390,
 		.name = "Marvell 88E6390X",
 		.num_databases = 4096,
+		.num_macs = 16384,
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.num_gpio = 16,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 52f7726cc099..65ce09bdcbcf 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -94,6 +94,7 @@ struct mv88e6xxx_info {
 	u16 prod_num;
 	const char *name;
 	unsigned int num_databases;
+	unsigned int num_macs;
 	unsigned int num_ports;
 	unsigned int num_internal_phys;
 	unsigned int num_gpio;
@@ -613,6 +614,11 @@ static inline unsigned int mv88e6xxx_num_databases(struct mv88e6xxx_chip *chip)
 	return chip->info->num_databases;
 }
 
+static inline unsigned int mv88e6xxx_num_macs(struct  mv88e6xxx_chip *chip)
+{
+	return chip->info->num_macs;
+}
+
 static inline unsigned int mv88e6xxx_num_ports(struct mv88e6xxx_chip *chip)
 {
 	return chip->info->num_ports;
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next 3/5] net: dsa: mv88e6xxx: global2: Expose ATU stats register
  2019-11-05  0:12 [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource Andrew Lunn
  2019-11-05  0:12 ` [PATCH net-next 1/5] net: dsa: Add support for devlink resources Andrew Lunn
  2019-11-05  0:12 ` [PATCH net-next 2/5] net: dsa: mv88e6xxx: Add number of MACs in the ATU Andrew Lunn
@ 2019-11-05  0:12 ` Andrew Lunn
  2019-11-06  4:32   ` Vivien Didelot
  2019-11-05  0:13 ` [PATCH net-next 4/5] net: dsa: mv88e6xxx: global1_atu: Add helper for get next Andrew Lunn
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2019-11-05  0:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, jiri, Andrew Lunn

Add helpers to set/get the ATU statistics register.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/global2.c | 20 ++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/global2.h | 24 +++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index bdbb72fc20ed..14954d92c564 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -280,6 +280,26 @@ int mv88e6xxx_g2_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr)
 	return err;
 }
 
+/* Offset 0x0E: ATU Statistics */
+
+int mv88e6xxx_g2_atu_stats_set(struct mv88e6xxx_chip *chip, u16 kind, u16 bin)
+{
+	return mv88e6xxx_g2_write(chip, MV88E6XXX_G2_ATU_STATS,
+				  kind | bin);
+}
+
+int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip)
+{
+	int err;
+	u16 val;
+
+	err = mv88e6xxx_g2_read(chip, MV88E6XXX_G2_ATU_STATS, &val);
+	if (err)
+		return err;
+
+	return val & MV88E6XXX_G2_ATU_STATS_MASK;
+}
+
 /* Offset 0x0F: Priority Override Table */
 
 static int mv88e6xxx_g2_pot_write(struct mv88e6xxx_chip *chip, int pointer,
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index 42da4bca73e8..a308ca7a7da6 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -113,7 +113,16 @@
 #define MV88E6XXX_G2_SWITCH_MAC_DATA_MASK	0x00ff
 
 /* Offset 0x0E: ATU Stats Register */
-#define MV88E6XXX_G2_ATU_STATS		0x0e
+#define MV88E6XXX_G2_ATU_STATS				0x0e
+#define MV88E6XXX_G2_ATU_STATS_BIN_0			(0x0 << 14)
+#define MV88E6XXX_G2_ATU_STATS_BIN_1			(0x1 << 14)
+#define MV88E6XXX_G2_ATU_STATS_BIN_2			(0x2 << 14)
+#define MV88E6XXX_G2_ATU_STATS_BIN_3			(0x3 << 14)
+#define MV88E6XXX_G2_ATU_STATS_MODE_ALL			(0x0 << 12)
+#define MV88E6XXX_G2_ATU_STATS_MODE_ALL_DYNAMIC		(0x1 << 12)
+#define MV88E6XXX_G2_ATU_STATS_MODE_FID_ALL		(0x2 << 12)
+#define MV88E6XXX_G2_ATU_STATS_MODE_FID_ALL_DYNAMIC	(0x3 << 12)
+#define MV88E6XXX_G2_ATU_STATS_MASK			0x0fff
 
 /* Offset 0x0F: Priority Override Table */
 #define MV88E6XXX_G2_PRIO_OVERRIDE		0x0f
@@ -353,6 +362,8 @@ extern const struct mv88e6xxx_gpio_ops mv88e6352_gpio_ops;
 
 int mv88e6xxx_g2_scratch_gpio_set_smi(struct mv88e6xxx_chip *chip,
 				      bool external);
+int mv88e6xxx_g2_atu_stats_set(struct mv88e6xxx_chip *chip, u16 kind, u16 bin);
+int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip);
 
 #else /* !CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 */
 
@@ -515,6 +526,17 @@ static inline int mv88e6xxx_g2_device_mapping_write(struct mv88e6xxx_chip *chip,
 	return -EOPNOTSUPP;
 }
 
+static inline int mv88e6xxx_g2_atu_stats_set(struct mv88e6xxx_chip *chip,
+					     u16 kind, u16 bin)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip)
+{
+	return -EOPNOTSUPP;
+}
+
 #endif /* CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 */
 
 #endif /* _MV88E6XXX_GLOBAL2_H */
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next 4/5] net: dsa: mv88e6xxx: global1_atu: Add helper for get next
  2019-11-05  0:12 [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource Andrew Lunn
                   ` (2 preceding siblings ...)
  2019-11-05  0:12 ` [PATCH net-next 3/5] net: dsa: mv88e6xxx: global2: Expose ATU stats register Andrew Lunn
@ 2019-11-05  0:13 ` Andrew Lunn
  2019-11-05  0:13 ` [PATCH net-next 5/5] net: dsa: mv88e6xxx: Add ATU occupancy via devlink resources Andrew Lunn
  2019-11-06  2:10 ` [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource David Miller
  5 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2019-11-05  0:13 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, jiri, Andrew Lunn

When retrieving the ATU statistics, and ATU get next has to be
performed to trigger the ATU to collect the statistics. Export a
helper from global1_atu to perform this.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/global1.h     |  1 +
 drivers/net/dsa/mv88e6xxx/global1_atu.c |  5 +++++
 drivers/net/dsa/mv88e6xxx/global2.c     | 11 ++---------
 drivers/net/dsa/mv88e6xxx/global2.h     |  2 +-
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index 40fc0e13fc45..342172275841 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -341,5 +341,6 @@ int mv88e6390_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 int mv88e6xxx_g1_vtu_flush(struct mv88e6xxx_chip *chip);
 int mv88e6xxx_g1_vtu_prob_irq_setup(struct mv88e6xxx_chip *chip);
 void mv88e6xxx_g1_vtu_prob_irq_free(struct mv88e6xxx_chip *chip);
+int mv88e6xxx_g1_atu_get_next(struct mv88e6xxx_chip *chip, u16 fid);
 
 #endif /* _MV88E6XXX_GLOBAL1_H */
diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index d8a03bbba83c..bdcd25560dd2 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -154,6 +154,11 @@ static int mv88e6xxx_g1_atu_op(struct mv88e6xxx_chip *chip, u16 fid, u16 op)
 	return mv88e6xxx_g1_atu_op_wait(chip);
 }
 
+int mv88e6xxx_g1_atu_get_next(struct mv88e6xxx_chip *chip, u16 fid)
+{
+	return mv88e6xxx_g1_atu_op(chip, fid, MV88E6XXX_G1_ATU_OP_GET_NEXT_DB);
+}
+
 /* Offset 0x0C: ATU Data Register */
 
 static int mv88e6xxx_g1_atu_data_read(struct mv88e6xxx_chip *chip,
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index 14954d92c564..87bfe7c8c9cd 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -288,16 +288,9 @@ int mv88e6xxx_g2_atu_stats_set(struct mv88e6xxx_chip *chip, u16 kind, u16 bin)
 				  kind | bin);
 }
 
-int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip)
+int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip, u16 *stats)
 {
-	int err;
-	u16 val;
-
-	err = mv88e6xxx_g2_read(chip, MV88E6XXX_G2_ATU_STATS, &val);
-	if (err)
-		return err;
-
-	return val & MV88E6XXX_G2_ATU_STATS_MASK;
+	return mv88e6xxx_g2_read(chip, MV88E6XXX_G2_ATU_STATS, stats);
 }
 
 /* Offset 0x0F: Priority Override Table */
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index a308ca7a7da6..d80ad203d126 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -363,7 +363,7 @@ extern const struct mv88e6xxx_gpio_ops mv88e6352_gpio_ops;
 int mv88e6xxx_g2_scratch_gpio_set_smi(struct mv88e6xxx_chip *chip,
 				      bool external);
 int mv88e6xxx_g2_atu_stats_set(struct mv88e6xxx_chip *chip, u16 kind, u16 bin);
-int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip);
+int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip, u16 *stats);
 
 #else /* !CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 */
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next 5/5] net: dsa: mv88e6xxx: Add ATU occupancy via devlink resources
  2019-11-05  0:12 [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource Andrew Lunn
                   ` (3 preceding siblings ...)
  2019-11-05  0:13 ` [PATCH net-next 4/5] net: dsa: mv88e6xxx: global1_atu: Add helper for get next Andrew Lunn
@ 2019-11-05  0:13 ` Andrew Lunn
  2019-11-05  8:47   ` Jiri Pirko
  2019-11-06  2:10 ` [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource David Miller
  5 siblings, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2019-11-05  0:13 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Vivien Didelot, jiri, Andrew Lunn

The ATU can report how many entries it contains. It does this per bin,
there being 4 bins in total. Export the ATU as a devlink resource, and
provide a method the needed callback to get the resource occupancy.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 190 ++++++++++++++++++++++++++++++-
 1 file changed, 186 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 3540ca2e3b6f..0dbe6c8b9dc0 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2720,9 +2720,179 @@ static void mv88e6xxx_teardown_devlink_params(struct dsa_switch *ds)
 				      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;
+	}
+
+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);
+	dsa_devlink_resources_unregister(ds);
 }
 
 static int mv88e6xxx_setup(struct dsa_switch *ds)
@@ -2841,11 +3011,23 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 unlock:
 	mv88e6xxx_reg_unlock(chip);
 
-	/* Has to be called without holding the register lock, since
-	 * it takes the devlink lock, and we later take the locks in
-	 * the reverse order when getting/setting parameters.
+	if (err)
+		return err;
+
+	/* Have to be called without holding the register lock, since
+	 * they take the devlink lock, and we later take the locks in
+	 * the reverse order when getting/setting parameters or
+	 * resource occupancy.
 	 */
-	return mv88e6xxx_setup_devlink_params(ds);
+	err = mv88e6xxx_setup_devlink_resources(ds);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_setup_devlink_params(ds);
+	if (err)
+		dsa_devlink_resources_unregister(ds);
+
+	return err;
 }
 
 static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 1/5] net: dsa: Add support for devlink resources
  2019-11-05  0:12 ` [PATCH net-next 1/5] net: dsa: Add support for devlink resources Andrew Lunn
@ 2019-11-05  8:47   ` Jiri Pirko
  0 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2019-11-05  8:47 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli, Vivien Didelot, jiri

Tue, Nov 05, 2019 at 01:12:57AM CET, andrew@lunn.ch wrote:
>Add wrappers around the devlink resource API, so that DSA drivers can
>register and unregister devlink resources.
>
>Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: Add ATU occupancy via devlink resources
  2019-11-05  0:13 ` [PATCH net-next 5/5] net: dsa: mv88e6xxx: Add ATU occupancy via devlink resources Andrew Lunn
@ 2019-11-05  8:47   ` Jiri Pirko
  0 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2019-11-05  8:47 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli, Vivien Didelot, jiri

Tue, Nov 05, 2019 at 01:13:01AM CET, andrew@lunn.ch wrote:
>The ATU can report how many entries it contains. It does this per bin,
>there being 4 bins in total. Export the ATU as a devlink resource, and
>provide a method the needed callback to get the resource occupancy.
>
>Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource
  2019-11-05  0:12 [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource Andrew Lunn
                   ` (4 preceding siblings ...)
  2019-11-05  0:13 ` [PATCH net-next 5/5] net: dsa: mv88e6xxx: Add ATU occupancy via devlink resources Andrew Lunn
@ 2019-11-06  2:10 ` David Miller
  2019-11-06 13:55   ` Andrew Lunn
  5 siblings, 1 reply; 13+ messages in thread
From: David Miller @ 2019-11-06  2:10 UTC (permalink / raw)
  To: andrew; +Cc: netdev, f.fainelli, vivien.didelot, jiri

From: Andrew Lunn <andrew@lunn.ch>
Date: Tue,  5 Nov 2019 01:12:56 +0100

> This patchset add generic support to DSA for devlink resources. The
> Marvell switch Address Translation Unit occupancy is then exported as
> a resource. In order to do this, the number of ATU entries is added to
> the per switch info structure. Helpers are added, and then the
> resource itself is then added.

Series applied, thanks Andrew.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 3/5] net: dsa: mv88e6xxx: global2: Expose ATU stats register
  2019-11-05  0:12 ` [PATCH net-next 3/5] net: dsa: mv88e6xxx: global2: Expose ATU stats register Andrew Lunn
@ 2019-11-06  4:32   ` Vivien Didelot
  2019-11-06  4:37     ` Vivien Didelot
  2019-11-07  0:12     ` Andrew Lunn
  0 siblings, 2 replies; 13+ messages in thread
From: Vivien Didelot @ 2019-11-06  4:32 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli, jiri, Andrew Lunn

On Tue,  5 Nov 2019 01:12:59 +0100, Andrew Lunn <andrew@lunn.ch> wrote:
> Add helpers to set/get the ATU statistics register.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/dsa/mv88e6xxx/global2.c | 20 ++++++++++++++++++++
>  drivers/net/dsa/mv88e6xxx/global2.h | 24 +++++++++++++++++++++++-
>  2 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
> index bdbb72fc20ed..14954d92c564 100644
> --- a/drivers/net/dsa/mv88e6xxx/global2.c
> +++ b/drivers/net/dsa/mv88e6xxx/global2.c
> @@ -280,6 +280,26 @@ int mv88e6xxx_g2_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr)
>  	return err;
>  }
>  
> +/* Offset 0x0E: ATU Statistics */
> +
> +int mv88e6xxx_g2_atu_stats_set(struct mv88e6xxx_chip *chip, u16 kind, u16 bin)
> +{
> +	return mv88e6xxx_g2_write(chip, MV88E6XXX_G2_ATU_STATS,
> +				  kind | bin);
> +}
> +
> +int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip)
> +{
> +	int err;
> +	u16 val;
> +
> +	err = mv88e6xxx_g2_read(chip, MV88E6XXX_G2_ATU_STATS, &val);
> +	if (err)
> +		return err;
> +
> +	return val & MV88E6XXX_G2_ATU_STATS_MASK;
> +}

I would use the logic commonly used in the mv88e6xxx driver for
functions that may fail, returning only an error code and taking a
pointer to the correct type as argument, so that we do as usual:

    err = mv88e6xxx_g2_atu_stats_get(chip, &val);
    if (err)
        return err;

> +
>  /* Offset 0x0F: Priority Override Table */
>  
>  static int mv88e6xxx_g2_pot_write(struct mv88e6xxx_chip *chip, int pointer,
> diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
> index 42da4bca73e8..a308ca7a7da6 100644
> --- a/drivers/net/dsa/mv88e6xxx/global2.h
> +++ b/drivers/net/dsa/mv88e6xxx/global2.h
> @@ -113,7 +113,16 @@
>  #define MV88E6XXX_G2_SWITCH_MAC_DATA_MASK	0x00ff
>  
>  /* Offset 0x0E: ATU Stats Register */
> -#define MV88E6XXX_G2_ATU_STATS		0x0e
> +#define MV88E6XXX_G2_ATU_STATS				0x0e
> +#define MV88E6XXX_G2_ATU_STATS_BIN_0			(0x0 << 14)
> +#define MV88E6XXX_G2_ATU_STATS_BIN_1			(0x1 << 14)
> +#define MV88E6XXX_G2_ATU_STATS_BIN_2			(0x2 << 14)
> +#define MV88E6XXX_G2_ATU_STATS_BIN_3			(0x3 << 14)
> +#define MV88E6XXX_G2_ATU_STATS_MODE_ALL			(0x0 << 12)
> +#define MV88E6XXX_G2_ATU_STATS_MODE_ALL_DYNAMIC		(0x1 << 12)
> +#define MV88E6XXX_G2_ATU_STATS_MODE_FID_ALL		(0x2 << 12)
> +#define MV88E6XXX_G2_ATU_STATS_MODE_FID_ALL_DYNAMIC	(0x3 << 12)
> +#define MV88E6XXX_G2_ATU_STATS_MASK			0x0fff

Please use the 0x1234 format for these 16-bit registers.

>  
>  /* Offset 0x0F: Priority Override Table */
>  #define MV88E6XXX_G2_PRIO_OVERRIDE		0x0f
> @@ -353,6 +362,8 @@ extern const struct mv88e6xxx_gpio_ops mv88e6352_gpio_ops;
>  
>  int mv88e6xxx_g2_scratch_gpio_set_smi(struct mv88e6xxx_chip *chip,
>  				      bool external);
> +int mv88e6xxx_g2_atu_stats_set(struct mv88e6xxx_chip *chip, u16 kind, u16 bin);
> +int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip);
>  
>  #else /* !CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 */
>  
> @@ -515,6 +526,17 @@ static inline int mv88e6xxx_g2_device_mapping_write(struct mv88e6xxx_chip *chip,
>  	return -EOPNOTSUPP;
>  }
>  
> +static inline int mv88e6xxx_g2_atu_stats_set(struct mv88e6xxx_chip *chip,
> +					     u16 kind, u16 bin)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  #endif /* CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 */
>  
>  #endif /* _MV88E6XXX_GLOBAL2_H */
> -- 
> 2.23.0
> 

Otherwise this looks good.


Thanks,

	Vivien

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 3/5] net: dsa: mv88e6xxx: global2: Expose ATU stats register
  2019-11-06  4:32   ` Vivien Didelot
@ 2019-11-06  4:37     ` Vivien Didelot
  2019-11-07  0:12     ` Andrew Lunn
  1 sibling, 0 replies; 13+ messages in thread
From: Vivien Didelot @ 2019-11-06  4:37 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli, jiri, Andrew Lunn

Hi Andrew,

On Tue, 5 Nov 2019 23:32:41 -0500, Vivien Didelot <vivien.didelot@gmail.com> wrote:
> > +int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip)
> > +{
> > +	int err;
> > +	u16 val;
> > +
> > +	err = mv88e6xxx_g2_read(chip, MV88E6XXX_G2_ATU_STATS, &val);
> > +	if (err)
> > +		return err;
> > +
> > +	return val & MV88E6XXX_G2_ATU_STATS_MASK;
> > +}
> 
> I would use the logic commonly used in the mv88e6xxx driver for
> functions that may fail, returning only an error code and taking a
> pointer to the correct type as argument, so that we do as usual:
> 
>     err = mv88e6xxx_g2_atu_stats_get(chip, &val);
>     if (err)
>         return err;
> 
> > +
> >  /* Offset 0x0F: Priority Override Table */
> >  
> >  static int mv88e6xxx_g2_pot_write(struct mv88e6xxx_chip *chip, int pointer,
> > diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
> > index 42da4bca73e8..a308ca7a7da6 100644
> > --- a/drivers/net/dsa/mv88e6xxx/global2.h
> > +++ b/drivers/net/dsa/mv88e6xxx/global2.h
> > @@ -113,7 +113,16 @@
> >  #define MV88E6XXX_G2_SWITCH_MAC_DATA_MASK	0x00ff
> >  
> >  /* Offset 0x0E: ATU Stats Register */
> > -#define MV88E6XXX_G2_ATU_STATS		0x0e
> > +#define MV88E6XXX_G2_ATU_STATS				0x0e
> > +#define MV88E6XXX_G2_ATU_STATS_BIN_0			(0x0 << 14)
> > +#define MV88E6XXX_G2_ATU_STATS_BIN_1			(0x1 << 14)
> > +#define MV88E6XXX_G2_ATU_STATS_BIN_2			(0x2 << 14)
> > +#define MV88E6XXX_G2_ATU_STATS_BIN_3			(0x3 << 14)
> > +#define MV88E6XXX_G2_ATU_STATS_MODE_ALL			(0x0 << 12)
> > +#define MV88E6XXX_G2_ATU_STATS_MODE_ALL_DYNAMIC		(0x1 << 12)
> > +#define MV88E6XXX_G2_ATU_STATS_MODE_FID_ALL		(0x2 << 12)
> > +#define MV88E6XXX_G2_ATU_STATS_MODE_FID_ALL_DYNAMIC	(0x3 << 12)
> > +#define MV88E6XXX_G2_ATU_STATS_MASK			0x0fff
> 
> Please use the 0x1234 format for these 16-bit registers.

Oops the series has already been applied. Andrew please consider
these comments for your future series, they always apply...

Thank a lot,

	Vivien

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource
  2019-11-06  2:10 ` [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource David Miller
@ 2019-11-06 13:55   ` Andrew Lunn
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2019-11-06 13:55 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, f.fainelli, vivien.didelot, jiri

On Tue, Nov 05, 2019 at 06:10:01PM -0800, David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Tue,  5 Nov 2019 01:12:56 +0100
> 
> > This patchset add generic support to DSA for devlink resources. The
> > Marvell switch Address Translation Unit occupancy is then exported as
> > a resource. In order to do this, the number of ATU entries is added to
> > the per switch info structure. Helpers are added, and then the
> > resource itself is then added.
> 
> Series applied, thanks Andrew.

Hi David

I will send a fix for the build error ASAP. It should not affect
normal builds, just random configs.

       Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 3/5] net: dsa: mv88e6xxx: global2: Expose ATU stats register
  2019-11-06  4:32   ` Vivien Didelot
  2019-11-06  4:37     ` Vivien Didelot
@ 2019-11-07  0:12     ` Andrew Lunn
  1 sibling, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2019-11-07  0:12 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: David Miller, netdev, Florian Fainelli, jiri

> > +int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip)
> > +{
> > +	int err;
> > +	u16 val;
> > +
> > +	err = mv88e6xxx_g2_read(chip, MV88E6XXX_G2_ATU_STATS, &val);
> > +	if (err)
> > +		return err;
> > +
> > +	return val & MV88E6XXX_G2_ATU_STATS_MASK;
> > +}
> 
> I would use the logic commonly used in the mv88e6xxx driver for
> functions that may fail, returning only an error code and taking a
> pointer to the correct type as argument, so that we do as usual:
> 
>     err = mv88e6xxx_g2_atu_stats_get(chip, &val);
>     if (err)
>         return err;

Well, actually. Take a look at the next patch. This function gets
changed there. After you reviewed the first devlink patchset adding
control of the ATU algorithm, i went through this patchset and applied
the same comments to it. So i change the implementation to how you
actually wanted it. But i messed up my git commands and that changed
ended up in the next patch :-(

> >  /* Offset 0x0E: ATU Stats Register */
> > -#define MV88E6XXX_G2_ATU_STATS		0x0e
> > +#define MV88E6XXX_G2_ATU_STATS				0x0e
> > +#define MV88E6XXX_G2_ATU_STATS_BIN_0			(0x0 << 14)
> > +#define MV88E6XXX_G2_ATU_STATS_BIN_1			(0x1 << 14)
> > +#define MV88E6XXX_G2_ATU_STATS_BIN_2			(0x2 << 14)
> > +#define MV88E6XXX_G2_ATU_STATS_BIN_3			(0x3 << 14)
> > +#define MV88E6XXX_G2_ATU_STATS_MODE_ALL			(0x0 << 12)
> > +#define MV88E6XXX_G2_ATU_STATS_MODE_ALL_DYNAMIC		(0x1 << 12)
> > +#define MV88E6XXX_G2_ATU_STATS_MODE_FID_ALL		(0x2 << 12)
> > +#define MV88E6XXX_G2_ATU_STATS_MODE_FID_ALL_DYNAMIC	(0x3 << 12)
> > +#define MV88E6XXX_G2_ATU_STATS_MASK			0x0fff
> 
> Please use the 0x1234 format for these 16-bit registers.

Ug, no. That is a lot less readable. The datasheet describes there
fields in terms of bit offsets in the registers. Bit 14 and 15 for the
bin, bits 12 and 13 for the mode. You can clearly see that when using
value and shift representation. 0x0200 is much harder to read, and
much more error prone.

      Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-11-07  0:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05  0:12 [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource Andrew Lunn
2019-11-05  0:12 ` [PATCH net-next 1/5] net: dsa: Add support for devlink resources Andrew Lunn
2019-11-05  8:47   ` Jiri Pirko
2019-11-05  0:12 ` [PATCH net-next 2/5] net: dsa: mv88e6xxx: Add number of MACs in the ATU Andrew Lunn
2019-11-05  0:12 ` [PATCH net-next 3/5] net: dsa: mv88e6xxx: global2: Expose ATU stats register Andrew Lunn
2019-11-06  4:32   ` Vivien Didelot
2019-11-06  4:37     ` Vivien Didelot
2019-11-07  0:12     ` Andrew Lunn
2019-11-05  0:13 ` [PATCH net-next 4/5] net: dsa: mv88e6xxx: global1_atu: Add helper for get next Andrew Lunn
2019-11-05  0:13 ` [PATCH net-next 5/5] net: dsa: mv88e6xxx: Add ATU occupancy via devlink resources Andrew Lunn
2019-11-05  8:47   ` Jiri Pirko
2019-11-06  2:10 ` [PATCH net-next 0/5] mv88e6xxx ATU occupancy as devlink resource David Miller
2019-11-06 13:55   ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).