| Message ID | 20200816194316.2291489-6-andrew@lunn.ch |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: dsa: mv88e6xxx: Add devlink regions support | expand |
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
> > +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
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.
> 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
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 */
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(-)