netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next 0/2] mv88e6xxx: Allow config of ATU hash algorithm
@ 2019-10-04 21:09 Andrew Lunn
  2019-10-04 21:09 ` [PATCH v2 net-next 1/2] net: dsa: Add support for devlink device parameters Andrew Lunn
  2019-10-04 21:09 ` [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Add devlink param for ATU hash algorithm Andrew Lunn
  0 siblings, 2 replies; 8+ messages in thread
From: Andrew Lunn @ 2019-10-04 21:09 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn

The Marvell switches allow the hash algorithm for MAC addresses in the
address translation unit to be configured. Add support to the DSA core
to allow DSA drivers to make use of devlink parameters, and allow the
ATU hash to be get/set via such a parameter.

v2:

Pass a pointer for where the hash should be stored, return a plain
errno, or 0.

Document the parameter.

 .../networking/devlink-params-mv88e6xxx.txt   |   6 +
 MAINTAINERS                                   |   1 +
 drivers/net/dsa/mv88e6xxx/chip.c              | 134 +++++++++++++++++-
 drivers/net/dsa/mv88e6xxx/chip.h              |   4 +
 drivers/net/dsa/mv88e6xxx/global1.h           |   3 +
 drivers/net/dsa/mv88e6xxx/global1_atu.c       |  32 +++++
 include/net/dsa.h                             |  23 +++
 net/dsa/dsa.c                                 |  48 +++++++
 net/dsa/dsa2.c                                |   7 +-
 9 files changed, 256 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/networking/devlink-params-mv88e6xxx.txt

-- 
2.23.0


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

* [PATCH v2 net-next 1/2] net: dsa: Add support for devlink device parameters
  2019-10-04 21:09 [PATCH v2 net-next 0/2] mv88e6xxx: Allow config of ATU hash algorithm Andrew Lunn
@ 2019-10-04 21:09 ` Andrew Lunn
  2019-10-05  0:21   ` Vivien Didelot
  2019-10-04 21:09 ` [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Add devlink param for ATU hash algorithm Andrew Lunn
  1 sibling, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2019-10-04 21:09 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn

Add plumbing to allow DSA drivers to register parameters with devlink.

To keep with the abstraction, the DSA drivers pass the ds structure to
these helpers, and the DSA core then translates that to the devlink
structure associated to the device.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/dsa.h | 23 +++++++++++++++++++++++
 net/dsa/dsa.c     | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 net/dsa/dsa2.c    |  7 ++++++-
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8c3ea0530f65..6623f4428930 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -541,6 +541,29 @@ struct dsa_switch_ops {
 	 */
 	netdev_tx_t (*port_deferred_xmit)(struct dsa_switch *ds, int port,
 					  struct sk_buff *skb);
+	/* Devlink parameters */
+	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);
+};
+
+#define DSA_DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes)		\
+	DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes,		\
+			     dsa_dl_param_get,	dsa_dl_param_set, NULL)
+
+int dsa_dl_param_get(struct devlink *dl, u32 id,
+		     struct devlink_param_gset_ctx *ctx);
+int dsa_dl_param_set(struct devlink *dl, u32 id,
+		     struct devlink_param_gset_ctx *ctx);
+int dsa_devlink_params_register(struct dsa_switch *ds,
+				const struct devlink_param *params,
+				size_t params_count);
+void dsa_devlink_params_unregister(struct dsa_switch *ds,
+				   const struct devlink_param *params,
+				   size_t params_count);
+struct dsa_devlink_priv {
+	struct dsa_switch *ds;
 };
 
 struct dsa_switch_driver {
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 43120a3fb06f..ea7678650d8c 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -329,6 +329,54 @@ int call_dsa_notifiers(unsigned long val, struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(call_dsa_notifiers);
 
+int dsa_dl_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;
+
+	if (!ds->ops->devlink_param_get)
+		return -EOPNOTSUPP;
+
+	return ds->ops->devlink_param_get(ds, id, ctx);
+}
+EXPORT_SYMBOL_GPL(dsa_dl_param_get);
+
+int dsa_dl_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;
+
+	if (!ds->ops->devlink_param_set)
+		return -EOPNOTSUPP;
+
+	return ds->ops->devlink_param_set(ds, id, ctx);
+}
+EXPORT_SYMBOL_GPL(dsa_dl_param_set);
+
+int dsa_devlink_params_register(struct dsa_switch *ds,
+				const struct devlink_param *params,
+				size_t params_count)
+{
+	return devlink_params_register(ds->devlink, params, params_count);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_params_register);
+
+void dsa_devlink_params_unregister(struct dsa_switch *ds,
+				   const struct devlink_param *params,
+				   size_t params_count)
+{
+	devlink_params_unregister(ds->devlink, params, params_count);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_params_unregister);
+
 static int __init dsa_init_module(void)
 {
 	int rc;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 73002022c9d8..d74cc82fb44a 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -367,6 +367,7 @@ static void dsa_port_teardown(struct dsa_port *dp)
 
 static int dsa_switch_setup(struct dsa_switch *ds)
 {
+	struct dsa_devlink_priv *dl_priv;
 	int err = 0;
 
 	/* Initialize ds->phys_mii_mask before registering the slave MDIO bus
@@ -379,9 +380,11 @@ static int dsa_switch_setup(struct dsa_switch *ds)
 	/* Add the switch to devlink before calling setup, so that setup can
 	 * add dpipe tables
 	 */
-	ds->devlink = devlink_alloc(&dsa_devlink_ops, 0);
+	ds->devlink = devlink_alloc(&dsa_devlink_ops, sizeof(*devlink_priv));
 	if (!ds->devlink)
 		return -ENOMEM;
+	dl_priv = devlink_priv(ds->devlink);
+	dl_priv->ds = ds;
 
 	err = devlink_register(ds->devlink, ds->dev);
 	if (err)
@@ -395,6 +398,8 @@ static int dsa_switch_setup(struct dsa_switch *ds)
 	if (err < 0)
 		goto unregister_notifier;
 
+	devlink_params_publish(ds->devlink);
+
 	if (!ds->slave_mii_bus && ds->ops->phy_read) {
 		ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
 		if (!ds->slave_mii_bus) {
-- 
2.23.0


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

* [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Add devlink param for ATU hash algorithm.
  2019-10-04 21:09 [PATCH v2 net-next 0/2] mv88e6xxx: Allow config of ATU hash algorithm Andrew Lunn
  2019-10-04 21:09 ` [PATCH v2 net-next 1/2] net: dsa: Add support for devlink device parameters Andrew Lunn
@ 2019-10-04 21:09 ` Andrew Lunn
  2019-10-04 21:32   ` Jakub Kicinski
  2019-10-05  0:32   ` Vivien Didelot
  1 sibling, 2 replies; 8+ messages in thread
From: Andrew Lunn @ 2019-10-04 21:09 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn

Some of the marvell switches have bits controlling the hash algorithm
the ATU uses for MAC addresses. In some industrial settings, where all
the devices are from the same manufacture, and hence use the same OUI,
the default hashing algorithm is not optimal. Allow the other
algorithms to be selected via devlink.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 .../networking/devlink-params-mv88e6xxx.txt   |   6 +
 MAINTAINERS                                   |   1 +
 drivers/net/dsa/mv88e6xxx/chip.c              | 134 +++++++++++++++++-
 drivers/net/dsa/mv88e6xxx/chip.h              |   4 +
 drivers/net/dsa/mv88e6xxx/global1.h           |   3 +
 drivers/net/dsa/mv88e6xxx/global1_atu.c       |  32 +++++
 6 files changed, 179 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/networking/devlink-params-mv88e6xxx.txt

diff --git a/Documentation/networking/devlink-params-mv88e6xxx.txt b/Documentation/networking/devlink-params-mv88e6xxx.txt
new file mode 100644
index 000000000000..cc5c1ac87c36
--- /dev/null
+++ b/Documentation/networking/devlink-params-mv88e6xxx.txt
@@ -0,0 +1,6 @@
+ATU_hash		[DEVICE, DRIVER-SPECIFIC]
+			Select one of four possible hashing algorithms for
+			MAC addresses in the Address Translation Unity.
+			A value of 3 seems to work better than the default of
+			1 when many MAC addresses have the same OUI.
+			Configuration mode: runtime
diff --git a/MAINTAINERS b/MAINTAINERS
index 496e8f156925..2246dc121c30 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9736,6 +9736,7 @@ S:	Maintained
 F:	drivers/net/dsa/mv88e6xxx/
 F:	include/linux/platform_data/mv88e6xxx.h
 F:	Documentation/devicetree/bindings/net/dsa/marvell.txt
+F:	Documentation/networking/devlink-params-mv88e6xxx.txt
 
 MARVELL ARMADA DRM SUPPORT
 M:	Russell King <linux@armlinux.org.uk>
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 6787d560e9e3..c9755a4285a9 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1370,6 +1370,22 @@ 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)
 {
@@ -2641,6 +2657,81 @@ 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;
+	u8 hash;
+
+	mv88e6xxx_reg_lock(chip);
+
+	switch (id) {
+	case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
+		err = mv88e6xxx_atu_get_hash(chip, &hash);
+		if (err < 0)
+			break;
+
+		ctx->val.vu8 = hash;
+		break;
+	default:
+		err = -EOPNOTSUPP;
+	}
+
+	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;
+	}
+
+	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));
+}
+
+static void mv88e6xxx_teardown(struct dsa_switch *ds)
+{
+	mv88e6xxx_teardown_devlink_params(ds);
+}
+
 static int mv88e6xxx_setup(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
@@ -2757,7 +2848,11 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 unlock:
 	mv88e6xxx_reg_unlock(chip);
 
-	return err;
+	/* 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.
+	 */
+	return mv88e6xxx_setup_devlink_params(ds);
 }
 
 static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
@@ -3117,6 +3212,8 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.phylink_validate = mv88e6185_phylink_validate,
@@ -3246,6 +3343,8 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.avb_ops = &mv88e6165_avb_ops,
@@ -3280,6 +3379,8 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.avb_ops = &mv88e6165_avb_ops,
@@ -3322,6 +3423,8 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.phylink_validate = mv88e6185_phylink_validate,
@@ -3366,6 +3469,8 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6352_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.serdes_get_lane = mv88e6352_serdes_get_lane,
@@ -3409,6 +3514,8 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.phylink_validate = mv88e6185_phylink_validate,
@@ -3453,6 +3560,8 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6352_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.serdes_get_lane = mv88e6352_serdes_get_lane,
@@ -3538,6 +3647,8 @@ static const struct mv88e6xxx_ops mv88e6190_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6390_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
 	.serdes_power = mv88e6390_serdes_power,
@@ -3587,6 +3698,8 @@ static const struct mv88e6xxx_ops mv88e6190x_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6390_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
 	.serdes_power = mv88e6390_serdes_power,
@@ -3635,6 +3748,8 @@ static const struct mv88e6xxx_ops mv88e6191_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6390_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
 	.serdes_power = mv88e6390_serdes_power,
@@ -3686,6 +3801,8 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6352_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.serdes_get_lane = mv88e6352_serdes_get_lane,
@@ -3777,6 +3894,8 @@ static const struct mv88e6xxx_ops mv88e6290_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6390_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
 	.serdes_power = mv88e6390_serdes_power,
@@ -3963,6 +4082,8 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.phylink_validate = mv88e6185_phylink_validate,
@@ -4003,6 +4124,8 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.avb_ops = &mv88e6352_avb_ops,
@@ -4049,6 +4172,8 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6352_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.serdes_get_lane = mv88e6352_serdes_get_lane,
@@ -4105,6 +4230,8 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6390_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
 	.serdes_power = mv88e6390_serdes_power,
@@ -4158,6 +4285,8 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6390_g1_rmu_disable,
+	.atu_get_hash = mv88e6165_g1_atu_get_hash,
+	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
 	.serdes_power = mv88e6390_serdes_power,
@@ -4933,6 +5062,7 @@ static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
 static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.get_tag_protocol	= mv88e6xxx_get_tag_protocol,
 	.setup			= mv88e6xxx_setup,
+	.teardown		= mv88e6xxx_teardown,
 	.phylink_validate	= mv88e6xxx_validate,
 	.phylink_mac_link_state	= mv88e6xxx_link_state,
 	.phylink_mac_config	= mv88e6xxx_mac_config,
@@ -4975,6 +5105,8 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.port_txtstamp		= mv88e6xxx_port_txtstamp,
 	.port_rxtstamp		= mv88e6xxx_port_rxtstamp,
 	.get_ts_info		= mv88e6xxx_get_ts_info,
+	.devlink_param_get	= mv88e6xxx_devlink_param_get,
+	.devlink_param_set	= mv88e6xxx_devlink_param_set,
 };
 
 static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index e9b1a1ac9a8e..52f7726cc099 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -497,6 +497,10 @@ struct mv88e6xxx_ops {
 	int (*serdes_get_stats)(struct mv88e6xxx_chip *chip,  int port,
 				uint64_t *data);
 
+	/* Address Translation Unit operations */
+	int (*atu_get_hash)(struct mv88e6xxx_chip *chip, u8 *hash);
+	int (*atu_set_hash)(struct mv88e6xxx_chip *chip, u8 hash);
+
 	/* VLAN Translation Unit operations */
 	int (*vtu_getnext)(struct mv88e6xxx_chip *chip,
 			   struct mv88e6xxx_vtu_entry *entry);
diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index 0870fcc8bfc8..9f2af711293b 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -109,6 +109,7 @@
 /* Offset 0x0A: ATU Control Register */
 #define MV88E6XXX_G1_ATU_CTL		0x0a
 #define MV88E6XXX_G1_ATU_CTL_LEARN2ALL	0x0008
+#define MV88E6161_G1_ATU_CTL_HASH_MASK	0x3
 
 /* Offset 0x0B: ATU Operation Register */
 #define MV88E6XXX_G1_ATU_OP				0x0b
@@ -318,6 +319,8 @@ int mv88e6xxx_g1_atu_remove(struct mv88e6xxx_chip *chip, u16 fid, int port,
 			    bool all);
 int mv88e6xxx_g1_atu_prob_irq_setup(struct mv88e6xxx_chip *chip);
 void mv88e6xxx_g1_atu_prob_irq_free(struct mv88e6xxx_chip *chip);
+int mv88e6165_g1_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash);
+int mv88e6165_g1_atu_set_hash(struct mv88e6xxx_chip *chip, u8 hash);
 
 int mv88e6185_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 			     struct mv88e6xxx_vtu_entry *entry);
diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index 792a96ef418f..d8a03bbba83c 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -73,6 +73,38 @@ int mv88e6xxx_g1_atu_set_age_time(struct mv88e6xxx_chip *chip,
 	return 0;
 }
 
+int mv88e6165_g1_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
+{
+	int err;
+	u16 val;
+
+	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_CTL, &val);
+	if (err)
+		return err;
+
+	*hash = val & MV88E6161_G1_ATU_CTL_HASH_MASK;
+
+	return 0;
+}
+
+int mv88e6165_g1_atu_set_hash(struct mv88e6xxx_chip *chip, u8 hash)
+{
+	int err;
+	u16 val;
+
+	if (hash & ~MV88E6161_G1_ATU_CTL_HASH_MASK)
+		return -EINVAL;
+
+	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_CTL, &val);
+	if (err)
+		return err;
+
+	val &= ~MV88E6161_G1_ATU_CTL_HASH_MASK;
+	val |= hash;
+
+	return mv88e6xxx_g1_write(chip, MV88E6XXX_G1_ATU_CTL, val);
+}
+
 /* Offset 0x0B: ATU Operation Register */
 
 static int mv88e6xxx_g1_atu_op_wait(struct mv88e6xxx_chip *chip)
-- 
2.23.0


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

* Re: [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Add devlink param for ATU hash algorithm.
  2019-10-04 21:09 ` [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Add devlink param for ATU hash algorithm Andrew Lunn
@ 2019-10-04 21:32   ` Jakub Kicinski
  2019-10-04 21:40     ` Andrew Lunn
  2019-10-05  0:32   ` Vivien Didelot
  1 sibling, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2019-10-04 21:32 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Vivien Didelot, Florian Fainelli

On Fri,  4 Oct 2019 23:09:34 +0200, Andrew Lunn wrote:
> diff --git a/Documentation/networking/devlink-params-mv88e6xxx.txt b/Documentation/networking/devlink-params-mv88e6xxx.txt
> new file mode 100644
> index 000000000000..cc5c1ac87c36
> --- /dev/null
> +++ b/Documentation/networking/devlink-params-mv88e6xxx.txt
> @@ -0,0 +1,6 @@
> +ATU_hash		[DEVICE, DRIVER-SPECIFIC]
> +			Select one of four possible hashing algorithms for
> +			MAC addresses in the Address Translation Unity.
> +			A value of 3 seems to work better than the default of
> +			1 when many MAC addresses have the same OUI.
> +			Configuration mode: runtime

I think it's common practice to specify the type here? Otherwise looks
good to me, thanks for adding it!

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

* Re: [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Add devlink param for ATU hash algorithm.
  2019-10-04 21:32   ` Jakub Kicinski
@ 2019-10-04 21:40     ` Andrew Lunn
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2019-10-04 21:40 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: David Miller, netdev, Vivien Didelot, Florian Fainelli

On Fri, Oct 04, 2019 at 02:32:36PM -0700, Jakub Kicinski wrote:
> On Fri,  4 Oct 2019 23:09:34 +0200, Andrew Lunn wrote:
> > diff --git a/Documentation/networking/devlink-params-mv88e6xxx.txt b/Documentation/networking/devlink-params-mv88e6xxx.txt
> > new file mode 100644
> > index 000000000000..cc5c1ac87c36
> > --- /dev/null
> > +++ b/Documentation/networking/devlink-params-mv88e6xxx.txt
> > @@ -0,0 +1,6 @@
> > +ATU_hash		[DEVICE, DRIVER-SPECIFIC]
> > +			Select one of four possible hashing algorithms for
> > +			MAC addresses in the Address Translation Unity.
> > +			A value of 3 seems to work better than the default of
> > +			1 when many MAC addresses have the same OUI.
> > +			Configuration mode: runtime
> 
> I think it's common practice to specify the type here? Otherwise looks
> good to me, thanks for adding it!

Hi Jakub

Ah, yes. It is a u8, but only values 0-3 are valid.

I will do a v3, but will wait a day for further comments.

    Andrew

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

* Re: [PATCH v2 net-next 1/2] net: dsa: Add support for devlink device parameters
  2019-10-04 21:09 ` [PATCH v2 net-next 1/2] net: dsa: Add support for devlink device parameters Andrew Lunn
@ 2019-10-05  0:21   ` Vivien Didelot
  2019-10-05  0:52     ` Andrew Lunn
  0 siblings, 1 reply; 8+ messages in thread
From: Vivien Didelot @ 2019-10-05  0:21 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli, Andrew Lunn

On Fri,  4 Oct 2019 23:09:33 +0200, Andrew Lunn <andrew@lunn.ch> wrote:
> Add plumbing to allow DSA drivers to register parameters with devlink.
> 
> To keep with the abstraction, the DSA drivers pass the ds structure to
> these helpers, and the DSA core then translates that to the devlink
> structure associated to the device.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/net/dsa.h | 23 +++++++++++++++++++++++
>  net/dsa/dsa.c     | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  net/dsa/dsa2.c    |  7 ++++++-
>  3 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 8c3ea0530f65..6623f4428930 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -541,6 +541,29 @@ struct dsa_switch_ops {
>  	 */
>  	netdev_tx_t (*port_deferred_xmit)(struct dsa_switch *ds, int port,
>  					  struct sk_buff *skb);
> +	/* Devlink parameters */
> +	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);

Unless that is how devlink is designed, shouldn't ctx be const on _set?


Thanks,

	Vivien

> +};
> +
> +#define DSA_DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes)		\
> +	DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes,		\
> +			     dsa_dl_param_get,	dsa_dl_param_set, NULL)
> +
> +int dsa_dl_param_get(struct devlink *dl, u32 id,
> +		     struct devlink_param_gset_ctx *ctx);
> +int dsa_dl_param_set(struct devlink *dl, u32 id,
> +		     struct devlink_param_gset_ctx *ctx);
> +int dsa_devlink_params_register(struct dsa_switch *ds,
> +				const struct devlink_param *params,
> +				size_t params_count);
> +void dsa_devlink_params_unregister(struct dsa_switch *ds,
> +				   const struct devlink_param *params,
> +				   size_t params_count);
> +struct dsa_devlink_priv {
> +	struct dsa_switch *ds;
>  };
>  
>  struct dsa_switch_driver {
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 43120a3fb06f..ea7678650d8c 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -329,6 +329,54 @@ int call_dsa_notifiers(unsigned long val, struct net_device *dev,
>  }
>  EXPORT_SYMBOL_GPL(call_dsa_notifiers);
>  
> +int dsa_dl_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;
> +
> +	if (!ds->ops->devlink_param_get)
> +		return -EOPNOTSUPP;
> +
> +	return ds->ops->devlink_param_get(ds, id, ctx);
> +}
> +EXPORT_SYMBOL_GPL(dsa_dl_param_get);
> +
> +int dsa_dl_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;
> +
> +	if (!ds->ops->devlink_param_set)
> +		return -EOPNOTSUPP;
> +
> +	return ds->ops->devlink_param_set(ds, id, ctx);
> +}
> +EXPORT_SYMBOL_GPL(dsa_dl_param_set);
> +
> +int dsa_devlink_params_register(struct dsa_switch *ds,
> +				const struct devlink_param *params,
> +				size_t params_count)
> +{
> +	return devlink_params_register(ds->devlink, params, params_count);
> +}
> +EXPORT_SYMBOL_GPL(dsa_devlink_params_register);
> +
> +void dsa_devlink_params_unregister(struct dsa_switch *ds,
> +				   const struct devlink_param *params,
> +				   size_t params_count)
> +{
> +	devlink_params_unregister(ds->devlink, params, params_count);
> +}
> +EXPORT_SYMBOL_GPL(dsa_devlink_params_unregister);
> +
>  static int __init dsa_init_module(void)
>  {
>  	int rc;
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 73002022c9d8..d74cc82fb44a 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -367,6 +367,7 @@ static void dsa_port_teardown(struct dsa_port *dp)
>  
>  static int dsa_switch_setup(struct dsa_switch *ds)
>  {
> +	struct dsa_devlink_priv *dl_priv;
>  	int err = 0;
>  
>  	/* Initialize ds->phys_mii_mask before registering the slave MDIO bus
> @@ -379,9 +380,11 @@ static int dsa_switch_setup(struct dsa_switch *ds)
>  	/* Add the switch to devlink before calling setup, so that setup can
>  	 * add dpipe tables
>  	 */
> -	ds->devlink = devlink_alloc(&dsa_devlink_ops, 0);
> +	ds->devlink = devlink_alloc(&dsa_devlink_ops, sizeof(*devlink_priv));
>  	if (!ds->devlink)
>  		return -ENOMEM;
> +	dl_priv = devlink_priv(ds->devlink);
> +	dl_priv->ds = ds;
>  
>  	err = devlink_register(ds->devlink, ds->dev);
>  	if (err)
> @@ -395,6 +398,8 @@ static int dsa_switch_setup(struct dsa_switch *ds)
>  	if (err < 0)
>  		goto unregister_notifier;
>  
> +	devlink_params_publish(ds->devlink);
> +
>  	if (!ds->slave_mii_bus && ds->ops->phy_read) {
>  		ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
>  		if (!ds->slave_mii_bus) {
> -- 
> 2.23.0
> 

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

* Re: [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Add devlink param for ATU hash algorithm.
  2019-10-04 21:09 ` [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Add devlink param for ATU hash algorithm Andrew Lunn
  2019-10-04 21:32   ` Jakub Kicinski
@ 2019-10-05  0:32   ` Vivien Didelot
  1 sibling, 0 replies; 8+ messages in thread
From: Vivien Didelot @ 2019-10-05  0:32 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli, Andrew Lunn

Hi Andrew,

On Fri,  4 Oct 2019 23:09:34 +0200, Andrew Lunn <andrew@lunn.ch> wrote:
> Some of the marvell switches have bits controlling the hash algorithm
> the ATU uses for MAC addresses. In some industrial settings, where all
> the devices are from the same manufacture, and hence use the same OUI,
> the default hashing algorithm is not optimal. Allow the other
> algorithms to be selected via devlink.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  .../networking/devlink-params-mv88e6xxx.txt   |   6 +
>  MAINTAINERS                                   |   1 +
>  drivers/net/dsa/mv88e6xxx/chip.c              | 134 +++++++++++++++++-
>  drivers/net/dsa/mv88e6xxx/chip.h              |   4 +
>  drivers/net/dsa/mv88e6xxx/global1.h           |   3 +
>  drivers/net/dsa/mv88e6xxx/global1_atu.c       |  32 +++++
>  6 files changed, 179 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/networking/devlink-params-mv88e6xxx.txt
> 
> diff --git a/Documentation/networking/devlink-params-mv88e6xxx.txt b/Documentation/networking/devlink-params-mv88e6xxx.txt
> new file mode 100644
> index 000000000000..cc5c1ac87c36
> --- /dev/null
> +++ b/Documentation/networking/devlink-params-mv88e6xxx.txt
> @@ -0,0 +1,6 @@
> +ATU_hash		[DEVICE, DRIVER-SPECIFIC]
> +			Select one of four possible hashing algorithms for
> +			MAC addresses in the Address Translation Unity.
> +			A value of 3 seems to work better than the default of
> +			1 when many MAC addresses have the same OUI.
> +			Configuration mode: runtime
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 496e8f156925..2246dc121c30 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9736,6 +9736,7 @@ S:	Maintained
>  F:	drivers/net/dsa/mv88e6xxx/
>  F:	include/linux/platform_data/mv88e6xxx.h
>  F:	Documentation/devicetree/bindings/net/dsa/marvell.txt
> +F:	Documentation/networking/devlink-params-mv88e6xxx.txt
>  
>  MARVELL ARMADA DRM SUPPORT
>  M:	Russell King <linux@armlinux.org.uk>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 6787d560e9e3..c9755a4285a9 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1370,6 +1370,22 @@ 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)
>  {
> @@ -2641,6 +2657,81 @@ 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;
> +	u8 hash;
> +
> +	mv88e6xxx_reg_lock(chip);
> +
> +	switch (id) {
> +	case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
> +		err = mv88e6xxx_atu_get_hash(chip, &hash);
> +		if (err < 0)
> +			break;

No need to check for error signedness, just do if (err).

> +
> +		ctx->val.vu8 = hash;

Is ctx->val.vu8 an u8 as well? If so you can just write:

                err = mv88e6xxx_atu_get_hash(chip, &ctx->val.vu8);
> +		break;
> +	default:
> +		err = -EOPNOTSUPP;

Missing a break statement here.

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

Missing a break statement here too.

> +	}
> +
> +	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));
> +}
> +
> +static void mv88e6xxx_teardown(struct dsa_switch *ds)
> +{
> +	mv88e6xxx_teardown_devlink_params(ds);
> +}
> +
>  static int mv88e6xxx_setup(struct dsa_switch *ds)
>  {
>  	struct mv88e6xxx_chip *chip = ds->priv;
> @@ -2757,7 +2848,11 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
>  unlock:
>  	mv88e6xxx_reg_unlock(chip);
>  
> -	return err;
> +	/* 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.
> +	 */
> +	return mv88e6xxx_setup_devlink_params(ds);
>  }
>  
>  static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
> @@ -3117,6 +3212,8 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
>  	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.phylink_validate = mv88e6185_phylink_validate,
> @@ -3246,6 +3343,8 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
>  	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.avb_ops = &mv88e6165_avb_ops,
> @@ -3280,6 +3379,8 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
>  	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.avb_ops = &mv88e6165_avb_ops,
> @@ -3322,6 +3423,8 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
>  	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.phylink_validate = mv88e6185_phylink_validate,
> @@ -3366,6 +3469,8 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6352_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.serdes_get_lane = mv88e6352_serdes_get_lane,
> @@ -3409,6 +3514,8 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
>  	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.phylink_validate = mv88e6185_phylink_validate,
> @@ -3453,6 +3560,8 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6352_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.serdes_get_lane = mv88e6352_serdes_get_lane,
> @@ -3538,6 +3647,8 @@ static const struct mv88e6xxx_ops mv88e6190_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6390_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6390_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
>  	.serdes_power = mv88e6390_serdes_power,
> @@ -3587,6 +3698,8 @@ static const struct mv88e6xxx_ops mv88e6190x_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6390_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6390_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
>  	.serdes_power = mv88e6390_serdes_power,
> @@ -3635,6 +3748,8 @@ static const struct mv88e6xxx_ops mv88e6191_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6390_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6390_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
>  	.serdes_power = mv88e6390_serdes_power,
> @@ -3686,6 +3801,8 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6352_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.serdes_get_lane = mv88e6352_serdes_get_lane,
> @@ -3777,6 +3894,8 @@ static const struct mv88e6xxx_ops mv88e6290_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6390_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6390_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
>  	.serdes_power = mv88e6390_serdes_power,
> @@ -3963,6 +4082,8 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
>  	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.phylink_validate = mv88e6185_phylink_validate,
> @@ -4003,6 +4124,8 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
>  	.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.avb_ops = &mv88e6352_avb_ops,
> @@ -4049,6 +4172,8 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6352_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6352_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
>  	.serdes_get_lane = mv88e6352_serdes_get_lane,
> @@ -4105,6 +4230,8 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6390_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6390_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
>  	.serdes_power = mv88e6390_serdes_power,
> @@ -4158,6 +4285,8 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
>  	.pot_clear = mv88e6xxx_g2_pot_clear,
>  	.reset = mv88e6352_g1_reset,
>  	.rmu_disable = mv88e6390_g1_rmu_disable,
> +	.atu_get_hash = mv88e6165_g1_atu_get_hash,
> +	.atu_set_hash = mv88e6165_g1_atu_set_hash,
>  	.vtu_getnext = mv88e6390_g1_vtu_getnext,
>  	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
>  	.serdes_power = mv88e6390_serdes_power,
> @@ -4933,6 +5062,7 @@ static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
>  static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
>  	.get_tag_protocol	= mv88e6xxx_get_tag_protocol,
>  	.setup			= mv88e6xxx_setup,
> +	.teardown		= mv88e6xxx_teardown,
>  	.phylink_validate	= mv88e6xxx_validate,
>  	.phylink_mac_link_state	= mv88e6xxx_link_state,
>  	.phylink_mac_config	= mv88e6xxx_mac_config,
> @@ -4975,6 +5105,8 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
>  	.port_txtstamp		= mv88e6xxx_port_txtstamp,
>  	.port_rxtstamp		= mv88e6xxx_port_rxtstamp,
>  	.get_ts_info		= mv88e6xxx_get_ts_info,
> +	.devlink_param_get	= mv88e6xxx_devlink_param_get,
> +	.devlink_param_set	= mv88e6xxx_devlink_param_set,
>  };
>  
>  static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
> index e9b1a1ac9a8e..52f7726cc099 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -497,6 +497,10 @@ struct mv88e6xxx_ops {
>  	int (*serdes_get_stats)(struct mv88e6xxx_chip *chip,  int port,
>  				uint64_t *data);
>  
> +	/* Address Translation Unit operations */
> +	int (*atu_get_hash)(struct mv88e6xxx_chip *chip, u8 *hash);
> +	int (*atu_set_hash)(struct mv88e6xxx_chip *chip, u8 hash);
> +
>  	/* VLAN Translation Unit operations */
>  	int (*vtu_getnext)(struct mv88e6xxx_chip *chip,
>  			   struct mv88e6xxx_vtu_entry *entry);
> diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
> index 0870fcc8bfc8..9f2af711293b 100644
> --- a/drivers/net/dsa/mv88e6xxx/global1.h
> +++ b/drivers/net/dsa/mv88e6xxx/global1.h
> @@ -109,6 +109,7 @@
>  /* Offset 0x0A: ATU Control Register */
>  #define MV88E6XXX_G1_ATU_CTL		0x0a
>  #define MV88E6XXX_G1_ATU_CTL_LEARN2ALL	0x0008
> +#define MV88E6161_G1_ATU_CTL_HASH_MASK	0x3
>  
>  /* Offset 0x0B: ATU Operation Register */
>  #define MV88E6XXX_G1_ATU_OP				0x0b
> @@ -318,6 +319,8 @@ int mv88e6xxx_g1_atu_remove(struct mv88e6xxx_chip *chip, u16 fid, int port,
>  			    bool all);
>  int mv88e6xxx_g1_atu_prob_irq_setup(struct mv88e6xxx_chip *chip);
>  void mv88e6xxx_g1_atu_prob_irq_free(struct mv88e6xxx_chip *chip);
> +int mv88e6165_g1_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash);
> +int mv88e6165_g1_atu_set_hash(struct mv88e6xxx_chip *chip, u8 hash);
>  
>  int mv88e6185_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
>  			     struct mv88e6xxx_vtu_entry *entry);
> diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
> index 792a96ef418f..d8a03bbba83c 100644
> --- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
> +++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
> @@ -73,6 +73,38 @@ int mv88e6xxx_g1_atu_set_age_time(struct mv88e6xxx_chip *chip,
>  	return 0;
>  }
>  
> +int mv88e6165_g1_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
> +{
> +	int err;
> +	u16 val;
> +
> +	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_CTL, &val);
> +	if (err)
> +		return err;
> +
> +	*hash = val & MV88E6161_G1_ATU_CTL_HASH_MASK;
> +
> +	return 0;
> +}
> +
> +int mv88e6165_g1_atu_set_hash(struct mv88e6xxx_chip *chip, u8 hash)
> +{
> +	int err;
> +	u16 val;
> +
> +	if (hash & ~MV88E6161_G1_ATU_CTL_HASH_MASK)
> +		return -EINVAL;
> +
> +	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_CTL, &val);
> +	if (err)
> +		return err;
> +
> +	val &= ~MV88E6161_G1_ATU_CTL_HASH_MASK;
> +	val |= hash;
> +
> +	return mv88e6xxx_g1_write(chip, MV88E6XXX_G1_ATU_CTL, val);
> +}
> +
>  /* Offset 0x0B: ATU Operation Register */
>  
>  static int mv88e6xxx_g1_atu_op_wait(struct mv88e6xxx_chip *chip)


Thanks,

	Vivien

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

* Re: [PATCH v2 net-next 1/2] net: dsa: Add support for devlink device parameters
  2019-10-05  0:21   ` Vivien Didelot
@ 2019-10-05  0:52     ` Andrew Lunn
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2019-10-05  0:52 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: David Miller, netdev, Florian Fainelli

On Fri, Oct 04, 2019 at 08:21:22PM -0400, Vivien Didelot wrote:
> On Fri,  4 Oct 2019 23:09:33 +0200, Andrew Lunn <andrew@lunn.ch> wrote:
> > Add plumbing to allow DSA drivers to register parameters with devlink.
> > 
> > To keep with the abstraction, the DSA drivers pass the ds structure to
> > these helpers, and the DSA core then translates that to the devlink
> > structure associated to the device.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  include/net/dsa.h | 23 +++++++++++++++++++++++
> >  net/dsa/dsa.c     | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> >  net/dsa/dsa2.c    |  7 ++++++-
> >  3 files changed, 77 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/net/dsa.h b/include/net/dsa.h
> > index 8c3ea0530f65..6623f4428930 100644
> > --- a/include/net/dsa.h
> > +++ b/include/net/dsa.h
> > @@ -541,6 +541,29 @@ struct dsa_switch_ops {
> >  	 */
> >  	netdev_tx_t (*port_deferred_xmit)(struct dsa_switch *ds, int port,
> >  					  struct sk_buff *skb);
> > +	/* Devlink parameters */
> > +	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);
> 
> Unless that is how devlink is designed, shouldn't ctx be const on _set?

It is the way devlink is designed. The devlink structure is

truct devlink_param {
        u32 id;
        const char *name;
        bool generic;
        enum devlink_param_type type;
        unsigned long supported_cmodes;
        int (*get)(struct devlink *devlink, u32 id,
                   struct devlink_param_gset_ctx *ctx);
        int (*set)(struct devlink *devlink, u32 id,
                   struct devlink_param_gset_ctx *ctx);
        int (*validate)(struct devlink *devlink, u32 id,
                        union devlink_param_value val,
                        struct netlink_ext_ack *extack);
};

No const on set.

   Andrew

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

end of thread, other threads:[~2019-10-05  0:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04 21:09 [PATCH v2 net-next 0/2] mv88e6xxx: Allow config of ATU hash algorithm Andrew Lunn
2019-10-04 21:09 ` [PATCH v2 net-next 1/2] net: dsa: Add support for devlink device parameters Andrew Lunn
2019-10-05  0:21   ` Vivien Didelot
2019-10-05  0:52     ` Andrew Lunn
2019-10-04 21:09 ` [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Add devlink param for ATU hash algorithm Andrew Lunn
2019-10-04 21:32   ` Jakub Kicinski
2019-10-04 21:40     ` Andrew Lunn
2019-10-05  0:32   ` Vivien Didelot

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).