* [PATCH net-next 1/7] net: dsa: Add helper to convert from devlink to ds
2020-08-16 19:43 [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Andrew Lunn
@ 2020-08-16 19:43 ` Andrew Lunn
2020-08-17 17:08 ` Florian Fainelli
2020-08-16 19:43 ` [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA Andrew Lunn
` (7 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 19:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Chris Healy, Vivien Didelot, Andrew Lunn
Given a devlink instance, return the dsa switch it is associated to.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
include/net/dsa.h | 2 ++
net/dsa/dsa.c | 20 ++++++++++----------
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 75c8fac82017..63ff6f717307 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -629,6 +629,8 @@ struct dsa_switch_ops {
int (*port_max_mtu)(struct dsa_switch *ds, int port);
};
+struct dsa_switch *dsa_devlink_to_ds(struct devlink *dl);
+
#define DSA_DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes) \
DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes, \
dsa_devlink_param_get, dsa_devlink_param_set, NULL)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 1ce9ba8cf545..86351da4e202 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -327,14 +327,18 @@ int call_dsa_notifiers(unsigned long val, struct net_device *dev,
}
EXPORT_SYMBOL_GPL(call_dsa_notifiers);
+struct dsa_switch *dsa_devlink_to_ds(struct devlink *dl)
+{
+ struct dsa_devlink_priv *dl_priv = devlink_priv(dl);
+
+ return dl_priv->ds;
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_to_ds);
+
int dsa_devlink_param_get(struct devlink *dl, u32 id,
struct devlink_param_gset_ctx *ctx)
{
- struct dsa_devlink_priv *dl_priv;
- struct dsa_switch *ds;
-
- dl_priv = devlink_priv(dl);
- ds = dl_priv->ds;
+ struct dsa_switch *ds = dsa_devlink_to_ds(dl);
if (!ds->ops->devlink_param_get)
return -EOPNOTSUPP;
@@ -346,11 +350,7 @@ EXPORT_SYMBOL_GPL(dsa_devlink_param_get);
int dsa_devlink_param_set(struct devlink *dl, u32 id,
struct devlink_param_gset_ctx *ctx)
{
- struct dsa_devlink_priv *dl_priv;
- struct dsa_switch *ds;
-
- dl_priv = devlink_priv(dl);
- ds = dl_priv->ds;
+ struct dsa_switch *ds = dsa_devlink_to_ds(dl);
if (!ds->ops->devlink_param_set)
return -EOPNOTSUPP;
--
2.28.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA
2020-08-16 19:43 [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Andrew Lunn
2020-08-16 19:43 ` [PATCH net-next 1/7] net: dsa: Add helper to convert from devlink to ds Andrew Lunn
@ 2020-08-16 19:43 ` Andrew Lunn
2020-08-16 21:50 ` Vladimir Oltean
2020-08-16 19:43 ` [PATCH net-next 3/7] net: dsa: mv88e6xxx: Move devlink code into its own file Andrew Lunn
` (6 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 19:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Chris Healy, Vivien Didelot, Andrew Lunn
Allow DSA drivers to make use of devlink regions, via simple wrappers.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
include/net/dsa.h | 6 ++++++
net/dsa/dsa.c | 16 ++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 63ff6f717307..8963440ec7f8 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -660,6 +660,12 @@ void dsa_devlink_resource_occ_get_register(struct dsa_switch *ds,
void *occ_get_priv);
void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
u64 resource_id);
+struct devlink_region *
+dsa_devlink_region_create(struct dsa_switch *ds,
+ const struct devlink_region_ops *ops,
+ u32 region_max_snapshots, u64 region_size);
+void dsa_devlink_region_destroy(struct devlink_region *region);
+
struct dsa_port *dsa_port_from_netdev(struct net_device *netdev);
struct dsa_devlink_priv {
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 86351da4e202..fea2efe5fe68 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -412,6 +412,22 @@ void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
}
EXPORT_SYMBOL_GPL(dsa_devlink_resource_occ_get_unregister);
+struct devlink_region *
+dsa_devlink_region_create(struct dsa_switch *ds,
+ const struct devlink_region_ops *ops,
+ u32 region_max_snapshots, u64 region_size)
+{
+ return devlink_region_create(ds->devlink, ops, region_max_snapshots,
+ region_size);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_region_create);
+
+void dsa_devlink_region_destroy(struct devlink_region *region)
+{
+ devlink_region_destroy(region);
+}
+EXPORT_SYMBOL_GPL(dsa_devlink_region_destroy);
+
struct dsa_port *dsa_port_from_netdev(struct net_device *netdev)
{
if (!netdev || !dsa_slave_dev_check(netdev))
--
2.28.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA
2020-08-16 19:43 ` [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA Andrew Lunn
@ 2020-08-16 21:50 ` Vladimir Oltean
2020-08-16 22:06 ` Andrew Lunn
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-08-16 21:50 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, Chris Healy, Vivien Didelot
On Sun, Aug 16, 2020 at 09:43:11PM +0200, Andrew Lunn wrote:
> Allow DSA drivers to make use of devlink regions, via simple wrappers.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> include/net/dsa.h | 6 ++++++
> net/dsa/dsa.c | 16 ++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 63ff6f717307..8963440ec7f8 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -660,6 +660,12 @@ void dsa_devlink_resource_occ_get_register(struct dsa_switch *ds,
> void *occ_get_priv);
> void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
> u64 resource_id);
> +struct devlink_region *
> +dsa_devlink_region_create(struct dsa_switch *ds,
> + const struct devlink_region_ops *ops,
> + u32 region_max_snapshots, u64 region_size);
> +void dsa_devlink_region_destroy(struct devlink_region *region);
> +
> struct dsa_port *dsa_port_from_netdev(struct net_device *netdev);
>
> struct dsa_devlink_priv {
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 86351da4e202..fea2efe5fe68 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -412,6 +412,22 @@ void dsa_devlink_resource_occ_get_unregister(struct dsa_switch *ds,
> }
> EXPORT_SYMBOL_GPL(dsa_devlink_resource_occ_get_unregister);
>
> +struct devlink_region *
> +dsa_devlink_region_create(struct dsa_switch *ds,
> + const struct devlink_region_ops *ops,
> + u32 region_max_snapshots, u64 region_size)
> +{
> + return devlink_region_create(ds->devlink, ops, region_max_snapshots,
> + region_size);
> +}
> +EXPORT_SYMBOL_GPL(dsa_devlink_region_create);
> +
> +void dsa_devlink_region_destroy(struct devlink_region *region)
> +{
> + devlink_region_destroy(region);
> +}
> +EXPORT_SYMBOL_GPL(dsa_devlink_region_destroy);
> +
> struct dsa_port *dsa_port_from_netdev(struct net_device *netdev)
> {
> if (!netdev || !dsa_slave_dev_check(netdev))
> --
> 2.28.0
>
Could we perhaps open-code these from the drivers themselves? There's
hardly any added value in DSA providing a "helper" for creation of
devlink resources (regions, shared buffers, etc).
Take the ocelot/felix driver for example. It is a DSA driver whose core
functionality is provided by drivers/net/ethernet/mscc/ocelot*.c, which
is non-DSA code. If it were to implement devlink regions, presumably
that code would live in drivers/net/ethernet/mscc/ and not in
drivers/net/dsa/.
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA
2020-08-16 21:50 ` Vladimir Oltean
@ 2020-08-16 22:06 ` Andrew Lunn
2020-08-16 22:17 ` Vladimir Oltean
0 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 22:06 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: David Miller, netdev, Chris Healy, Vivien Didelot
> Could we perhaps open-code these from the drivers themselves? There's
> hardly any added value in DSA providing a "helper" for creation of
> devlink resources (regions, shared buffers, etc).
It is something i considered. But we already have devlink wrappers. It
would be odd to have some parts of devlink wrapped and other parts
not.
The wrapping of phys is also causing Russell King problems. Both
phylink and devlink structures are mostly hidden away in the DSA core,
but do leak a bit into the drivers.
If we do change to open coding, would we remove the existing wrappers
as well?
> Take the ocelot/felix driver for example.
ocelot/felix is just plain odd. We have to do a balancing act for
it. We don't want to take stuff out of the core just for this one odd
switch, at the detriment for other normal DSA drivers.
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA
2020-08-16 22:06 ` Andrew Lunn
@ 2020-08-16 22:17 ` Vladimir Oltean
0 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-08-16 22:17 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, Chris Healy, Vivien Didelot
On Mon, Aug 17, 2020 at 12:06:32AM +0200, Andrew Lunn wrote:
> > Could we perhaps open-code these from the drivers themselves? There's
> > hardly any added value in DSA providing a "helper" for creation of
> > devlink resources (regions, shared buffers, etc).
>
> If we do change to open coding, would we remove the existing wrappers
> as well?
>
Maybe?
I reckon one of the main reasons why DSA hides struct net_device is to
present a unified API for the ports that don't have one.
But with devlink we don't have that problem.
> > Take the ocelot/felix driver for example.
>
> ocelot/felix is just plain odd. We have to do a balancing act for
> it. We don't want to take stuff out of the core just for this one odd
> switch, at the detriment for other normal DSA drivers.
>
Yes, the ocelot/felix driver _is_ odd, but in my defence it's only as
odd as the hardware was integrated.
On the other hand, the model you're proposing would be forcing me to
register devlink regions in one way for felix DSA, and in another way
for ocelot switchdev. Or could I just ignore the helper, and call
devlink directly, even if there's a helper in place?
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-next 3/7] net: dsa: mv88e6xxx: Move devlink code into its own file
2020-08-16 19:43 [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Andrew Lunn
2020-08-16 19:43 ` [PATCH net-next 1/7] net: dsa: Add helper to convert from devlink to ds Andrew Lunn
2020-08-16 19:43 ` [PATCH net-next 2/7] net: dsa: Add devlink regions support to DSA Andrew Lunn
@ 2020-08-16 19:43 ` Andrew Lunn
2020-08-16 19:43 ` [PATCH net-next 4/7] net: dsa: mv88e6xxx: Create helper for FIDs in use Andrew Lunn
` (5 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 19:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Chris Healy, Vivien Didelot, Andrew Lunn
There will soon be more devlink code. Move the existing code into a
file of its own, before we start adding this new code.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/Makefile | 1 +
drivers/net/dsa/mv88e6xxx/chip.c | 255 +--------------------------
drivers/net/dsa/mv88e6xxx/devlink.c | 262 ++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/devlink.h | 16 ++
4 files changed, 280 insertions(+), 254 deletions(-)
create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.c
create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.h
diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
index aa645ff86f64..4b080b448ce7 100644
--- a/drivers/net/dsa/mv88e6xxx/Makefile
+++ b/drivers/net/dsa/mv88e6xxx/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_NET_DSA_MV88E6XXX) += mv88e6xxx.o
mv88e6xxx-objs := chip.o
+mv88e6xxx-objs += devlink.o
mv88e6xxx-objs += global1.o
mv88e6xxx-objs += global1_atu.o
mv88e6xxx-objs += global1_vtu.o
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 7a71c9902e73..940dd0c55568 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -32,6 +32,7 @@
#include <net/dsa.h>
#include "chip.h"
+#include "devlink.h"
#include "global1.h"
#include "global2.h"
#include "hwtstamp.h"
@@ -1508,22 +1509,6 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
return mv88e6xxx_g1_atu_flush(chip, *fid, true);
}
-static int mv88e6xxx_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
-{
- if (chip->info->ops->atu_get_hash)
- return chip->info->ops->atu_get_hash(chip, hash);
-
- return -EOPNOTSUPP;
-}
-
-static int mv88e6xxx_atu_set_hash(struct mv88e6xxx_chip *chip, u8 hash)
-{
- if (chip->info->ops->atu_set_hash)
- return chip->info->ops->atu_set_hash(chip, hash);
-
- return -EOPNOTSUPP;
-}
-
static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
u16 vid_begin, u16 vid_end)
{
@@ -2837,244 +2822,6 @@ static int mv88e6390_setup_errata(struct mv88e6xxx_chip *chip)
return mv88e6xxx_software_reset(chip);
}
-enum mv88e6xxx_devlink_param_id {
- MV88E6XXX_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
- MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH,
-};
-
-static int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
- struct devlink_param_gset_ctx *ctx)
-{
- struct mv88e6xxx_chip *chip = ds->priv;
- int err;
-
- mv88e6xxx_reg_lock(chip);
-
- switch (id) {
- case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
- err = mv88e6xxx_atu_get_hash(chip, &ctx->val.vu8);
- break;
- default:
- err = -EOPNOTSUPP;
- break;
- }
-
- mv88e6xxx_reg_unlock(chip);
-
- return err;
-}
-
-static int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
- struct devlink_param_gset_ctx *ctx)
-{
- struct mv88e6xxx_chip *chip = ds->priv;
- int err;
-
- mv88e6xxx_reg_lock(chip);
-
- switch (id) {
- case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
- err = mv88e6xxx_atu_set_hash(chip, ctx->val.vu8);
- break;
- default:
- err = -EOPNOTSUPP;
- break;
- }
-
- mv88e6xxx_reg_unlock(chip);
-
- return err;
-}
-
-static const struct devlink_param mv88e6xxx_devlink_params[] = {
- DSA_DEVLINK_PARAM_DRIVER(MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH,
- "ATU_hash", DEVLINK_PARAM_TYPE_U8,
- BIT(DEVLINK_PARAM_CMODE_RUNTIME)),
-};
-
-static int mv88e6xxx_setup_devlink_params(struct dsa_switch *ds)
-{
- return dsa_devlink_params_register(ds, mv88e6xxx_devlink_params,
- ARRAY_SIZE(mv88e6xxx_devlink_params));
-}
-
-static void mv88e6xxx_teardown_devlink_params(struct dsa_switch *ds)
-{
- dsa_devlink_params_unregister(ds, mv88e6xxx_devlink_params,
- ARRAY_SIZE(mv88e6xxx_devlink_params));
-}
-
-enum mv88e6xxx_devlink_resource_id {
- MV88E6XXX_RESOURCE_ID_ATU,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
-};
-
-static u64 mv88e6xxx_devlink_atu_bin_get(struct mv88e6xxx_chip *chip,
- u16 bin)
-{
- u16 occupancy = 0;
- int err;
-
- mv88e6xxx_reg_lock(chip);
-
- err = mv88e6xxx_g2_atu_stats_set(chip, MV88E6XXX_G2_ATU_STATS_MODE_ALL,
- bin);
- if (err) {
- dev_err(chip->dev, "failed to set ATU stats kind/bin\n");
- goto unlock;
- }
-
- err = mv88e6xxx_g1_atu_get_next(chip, 0);
- if (err) {
- dev_err(chip->dev, "failed to perform ATU get next\n");
- goto unlock;
- }
-
- err = mv88e6xxx_g2_atu_stats_get(chip, &occupancy);
- if (err) {
- dev_err(chip->dev, "failed to get ATU stats\n");
- goto unlock;
- }
-
- occupancy &= MV88E6XXX_G2_ATU_STATS_MASK;
-
-unlock:
- mv88e6xxx_reg_unlock(chip);
-
- return occupancy;
-}
-
-static u64 mv88e6xxx_devlink_atu_bin_0_get(void *priv)
-{
- struct mv88e6xxx_chip *chip = priv;
-
- return mv88e6xxx_devlink_atu_bin_get(chip,
- MV88E6XXX_G2_ATU_STATS_BIN_0);
-}
-
-static u64 mv88e6xxx_devlink_atu_bin_1_get(void *priv)
-{
- struct mv88e6xxx_chip *chip = priv;
-
- return mv88e6xxx_devlink_atu_bin_get(chip,
- MV88E6XXX_G2_ATU_STATS_BIN_1);
-}
-
-static u64 mv88e6xxx_devlink_atu_bin_2_get(void *priv)
-{
- struct mv88e6xxx_chip *chip = priv;
-
- return mv88e6xxx_devlink_atu_bin_get(chip,
- MV88E6XXX_G2_ATU_STATS_BIN_2);
-}
-
-static u64 mv88e6xxx_devlink_atu_bin_3_get(void *priv)
-{
- struct mv88e6xxx_chip *chip = priv;
-
- return mv88e6xxx_devlink_atu_bin_get(chip,
- MV88E6XXX_G2_ATU_STATS_BIN_3);
-}
-
-static u64 mv88e6xxx_devlink_atu_get(void *priv)
-{
- return mv88e6xxx_devlink_atu_bin_0_get(priv) +
- mv88e6xxx_devlink_atu_bin_1_get(priv) +
- mv88e6xxx_devlink_atu_bin_2_get(priv) +
- mv88e6xxx_devlink_atu_bin_3_get(priv);
-}
-
-static int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds)
-{
- struct devlink_resource_size_params size_params;
- struct mv88e6xxx_chip *chip = ds->priv;
- int err;
-
- devlink_resource_size_params_init(&size_params,
- mv88e6xxx_num_macs(chip),
- mv88e6xxx_num_macs(chip),
- 1, DEVLINK_RESOURCE_UNIT_ENTRY);
-
- err = dsa_devlink_resource_register(ds, "ATU",
- mv88e6xxx_num_macs(chip),
- MV88E6XXX_RESOURCE_ID_ATU,
- DEVLINK_RESOURCE_ID_PARENT_TOP,
- &size_params);
- if (err)
- goto out;
-
- devlink_resource_size_params_init(&size_params,
- mv88e6xxx_num_macs(chip) / 4,
- mv88e6xxx_num_macs(chip) / 4,
- 1, DEVLINK_RESOURCE_UNIT_ENTRY);
-
- err = dsa_devlink_resource_register(ds, "ATU_bin_0",
- mv88e6xxx_num_macs(chip) / 4,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
- MV88E6XXX_RESOURCE_ID_ATU,
- &size_params);
- if (err)
- goto out;
-
- err = dsa_devlink_resource_register(ds, "ATU_bin_1",
- mv88e6xxx_num_macs(chip) / 4,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
- MV88E6XXX_RESOURCE_ID_ATU,
- &size_params);
- if (err)
- goto out;
-
- err = dsa_devlink_resource_register(ds, "ATU_bin_2",
- mv88e6xxx_num_macs(chip) / 4,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
- MV88E6XXX_RESOURCE_ID_ATU,
- &size_params);
- if (err)
- goto out;
-
- err = dsa_devlink_resource_register(ds, "ATU_bin_3",
- mv88e6xxx_num_macs(chip) / 4,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
- MV88E6XXX_RESOURCE_ID_ATU,
- &size_params);
- if (err)
- goto out;
-
- dsa_devlink_resource_occ_get_register(ds,
- MV88E6XXX_RESOURCE_ID_ATU,
- mv88e6xxx_devlink_atu_get,
- chip);
-
- dsa_devlink_resource_occ_get_register(ds,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
- mv88e6xxx_devlink_atu_bin_0_get,
- chip);
-
- dsa_devlink_resource_occ_get_register(ds,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
- mv88e6xxx_devlink_atu_bin_1_get,
- chip);
-
- dsa_devlink_resource_occ_get_register(ds,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
- mv88e6xxx_devlink_atu_bin_2_get,
- chip);
-
- dsa_devlink_resource_occ_get_register(ds,
- MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
- mv88e6xxx_devlink_atu_bin_3_get,
- chip);
-
- return 0;
-
-out:
- dsa_devlink_resources_unregister(ds);
- return err;
-}
-
static void mv88e6xxx_teardown(struct dsa_switch *ds)
{
mv88e6xxx_teardown_devlink_params(ds);
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
new file mode 100644
index 000000000000..91e02024c5cf
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <net/dsa.h>
+
+#include "chip.h"
+#include "devlink.h"
+#include "global1.h"
+#include "global2.h"
+
+static int mv88e6xxx_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
+{
+ if (chip->info->ops->atu_get_hash)
+ return chip->info->ops->atu_get_hash(chip, hash);
+
+ return -EOPNOTSUPP;
+}
+
+static int mv88e6xxx_atu_set_hash(struct mv88e6xxx_chip *chip, u8 hash)
+{
+ if (chip->info->ops->atu_set_hash)
+ return chip->info->ops->atu_set_hash(chip, hash);
+
+ return -EOPNOTSUPP;
+}
+
+enum mv88e6xxx_devlink_param_id {
+ MV88E6XXX_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+ MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH,
+};
+
+int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+ int err;
+
+ mv88e6xxx_reg_lock(chip);
+
+ switch (id) {
+ case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
+ err = mv88e6xxx_atu_get_hash(chip, &ctx->val.vu8);
+ break;
+ default:
+ err = -EOPNOTSUPP;
+ break;
+ }
+
+ mv88e6xxx_reg_unlock(chip);
+
+ return err;
+}
+
+int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+ int err;
+
+ mv88e6xxx_reg_lock(chip);
+
+ switch (id) {
+ case MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH:
+ err = mv88e6xxx_atu_set_hash(chip, ctx->val.vu8);
+ break;
+ default:
+ err = -EOPNOTSUPP;
+ break;
+ }
+
+ mv88e6xxx_reg_unlock(chip);
+
+ return err;
+}
+
+static const struct devlink_param mv88e6xxx_devlink_params[] = {
+ DSA_DEVLINK_PARAM_DRIVER(MV88E6XXX_DEVLINK_PARAM_ID_ATU_HASH,
+ "ATU_hash", DEVLINK_PARAM_TYPE_U8,
+ BIT(DEVLINK_PARAM_CMODE_RUNTIME)),
+};
+
+int mv88e6xxx_setup_devlink_params(struct dsa_switch *ds)
+{
+ return dsa_devlink_params_register(ds, mv88e6xxx_devlink_params,
+ ARRAY_SIZE(mv88e6xxx_devlink_params));
+}
+
+void mv88e6xxx_teardown_devlink_params(struct dsa_switch *ds)
+{
+ dsa_devlink_params_unregister(ds, mv88e6xxx_devlink_params,
+ ARRAY_SIZE(mv88e6xxx_devlink_params));
+}
+
+enum mv88e6xxx_devlink_resource_id {
+ MV88E6XXX_RESOURCE_ID_ATU,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
+};
+
+static u64 mv88e6xxx_devlink_atu_bin_get(struct mv88e6xxx_chip *chip,
+ u16 bin)
+{
+ u16 occupancy = 0;
+ int err;
+
+ mv88e6xxx_reg_lock(chip);
+
+ err = mv88e6xxx_g2_atu_stats_set(chip, MV88E6XXX_G2_ATU_STATS_MODE_ALL,
+ bin);
+ if (err) {
+ dev_err(chip->dev, "failed to set ATU stats kind/bin\n");
+ goto unlock;
+ }
+
+ err = mv88e6xxx_g1_atu_get_next(chip, 0);
+ if (err) {
+ dev_err(chip->dev, "failed to perform ATU get next\n");
+ goto unlock;
+ }
+
+ err = mv88e6xxx_g2_atu_stats_get(chip, &occupancy);
+ if (err) {
+ dev_err(chip->dev, "failed to get ATU stats\n");
+ goto unlock;
+ }
+
+ occupancy &= MV88E6XXX_G2_ATU_STATS_MASK;
+
+unlock:
+ mv88e6xxx_reg_unlock(chip);
+
+ return occupancy;
+}
+
+static u64 mv88e6xxx_devlink_atu_bin_0_get(void *priv)
+{
+ struct mv88e6xxx_chip *chip = priv;
+
+ return mv88e6xxx_devlink_atu_bin_get(chip,
+ MV88E6XXX_G2_ATU_STATS_BIN_0);
+}
+
+static u64 mv88e6xxx_devlink_atu_bin_1_get(void *priv)
+{
+ struct mv88e6xxx_chip *chip = priv;
+
+ return mv88e6xxx_devlink_atu_bin_get(chip,
+ MV88E6XXX_G2_ATU_STATS_BIN_1);
+}
+
+static u64 mv88e6xxx_devlink_atu_bin_2_get(void *priv)
+{
+ struct mv88e6xxx_chip *chip = priv;
+
+ return mv88e6xxx_devlink_atu_bin_get(chip,
+ MV88E6XXX_G2_ATU_STATS_BIN_2);
+}
+
+static u64 mv88e6xxx_devlink_atu_bin_3_get(void *priv)
+{
+ struct mv88e6xxx_chip *chip = priv;
+
+ return mv88e6xxx_devlink_atu_bin_get(chip,
+ MV88E6XXX_G2_ATU_STATS_BIN_3);
+}
+
+static u64 mv88e6xxx_devlink_atu_get(void *priv)
+{
+ return mv88e6xxx_devlink_atu_bin_0_get(priv) +
+ mv88e6xxx_devlink_atu_bin_1_get(priv) +
+ mv88e6xxx_devlink_atu_bin_2_get(priv) +
+ mv88e6xxx_devlink_atu_bin_3_get(priv);
+}
+
+int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds)
+{
+ struct devlink_resource_size_params size_params;
+ struct mv88e6xxx_chip *chip = ds->priv;
+ int err;
+
+ devlink_resource_size_params_init(&size_params,
+ mv88e6xxx_num_macs(chip),
+ mv88e6xxx_num_macs(chip),
+ 1, DEVLINK_RESOURCE_UNIT_ENTRY);
+
+ err = dsa_devlink_resource_register(ds, "ATU",
+ mv88e6xxx_num_macs(chip),
+ MV88E6XXX_RESOURCE_ID_ATU,
+ DEVLINK_RESOURCE_ID_PARENT_TOP,
+ &size_params);
+ if (err)
+ goto out;
+
+ devlink_resource_size_params_init(&size_params,
+ mv88e6xxx_num_macs(chip) / 4,
+ mv88e6xxx_num_macs(chip) / 4,
+ 1, DEVLINK_RESOURCE_UNIT_ENTRY);
+
+ err = dsa_devlink_resource_register(ds, "ATU_bin_0",
+ mv88e6xxx_num_macs(chip) / 4,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
+ MV88E6XXX_RESOURCE_ID_ATU,
+ &size_params);
+ if (err)
+ goto out;
+
+ err = dsa_devlink_resource_register(ds, "ATU_bin_1",
+ mv88e6xxx_num_macs(chip) / 4,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
+ MV88E6XXX_RESOURCE_ID_ATU,
+ &size_params);
+ if (err)
+ goto out;
+
+ err = dsa_devlink_resource_register(ds, "ATU_bin_2",
+ mv88e6xxx_num_macs(chip) / 4,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
+ MV88E6XXX_RESOURCE_ID_ATU,
+ &size_params);
+ if (err)
+ goto out;
+
+ err = dsa_devlink_resource_register(ds, "ATU_bin_3",
+ mv88e6xxx_num_macs(chip) / 4,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
+ MV88E6XXX_RESOURCE_ID_ATU,
+ &size_params);
+ if (err)
+ goto out;
+
+ dsa_devlink_resource_occ_get_register(ds,
+ MV88E6XXX_RESOURCE_ID_ATU,
+ mv88e6xxx_devlink_atu_get,
+ chip);
+
+ dsa_devlink_resource_occ_get_register(ds,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_0,
+ mv88e6xxx_devlink_atu_bin_0_get,
+ chip);
+
+ dsa_devlink_resource_occ_get_register(ds,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_1,
+ mv88e6xxx_devlink_atu_bin_1_get,
+ chip);
+
+ dsa_devlink_resource_occ_get_register(ds,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_2,
+ mv88e6xxx_devlink_atu_bin_2_get,
+ chip);
+
+ dsa_devlink_resource_occ_get_register(ds,
+ MV88E6XXX_RESOURCE_ID_ATU_BIN_3,
+ mv88e6xxx_devlink_atu_bin_3_get,
+ chip);
+
+ return 0;
+
+out:
+ dsa_devlink_resources_unregister(ds);
+ return err;
+}
+
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.h b/drivers/net/dsa/mv88e6xxx/devlink.h
new file mode 100644
index 000000000000..f6254e049653
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/devlink.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/* Marvell 88E6xxx Switch devlink support. */
+
+#ifndef _MV88E6XXX_DEVLINK_H
+#define _MV88E6XXX_DEVLINK_H
+
+int mv88e6xxx_setup_devlink_params(struct dsa_switch *ds);
+void mv88e6xxx_teardown_devlink_params(struct dsa_switch *ds);
+int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds);
+int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+
+#endif /* _MV88E6XXX_DEVLINK_H */
--
2.28.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH net-next 4/7] net: dsa: mv88e6xxx: Create helper for FIDs in use
2020-08-16 19:43 [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Andrew Lunn
` (2 preceding siblings ...)
2020-08-16 19:43 ` [PATCH net-next 3/7] net: dsa: mv88e6xxx: Move devlink code into its own file Andrew Lunn
@ 2020-08-16 19:43 ` Andrew Lunn
2020-08-16 19:43 ` [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions Andrew Lunn
` (4 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 19:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Chris Healy, Vivien Didelot, Andrew Lunn
Refactor the code in mv88e6xxx_atu_new() which builds a bitmaps of
FIDs in use into a helper function. This will be reused by the devlink
code when dumping the ATU.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 20 ++++++++++++++++----
drivers/net/dsa/mv88e6xxx/chip.h | 2 ++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 940dd0c55568..2b3ddf2bfcea 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1466,21 +1466,21 @@ static int mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_chip *chip,
return chip->info->ops->vtu_loadpurge(chip, entry);
}
-static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
+int mv88e6xxx_fid_map(struct mv88e6xxx_chip *chip, unsigned long *fid_bitmap)
{
- DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
struct mv88e6xxx_vtu_entry vlan;
int i, err;
+ u16 fid;
bitmap_zero(fid_bitmap, MV88E6XXX_N_FID);
/* Set every FID bit used by the (un)bridged ports */
for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
- err = mv88e6xxx_port_get_fid(chip, i, fid);
+ err = mv88e6xxx_port_get_fid(chip, i, &fid);
if (err)
return err;
- set_bit(*fid, fid_bitmap);
+ set_bit(fid, fid_bitmap);
}
/* Set every FID bit used by the VLAN entries */
@@ -1498,6 +1498,18 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
set_bit(vlan.fid, fid_bitmap);
} while (vlan.vid < chip->info->max_vid);
+ return 0;
+}
+
+static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
+{
+ DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
+ int err;
+
+ err = mv88e6xxx_fid_map(chip, fid_bitmap);
+ if (err)
+ return err;
+
/* The reset value 0x000 is used to indicate that multiple address
* databases are not needed. Return the next positive available.
*/
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 823ae89e5fca..77d81aa99f37 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -689,4 +689,6 @@ static inline void mv88e6xxx_reg_unlock(struct mv88e6xxx_chip *chip)
mutex_unlock(&chip->reg_lock);
}
+int mv88e6xxx_fid_map(struct mv88e6xxx_chip *chip, unsigned long *bitmap);
+
#endif /* _MV88E6XXX_CHIP_H */
--
2.28.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions
2020-08-16 19:43 [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Andrew Lunn
` (3 preceding siblings ...)
2020-08-16 19:43 ` [PATCH net-next 4/7] net: dsa: mv88e6xxx: Create helper for FIDs in use Andrew Lunn
@ 2020-08-16 19:43 ` Andrew Lunn
2020-08-16 22:12 ` Vladimir Oltean
2020-08-16 19:43 ` [PATCH net-next 6/7] net: dsa: wire up devlink info get Andrew Lunn
` (3 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 19:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Chris Healy, Vivien Didelot, Andrew Lunn
Allow ports, the global registers, and the ATU to be snapshot via
devlink regions.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 14 +-
drivers/net/dsa/mv88e6xxx/chip.h | 12 +
drivers/net/dsa/mv88e6xxx/devlink.c | 413 ++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/devlink.h | 2 +
4 files changed, 440 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 2b3ddf2bfcea..33e4518736a2 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2838,6 +2838,7 @@ static void mv88e6xxx_teardown(struct dsa_switch *ds)
{
mv88e6xxx_teardown_devlink_params(ds);
dsa_devlink_resources_unregister(ds);
+ mv88e6xxx_teardown_devlink_regions(ds);
}
static int mv88e6xxx_setup(struct dsa_switch *ds)
@@ -2970,7 +2971,18 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
err = mv88e6xxx_setup_devlink_params(ds);
if (err)
- dsa_devlink_resources_unregister(ds);
+ goto out_resources;
+
+ err = mv88e6xxx_setup_devlink_regions(ds);
+ if (err)
+ goto out_params;
+
+ return 0;
+
+out_params:
+ mv88e6xxx_teardown_devlink_params(ds);
+out_resources:
+ dsa_devlink_resources_unregister(ds);
return err;
}
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 77d81aa99f37..d8bd211afcec 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -238,6 +238,15 @@ struct mv88e6xxx_port {
bool mirror_egress;
unsigned int serdes_irq;
char serdes_irq_name[64];
+ struct devlink_region *region;
+};
+
+enum mv88e6xxx_region_id {
+ MV88E6XXX_REGION_GLOBAL1 = 0,
+ MV88E6XXX_REGION_GLOBAL2,
+ MV88E6XXX_REGION_ATU,
+
+ _MV88E6XXX_REGION_MAX,
};
struct mv88e6xxx_chip {
@@ -334,6 +343,9 @@ struct mv88e6xxx_chip {
/* Array of port structures. */
struct mv88e6xxx_port ports[DSA_MAX_PORTS];
+
+ /* devlink regions */
+ struct devlink_region *regions[_MV88E6XXX_REGION_MAX];
};
struct mv88e6xxx_bus_ops {
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
index 91e02024c5cf..c6ebadcfa63f 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -5,6 +5,7 @@
#include "devlink.h"
#include "global1.h"
#include "global2.h"
+#include "port.h"
static int mv88e6xxx_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
{
@@ -33,6 +34,8 @@ int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
struct mv88e6xxx_chip *chip = ds->priv;
int err;
+ dev_info(ds->dev, "%s: enter\n", __func__);
+
mv88e6xxx_reg_lock(chip);
switch (id) {
@@ -55,6 +58,8 @@ int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
struct mv88e6xxx_chip *chip = ds->priv;
int err;
+ dev_info(ds->dev, "%s: enter\n", __func__);
+
mv88e6xxx_reg_lock(chip);
switch (id) {
@@ -260,3 +265,411 @@ int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds)
return err;
}
+static int mv88e6xxx_region_port_snapshot(struct devlink *dl,
+ struct netlink_ext_ack *extack,
+ u8 **data,
+ int port)
+{
+ struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+ struct mv88e6xxx_chip *chip = ds->priv;
+ u16 *registers;
+ int i, err;
+
+ registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
+ if (!registers)
+ return -ENOMEM;
+
+ mv88e6xxx_reg_lock(chip);
+ for (i = 0; i < 32; i++) {
+ err = mv88e6xxx_port_read(chip, port, i, ®isters[i]);
+ if (err) {
+ kfree(registers);
+ goto out;
+ }
+ }
+ *data = (u8 *)registers;
+out:
+ mv88e6xxx_reg_unlock(chip);
+
+ return err;
+}
+
+#define PORT_SNAPSHOT(_X_) \
+static int mv88e6xxx_region_port_ ## _X_ ## _snapshot( \
+ struct devlink *dl, \
+ struct netlink_ext_ack *extack, \
+ u8 **data) \
+{ \
+ return mv88e6xxx_region_port_snapshot(dl, extack, data, _X_); \
+}
+
+PORT_SNAPSHOT(0);
+PORT_SNAPSHOT(1);
+PORT_SNAPSHOT(2);
+PORT_SNAPSHOT(3);
+PORT_SNAPSHOT(4);
+PORT_SNAPSHOT(5);
+PORT_SNAPSHOT(6);
+PORT_SNAPSHOT(7);
+PORT_SNAPSHOT(8);
+PORT_SNAPSHOT(9);
+PORT_SNAPSHOT(10);
+PORT_SNAPSHOT(11);
+
+#define PORT_REGION_OPS(_X_) \
+static struct devlink_region_ops mv88e6xxx_region_port_ ## _X_ ## _ops = { \
+ .name = "port" #_X_, \
+ .snapshot = mv88e6xxx_region_port_ ## _X_ ## _snapshot, \
+ .destructor = kfree, \
+}
+
+PORT_REGION_OPS(0);
+PORT_REGION_OPS(1);
+PORT_REGION_OPS(2);
+PORT_REGION_OPS(3);
+PORT_REGION_OPS(4);
+PORT_REGION_OPS(5);
+PORT_REGION_OPS(6);
+PORT_REGION_OPS(7);
+PORT_REGION_OPS(8);
+PORT_REGION_OPS(9);
+PORT_REGION_OPS(10);
+PORT_REGION_OPS(11);
+
+static const struct devlink_region_ops *mv88e6xxx_region_port_ops[] = {
+ &mv88e6xxx_region_port_0_ops,
+ &mv88e6xxx_region_port_1_ops,
+ &mv88e6xxx_region_port_2_ops,
+ &mv88e6xxx_region_port_3_ops,
+ &mv88e6xxx_region_port_4_ops,
+ &mv88e6xxx_region_port_5_ops,
+ &mv88e6xxx_region_port_6_ops,
+ &mv88e6xxx_region_port_7_ops,
+ &mv88e6xxx_region_port_8_ops,
+ &mv88e6xxx_region_port_9_ops,
+ &mv88e6xxx_region_port_10_ops,
+ &mv88e6xxx_region_port_11_ops,
+};
+
+static int mv88e6xxx_region_global1_snapshot(struct devlink *dl,
+ struct netlink_ext_ack *extack,
+ u8 **data)
+{
+ struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+ struct mv88e6xxx_chip *chip = ds->priv;
+ u16 *registers;
+ int i, err;
+
+ registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
+ if (!registers)
+ return -ENOMEM;
+
+ mv88e6xxx_reg_lock(chip);
+ for (i = 0; i < 32; i++) {
+ err = mv88e6xxx_g1_read(chip, i, ®isters[i]);
+ if (err) {
+ kfree(registers);
+ goto out;
+ }
+ }
+ *data = (u8 *)registers;
+out:
+ mv88e6xxx_reg_unlock(chip);
+
+ return err;
+}
+
+static int mv88e6xxx_region_global2_snapshot(struct devlink *dl,
+ struct netlink_ext_ack *extack,
+ u8 **data)
+{
+ struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+ struct mv88e6xxx_chip *chip = ds->priv;
+ u16 *registers;
+ int i, err;
+
+ registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
+ if (!registers)
+ return -ENOMEM;
+
+ mv88e6xxx_reg_lock(chip);
+ for (i = 0; i < 32; i++) {
+ err = mv88e6xxx_g2_read(chip, i, ®isters[i]);
+ if (err) {
+ kfree(registers);
+ goto out;
+ }
+ }
+ *data = (u8 *)registers;
+out:
+ mv88e6xxx_reg_unlock(chip);
+
+ return err;
+}
+
+/* The ATU entry varies between chipset generations. Define a generic
+ * format which covers all the current and hopefully future
+ * generations
+ */
+
+struct mv88e6xxx_devlink_atu_entry {
+ /* The FID is scattered over multiple registers. */
+ u16 fid;
+ u16 atu_op;
+ u16 atu_data;
+ u16 atu_01;
+ u16 atu_23;
+ u16 atu_45;
+};
+
+static int mv88e6xxx_region_atu_snapshot_fid(struct mv88e6xxx_chip *chip,
+ int fid,
+ struct mv88e6xxx_devlink_atu_entry *table,
+ int *count)
+{
+ u16 atu_op, atu_data, atu_01, atu_23, atu_45;
+ struct mv88e6xxx_atu_entry addr;
+ int err;
+
+ addr.state = 0;
+ eth_broadcast_addr(addr.mac);
+
+ do {
+ err = mv88e6xxx_g1_atu_getnext(chip, fid, &addr);
+ if (err)
+ return err;
+
+ if (!addr.state)
+ break;
+
+ err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_OP, &atu_op);
+ if (err)
+ return err;
+
+ err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_DATA, &atu_data);
+ if (err)
+ return err;
+
+ err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC01, &atu_01);
+ if (err)
+ return err;
+
+ err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC23, &atu_23);
+ if (err)
+ return err;
+
+ err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC45, &atu_45);
+ if (err)
+ return err;
+
+ table[*count].fid = fid;
+ table[*count].atu_op = atu_op;
+ table[*count].atu_data = atu_data;
+ table[*count].atu_01 = atu_01;
+ table[*count].atu_23 = atu_23;
+ table[*count].atu_45 = atu_45;
+ (*count)++;
+ } while (!is_broadcast_ether_addr(addr.mac));
+
+ return 0;
+}
+
+static int mv88e6xxx_region_atu_snapshot(struct devlink *dl,
+ struct netlink_ext_ack *extack,
+ u8 **data)
+{
+ struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+ DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
+ struct mv88e6xxx_devlink_atu_entry *table;
+ struct mv88e6xxx_chip *chip = ds->priv;
+ int fid = -1, count, err;
+
+ table = kmalloc_array(mv88e6xxx_num_databases(chip),
+ sizeof(struct mv88e6xxx_devlink_atu_entry),
+ GFP_KERNEL);
+ if (!table)
+ return -ENOMEM;
+
+ memset(table, 0, mv88e6xxx_num_databases(chip) *
+ sizeof(struct mv88e6xxx_devlink_atu_entry));
+
+ count = 0;
+
+ mv88e6xxx_reg_lock(chip);
+
+ err = mv88e6xxx_fid_map(chip, fid_bitmap);
+ if (err)
+ goto out;
+
+ while (1) {
+ fid = find_next_bit(fid_bitmap, MV88E6XXX_N_FID, fid + 1);
+ if (fid == MV88E6XXX_N_FID)
+ break;
+
+ err = mv88e6xxx_region_atu_snapshot_fid(chip, fid, table,
+ &count);
+ if (err) {
+ kfree(table);
+ goto out;
+ }
+ }
+ *data = (u8 *)table;
+out:
+ mv88e6xxx_reg_unlock(chip);
+
+ return err;
+}
+
+static struct devlink_region_ops mv88e6xxx_region_global1_ops = {
+ .name = "global1",
+ .snapshot = mv88e6xxx_region_global1_snapshot,
+ .destructor = kfree,
+};
+
+static struct devlink_region_ops mv88e6xxx_region_global2_ops = {
+ .name = "global2",
+ .snapshot = mv88e6xxx_region_global2_snapshot,
+ .destructor = kfree,
+};
+
+static struct devlink_region_ops mv88e6xxx_region_atu_ops = {
+ .name = "atu",
+ .snapshot = mv88e6xxx_region_atu_snapshot,
+ .destructor = kfree,
+};
+
+struct mv88e6xxx_region {
+ struct devlink_region_ops *ops;
+ u64 size;
+};
+
+static struct mv88e6xxx_region mv88e6xxx_regions[] = {
+ [MV88E6XXX_REGION_GLOBAL1] = {
+ .ops = &mv88e6xxx_region_global1_ops,
+ .size = 32 * sizeof(u16)
+ },
+ [MV88E6XXX_REGION_GLOBAL2] = {
+ .ops = &mv88e6xxx_region_global2_ops,
+ .size = 32 * sizeof(u16) },
+ [MV88E6XXX_REGION_ATU] = {
+ .ops = &mv88e6xxx_region_atu_ops
+ /* calculated at runtime */
+ },
+};
+
+static void
+mv88e6xxx_teardown_devlink_regions_port(struct mv88e6xxx_chip *chip, int port)
+{
+ dsa_devlink_region_destroy(chip->ports[port].region);
+}
+
+static void
+mv88e6xxx_teardown_devlink_regions_ports(struct mv88e6xxx_chip *chip)
+{
+ int port;
+
+ for (port = 0; port < mv88e6xxx_num_ports(chip); port++)
+ mv88e6xxx_teardown_devlink_regions_port(chip, port);
+}
+
+static void
+mv88e6xxx_teardown_devlink_regions_global(struct mv88e6xxx_chip *chip)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(mv88e6xxx_regions); i++)
+ dsa_devlink_region_destroy(chip->regions[i]);
+}
+
+void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+
+ mv88e6xxx_teardown_devlink_regions_ports(chip);
+ mv88e6xxx_teardown_devlink_regions_global(chip);
+}
+
+static int mv88e6xxx_setup_devlink_regions_port(struct dsa_switch *ds,
+ struct mv88e6xxx_chip *chip,
+ int port)
+{
+ struct devlink_region *region;
+
+ region = dsa_devlink_region_create(ds,
+ mv88e6xxx_region_port_ops[port], 1,
+ 32 * sizeof(u16));
+ if (IS_ERR(region))
+ return PTR_ERR(region);
+
+ chip->ports[port].region = region;
+ return 0;
+}
+
+static int mv88e6xxx_setup_devlink_regions_ports(struct dsa_switch *ds,
+ struct mv88e6xxx_chip *chip)
+{
+ int port, port_err;
+ int err;
+
+ for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
+ err = mv88e6xxx_setup_devlink_regions_port(ds, chip, port);
+ if (err)
+ goto out;
+ }
+ return 0;
+
+out:
+ for (port_err = 0; port_err < port; port_err++)
+ mv88e6xxx_teardown_devlink_regions_port(chip, port_err);
+
+ return err;
+}
+
+static int mv88e6xxx_setup_devlink_regions_global(struct dsa_switch *ds,
+ struct mv88e6xxx_chip *chip)
+{
+ struct devlink_region_ops *ops;
+ struct devlink_region *region;
+ u64 size;
+ int i, j;
+
+ for (i = 0; i < ARRAY_SIZE(mv88e6xxx_regions); i++) {
+ ops = mv88e6xxx_regions[i].ops;
+ size = mv88e6xxx_regions[i].size;
+
+ if (i == MV88E6XXX_REGION_ATU)
+ size = mv88e6xxx_num_databases(chip) *
+ sizeof(struct mv88e6xxx_devlink_atu_entry);
+
+ region = dsa_devlink_region_create(ds, ops, 1, size);
+ if (IS_ERR(region))
+ goto out;
+ chip->regions[i] = region;
+ }
+ return 0;
+
+out:
+ for (j = 0; j < i; j++)
+ dsa_devlink_region_destroy(chip->regions[j]);
+
+ return PTR_ERR(region);
+}
+
+int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+ int err;
+
+ err = mv88e6xxx_setup_devlink_regions_ports(ds, chip);
+ if (err)
+ return err;
+
+ err = mv88e6xxx_setup_devlink_regions_global(ds, chip);
+ if (err) {
+ mv88e6xxx_teardown_devlink_regions_ports(chip);
+ return err;
+ }
+
+ return 0;
+}
+
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.h b/drivers/net/dsa/mv88e6xxx/devlink.h
index f6254e049653..da83c25d944b 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.h
+++ b/drivers/net/dsa/mv88e6xxx/devlink.h
@@ -12,5 +12,7 @@ int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
struct devlink_param_gset_ctx *ctx);
int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
struct devlink_param_gset_ctx *ctx);
+int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds);
+void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds);
#endif /* _MV88E6XXX_DEVLINK_H */
--
2.28.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions
2020-08-16 19:43 ` [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions Andrew Lunn
@ 2020-08-16 22:12 ` Vladimir Oltean
2020-08-16 22:39 ` Andrew Lunn
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-08-16 22:12 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, Chris Healy, Vivien Didelot
On Sun, Aug 16, 2020 at 09:43:14PM +0200, Andrew Lunn wrote:
> Allow ports, the global registers, and the ATU to be snapshot via
> devlink regions.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> drivers/net/dsa/mv88e6xxx/chip.c | 14 +-
> drivers/net/dsa/mv88e6xxx/chip.h | 12 +
> drivers/net/dsa/mv88e6xxx/devlink.c | 413 ++++++++++++++++++++++++++++
> drivers/net/dsa/mv88e6xxx/devlink.h | 2 +
> 4 files changed, 440 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 2b3ddf2bfcea..33e4518736a2 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2838,6 +2838,7 @@ static void mv88e6xxx_teardown(struct dsa_switch *ds)
> {
> mv88e6xxx_teardown_devlink_params(ds);
> dsa_devlink_resources_unregister(ds);
> + mv88e6xxx_teardown_devlink_regions(ds);
> }
>
> static int mv88e6xxx_setup(struct dsa_switch *ds)
> @@ -2970,7 +2971,18 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
>
> err = mv88e6xxx_setup_devlink_params(ds);
> if (err)
> - dsa_devlink_resources_unregister(ds);
> + goto out_resources;
> +
> + err = mv88e6xxx_setup_devlink_regions(ds);
> + if (err)
> + goto out_params;
> +
> + return 0;
> +
> +out_params:
> + mv88e6xxx_teardown_devlink_params(ds);
> +out_resources:
> + dsa_devlink_resources_unregister(ds);
>
> return err;
> }
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
> index 77d81aa99f37..d8bd211afcec 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -238,6 +238,15 @@ struct mv88e6xxx_port {
> bool mirror_egress;
> unsigned int serdes_irq;
> char serdes_irq_name[64];
> + struct devlink_region *region;
> +};
> +
> +enum mv88e6xxx_region_id {
> + MV88E6XXX_REGION_GLOBAL1 = 0,
> + MV88E6XXX_REGION_GLOBAL2,
> + MV88E6XXX_REGION_ATU,
> +
> + _MV88E6XXX_REGION_MAX,
> };
>
> struct mv88e6xxx_chip {
> @@ -334,6 +343,9 @@ struct mv88e6xxx_chip {
>
> /* Array of port structures. */
> struct mv88e6xxx_port ports[DSA_MAX_PORTS];
> +
> + /* devlink regions */
> + struct devlink_region *regions[_MV88E6XXX_REGION_MAX];
> };
>
> struct mv88e6xxx_bus_ops {
> diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
> index 91e02024c5cf..c6ebadcfa63f 100644
> --- a/drivers/net/dsa/mv88e6xxx/devlink.c
> +++ b/drivers/net/dsa/mv88e6xxx/devlink.c
> @@ -5,6 +5,7 @@
> #include "devlink.h"
> #include "global1.h"
> #include "global2.h"
> +#include "port.h"
>
> static int mv88e6xxx_atu_get_hash(struct mv88e6xxx_chip *chip, u8 *hash)
> {
> @@ -33,6 +34,8 @@ int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
> struct mv88e6xxx_chip *chip = ds->priv;
> int err;
>
> + dev_info(ds->dev, "%s: enter\n", __func__);
Debugging leftovers (although it's curious that this patch is not about devlink
params...).
> +
> mv88e6xxx_reg_lock(chip);
>
> switch (id) {
> @@ -55,6 +58,8 @@ int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
> struct mv88e6xxx_chip *chip = ds->priv;
> int err;
>
> + dev_info(ds->dev, "%s: enter\n", __func__);
Likewise.
> +
> mv88e6xxx_reg_lock(chip);
>
> switch (id) {
> @@ -260,3 +265,411 @@ int mv88e6xxx_setup_devlink_resources(struct dsa_switch *ds)
> return err;
> }
>
> +static int mv88e6xxx_region_port_snapshot(struct devlink *dl,
> + struct netlink_ext_ack *extack,
> + u8 **data,
> + int port)
> +{
> + struct dsa_switch *ds = dsa_devlink_to_ds(dl);
> + struct mv88e6xxx_chip *chip = ds->priv;
> + u16 *registers;
> + int i, err;
> +
> + registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
> + if (!registers)
> + return -ENOMEM;
> +
> + mv88e6xxx_reg_lock(chip);
> + for (i = 0; i < 32; i++) {
> + err = mv88e6xxx_port_read(chip, port, i, ®isters[i]);
> + if (err) {
> + kfree(registers);
> + goto out;
> + }
> + }
> + *data = (u8 *)registers;
> +out:
> + mv88e6xxx_reg_unlock(chip);
> +
> + return err;
> +}
> +
> +#define PORT_SNAPSHOT(_X_) \
> +static int mv88e6xxx_region_port_ ## _X_ ## _snapshot( \
> + struct devlink *dl, \
> + struct netlink_ext_ack *extack, \
> + u8 **data) \
> +{ \
> + return mv88e6xxx_region_port_snapshot(dl, extack, data, _X_); \
> +}
> +
> +PORT_SNAPSHOT(0);
> +PORT_SNAPSHOT(1);
> +PORT_SNAPSHOT(2);
> +PORT_SNAPSHOT(3);
> +PORT_SNAPSHOT(4);
> +PORT_SNAPSHOT(5);
> +PORT_SNAPSHOT(6);
> +PORT_SNAPSHOT(7);
> +PORT_SNAPSHOT(8);
> +PORT_SNAPSHOT(9);
> +PORT_SNAPSHOT(10);
> +PORT_SNAPSHOT(11);
> +
> +#define PORT_REGION_OPS(_X_) \
> +static struct devlink_region_ops mv88e6xxx_region_port_ ## _X_ ## _ops = { \
> + .name = "port" #_X_, \
> + .snapshot = mv88e6xxx_region_port_ ## _X_ ## _snapshot, \
> + .destructor = kfree, \
> +}
> +
> +PORT_REGION_OPS(0);
> +PORT_REGION_OPS(1);
> +PORT_REGION_OPS(2);
> +PORT_REGION_OPS(3);
> +PORT_REGION_OPS(4);
> +PORT_REGION_OPS(5);
> +PORT_REGION_OPS(6);
> +PORT_REGION_OPS(7);
> +PORT_REGION_OPS(8);
> +PORT_REGION_OPS(9);
> +PORT_REGION_OPS(10);
> +PORT_REGION_OPS(11);
> +
> +static const struct devlink_region_ops *mv88e6xxx_region_port_ops[] = {
> + &mv88e6xxx_region_port_0_ops,
> + &mv88e6xxx_region_port_1_ops,
> + &mv88e6xxx_region_port_2_ops,
> + &mv88e6xxx_region_port_3_ops,
> + &mv88e6xxx_region_port_4_ops,
> + &mv88e6xxx_region_port_5_ops,
> + &mv88e6xxx_region_port_6_ops,
> + &mv88e6xxx_region_port_7_ops,
> + &mv88e6xxx_region_port_8_ops,
> + &mv88e6xxx_region_port_9_ops,
> + &mv88e6xxx_region_port_10_ops,
> + &mv88e6xxx_region_port_11_ops,
> +};
> +
Sounds like there should maybe be an abstraction for 'per-port regions' in
devlink? I think your approach hardly scales if you start having
switches with more than 11 ports.
> +static int mv88e6xxx_region_global1_snapshot(struct devlink *dl,
> + struct netlink_ext_ack *extack,
> + u8 **data)
> +{
> + struct dsa_switch *ds = dsa_devlink_to_ds(dl);
> + struct mv88e6xxx_chip *chip = ds->priv;
> + u16 *registers;
> + int i, err;
> +
> + registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
> + if (!registers)
> + return -ENOMEM;
> +
> + mv88e6xxx_reg_lock(chip);
> + for (i = 0; i < 32; i++) {
> + err = mv88e6xxx_g1_read(chip, i, ®isters[i]);
> + if (err) {
> + kfree(registers);
> + goto out;
> + }
> + }
> + *data = (u8 *)registers;
> +out:
> + mv88e6xxx_reg_unlock(chip);
> +
> + return err;
> +}
> +
> +static int mv88e6xxx_region_global2_snapshot(struct devlink *dl,
> + struct netlink_ext_ack *extack,
> + u8 **data)
> +{
> + struct dsa_switch *ds = dsa_devlink_to_ds(dl);
> + struct mv88e6xxx_chip *chip = ds->priv;
> + u16 *registers;
> + int i, err;
> +
> + registers = kmalloc_array(32, sizeof(u16), GFP_KERNEL);
> + if (!registers)
> + return -ENOMEM;
> +
> + mv88e6xxx_reg_lock(chip);
> + for (i = 0; i < 32; i++) {
> + err = mv88e6xxx_g2_read(chip, i, ®isters[i]);
> + if (err) {
> + kfree(registers);
> + goto out;
> + }
> + }
> + *data = (u8 *)registers;
> +out:
> + mv88e6xxx_reg_unlock(chip);
> +
> + return err;
> +}
> +
> +/* The ATU entry varies between chipset generations. Define a generic
> + * format which covers all the current and hopefully future
> + * generations
> + */
Could you please present this generic format to us? Maybe my interpretation of
the word "generic" is incorrect in this context? Is it even desirable to expose
regions like the ATU in a really generic (cross-vendor) fashion?
> +
> +struct mv88e6xxx_devlink_atu_entry {
> + /* The FID is scattered over multiple registers. */
> + u16 fid;
> + u16 atu_op;
> + u16 atu_data;
> + u16 atu_01;
> + u16 atu_23;
> + u16 atu_45;
> +};
> +
> +static int mv88e6xxx_region_atu_snapshot_fid(struct mv88e6xxx_chip *chip,
> + int fid,
> + struct mv88e6xxx_devlink_atu_entry *table,
> + int *count)
> +{
> + u16 atu_op, atu_data, atu_01, atu_23, atu_45;
> + struct mv88e6xxx_atu_entry addr;
> + int err;
> +
> + addr.state = 0;
> + eth_broadcast_addr(addr.mac);
> +
> + do {
> + err = mv88e6xxx_g1_atu_getnext(chip, fid, &addr);
> + if (err)
> + return err;
> +
> + if (!addr.state)
> + break;
> +
> + err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_OP, &atu_op);
> + if (err)
> + return err;
> +
> + err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_DATA, &atu_data);
> + if (err)
> + return err;
> +
> + err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC01, &atu_01);
> + if (err)
> + return err;
> +
> + err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC23, &atu_23);
> + if (err)
> + return err;
> +
> + err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_MAC45, &atu_45);
> + if (err)
> + return err;
> +
> + table[*count].fid = fid;
> + table[*count].atu_op = atu_op;
> + table[*count].atu_data = atu_data;
> + table[*count].atu_01 = atu_01;
> + table[*count].atu_23 = atu_23;
> + table[*count].atu_45 = atu_45;
> + (*count)++;
> + } while (!is_broadcast_ether_addr(addr.mac));
> +
> + return 0;
> +}
> +
> +static int mv88e6xxx_region_atu_snapshot(struct devlink *dl,
> + struct netlink_ext_ack *extack,
> + u8 **data)
> +{
> + struct dsa_switch *ds = dsa_devlink_to_ds(dl);
> + DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
> + struct mv88e6xxx_devlink_atu_entry *table;
> + struct mv88e6xxx_chip *chip = ds->priv;
> + int fid = -1, count, err;
> +
> + table = kmalloc_array(mv88e6xxx_num_databases(chip),
> + sizeof(struct mv88e6xxx_devlink_atu_entry),
> + GFP_KERNEL);
> + if (!table)
> + return -ENOMEM;
> +
> + memset(table, 0, mv88e6xxx_num_databases(chip) *
> + sizeof(struct mv88e6xxx_devlink_atu_entry));
> +
> + count = 0;
> +
> + mv88e6xxx_reg_lock(chip);
> +
> + err = mv88e6xxx_fid_map(chip, fid_bitmap);
> + if (err)
> + goto out;
> +
> + while (1) {
> + fid = find_next_bit(fid_bitmap, MV88E6XXX_N_FID, fid + 1);
> + if (fid == MV88E6XXX_N_FID)
> + break;
> +
> + err = mv88e6xxx_region_atu_snapshot_fid(chip, fid, table,
> + &count);
> + if (err) {
> + kfree(table);
> + goto out;
> + }
> + }
> + *data = (u8 *)table;
> +out:
> + mv88e6xxx_reg_unlock(chip);
> +
> + return err;
> +}
> +
> +static struct devlink_region_ops mv88e6xxx_region_global1_ops = {
> + .name = "global1",
> + .snapshot = mv88e6xxx_region_global1_snapshot,
> + .destructor = kfree,
> +};
> +
> +static struct devlink_region_ops mv88e6xxx_region_global2_ops = {
> + .name = "global2",
> + .snapshot = mv88e6xxx_region_global2_snapshot,
> + .destructor = kfree,
> +};
> +
> +static struct devlink_region_ops mv88e6xxx_region_atu_ops = {
> + .name = "atu",
> + .snapshot = mv88e6xxx_region_atu_snapshot,
> + .destructor = kfree,
> +};
> +
> +struct mv88e6xxx_region {
> + struct devlink_region_ops *ops;
> + u64 size;
> +};
> +
> +static struct mv88e6xxx_region mv88e6xxx_regions[] = {
> + [MV88E6XXX_REGION_GLOBAL1] = {
> + .ops = &mv88e6xxx_region_global1_ops,
> + .size = 32 * sizeof(u16)
> + },
> + [MV88E6XXX_REGION_GLOBAL2] = {
> + .ops = &mv88e6xxx_region_global2_ops,
> + .size = 32 * sizeof(u16) },
> + [MV88E6XXX_REGION_ATU] = {
> + .ops = &mv88e6xxx_region_atu_ops
> + /* calculated at runtime */
> + },
> +};
> +
> +static void
> +mv88e6xxx_teardown_devlink_regions_port(struct mv88e6xxx_chip *chip, int port)
> +{
> + dsa_devlink_region_destroy(chip->ports[port].region);
> +}
> +
> +static void
> +mv88e6xxx_teardown_devlink_regions_ports(struct mv88e6xxx_chip *chip)
> +{
> + int port;
> +
> + for (port = 0; port < mv88e6xxx_num_ports(chip); port++)
> + mv88e6xxx_teardown_devlink_regions_port(chip, port);
> +}
> +
> +static void
> +mv88e6xxx_teardown_devlink_regions_global(struct mv88e6xxx_chip *chip)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(mv88e6xxx_regions); i++)
> + dsa_devlink_region_destroy(chip->regions[i]);
> +}
> +
> +void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds)
> +{
> + struct mv88e6xxx_chip *chip = ds->priv;
> +
> + mv88e6xxx_teardown_devlink_regions_ports(chip);
> + mv88e6xxx_teardown_devlink_regions_global(chip);
> +}
> +
> +static int mv88e6xxx_setup_devlink_regions_port(struct dsa_switch *ds,
> + struct mv88e6xxx_chip *chip,
> + int port)
> +{
> + struct devlink_region *region;
> +
> + region = dsa_devlink_region_create(ds,
> + mv88e6xxx_region_port_ops[port], 1,
> + 32 * sizeof(u16));
> + if (IS_ERR(region))
> + return PTR_ERR(region);
> +
> + chip->ports[port].region = region;
> + return 0;
> +}
> +
> +static int mv88e6xxx_setup_devlink_regions_ports(struct dsa_switch *ds,
> + struct mv88e6xxx_chip *chip)
> +{
> + int port, port_err;
> + int err;
> +
> + for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
> + err = mv88e6xxx_setup_devlink_regions_port(ds, chip, port);
> + if (err)
> + goto out;
> + }
> + return 0;
> +
> +out:
> + for (port_err = 0; port_err < port; port_err++)
> + mv88e6xxx_teardown_devlink_regions_port(chip, port_err);
> +
> + return err;
> +}
> +
> +static int mv88e6xxx_setup_devlink_regions_global(struct dsa_switch *ds,
> + struct mv88e6xxx_chip *chip)
> +{
> + struct devlink_region_ops *ops;
> + struct devlink_region *region;
> + u64 size;
> + int i, j;
> +
> + for (i = 0; i < ARRAY_SIZE(mv88e6xxx_regions); i++) {
> + ops = mv88e6xxx_regions[i].ops;
> + size = mv88e6xxx_regions[i].size;
> +
> + if (i == MV88E6XXX_REGION_ATU)
> + size = mv88e6xxx_num_databases(chip) *
> + sizeof(struct mv88e6xxx_devlink_atu_entry);
> +
> + region = dsa_devlink_region_create(ds, ops, 1, size);
> + if (IS_ERR(region))
> + goto out;
> + chip->regions[i] = region;
> + }
> + return 0;
> +
> +out:
> + for (j = 0; j < i; j++)
> + dsa_devlink_region_destroy(chip->regions[j]);
> +
> + return PTR_ERR(region);
> +}
> +
> +int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds)
> +{
> + struct mv88e6xxx_chip *chip = ds->priv;
> + int err;
> +
> + err = mv88e6xxx_setup_devlink_regions_ports(ds, chip);
> + if (err)
> + return err;
> +
> + err = mv88e6xxx_setup_devlink_regions_global(ds, chip);
> + if (err) {
> + mv88e6xxx_teardown_devlink_regions_ports(chip);
> + return err;
> + }
> +
> + return 0;
> +}
> +
> diff --git a/drivers/net/dsa/mv88e6xxx/devlink.h b/drivers/net/dsa/mv88e6xxx/devlink.h
> index f6254e049653..da83c25d944b 100644
> --- a/drivers/net/dsa/mv88e6xxx/devlink.h
> +++ b/drivers/net/dsa/mv88e6xxx/devlink.h
> @@ -12,5 +12,7 @@ int mv88e6xxx_devlink_param_get(struct dsa_switch *ds, u32 id,
> struct devlink_param_gset_ctx *ctx);
> int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
> struct devlink_param_gset_ctx *ctx);
> +int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds);
> +void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds);
>
> #endif /* _MV88E6XXX_DEVLINK_H */
> --
> 2.28.0
>
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions
2020-08-16 22:12 ` Vladimir Oltean
@ 2020-08-16 22:39 ` Andrew Lunn
2020-08-17 17:15 ` Florian Fainelli
0 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 22:39 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: David Miller, netdev, Chris Healy, Vivien Didelot
> > +static const struct devlink_region_ops *mv88e6xxx_region_port_ops[] = {
> > + &mv88e6xxx_region_port_0_ops,
> > + &mv88e6xxx_region_port_1_ops,
> > + &mv88e6xxx_region_port_2_ops,
> > + &mv88e6xxx_region_port_3_ops,
> > + &mv88e6xxx_region_port_4_ops,
> > + &mv88e6xxx_region_port_5_ops,
> > + &mv88e6xxx_region_port_6_ops,
> > + &mv88e6xxx_region_port_7_ops,
> > + &mv88e6xxx_region_port_8_ops,
> > + &mv88e6xxx_region_port_9_ops,
> > + &mv88e6xxx_region_port_10_ops,
> > + &mv88e6xxx_region_port_11_ops,
> > +};
> > +
>
> Sounds like there should maybe be an abstraction for 'per-port regions' in
> devlink? I think your approach hardly scales if you start having
> switches with more than 11 ports.
mv88e6xxx is unlikely to have more an 11 ports. Marvell had to move
bits around in registers in non-compatible ways to support the 6390
family with this number of ports. I doubt we will ever see a 16 port
mv88e6xxx switch, the registers are just too congested.
So this scales as far as it needs to scale.
> > +/* The ATU entry varies between chipset generations. Define a generic
> > + * format which covers all the current and hopefully future
> > + * generations
> > + */
>
> Could you please present this generic format to us? Maybe my interpretation of
> the word "generic" is incorrect in this context?
I mean generic across all mv88e6xxx switches. The fid has been slowly
getting bigger from generation to generation. If i remember correctly,
it start off as 6 bits. 2 more bits we added, in a different
register. Then it got moved into a new register and made 14 bits in
size. There are also some bits in the atu_op register which changed
meaning over time.
In order to decode any of this information in the regions, you need to
known the specific switch the dump came from. But that is the whole
point of regions.
https://www.kernel.org/doc/html/latest/networking/devlink/devlink-region.html
As regions are likely very device or driver specific, no generic
regions are defined. See the driver-specific documentation files
for information on the specific regions a driver supports.
This should also make the context of 'generic' more clear.
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions
2020-08-16 22:39 ` Andrew Lunn
@ 2020-08-17 17:15 ` Florian Fainelli
2020-08-17 19:02 ` Andrew Lunn
0 siblings, 1 reply; 22+ messages in thread
From: Florian Fainelli @ 2020-08-17 17:15 UTC (permalink / raw)
To: Andrew Lunn, Vladimir Oltean
Cc: David Miller, netdev, Chris Healy, Vivien Didelot
On 8/16/20 3:39 PM, Andrew Lunn wrote:
>>> +static const struct devlink_region_ops *mv88e6xxx_region_port_ops[] = {
>>> + &mv88e6xxx_region_port_0_ops,
>>> + &mv88e6xxx_region_port_1_ops,
>>> + &mv88e6xxx_region_port_2_ops,
>>> + &mv88e6xxx_region_port_3_ops,
>>> + &mv88e6xxx_region_port_4_ops,
>>> + &mv88e6xxx_region_port_5_ops,
>>> + &mv88e6xxx_region_port_6_ops,
>>> + &mv88e6xxx_region_port_7_ops,
>>> + &mv88e6xxx_region_port_8_ops,
>>> + &mv88e6xxx_region_port_9_ops,
>>> + &mv88e6xxx_region_port_10_ops,
>>> + &mv88e6xxx_region_port_11_ops,
>>> +};
>>> +
>>
>> Sounds like there should maybe be an abstraction for 'per-port regions' in
>> devlink? I think your approach hardly scales if you start having
>> switches with more than 11 ports.
>
> mv88e6xxx is unlikely to have more an 11 ports. Marvell had to move
> bits around in registers in non-compatible ways to support the 6390
> family with this number of ports. I doubt we will ever see a 16 port
> mv88e6xxx switch, the registers are just too congested.
Any number greater than 1 could justify finding a solution that scales.
>
> So this scales as far as it needs to scale.
>
>>> +/* The ATU entry varies between chipset generations. Define a generic
>>> + * format which covers all the current and hopefully future
>>> + * generations
>>> + */
>>
>> Could you please present this generic format to us? Maybe my interpretation of
>> the word "generic" is incorrect in this context?
>
> I mean generic across all mv88e6xxx switches. The fid has been slowly
> getting bigger from generation to generation. If i remember correctly,
> it start off as 6 bits. 2 more bits we added, in a different
> register. Then it got moved into a new register and made 14 bits in
> size. There are also some bits in the atu_op register which changed
> meaning over time.
>
> In order to decode any of this information in the regions, you need to
> known the specific switch the dump came from. But that is the whole
> point of regions.
>
> https://www.kernel.org/doc/html/latest/networking/devlink/devlink-region.html
>
> As regions are likely very device or driver specific, no generic
> regions are defined. See the driver-specific documentation files
> for information on the specific regions a driver supports.
>
> This should also make the context of 'generic' more clear.
Looking at the documentation above (assuming it is up to date), these
are raw hex dumps of the region, which is mildly useful.
If we were to pretty print those regions such that they can fully
replace the infamous debugfs interface patch from Vivien that has been
floated around before, what other information is available (besides the
driver name) for the user-space tools to do that pretty printing?
Right now, as with any single user facility it is a bit difficult to
determine whether a DSA common representation would be warranted.
--
Florian
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions
2020-08-17 17:15 ` Florian Fainelli
@ 2020-08-17 19:02 ` Andrew Lunn
0 siblings, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2020-08-17 19:02 UTC (permalink / raw)
To: Florian Fainelli
Cc: Vladimir Oltean, David Miller, netdev, Chris Healy, Vivien Didelot
> Looking at the documentation above (assuming it is up to date), these
> are raw hex dumps of the region, which is mildly useful.
>
> If we were to pretty print those regions such that they can fully
> replace the infamous debugfs interface patch from Vivien that has been
> floated around before, what other information is available (besides the
> driver name) for the user-space tools to do that pretty printing?
Hi Florian
https://github.com/lunn/mv88e6xx_dump
root@rap:~# /home/andrew/mv88e6xxx_dump/mv88e6xxx_dump --atu --global1 --global2 --ports --port 1
Using device <mdio_bus/gpio-0:00>
00 Port status 0x1e4f
Transmit Pause Enable bit 0
Receive Pause Enable bit 0
802.3 PHY Detected 1
Link Status Up
Duplex Full
Speed 1000 Mbps
Duplex Fixed 0
EEE Enabled 1
Transmitter Paused 0
Flow Control 0
Config Mode 0xf
01 Physical control 0x203e
RGMII Receive Timing Control Default
RGMII Transmit Timing Control Default
Force Speed 1
Alternate Speed Mode Normal
MII PHY Mode MAC
EEE force value 0
Force EEE 0
Link's Forced value Up
Force Link 1
Duplex's Forced value Full
Force Duplex 1
Force Speed 1000 Mbps
02 Flow control 0x0100
03 Switch ID 0x3901
04 Port control 0x053f
Source Address Filtering controls Disabled
Egress Mode Unmodified
Ingress & Egress Header Mode 0
IGMP and MLD Snooping 1
Frame Mode DSA
VLAN Tunnel 0
TagIfBoth 0
Initial Priority assignment Tag & IP Priority
Egress Flooding mode Allow unknown DA
Port State Forwarding
05 Port control 1 0x0000
Message Port 0
LAG Port 0
VTU Page 0
LAG ID 0
FID[11:4] 0x000
06 Port base VLAN map 0x07fd
FID[3:0] 0x000
Force Mapping 0
VLANTable 0 2 3 4 5 6 7 8 9 10
07 Def VLAN ID & Prio 0x0000
Default Priority 0x0
Force to use Default VID 0
Default VLAN Identifier 0
08 Port control 2 0x0080
Force good FCS in the frame 0
Allow bad FCS 0
Jumbo Mode 1522
802.1QMode Disabled
Discard Tagged Frames 0
Discard Untagged Frames 0
Map using DA hits 1
ARP Mirror enable 0
Egress Monitor Source Port 0
Ingress Monitor Source Port 0
Allow VID of Zero 0
Default Queue Priority 0x0
09 Egress rate control 0x0001
0a Egress rate control 2 0x0000
0b Port association vec 0x0000
0c Port ATU control 0x0000
0d Override 0x0000
0e Policy control 0x0000
0f Port ether type 0x9100
10 Reserved 0x0000
11 Reserved 0x0000
12 Reserved 0x0000
13 Reserved 0x0000
14 Reserved 0x0000
15 Reserved 0x0000
16 LED control 0x0022
17 IP prio map table 0x0000
18 IEEE prio map table 0x3e07
19 Port control 3 0x0000
1a Reserved 0x0000
1b Queue counters 0x8000
1c Queue control 0x0000
1d Reserved 0x0000
1e Cut through control 0x0000
1f Debug counters 0x003d
0 1 2 3 4 5 6 7 8 9 10
00 Port status 0e07 1e4f 100f 100f de4f 100f 100f 100f 1d0f 0049 0049
01 Physical control 0003 203e 0003 0003 0003 0003 0003 0003 0003 0003 0003
02 Flow control 0000 0100 0100 0100 0100 0000 0000 0000 0100 0000 0000
03 Switch ID 3901 3901 3901 3901 3901 3901 3901 3901 3901 3901 3901
04 Port control 007c 053f 0433 0433 0433 007c 007c 007c 0433 007c 007c
05 Port control 1 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
06 Port base VLAN map 07fe 07fd 0002 0002 0002 07df 07bf 077f 0002 05ff 03ff
07 Def VLAN ID & Prio 0001 0000 0000 0000 0000 0001 0001 0001 0000 0001 0001
08 Port control 2 2080 0080 0080 0080 0080 2080 2080 2080 0080 2080 2080
09 Egress rate control 0001 0001 0001 0001 0001 0001 0001 0001 0001 0001 0001
0a Egress rate control 2 8000 0000 0000 0000 0000 8000 8000 8000 0000 8000 8000
0b Port association vec 0001 0000 0004 0008 0010 0020 0040 0080 0100 0200 0400
0c Port ATU control 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
0d Override 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
0e Policy control 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
0f Port ether type 9100 9100 9100 9100 9100 9100 9100 9100 9100 9100 9100
10 Reserved 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
11 Reserved 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
12 Reserved 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
13 Reserved 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
14 Reserved 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
15 Reserved 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
16 LED control 0000 0022 0022 0022 0022 0022 0022 0022 0022 0022 0022
17 IP prio map table 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
18 IEEE prio map table 0000 3e07 3e07 3e07 3e07 0000 0000 0000 3e07 0000 0000
19 Port control 3 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
1a Reserved 0000 0000 0000 0000 3d40 01c0 0000 0000 0000 0000 0000
1b Queue counters 8000 8000 8000 8000 8000 8000 8000 8000 8000 8000 8000
1c Queue control 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
1d Reserved 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
1e Cut through control 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
1f Debug counters 0000 003d 0000 0000 0077 0000 0000 0000 0000 0000 0000
ATU:
FID MAC T 0123456789A Prio State
0 ff:ff:ff:ff:ff:ff 11111100000 0 Static
Global1:
0 c814
1 0000
2 0000
3 0000
4 40a8
5 4000
6 2fff
7 0000
8 0000
9 0000
10 0509
11 4000
12 7ff7
13 ffff
14 ffff
15 ffff
16 0000
17 0000
18 0000
19 0000
20 0000
21 0000
22 0000
23 0000
24 0000
25 0000
26 03ff
27 03fd
28 07c0
29 1000
30 0000
31 0000
Global2:
0 0000
1 811c
2 0000
3 0000
4 0258
5 0400
6 1f1f
7 77ff
8 7800
9 2a00
10 0000
11 31ff
12 0000
13 0589
14 0001
15 0f00
16 0000
17 0000
18 0000
19 0300
20 0000
21 0000
22 5e0e
23 0000
24 1885
25 c5e1
26 0000
27 110f
28 0000
29 0000
30 0000
31 0000
Still WIP. I want to add at least names for the global1 and glabel2.
> Right now, as with any single user facility it is a bit difficult to
> determine whether a DSA common representation would be warranted.
This is all specific to the mv88e6xxx. Vivien had two debugfs
patchsets. One for generic DSA properties and a second one for
mv88e6xxx specific stuff. The regions i've added only cover the
mv88e6xxx specific stuff.
The recent devlink metric's should help with some parts of the generic
debugfs code.
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-next 6/7] net: dsa: wire up devlink info get
2020-08-16 19:43 [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Andrew Lunn
` (4 preceding siblings ...)
2020-08-16 19:43 ` [PATCH net-next 5/7] net: dsa: mv88e6xxx: Add devlink regions Andrew Lunn
@ 2020-08-16 19:43 ` Andrew Lunn
2020-08-16 21:56 ` Vladimir Oltean
2020-08-16 19:43 ` [PATCH net-next 7/7] net: dsa: mv88e6xxx: Implement devlink info get callback Andrew Lunn
` (2 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 19:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Chris Healy, Vivien Didelot, Andrew Lunn
Allow the DSA drivers to implement the devlink call to get info info,
e.g. driver name, firmware version, ASIC ID, etc.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
include/net/dsa.h | 5 ++++-
net/dsa/dsa2.c | 21 ++++++++++++++++++---
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8963440ec7f8..0e34193d15ba 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -612,11 +612,14 @@ struct dsa_switch_ops {
bool (*port_rxtstamp)(struct dsa_switch *ds, int port,
struct sk_buff *skb, unsigned int type);
- /* Devlink parameters */
+ /* Devlink parameters, etc */
int (*devlink_param_get)(struct dsa_switch *ds, u32 id,
struct devlink_param_gset_ctx *ctx);
int (*devlink_param_set)(struct dsa_switch *ds, u32 id,
struct devlink_param_gset_ctx *ctx);
+ int (*devlink_info_get)(struct dsa_switch *ds,
+ struct devlink_info_req *req,
+ struct netlink_ext_ack *extack);
/*
* MTU change functionality. Switches can also adjust their MRU through
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index c0ffc7a2b65f..860f2fb22fe0 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -21,9 +21,6 @@
static DEFINE_MUTEX(dsa2_mutex);
LIST_HEAD(dsa_tree_list);
-static const struct devlink_ops dsa_devlink_ops = {
-};
-
struct dsa_switch *dsa_switch_find(int tree_index, int sw_index)
{
struct dsa_switch_tree *dst;
@@ -382,6 +379,24 @@ static void dsa_port_teardown(struct dsa_port *dp)
dp->setup = false;
}
+static int dsa_devlink_info_get(struct devlink *dl,
+ struct devlink_info_req *req,
+ struct netlink_ext_ack *extack)
+{
+ struct dsa_switch *ds;
+
+ ds = dsa_devlink_to_ds(dl);
+
+ if (ds->ops->devlink_info_get)
+ return ds->ops->devlink_info_get(ds, req, extack);
+
+ return -EOPNOTSUPP;
+}
+
+static const struct devlink_ops dsa_devlink_ops = {
+ .info_get = dsa_devlink_info_get,
+};
+
static int dsa_switch_setup(struct dsa_switch *ds)
{
struct dsa_devlink_priv *dl_priv;
--
2.28.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next 6/7] net: dsa: wire up devlink info get
2020-08-16 19:43 ` [PATCH net-next 6/7] net: dsa: wire up devlink info get Andrew Lunn
@ 2020-08-16 21:56 ` Vladimir Oltean
2020-08-16 22:16 ` Andrew Lunn
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-08-16 21:56 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, Chris Healy, Vivien Didelot
On Sun, Aug 16, 2020 at 09:43:15PM +0200, Andrew Lunn wrote:
> Allow the DSA drivers to implement the devlink call to get info info,
> e.g. driver name, firmware version, ASIC ID, etc.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> include/net/dsa.h | 5 ++++-
> net/dsa/dsa2.c | 21 ++++++++++++++++++---
> 2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 8963440ec7f8..0e34193d15ba 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -612,11 +612,14 @@ struct dsa_switch_ops {
> bool (*port_rxtstamp)(struct dsa_switch *ds, int port,
> struct sk_buff *skb, unsigned int type);
>
> - /* Devlink parameters */
> + /* Devlink parameters, etc */
> int (*devlink_param_get)(struct dsa_switch *ds, u32 id,
> struct devlink_param_gset_ctx *ctx);
> int (*devlink_param_set)(struct dsa_switch *ds, u32 id,
> struct devlink_param_gset_ctx *ctx);
> + int (*devlink_info_get)(struct dsa_switch *ds,
> + struct devlink_info_req *req,
> + struct netlink_ext_ack *extack);
>
> /*
> * MTU change functionality. Switches can also adjust their MRU through
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index c0ffc7a2b65f..860f2fb22fe0 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -21,9 +21,6 @@
> static DEFINE_MUTEX(dsa2_mutex);
> LIST_HEAD(dsa_tree_list);
>
> -static const struct devlink_ops dsa_devlink_ops = {
> -};
> -
> struct dsa_switch *dsa_switch_find(int tree_index, int sw_index)
> {
> struct dsa_switch_tree *dst;
> @@ -382,6 +379,24 @@ static void dsa_port_teardown(struct dsa_port *dp)
> dp->setup = false;
> }
>
> +static int dsa_devlink_info_get(struct devlink *dl,
> + struct devlink_info_req *req,
> + struct netlink_ext_ack *extack)
> +{
> + struct dsa_switch *ds;
> +
> + ds = dsa_devlink_to_ds(dl);
> +
Why not place the declaration and the assignment on a single line?
> + if (ds->ops->devlink_info_get)
> + return ds->ops->devlink_info_get(ds, req, extack);
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct devlink_ops dsa_devlink_ops = {
> + .info_get = dsa_devlink_info_get,
> +};
> +
> static int dsa_switch_setup(struct dsa_switch *ds)
> {
> struct dsa_devlink_priv *dl_priv;
> --
> 2.28.0
>
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next 6/7] net: dsa: wire up devlink info get
2020-08-16 21:56 ` Vladimir Oltean
@ 2020-08-16 22:16 ` Andrew Lunn
0 siblings, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 22:16 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: David Miller, netdev, Chris Healy, Vivien Didelot
> > +static int dsa_devlink_info_get(struct devlink *dl,
> > + struct devlink_info_req *req,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct dsa_switch *ds;
> > +
> > + ds = dsa_devlink_to_ds(dl);
> > +
>
> Why not place the declaration and the assignment on a single line?
This code went through a few refactors. Probably at some point i could
not keep reverse christmass tree so had to split it. And it then never
got put back together again.
I will fix it up for v2.
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-next 7/7] net: dsa: mv88e6xxx: Implement devlink info get callback
2020-08-16 19:43 [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Andrew Lunn
` (5 preceding siblings ...)
2020-08-16 19:43 ` [PATCH net-next 6/7] net: dsa: wire up devlink info get Andrew Lunn
@ 2020-08-16 19:43 ` Andrew Lunn
2020-08-17 16:03 ` Jakub Kicinski
2020-08-16 20:17 ` [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Chris Healy
2020-08-17 17:08 ` Florian Fainelli
8 siblings, 1 reply; 22+ messages in thread
From: Andrew Lunn @ 2020-08-16 19:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Chris Healy, Vivien Didelot, Andrew Lunn
Return the driver name and the asic.id with the switch name.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 1 +
drivers/net/dsa/mv88e6xxx/devlink.c | 15 +++++++++++++++
drivers/net/dsa/mv88e6xxx/devlink.h | 3 +++
3 files changed, 19 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 33e4518736a2..ee3b1a6b1ea8 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -5385,6 +5385,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
.get_ts_info = mv88e6xxx_get_ts_info,
.devlink_param_get = mv88e6xxx_devlink_param_get,
.devlink_param_set = mv88e6xxx_devlink_param_set,
+ .devlink_info_get = mv88e6xxx_devlink_info_get,
};
static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
index c6ebadcfa63f..8cb167d737e2 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -673,3 +673,18 @@ int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds)
return 0;
}
+int mv88e6xxx_devlink_info_get(struct dsa_switch *ds,
+ struct devlink_info_req *req,
+ struct netlink_ext_ack *extack)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+ int err;
+
+ err = devlink_info_driver_name_put(req, "mv88e6xxx");
+ if (err)
+ return err;
+
+ return devlink_info_version_fixed_put(req,
+ DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
+ chip->info->name);
+}
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.h b/drivers/net/dsa/mv88e6xxx/devlink.h
index da83c25d944b..3d72db3dcf95 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.h
+++ b/drivers/net/dsa/mv88e6xxx/devlink.h
@@ -15,4 +15,7 @@ int mv88e6xxx_devlink_param_set(struct dsa_switch *ds, u32 id,
int mv88e6xxx_setup_devlink_regions(struct dsa_switch *ds);
void mv88e6xxx_teardown_devlink_regions(struct dsa_switch *ds);
+int mv88e6xxx_devlink_info_get(struct dsa_switch *ds,
+ struct devlink_info_req *req,
+ struct netlink_ext_ack *extack);
#endif /* _MV88E6XXX_DEVLINK_H */
--
2.28.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support
2020-08-16 19:43 [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Andrew Lunn
` (6 preceding siblings ...)
2020-08-16 19:43 ` [PATCH net-next 7/7] net: dsa: mv88e6xxx: Implement devlink info get callback Andrew Lunn
@ 2020-08-16 20:17 ` Chris Healy
2020-08-17 17:08 ` Florian Fainelli
8 siblings, 0 replies; 22+ messages in thread
From: Chris Healy @ 2020-08-16 20:17 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, Vivien Didelot
I tested this on the Rev C Zii Vybrid Dev Board which has two Marvell
88E6390X switches.
Both switches worth of port, global, and atu data show up when running
the devlink show command and for each of the regions, I can dump the
contents.
I also tested on a different platform with a single 88E6352.
Full series is:
Tested-by: Chris Healy <cphealy@gmail.com>
On Sun, Aug 16, 2020 at 12:44 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> Make use of devlink regions to allow read access to some of the
> internal of the switches. The switch itself will never trigger a
> region snapshot, it is assumed it is performed from user space as
> needed.
>
> Andrew Lunn (7):
> net: dsa: Add helper to convert from devlink to ds
> net: dsa: Add devlink regions support to DSA
> net: dsa: mv88e6xxx: Move devlink code into its own file
> net: dsa: mv88e6xxx: Create helper for FIDs in use
> net: dsa: mv88e6xxx: Add devlink regions
> net: dsa: wire up devlink info get
> net: dsa: mv88e6xxx: Implement devlink info get callback
>
> drivers/net/dsa/mv88e6xxx/Makefile | 1 +
> drivers/net/dsa/mv88e6xxx/chip.c | 290 ++----------
> drivers/net/dsa/mv88e6xxx/chip.h | 14 +
> drivers/net/dsa/mv88e6xxx/devlink.c | 690 ++++++++++++++++++++++++++++
> drivers/net/dsa/mv88e6xxx/devlink.h | 21 +
> include/net/dsa.h | 13 +-
> net/dsa/dsa.c | 36 +-
> net/dsa/dsa2.c | 21 +-
> 8 files changed, 813 insertions(+), 273 deletions(-)
> create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.c
> create mode 100644 drivers/net/dsa/mv88e6xxx/devlink.h
>
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support
2020-08-16 19:43 [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Andrew Lunn
` (7 preceding siblings ...)
2020-08-16 20:17 ` [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support Chris Healy
@ 2020-08-17 17:08 ` Florian Fainelli
2020-08-17 19:08 ` Andrew Lunn
8 siblings, 1 reply; 22+ messages in thread
From: Florian Fainelli @ 2020-08-17 17:08 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Chris Healy, Vivien Didelot
On 8/16/20 12:43 PM, Andrew Lunn wrote:
> Make use of devlink regions to allow read access to some of the
> internal of the switches. The switch itself will never trigger a
> region snapshot, it is assumed it is performed from user space as
> needed.
>
> Andrew Lunn (7):
> net: dsa: Add helper to convert from devlink to ds
> net: dsa: Add devlink regions support to DSA
> net: dsa: mv88e6xxx: Move devlink code into its own file
> net: dsa: mv88e6xxx: Create helper for FIDs in use
> net: dsa: mv88e6xxx: Add devlink regions
> net: dsa: wire up devlink info get
> net: dsa: mv88e6xxx: Implement devlink info get callback
Andrew, do you mind copying all DSA maintainers on this patch series
since it potentially affects other drivers given the standard
representation you want to see adopted?
Thanks
--
Florian
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next 0/7] net: dsa: mv88e6xxx: Add devlink regions support
2020-08-17 17:08 ` Florian Fainelli
@ 2020-08-17 19:08 ` Andrew Lunn
0 siblings, 0 replies; 22+ messages in thread
From: Andrew Lunn @ 2020-08-17 19:08 UTC (permalink / raw)
To: Florian Fainelli; +Cc: David Miller, netdev, Chris Healy, Vivien Didelot
On Mon, Aug 17, 2020 at 10:08:07AM -0700, Florian Fainelli wrote:
> On 8/16/20 12:43 PM, Andrew Lunn wrote:
> > Make use of devlink regions to allow read access to some of the
> > internal of the switches. The switch itself will never trigger a
> > region snapshot, it is assumed it is performed from user space as
> > needed.
> >
> > Andrew Lunn (7):
> > net: dsa: Add helper to convert from devlink to ds
> > net: dsa: Add devlink regions support to DSA
> > net: dsa: mv88e6xxx: Move devlink code into its own file
> > net: dsa: mv88e6xxx: Create helper for FIDs in use
> > net: dsa: mv88e6xxx: Add devlink regions
> > net: dsa: wire up devlink info get
> > net: dsa: mv88e6xxx: Implement devlink info get callback
>
> Andrew, do you mind copying all DSA maintainers on this patch series
> since it potentially affects other drivers given the standard
> representation you want to see adopted?
Hi Florian
I'm not proposing anything standard at all here. This is all specific
to the mv88e6xxx. As i pointed out to Vladimir, devlink regions are
supposed to be very device or driver specific.
Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread