Messages in this thread | | | Date | Thu, 14 Dec 2023 12:04:35 +0200 | Subject | Re: [PATCH v8 net-next 11/11] net: ethernet: ti: am65-cpsw: Fix get_eth_mac_stats | From | Roger Quadros <> |
| |
On 13/12/2023 15:54, Vladimir Oltean wrote: > On Wed, Dec 13, 2023 at 01:07:21PM +0200, Roger Quadros wrote: >> We do not support individual stats for PMAC and EMAC so >> report only aggregate stats. >> >> Fixes: 67372d7a85fc ("net: ethernet: am65-cpsw: Add standard Ethernet MAC stats to ethtool") >> Signed-off-by: Roger Quadros <rogerq@kernel.org> >> --- >> drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> Changelog: >> >> v8: initial commit >> >> diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c >> index d2baffb05d55..35e318458b0c 100644 >> --- a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c >> +++ b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c >> @@ -671,6 +671,9 @@ static void am65_cpsw_get_eth_mac_stats(struct net_device *ndev, >> >> stats = port->stat_base; >> >> + if (s->src != ETHTOOL_MAC_STATS_SRC_AGGREGATE) >> + return; >> + >> s->FramesTransmittedOK = readl_relaxed(&stats->tx_good_frames); >> s->SingleCollisionFrames = readl_relaxed(&stats->tx_single_coll_frames); >> s->MultipleCollisionFrames = readl_relaxed(&stats->tx_mult_coll_frames); >> -- >> 2.34.1 >> > > Fixes are only fixes if they address a visible issue. And the blamed > commit is the one that made the issue visible - the same one that > "git bisect" would lead to - not necessarily the commit that introduced > the code being changed. > > If you look at net/ethtool/stats.c, it will only accept ETHTOOL_MAC_STATS_SRC_AGGREGATE > for drivers that don't support the MAC Merge layer. > > if ((src == ETHTOOL_MAC_STATS_SRC_EMAC || > src == ETHTOOL_MAC_STATS_SRC_PMAC) && > !__ethtool_dev_mm_supported(dev)) { > NL_SET_ERR_MSG_MOD(info->extack, > "Device does not support MAC merge layer"); > ethnl_ops_complete(dev); > return -EOPNOTSUPP; > }
Got it.
> > So, there was nothing broken in commit 67372d7a85fc ("net: ethernet: > am65-cpsw: Add standard Ethernet MAC stats to ethtool"). > > The first broken commit is when you add support for get_mm(), such that > __ethtool_dev_mm_supported() returns true. > > And because you don't add bugs in the code just to fix them later in the > series, you need to order the patches such that all the dependencies for > get_mm() are in place before the get_mm() support is added. > > Translated to your case, this patch must not be 11/11, and it must have > the Fixes: tag dropped, and it must explain in the commit message that > it is preparatory work.
OK. I take care of this in next spin. Thanks. > > -- > pw-bot: changes-requested
-- cheers, -roger
| |