| Message ID | 20221111204924.1442282-2-colin.foster@in-advantage.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | cleanup ocelot_stats exposure | expand |
Hi Colin,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Colin-Foster/cleanup-ocelot_stats-exposure/20221112-045112
patch link: https://lore.kernel.org/r/20221111204924.1442282-2-colin.foster%40in-advantage.com
patch subject: [PATCH v1 net-next 1/2] net: mscc: ocelot: remove redundant stats_layout pointers
config: x86_64-allyesconfig
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/08340d5695cf703f1714df628183554016c35107
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Colin-Foster/cleanup-ocelot_stats-exposure/20221112-045112
git checkout 08340d5695cf703f1714df628183554016c35107
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/ethernet/mscc/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/mscc/ocelot_vsc7514.c:103:40: warning: 'ocelot_stats_layout' defined but not used [-Wunused-const-variable=]
103 | static const struct ocelot_stat_layout ocelot_stats_layout[OCELOT_NUM_STATS] = {
| ^~~~~~~~~~~~~~~~~~~
vim +/ocelot_stats_layout +103 drivers/net/ethernet/mscc/ocelot_vsc7514.c
d9feb904997333 Vladimir Oltean 2020-06-20 102
9190460084ddd0 Vladimir Oltean 2022-08-16 @103 static const struct ocelot_stat_layout ocelot_stats_layout[OCELOT_NUM_STATS] = {
4d1d157fb6a49f Vladimir Oltean 2022-09-08 104 OCELOT_COMMON_STATS,
d9feb904997333 Vladimir Oltean 2020-06-20 105 };
d9feb904997333 Vladimir Oltean 2020-06-20 106
On Fri, Nov 11, 2022 at 12:49:23PM -0800, Colin Foster wrote: > Ever since commit 4d1d157fb6a4 ("net: mscc: ocelot: share the common stat > definitions between all drivers") the stats_layout entry in ocelot and > felix drivers have become redundant. Remove the unnecessary code. Oops - I didn't remove the unused struct from ocelot_vsc7514.c. That'll be in v2.
Hi Colin, On Fri, Nov 11, 2022 at 12:49:23PM -0800, Colin Foster wrote: > Ever since commit 4d1d157fb6a4 ("net: mscc: ocelot: share the common stat > definitions between all drivers") the stats_layout entry in ocelot and > felix drivers have become redundant. Remove the unnecessary code. > > Suggested-by: Vladimir Oltean <olteanv@gmail.com> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > --- (please also the Microchip development list, people do seem to follow it and do respond sometimes) Before saying anything about this patch set, I wanted to check what's the status with my downstream patches for the MAC Merge Layer counters. There, vsc9959_stats_layout would get expanded to look like this: static const struct ocelot_stat_layout vsc9959_stats_layout[OCELOT_NUM_STATS] = { OCELOT_COMMON_STATS, OCELOT_STAT(RX_ASSEMBLY_ERRS), OCELOT_STAT(RX_SMD_ERRS), OCELOT_STAT(RX_ASSEMBLY_OK), OCELOT_STAT(RX_MERGE_FRAGMENTS), OCELOT_STAT(TX_MERGE_FRAGMENTS), OCELOT_STAT(RX_PMAC_OCTETS), OCELOT_STAT(RX_PMAC_UNICAST), OCELOT_STAT(RX_PMAC_MULTICAST), OCELOT_STAT(RX_PMAC_BROADCAST), OCELOT_STAT(RX_PMAC_SHORTS), OCELOT_STAT(RX_PMAC_FRAGMENTS), OCELOT_STAT(RX_PMAC_JABBERS), OCELOT_STAT(RX_PMAC_CRC_ALIGN_ERRS), OCELOT_STAT(RX_PMAC_SYM_ERRS), OCELOT_STAT(RX_PMAC_64), OCELOT_STAT(RX_PMAC_65_127), OCELOT_STAT(RX_PMAC_128_255), OCELOT_STAT(RX_PMAC_256_511), OCELOT_STAT(RX_PMAC_512_1023), OCELOT_STAT(RX_PMAC_1024_1526), OCELOT_STAT(RX_PMAC_1527_MAX), OCELOT_STAT(RX_PMAC_PAUSE), OCELOT_STAT(RX_PMAC_CONTROL), OCELOT_STAT(RX_PMAC_LONGS), OCELOT_STAT(TX_PMAC_OCTETS), OCELOT_STAT(TX_PMAC_UNICAST), OCELOT_STAT(TX_PMAC_MULTICAST), OCELOT_STAT(TX_PMAC_BROADCAST), OCELOT_STAT(TX_PMAC_PAUSE), OCELOT_STAT(TX_PMAC_64), OCELOT_STAT(TX_PMAC_65_127), OCELOT_STAT(TX_PMAC_128_255), OCELOT_STAT(TX_PMAC_256_511), OCELOT_STAT(TX_PMAC_512_1023), OCELOT_STAT(TX_PMAC_1024_1526), OCELOT_STAT(TX_PMAC_1527_MAX), }; The issue is that not all Ocelot family switches support the MAC merge layer. Namely, only vsc9959 does. With your removal of the ability to have a custom per-switch stats layout, the only remaining thing for vsc9959 to do is to add a "bool mm_supported" to the common struct ocelot, and all the above extra stats will only be read from the common code in ocelot_stats.c only if mm_supported is set to true. What do you think, is this acceptable?
Hi Vladimir, On Mon, Nov 14, 2022 at 03:15:36PM +0000, Vladimir Oltean wrote: > Hi Colin, > > On Fri, Nov 11, 2022 at 12:49:23PM -0800, Colin Foster wrote: > > Ever since commit 4d1d157fb6a4 ("net: mscc: ocelot: share the common stat > > definitions between all drivers") the stats_layout entry in ocelot and > > felix drivers have become redundant. Remove the unnecessary code. > > > > Suggested-by: Vladimir Oltean <olteanv@gmail.com> > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > > --- > > (please also the Microchip development list, people do seem to follow it > and do respond sometimes) Apologies there. Honest mistake, as I see get_maintainer was working. > > Before saying anything about this patch set, I wanted to check what's > the status with my downstream patches for the MAC Merge Layer counters. > > There, vsc9959_stats_layout would get expanded to look like this: > > static const struct ocelot_stat_layout vsc9959_stats_layout[OCELOT_NUM_STATS] = { > OCELOT_COMMON_STATS, > OCELOT_STAT(RX_ASSEMBLY_ERRS), > OCELOT_STAT(RX_SMD_ERRS), > OCELOT_STAT(RX_ASSEMBLY_OK), > OCELOT_STAT(RX_MERGE_FRAGMENTS), > OCELOT_STAT(TX_MERGE_FRAGMENTS), > OCELOT_STAT(RX_PMAC_OCTETS), > OCELOT_STAT(RX_PMAC_UNICAST), > OCELOT_STAT(RX_PMAC_MULTICAST), > OCELOT_STAT(RX_PMAC_BROADCAST), > OCELOT_STAT(RX_PMAC_SHORTS), > OCELOT_STAT(RX_PMAC_FRAGMENTS), > OCELOT_STAT(RX_PMAC_JABBERS), > OCELOT_STAT(RX_PMAC_CRC_ALIGN_ERRS), > OCELOT_STAT(RX_PMAC_SYM_ERRS), > OCELOT_STAT(RX_PMAC_64), > OCELOT_STAT(RX_PMAC_65_127), > OCELOT_STAT(RX_PMAC_128_255), > OCELOT_STAT(RX_PMAC_256_511), > OCELOT_STAT(RX_PMAC_512_1023), > OCELOT_STAT(RX_PMAC_1024_1526), > OCELOT_STAT(RX_PMAC_1527_MAX), > OCELOT_STAT(RX_PMAC_PAUSE), > OCELOT_STAT(RX_PMAC_CONTROL), > OCELOT_STAT(RX_PMAC_LONGS), > OCELOT_STAT(TX_PMAC_OCTETS), > OCELOT_STAT(TX_PMAC_UNICAST), > OCELOT_STAT(TX_PMAC_MULTICAST), > OCELOT_STAT(TX_PMAC_BROADCAST), > OCELOT_STAT(TX_PMAC_PAUSE), > OCELOT_STAT(TX_PMAC_64), > OCELOT_STAT(TX_PMAC_65_127), > OCELOT_STAT(TX_PMAC_128_255), > OCELOT_STAT(TX_PMAC_256_511), > OCELOT_STAT(TX_PMAC_512_1023), > OCELOT_STAT(TX_PMAC_1024_1526), > OCELOT_STAT(TX_PMAC_1527_MAX), > }; > > The issue is that not all Ocelot family switches support the MAC merge > layer. Namely, only vsc9959 does. > > With your removal of the ability to have a custom per-switch stats layout, > the only remaining thing for vsc9959 to do is to add a "bool mm_supported" > to the common struct ocelot, and all the above extra stats will only be read > from the common code in ocelot_stats.c only if mm_supported is set to true. > > What do you think, is this acceptable? That's an interesting solution. I don't really have any strong opinions on this one. I remember we'd had the discussion about making sure the stats are ordered (so that bulk stat reads don't get fragmented) and that wasn't an issue here. So I'm happy to go any route, either: 1. I fix up this patch and resubmit 2. I wait until the 9959 code lands, and do some tweaks for mac merge stats 3. Maybe we deem this patch set unnecessary and drop it, since 9959 will start using custom stats again. Or maybe a 4th route, where ocelot->stats_layout remains in tact and felix->info->stats_layout defaults to the common stats. Only the 9959 would have to override it?
On Mon, Nov 14, 2022 at 07:43:48PM -0800, Colin Foster wrote: > > The issue is that not all Ocelot family switches support the MAC merge > > layer. Namely, only vsc9959 does. > > > > With your removal of the ability to have a custom per-switch stats layout, > > the only remaining thing for vsc9959 to do is to add a "bool mm_supported" > > to the common struct ocelot, and all the above extra stats will only be read > > from the common code in ocelot_stats.c only if mm_supported is set to true. > > > > What do you think, is this acceptable? > > That's an interesting solution. I don't really have any strong opinions > on this one. I remember we'd had the discussion about making sure the > stats are ordered (so that bulk stat reads don't get fragmented) and that > wasn't an issue here. So I'm happy to go any route, either: Oops, I completely forgot about this patch, which I promised I'd submit to net-next and I never did: https://patchwork.kernel.org/project/netdevbpf/patch/20220816135352.1431497-7-vladimir.oltean@nxp.com/#24973682 Would you mind picking it up since you're dealing with stats ATM anyway? > > 1. I fix up this patch and resubmit Honestly, I don't quite remember today what I had in mind yesterday with "mm_supported" - I'm not sure how that would work. I guess it involves creating an extra struct ocelot_stat_layout array beyond ocelot_stats_layout[], which would be called ocelot_mm_stats_layout[]. What you mentioned just above with the stats ordering is going to be a problem with this approach, because we'd need to modify ocelot_prepare_stats_regions() to construct the regions based on 2 distinct struct ocelot_stat_layout arrays, depending on whether ocelot->mm_supported is set (at least that's what I believe I was saying yesterday). But if we merge the arrays if mm_supported is set, we need to merge them in a sorted way. Complicates a lot of things. > 2. I wait until the 9959 code lands, and do some tweaks for mac merge stats Hmm, waiting for me to do something sounds like a potentially long wait. Why do you need to make these changes exactly? To reduce the amount of stuff exposed for vsc7512, right? > 3. Maybe we deem this patch set unnecessary and drop it, since 9959 will > start using custom stats again. > > > Or maybe a 4th route, where ocelot->stats_layout remains in tact and > felix->info->stats_layout defaults to the common stats. Only the 9959 > would have to override it? Something like that, maybe we could have a helper that is used in ocelot_stats.c like this: static const struct ocelot_stat_layout * ocelot_get_stats_layout(struct ocelot *ocelot) { if (ocelot->stats_layout) return ocelot->stats_layout; return ocelot_stats_layout; // common for everyone except VSC9959 } and we keep exposing to the world the OCELOT_COMMON_STATS macro and whatever else is needed for VSC9959 to construct its own vsc9959_stats_layout. Or..... hmm (sorry, this is a single-pass email, not gonna delete anything previous), maybe we could implement the helper function like this: static const struct ocelot_stat_layout ocelot_stats_layout[OCELOT_NUM_STATS] = { OCELOT_COMMON_STATS, }; static const struct ocelot_stat_layout ocelot_mm_stats_layout[OCELOT_NUM_STATS] = { OCELOT_COMMON_STATS, OCELOT_STAT(RX_ASSEMBLY_ERRS), OCELOT_STAT(RX_SMD_ERRS), OCELOT_STAT(RX_ASSEMBLY_OK), OCELOT_STAT(RX_MERGE_FRAGMENTS), OCELOT_STAT(TX_MERGE_FRAGMENTS), OCELOT_STAT(RX_PMAC_OCTETS), OCELOT_STAT(RX_PMAC_UNICAST), OCELOT_STAT(RX_PMAC_MULTICAST), OCELOT_STAT(RX_PMAC_BROADCAST), OCELOT_STAT(RX_PMAC_SHORTS), OCELOT_STAT(RX_PMAC_FRAGMENTS), OCELOT_STAT(RX_PMAC_JABBERS), OCELOT_STAT(RX_PMAC_CRC_ALIGN_ERRS), OCELOT_STAT(RX_PMAC_SYM_ERRS), OCELOT_STAT(RX_PMAC_64), OCELOT_STAT(RX_PMAC_65_127), OCELOT_STAT(RX_PMAC_128_255), OCELOT_STAT(RX_PMAC_256_511), OCELOT_STAT(RX_PMAC_512_1023), OCELOT_STAT(RX_PMAC_1024_1526), OCELOT_STAT(RX_PMAC_1527_MAX), OCELOT_STAT(RX_PMAC_PAUSE), OCELOT_STAT(RX_PMAC_CONTROL), OCELOT_STAT(RX_PMAC_LONGS), OCELOT_STAT(TX_PMAC_OCTETS), OCELOT_STAT(TX_PMAC_UNICAST), OCELOT_STAT(TX_PMAC_MULTICAST), OCELOT_STAT(TX_PMAC_BROADCAST), OCELOT_STAT(TX_PMAC_PAUSE), OCELOT_STAT(TX_PMAC_64), OCELOT_STAT(TX_PMAC_65_127), OCELOT_STAT(TX_PMAC_128_255), OCELOT_STAT(TX_PMAC_256_511), OCELOT_STAT(TX_PMAC_512_1023), OCELOT_STAT(TX_PMAC_1024_1526), OCELOT_STAT(TX_PMAC_1527_MAX), }; static const struct ocelot_stat_layout * ocelot_get_stats_layout(struct ocelot *ocelot) { if (ocelot->mm_supported) return ocelot_mm_stats_layout; // common + MM stats return ocelot_stats_layout; // just common stats } Then, setting mm_supported = true from vsc9959 would be enough, no need to provide its own stats layout, no need to sort/merge anything. How does this sound?
On Tue, Nov 15, 2022 at 04:08:40PM +0000, Vladimir Oltean wrote: > On Mon, Nov 14, 2022 at 07:43:48PM -0800, Colin Foster wrote: > > > The issue is that not all Ocelot family switches support the MAC merge > > > layer. Namely, only vsc9959 does. > > > > > > With your removal of the ability to have a custom per-switch stats layout, > > > the only remaining thing for vsc9959 to do is to add a "bool mm_supported" > > > to the common struct ocelot, and all the above extra stats will only be read > > > from the common code in ocelot_stats.c only if mm_supported is set to true. > > > > > > What do you think, is this acceptable? > > > > That's an interesting solution. I don't really have any strong opinions > > on this one. I remember we'd had the discussion about making sure the > > stats are ordered (so that bulk stat reads don't get fragmented) and that > > wasn't an issue here. So I'm happy to go any route, either: > > Oops, I completely forgot about this patch, which I promised I'd submit > to net-next and I never did: > https://patchwork.kernel.org/project/netdevbpf/patch/20220816135352.1431497-7-vladimir.oltean@nxp.com/#24973682 > > Would you mind picking it up since you're dealing with stats ATM anyway? I'll bring that patch into v2 of this set. I plan to get that out late this week / end. > > > > > 1. I fix up this patch and resubmit > > Honestly, I don't quite remember today what I had in mind yesterday with > "mm_supported" - I'm not sure how that would work. I guess it involves > creating an extra struct ocelot_stat_layout array beyond ocelot_stats_layout[], > which would be called ocelot_mm_stats_layout[]. > > What you mentioned just above with the stats ordering is going to be a > problem with this approach, because we'd need to modify ocelot_prepare_stats_regions() > to construct the regions based on 2 distinct struct ocelot_stat_layout > arrays, depending on whether ocelot->mm_supported is set (at least that's > what I believe I was saying yesterday). But if we merge the arrays if > mm_supported is set, we need to merge them in a sorted way. Complicates > a lot of things. > > > 2. I wait until the 9959 code lands, and do some tweaks for mac merge stats > > Hmm, waiting for me to do something sounds like a potentially long wait. > Why do you need to make these changes exactly? To reduce the amount of > stuff exposed for vsc7512, right? > > > 3. Maybe we deem this patch set unnecessary and drop it, since 9959 will > > start using custom stats again. > > > > > > Or maybe a 4th route, where ocelot->stats_layout remains in tact and > > felix->info->stats_layout defaults to the common stats. Only the 9959 > > would have to override it? > > Something like that, maybe we could have a helper that is used in > ocelot_stats.c like this: > > static const struct ocelot_stat_layout * > ocelot_get_stats_layout(struct ocelot *ocelot) > { > if (ocelot->stats_layout) > return ocelot->stats_layout; > > return ocelot_stats_layout; // common for everyone except VSC9959 > } > > and we keep exposing to the world the OCELOT_COMMON_STATS macro and > whatever else is needed for VSC9959 to construct its own vsc9959_stats_layout. > > Or..... hmm (sorry, this is a single-pass email, not gonna delete > anything previous), maybe we could implement the helper function like > this: > > static const struct ocelot_stat_layout ocelot_stats_layout[OCELOT_NUM_STATS] = { > OCELOT_COMMON_STATS, > }; > > static const struct ocelot_stat_layout ocelot_mm_stats_layout[OCELOT_NUM_STATS] = { > OCELOT_COMMON_STATS, > OCELOT_STAT(RX_ASSEMBLY_ERRS), > OCELOT_STAT(RX_SMD_ERRS), > OCELOT_STAT(RX_ASSEMBLY_OK), > OCELOT_STAT(RX_MERGE_FRAGMENTS), > OCELOT_STAT(TX_MERGE_FRAGMENTS), > OCELOT_STAT(RX_PMAC_OCTETS), > OCELOT_STAT(RX_PMAC_UNICAST), > OCELOT_STAT(RX_PMAC_MULTICAST), > OCELOT_STAT(RX_PMAC_BROADCAST), > OCELOT_STAT(RX_PMAC_SHORTS), > OCELOT_STAT(RX_PMAC_FRAGMENTS), > OCELOT_STAT(RX_PMAC_JABBERS), > OCELOT_STAT(RX_PMAC_CRC_ALIGN_ERRS), > OCELOT_STAT(RX_PMAC_SYM_ERRS), > OCELOT_STAT(RX_PMAC_64), > OCELOT_STAT(RX_PMAC_65_127), > OCELOT_STAT(RX_PMAC_128_255), > OCELOT_STAT(RX_PMAC_256_511), > OCELOT_STAT(RX_PMAC_512_1023), > OCELOT_STAT(RX_PMAC_1024_1526), > OCELOT_STAT(RX_PMAC_1527_MAX), > OCELOT_STAT(RX_PMAC_PAUSE), > OCELOT_STAT(RX_PMAC_CONTROL), > OCELOT_STAT(RX_PMAC_LONGS), > OCELOT_STAT(TX_PMAC_OCTETS), > OCELOT_STAT(TX_PMAC_UNICAST), > OCELOT_STAT(TX_PMAC_MULTICAST), > OCELOT_STAT(TX_PMAC_BROADCAST), > OCELOT_STAT(TX_PMAC_PAUSE), > OCELOT_STAT(TX_PMAC_64), > OCELOT_STAT(TX_PMAC_65_127), > OCELOT_STAT(TX_PMAC_128_255), > OCELOT_STAT(TX_PMAC_256_511), > OCELOT_STAT(TX_PMAC_512_1023), > OCELOT_STAT(TX_PMAC_1024_1526), > OCELOT_STAT(TX_PMAC_1527_MAX), > }; > > static const struct ocelot_stat_layout * > ocelot_get_stats_layout(struct ocelot *ocelot) > { > if (ocelot->mm_supported) > return ocelot_mm_stats_layout; // common + MM stats > > return ocelot_stats_layout; // just common stats > } > > Then, setting mm_supported = true from vsc9959 would be enough, no need > to provide its own stats layout, no need to sort/merge anything. > > How does this sound? That should work. If there end up being 10 different struct ocelot_stat_layout[]s, we might reconsider... but in the foreseeable future there will only be two. So this applies to patch 2 of my set, which means I'll pretty much keep it as-is. The get_stats_layout and the ocelot_mm_stats_layout can be added when the 9959 stuff gets applied. Thanks for the feedback / suggestions as always!
On Tue, Nov 15, 2022 at 09:10:57AM -0800, Colin Foster wrote: > That should work. If there end up being 10 different struct > ocelot_stat_layout[]s, we might reconsider... but in the foreseeable > future there will only be two. > > So this applies to patch 2 of my set, which means I'll pretty much keep > it as-is. The get_stats_layout and the ocelot_mm_stats_layout can be > added when the 9959 stuff gets applied. Sounds good.
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c index dd3a18cc89dd..e2ad9be11287 100644 --- a/drivers/net/dsa/ocelot/felix.c +++ b/drivers/net/dsa/ocelot/felix.c @@ -1370,7 +1370,6 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports) return -ENOMEM; ocelot->map = felix->info->map; - ocelot->stats_layout = felix->info->stats_layout; ocelot->num_mact_rows = felix->info->num_mact_rows; ocelot->vcap = felix->info->vcap; ocelot->vcap_pol.base = felix->info->vcap_pol_base; diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h index c9c29999c336..31fdbe75654d 100644 --- a/drivers/net/dsa/ocelot/felix.h +++ b/drivers/net/dsa/ocelot/felix.h @@ -28,7 +28,6 @@ struct felix_info { const struct ocelot_ops *ops; const u32 *port_modes; int num_mact_rows; - const struct ocelot_stat_layout *stats_layout; int num_ports; int num_tx_queues; struct vcap_props *vcap; diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c index 26a35ae322d1..e742cf48bc54 100644 --- a/drivers/net/dsa/ocelot/felix_vsc9959.c +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c @@ -565,10 +565,6 @@ static const struct reg_field vsc9959_regfields[REGFIELD_MAX] = { [SYS_PAUSE_CFG_PAUSE_ENA] = REG_FIELD_ID(SYS_PAUSE_CFG, 0, 1, 7, 4), }; -static const struct ocelot_stat_layout vsc9959_stats_layout[OCELOT_NUM_STATS] = { - OCELOT_COMMON_STATS, -}; - static const struct vcap_field vsc9959_vcap_es0_keys[] = { [VCAP_ES0_EGR_PORT] = { 0, 3}, [VCAP_ES0_IGR_PORT] = { 3, 3}, @@ -2575,7 +2571,6 @@ static const struct felix_info felix_info_vsc9959 = { .regfields = vsc9959_regfields, .map = vsc9959_regmap, .ops = &vsc9959_ops, - .stats_layout = vsc9959_stats_layout, .vcap = vsc9959_vcap_props, .vcap_pol_base = VSC9959_VCAP_POLICER_BASE, .vcap_pol_max = VSC9959_VCAP_POLICER_MAX, diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c index 7af33b2c685d..a383fae4e218 100644 --- a/drivers/net/dsa/ocelot/seville_vsc9953.c +++ b/drivers/net/dsa/ocelot/seville_vsc9953.c @@ -543,10 +543,6 @@ static const struct reg_field vsc9953_regfields[REGFIELD_MAX] = { [SYS_PAUSE_CFG_PAUSE_ENA] = REG_FIELD_ID(SYS_PAUSE_CFG, 0, 1, 11, 4), }; -static const struct ocelot_stat_layout vsc9953_stats_layout[OCELOT_NUM_STATS] = { - OCELOT_COMMON_STATS, -}; - static const struct vcap_field vsc9953_vcap_es0_keys[] = { [VCAP_ES0_EGR_PORT] = { 0, 4}, [VCAP_ES0_IGR_PORT] = { 4, 4}, @@ -996,7 +992,6 @@ static const struct felix_info seville_info_vsc9953 = { .regfields = vsc9953_regfields, .map = vsc9953_regmap, .ops = &vsc9953_ops, - .stats_layout = vsc9953_stats_layout, .vcap = vsc9953_vcap_props, .vcap_pol_base = VSC9953_VCAP_POLICER_BASE, .vcap_pol_max = VSC9953_VCAP_POLICER_MAX, diff --git a/drivers/net/ethernet/mscc/ocelot_stats.c b/drivers/net/ethernet/mscc/ocelot_stats.c index dbd20b125cea..5dc132f61d6a 100644 --- a/drivers/net/ethernet/mscc/ocelot_stats.c +++ b/drivers/net/ethernet/mscc/ocelot_stats.c @@ -9,6 +9,10 @@ #include <linux/workqueue.h> #include "ocelot.h" +static const struct ocelot_stat_layout ocelot_stats_layout[OCELOT_NUM_STATS] = { + OCELOT_COMMON_STATS, +}; + /* Read the counters from hardware and keep them in region->buf. * Caller must hold &ocelot->stat_view_lock. */ @@ -93,10 +97,10 @@ void ocelot_get_strings(struct ocelot *ocelot, int port, u32 sset, u8 *data) return; for (i = 0; i < OCELOT_NUM_STATS; i++) { - if (ocelot->stats_layout[i].name[0] == '\0') + if (ocelot_stats_layout[i].name[0] == '\0') continue; - memcpy(data + i * ETH_GSTRING_LEN, ocelot->stats_layout[i].name, + memcpy(data + i * ETH_GSTRING_LEN, ocelot_stats_layout[i].name, ETH_GSTRING_LEN); } } @@ -137,7 +141,7 @@ int ocelot_get_sset_count(struct ocelot *ocelot, int port, int sset) return -EOPNOTSUPP; for (i = 0; i < OCELOT_NUM_STATS; i++) - if (ocelot->stats_layout[i].name[0] != '\0') + if (ocelot_stats_layout[i].name[0] != '\0') num_stats++; return num_stats; @@ -154,7 +158,7 @@ static void ocelot_port_ethtool_stats_cb(struct ocelot *ocelot, int port, for (i = 0; i < OCELOT_NUM_STATS; i++) { int index = port * OCELOT_NUM_STATS + i; - if (ocelot->stats_layout[i].name[0] == '\0') + if (ocelot_stats_layout[i].name[0] == '\0') continue; *data++ = ocelot->stats[index]; @@ -389,10 +393,10 @@ static int ocelot_prepare_stats_regions(struct ocelot *ocelot) INIT_LIST_HEAD(&ocelot->stats_regions); for (i = 0; i < OCELOT_NUM_STATS; i++) { - if (!ocelot->stats_layout[i].reg) + if (!ocelot_stats_layout[i].reg) continue; - if (region && ocelot->stats_layout[i].reg == last + 4) { + if (region && ocelot_stats_layout[i].reg == last + 4) { region->count++; } else { region = devm_kzalloc(ocelot->dev, sizeof(*region), @@ -400,12 +404,12 @@ static int ocelot_prepare_stats_regions(struct ocelot *ocelot) if (!region) return -ENOMEM; - region->base = ocelot->stats_layout[i].reg; + region->base = ocelot_stats_layout[i].reg; region->count = 1; list_add_tail(®ion->node, &ocelot->stats_regions); } - last = ocelot->stats_layout[i].reg; + last = ocelot_stats_layout[i].reg; } list_for_each_entry(region, &ocelot->stats_regions, node) { diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c index 93431d2ff4f1..125262e16351 100644 --- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c +++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c @@ -138,7 +138,6 @@ static int ocelot_chip_init(struct ocelot *ocelot, const struct ocelot_ops *ops) int ret; ocelot->map = ocelot_regmap; - ocelot->stats_layout = ocelot_stats_layout; ocelot->num_mact_rows = 1024; ocelot->ops = ops; diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h index 967ba30ea636..995b5950afe6 100644 --- a/include/soc/mscc/ocelot.h +++ b/include/soc/mscc/ocelot.h @@ -967,7 +967,6 @@ struct ocelot { struct regmap *targets[TARGET_MAX]; struct regmap_field *regfields[REGFIELD_MAX]; const u32 *const *map; - const struct ocelot_stat_layout *stats_layout; struct list_head stats_regions; u32 pool_size[OCELOT_SB_NUM][OCELOT_SB_POOL_NUM];
Ever since commit 4d1d157fb6a4 ("net: mscc: ocelot: share the common stat definitions between all drivers") the stats_layout entry in ocelot and felix drivers have become redundant. Remove the unnecessary code. Suggested-by: Vladimir Oltean <olteanv@gmail.com> Signed-off-by: Colin Foster <colin.foster@in-advantage.com> --- drivers/net/dsa/ocelot/felix.c | 1 - drivers/net/dsa/ocelot/felix.h | 1 - drivers/net/dsa/ocelot/felix_vsc9959.c | 5 ----- drivers/net/dsa/ocelot/seville_vsc9953.c | 5 ----- drivers/net/ethernet/mscc/ocelot_stats.c | 20 ++++++++++++-------- drivers/net/ethernet/mscc/ocelot_vsc7514.c | 1 - include/soc/mscc/ocelot.h | 1 - 7 files changed, 12 insertions(+), 22 deletions(-)