| Message ID | 20220908132109.3213080-6-mattias.forsblad@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: qca8k, mv88e6xxx: rmon: Add RMU support | expand |
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index bbdf229c9e71..bd16afa2e1a5 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -1234,16 +1234,30 @@ static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port, > u16 bank1_select, u16 histogram) > { > struct mv88e6xxx_hw_stat *stat; > + int offset = 0; > + u64 high; > int i, j; > > for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) { > stat = &mv88e6xxx_hw_stats[i]; > if (stat->type & types) { > - mv88e6xxx_reg_lock(chip); > - data[j] = _mv88e6xxx_get_ethtool_stat(chip, stat, port, > - bank1_select, > - histogram); > - mv88e6xxx_reg_unlock(chip); > + if (mv88e6xxx_rmu_available(chip) && I was trying to avoid code like this, by the use of the ops. In terms of code, you are actually reusing very little here. What you are re-using is the table of statistics. I would add a new function, not change this function. > + !(stat->type & STATS_TYPE_PORT)) { > + if (stat->type & STATS_TYPE_BANK1) > + offset = 32; > + > + data[j] = chip->ports[port].rmu_raw_stats[stat->reg + offset]; > + if (stat->size == 8) { > + high = chip->ports[port].rmu_raw_stats[stat->reg + offset > + + 1]; > + data[j] += (high << 32); > + } > + } else { > + mv88e6xxx_reg_lock(chip); > + data[j] = _mv88e6xxx_get_ethtool_stat(chip, stat, port, > + bank1_select, histogram); > + mv88e6xxx_reg_unlock(chip); > + } > > j++; > } > diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h > index 566d18cf5170..5459037067e6 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.h > +++ b/drivers/net/dsa/mv88e6xxx/chip.h > @@ -266,6 +266,7 @@ struct mv88e6xxx_vlan { > struct mv88e6xxx_port { > struct mv88e6xxx_chip *chip; > int port; > + u64 rmu_raw_stats[64]; That is a lot of space you don't actually need. Once you have a clean set of functions, you can simply pass the raw data as a parameter. Andrew
On 2022-09-09 03:49, Andrew Lunn wrote: >> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c >> index bbdf229c9e71..bd16afa2e1a5 100644 >> --- a/drivers/net/dsa/mv88e6xxx/chip.c >> +++ b/drivers/net/dsa/mv88e6xxx/chip.c >> @@ -1234,16 +1234,30 @@ static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port, >> u16 bank1_select, u16 histogram) >> { >> struct mv88e6xxx_hw_stat *stat; >> + int offset = 0; >> + u64 high; >> int i, j; >> >> for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) { >> stat = &mv88e6xxx_hw_stats[i]; >> if (stat->type & types) { >> - mv88e6xxx_reg_lock(chip); >> - data[j] = _mv88e6xxx_get_ethtool_stat(chip, stat, port, >> - bank1_select, >> - histogram); >> - mv88e6xxx_reg_unlock(chip); >> + if (mv88e6xxx_rmu_available(chip) && > > I was trying to avoid code like this, by the use of the ops. > The call path with this patch is: dsa_slave_get_ethtool_stats-> get_ethtool_stats(ops)-> mv88e6xxx_get_ethtool_stats-> get_rmon(ops)-> (1) mv88e6xxx_rmu_stats_get-> stats_get_stats(ops)-> (per chip implementation:mv88e6095/6250/6320/6390_stats_get_stats-> mv88e6xxx_stats_get_stats(with different parameters) Here we want to decode the raw RMU data according to specific chip. This function is not an ops and furthermore some RMON data is still fetched through MDIO, i.e. !(stat->type & STATS_TYPE_PORT). I'm not sure what you want me to do? The ops I've changed is at (1) /Mattias
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index bbdf229c9e71..bd16afa2e1a5 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1234,16 +1234,30 @@ static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port, u16 bank1_select, u16 histogram) { struct mv88e6xxx_hw_stat *stat; + int offset = 0; + u64 high; int i, j; for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) { stat = &mv88e6xxx_hw_stats[i]; if (stat->type & types) { - mv88e6xxx_reg_lock(chip); - data[j] = _mv88e6xxx_get_ethtool_stat(chip, stat, port, - bank1_select, - histogram); - mv88e6xxx_reg_unlock(chip); + if (mv88e6xxx_rmu_available(chip) && + !(stat->type & STATS_TYPE_PORT)) { + if (stat->type & STATS_TYPE_BANK1) + offset = 32; + + data[j] = chip->ports[port].rmu_raw_stats[stat->reg + offset]; + if (stat->size == 8) { + high = chip->ports[port].rmu_raw_stats[stat->reg + offset + + 1]; + data[j] += (high << 32); + } + } else { + mv88e6xxx_reg_lock(chip); + data[j] = _mv88e6xxx_get_ethtool_stat(chip, stat, port, + bank1_select, histogram); + mv88e6xxx_reg_unlock(chip); + } j++; } @@ -1312,10 +1326,9 @@ static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port, mv88e6xxx_reg_unlock(chip); } -static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port, - uint64_t *data) +void mv88e6xxx_get_ethtool_stats_mdio(struct mv88e6xxx_chip *chip, int port, + uint64_t *data) { - struct mv88e6xxx_chip *chip = ds->priv; int ret; mv88e6xxx_reg_lock(chip); @@ -1327,7 +1340,14 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port, return; mv88e6xxx_get_stats(chip, port, data); +} + +static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port, + uint64_t *data) +{ + struct mv88e6xxx_chip *chip = ds->priv; + chip->smi_ops->get_rmon(chip, port, data); } static int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port) diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h index 566d18cf5170..5459037067e6 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.h +++ b/drivers/net/dsa/mv88e6xxx/chip.h @@ -266,6 +266,7 @@ struct mv88e6xxx_vlan { struct mv88e6xxx_port { struct mv88e6xxx_chip *chip; int port; + u64 rmu_raw_stats[64]; struct mv88e6xxx_vlan bridge_pvid; u64 serdes_stats[2]; u64 atu_member_violation; @@ -431,6 +432,7 @@ struct mv88e6xxx_bus_ops { int (*read)(struct mv88e6xxx_chip *chip, int addr, int reg, u16 *val); int (*write)(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val); int (*init)(struct mv88e6xxx_chip *chip); + void (*get_rmon)(struct mv88e6xxx_chip *chip, int port, uint64_t *data); }; struct mv88e6xxx_mdio_bus { @@ -807,6 +809,8 @@ int mv88e6xxx_wait_mask(struct mv88e6xxx_chip *chip, int addr, int reg, int mv88e6xxx_wait_bit(struct mv88e6xxx_chip *chip, int addr, int reg, int bit, int val); struct mii_bus *mv88e6xxx_default_mdio_bus(struct mv88e6xxx_chip *chip); +void mv88e6xxx_get_ethtool_stats_mdio(struct mv88e6xxx_chip *chip, int port, + uint64_t *data); static inline void mv88e6xxx_reg_lock(struct mv88e6xxx_chip *chip) { diff --git a/drivers/net/dsa/mv88e6xxx/smi.c b/drivers/net/dsa/mv88e6xxx/smi.c index a990271b7482..ae805c449b85 100644 --- a/drivers/net/dsa/mv88e6xxx/smi.c +++ b/drivers/net/dsa/mv88e6xxx/smi.c @@ -83,6 +83,7 @@ static int mv88e6xxx_smi_direct_wait(struct mv88e6xxx_chip *chip, static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_direct_ops = { .read = mv88e6xxx_smi_direct_read, .write = mv88e6xxx_smi_direct_write, + .get_rmon = mv88e6xxx_get_ethtool_stats_mdio, }; static int mv88e6xxx_smi_dual_direct_read(struct mv88e6xxx_chip *chip, @@ -100,6 +101,7 @@ static int mv88e6xxx_smi_dual_direct_write(struct mv88e6xxx_chip *chip, static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_dual_direct_ops = { .read = mv88e6xxx_smi_dual_direct_read, .write = mv88e6xxx_smi_dual_direct_write, + .get_rmon = mv88e6xxx_get_ethtool_stats_mdio, }; /* Offset 0x00: SMI Command Register @@ -166,6 +168,7 @@ static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_indirect_ops = { .read = mv88e6xxx_smi_indirect_read, .write = mv88e6xxx_smi_indirect_write, .init = mv88e6xxx_smi_indirect_init, + .get_rmon = mv88e6xxx_get_ethtool_stats_mdio, }; int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
Use the Remote Management Unit for efficiently accessing the RMON data. Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com> --- drivers/net/dsa/mv88e6xxx/chip.c | 36 +++++++++++++++++++++++++------- drivers/net/dsa/mv88e6xxx/chip.h | 4 ++++ drivers/net/dsa/mv88e6xxx/smi.c | 3 +++ 3 files changed, 35 insertions(+), 8 deletions(-)