On 03/27/2018 02:59 PM, Andrew Lunn wrote: > Count the numbers of various ATU and VTU violation statistics and > return them as part of the ethtool -S statistics. > > Signed-off-by: Andrew Lunn <and...@lunn.ch> > --- > drivers/net/dsa/mv88e6xxx/chip.c | 50 > ++++++++++++++++++++++++++++----- > drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++--- > drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++--- > drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 ++++-- > drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++---- > drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++--- > 6 files changed, 78 insertions(+), 28 deletions(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c > b/drivers/net/dsa/mv88e6xxx/chip.c > index 9a5d786b4885..186021f98c5d 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -723,6 +723,24 @@ static int mv88e6320_stats_get_strings(struct > mv88e6xxx_chip *chip, > STATS_TYPE_BANK0 | STATS_TYPE_BANK1); > } > > +static const uint8_t *mv88e6xxx_atu_vtu_stats_strings[] = {
Why not const char *? > + "atu_member_violation", > + "atu_miss_violation", > + "atu_full_violation", > + "vtu_member_violation", > + "vtu_miss_violation", > +}; > + > +static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data) > +{ > + int i; unsigned int i? > + > + for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++) > + strlcpy(data + i * ETH_GSTRING_LEN, > + mv88e6xxx_atu_vtu_stats_strings[i], > + ETH_GSTRING_LEN); > +} > + > static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, > uint8_t *data) > { > @@ -736,9 +754,12 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, > int port, > > if (chip->info->ops->serdes_get_strings) { > data += count * ETH_GSTRING_LEN; > - chip->info->ops->serdes_get_strings(chip, port, data); > + count = chip->info->ops->serdes_get_strings(chip, port, data); > } > > + data += count * ETH_GSTRING_LEN; > + mv88e6xxx_atu_vtu_get_strings(data); > + > mutex_unlock(&chip->reg_lock); > } > > @@ -783,10 +804,13 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch > *ds, int port) > if (chip->info->ops->serdes_get_sset_count) > serdes_count = chip->info->ops->serdes_get_sset_count(chip, > port); > - if (serdes_count < 0) > + if (serdes_count < 0) { > count = serdes_count; > - else > - count += serdes_count; > + goto out; > + } > + count += serdes_count; > + count += ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); > + > out: > mutex_unlock(&chip->reg_lock); > > @@ -841,6 +865,16 @@ static int mv88e6390_stats_get_stats(struct > mv88e6xxx_chip *chip, int port, > 0); > } > > +static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int > port, > + uint64_t *data) > +{ > + *data++ = chip->ports[port].atu_member_violation; > + *data++ = chip->ports[port].atu_miss_violation; > + *data++ = chip->ports[port].atu_full_violation; > + *data++ = chip->ports[port].vtu_member_violation; > + *data++ = chip->ports[port].vtu_miss_violation; This looks fine, but I suppose you could just have an u64 pointer which is initialized to point to atu_member_violation, and then just do pointer arithmetics to iterate, this would avoid possibly missing that function in case new ATU/VTU violations are handled in the future? -- Florian