From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,UNWANTED_LANGUAGE_BODY,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E0858C282C4 for ; Sat, 9 Feb 2019 17:22:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A9DB82192B for ; Sat, 9 Feb 2019 17:22:57 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=lunn.ch header.i=@lunn.ch header.b="QTOab21C" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727328AbfBIRW4 (ORCPT ); Sat, 9 Feb 2019 12:22:56 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:45304 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727021AbfBIRW4 (ORCPT ); Sat, 9 Feb 2019 12:22:56 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=hg4iJvGC0EqZQzbow1QlQIjMSeYqQ/t8IpdGq/4deA4=; b=QTOab21CIaLeb4EMM4AMvcWIUz 9uKaYOuTOM6xmxRUsmtzVd9mlCX+tdO7s7MnvIx+tHfomIwcYjqv4XcOhSD5fm50lCGr9XXKY0jJG +V2t1L2dmlrtfoF6Sic+aRE4d51VH/HCdvkOfcV9gcXXn3UKY2Yy+gXL8J60InsYhAW0=; Received: from andrew by vps0.lunn.ch with local (Exim 4.89) (envelope-from ) id 1gsWL3-0000Dg-SA; Sat, 09 Feb 2019 18:22:49 +0100 Date: Sat, 9 Feb 2019 18:22:49 +0100 From: Andrew Lunn To: Tristram.Ha@microchip.com Cc: Sergio Paracuellos , Florian Fainelli , Pavel Machek , UNGLinuxDriver@microchip.com, netdev@vger.kernel.org Subject: Re: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support Message-ID: <20190209172249.GG30856@lunn.ch> References: <1549598829-25970-1-git-send-email-Tristram.Ha@microchip.com> <1549598829-25970-3-git-send-email-Tristram.Ha@microchip.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1549598829-25970-3-git-send-email-Tristram.Ha@microchip.com> 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 Thu, Feb 07, 2019 at 08:07:07PM -0800, Tristram.Ha@microchip.com wrote: > From: Tristram Ha > > Add MIB counter reading support. > > Signed-off-by: Tristram Ha > --- > drivers/net/dsa/microchip/ksz9477.c | 139 +++++++++++++++++++++++---------- > drivers/net/dsa/microchip/ksz_common.c | 96 +++++++++++++++++++++++ > drivers/net/dsa/microchip/ksz_common.h | 2 + > drivers/net/dsa/microchip/ksz_priv.h | 7 +- > 4 files changed, 198 insertions(+), 46 deletions(-) > > diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c > index 0fdb22d..4502e13 100644 > --- a/drivers/net/dsa/microchip/ksz9477.c > +++ b/drivers/net/dsa/microchip/ksz9477.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -18,8 +19,8 @@ > #include > > #include "ksz_priv.h" > -#include "ksz_common.h" > #include "ksz9477_reg.h" > +#include "ksz_common.h" > > static const struct { > int index; > @@ -92,6 +93,27 @@ static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset, > ksz_write32(dev, addr, data); > } > > +#define read8_op(addr) \ > +({ \ > + u8 data; \ > + ksz_read8(dev, addr, &data); \ > + data; \ > +}) > + > +#define read32_op(addr) \ > +({ \ > + u32 data; \ > + ksz_read32(dev, addr, &data); \ > + data; \ > +}) These two are not used. Please remove them. > + > +#define pread32_op(addr) \ > +({ \ > + u32 data; \ > + ksz_pread32(dev, port, addr, &data); \ > + data; \ > +}) It works, but it is not nice, and it makes assumptions about how readx_poll_timeout is implemented. > + ret = readx_poll_timeout(pread32_op, REG_PORT_MIB_CTRL_STAT__4, data, > + !(data & MIB_COUNTER_READ), 10, 1000); The macro is only used one, and addr is fixed, REG_PORT_MIB_CTRL_STAT__4. So you can at least replace addr with port, and rename the macro pread32_stat(port). > + /* failed to read MIB. get out of loop */ > + if (ret < 0) { > + dev_dbg(dev->dev, "Failed to get MIB\n"); > + return; > + } > + > + /* count resets upon read */ > + ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data); > + *cnt += data; > +} > + > +static void ksz9477_r_mib_pkt(struct ksz_device *dev, int port, u16 addr, > + u64 *dropped, u64 *cnt) > +{ > + addr = ksz9477_mib_names[addr].index; > + ksz9477_r_mib_cnt(dev, port, addr, cnt); > +} > + > +static void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool freeze) > +{ > + struct ksz_port *p = &dev->ports[port]; > + u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0; Reverse Christmas tree. > + > + /* enable/disable the port for flush/freeze function */ > + mutex_lock(&p->mib.cnt_mutex); > + ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, val); > + > + /* used by MIB counter reading code to know freeze is enabled */ > + p->freeze = freeze; > + mutex_unlock(&p->mib.cnt_mutex); > +} > +static void ksz_mib_read_work(struct work_struct *work) > +{ > + struct ksz_device *dev = > + container_of(work, struct ksz_device, mib_read); > + struct ksz_port *p; > + struct ksz_port_mib *mib; > + int i; > + > + for (i = 0; i < dev->mib_port_cnt; i++) { > + p = &dev->ports[i]; > + if (!p->on) > + continue; > + mib = &p->mib; > + mutex_lock(&mib->cnt_mutex); > + > + /* read only dropped counters when link is not up */ > + if (p->link_just_down) > + p->link_just_down = 0; > + else if (!p->phydev.link) > + mib->cnt_ptr = dev->reg_mib_cnt; This link_just_down stuff is not clear at all. Why can the drop counters not be read when the link is up? > + port_r_cnt(dev, i); > + mutex_unlock(&mib->cnt_mutex); > + } > +}