From patchwork Wed Aug 19 19:24:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Corinna Vinschen X-Patchwork-Id: 508835 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 9621E14032C for ; Thu, 20 Aug 2015 05:24:22 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751477AbbHSTYS (ORCPT ); Wed, 19 Aug 2015 15:24:18 -0400 Received: from mail-n.franken.de ([193.175.24.27]:37157 "EHLO mail-n.franken.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750933AbbHSTYR (ORCPT ); Wed, 19 Aug 2015 15:24:17 -0400 Received: from aqua.hirmke.de (aquarius.franken.de [193.175.24.89]) by mail-n.franken.de (Postfix) with ESMTP id DAD8A1C0C0BCF; Wed, 19 Aug 2015 21:24:14 +0200 (CEST) Received: from calimero.vinschen.de (calimero.vinschen.de [192.168.129.6]) by aqua.hirmke.de (Postfix) with ESMTP id 3D7745E01EC; Wed, 19 Aug 2015 21:24:14 +0200 (CEST) Received: by calimero.vinschen.de (Postfix, from userid 500) id 35865A807C8; Wed, 19 Aug 2015 21:24:14 +0200 (CEST) Date: Wed, 19 Aug 2015 21:24:14 +0200 From: Corinna Vinschen To: "netdev@vger.kernel.org" Cc: Hayes Wang , Francois Romieu , nic_swsd , Ivan Vecera , David Miller Subject: Re: [PATCH] r8169: Add values missing in @get_stats64 from HW counters Message-ID: <20150819192414.GC30904@calimero.vinschen.de> Mail-Followup-To: "netdev@vger.kernel.org" , Hayes Wang , Francois Romieu , nic_swsd , Ivan Vecera , David Miller References: <1439888664-28620-1-git-send-email-vinschen@redhat.com> <20150818214017.GA14914@electric-eye.fr.zoreil.com> <0835B3720019904CB8F7AA43166CEEB2F4B846@RTITMBSV03.realtek.com.tw> <20150819091318.GV17917@calimero.vinschen.de> <0835B3720019904CB8F7AA43166CEEB2F4BF49@RTITMBSV03.realtek.com.tw> <20150819130744.GA24832@calimero.vinschen.de> <20150819142416.GA26271@calimero.vinschen.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20150819142416.GA26271@calimero.vinschen.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Aug 19 16:24, Corinna Vinschen wrote: > On Aug 19 15:07, Corinna Vinschen wrote: > > On Aug 19 09:31, Hayes Wang wrote: > > > Corinna Vinschen [mailto:vinschen@redhat.com] > > > > Sent: Wednesday, August 19, 2015 5:13 PM > > > [...] > > > > > It could be cleared by setting bit 0, such as rtl_tally_reset() of r8152. > > > > > > > > Is it safe to assume that this is implemented in all NICs covered by r8169? > > > > > > It is supported from RTL8111C. That is, RTL_GIGA_MAC_VER_19 and later. > > > > Thanks. In that case I would prefer the same generic method for all > > chip versions, so I'd opt for storing the offset values at rtl_open > > time as my patch is doing right now. Is that acceptable? > > > > If so, wouldn't it make even more sense to use the hardware collected > > information in @get_stats64 throughout, except for the numbers collected > > *only* in software? > > > > I would be willing to propose a matching patch. > > It just occured to me that the combination of resetting the counters on > post-RTL_GIGA_MAC_VER_19 chips plus offset handling would be quite > nice, because it would reset also the small 16 and 32 bit counters. > > So I'd like to propose a patch which combines both techniques, if that's > an acceptable way to go forward. > > Btw., does setting the reset bit in CounterAddrLow work the same way as > setting the CounterDump flag? I.e, does the driver have to wait for the > hardware to set the bit to 0 again to be sure the reset is finished? Here's a preliminary implementation of the counter reset code. It's based on my previous patch. It calls the new rtl8169_reset_counters prior to rtl8169_update_counters from rtl_open. I tested the patch successfully on a RTL8111f NIC (RTL_GIGA_MAC_VER_35). This isn't an official patch submission, I'm just sending this for further discussion. I'll make a full patch submission if the code is acceptable. Thanks, Corinna diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index e7c7955..204f7e7 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -637,6 +637,9 @@ enum rtl_register_content { /* _TBICSRBit */ TBILinkOK = 0x02000000, + /* ResetCounterCommand */ + CounterReset = 0x1, + /* DumpCounterCommand */ CounterDump = 0x8, @@ -2188,6 +2191,31 @@ static int rtl8169_get_sset_count(struct net_device *dev, int sset) } } +DECLARE_RTL_COND(rtl_reset_counters_cond) +{ + void __iomem *ioaddr = tp->mmio_addr; + + return RTL_R32(CounterAddrLow) & CounterReset; +} + +static void rtl8169_reset_counters(struct net_device *dev) +{ + struct rtl8169_private *tp = netdev_priv(dev); + void __iomem *ioaddr = tp->mmio_addr; + + /* + * Versions prior to RTL_GIGA_MAC_VER_19 don't support resetting the + * tally counters. + */ + if (tp->mac_version >= RTL_GIGA_MAC_VER_19) { + RTL_W32(CounterAddrHigh, 0); + RTL_W32(CounterAddrLow, CounterReset); + if (!rtl_udelay_loop_wait_low(tp, &rtl_reset_counters_cond, + 10, 1000)) + netif_warn(tp, hw, dev, "counter reset failed\n"); + } +} + DECLARE_RTL_COND(rtl_counters_cond) { void __iomem *ioaddr = tp->mmio_addr; @@ -2234,11 +2262,10 @@ static void rtl8169_init_tc_counter_offset(struct net_device *dev) struct rtl8169_private *tp = netdev_priv(dev); /* - * rtl8169_init_tc_counter_offset is called from rtl_open. The tally - * counters are only reset by a power cycle, while the counter values - * collected by the driver are reset at every driver unload/load cycle. - * There's also no (documented?) way to reset the tally counters - * programatically. + * rtl8169_init_tc_counter_offset is called from rtl_open. On chip + * versions prior to RTL_GIGA_MAC_VER_19 the tally counters are only + * reset by a power cycle, while the counter values collected by the + * driver are reset at every driver unload/load cycle. * * To make sure the HW values returned by @get_stats64 match the SW * values, we collect the initial values at first open(*) and use them @@ -2252,6 +2279,8 @@ static void rtl8169_init_tc_counter_offset(struct net_device *dev) if (tp->tc_offset.inited) return; + rtl8169_reset_counters(dev); + rtl8169_update_counters(dev); tp->tc_offset.tx_errors = tp->counters.tx_errors;