From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com [209.85.208.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2B4AA131722; Fri, 16 Feb 2024 18:24:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708107848; cv=none; b=PFXuEOuny1WLafaJwC67g/2guzNYNMyIjV7frtZJ+jHck/kBdVAuzNpUi+GcbBJawPRr+jSN2D8xeW0ihmrH51p4HnMv6Eb0DuRZ+/PBoAkvxqy5PuzemF7crDXtSy2wHNV2ojKCd3Xj/ZUNv0sgxwmwK4sKYb3My1ieRXNLajc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708107848; c=relaxed/simple; bh=SqYPKWDLnuGJK5U2Ewke7cCIfy9hQPIkVCWR4ZEMhwk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lsbUTFKfP6DRcKEOvq7QcN2r/JEQsP2acfnn8QBcrQcgRjwcsB/+rfb3YM71ThOsJN3Na5wHKNYVZ4xol7wVDJ7hbIXwXi4roiaKwPkXfY6V9rm+5ReAxqPBcgAcR+sKpIdmg6KG5fw4ef/cwVfto69LF37vmQNvYSCzLFQTqvI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ERjQRRjg; arc=none smtp.client-ip=209.85.208.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ERjQRRjg" Received: by mail-lj1-f172.google.com with SMTP id 38308e7fff4ca-2d0b4ea773eso29564231fa.0; Fri, 16 Feb 2024 10:24:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708107845; x=1708712645; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=cVDKfJQ+tCuSdXhY6TiavPFjaspEvu7Ad4BGUjaCUAE=; b=ERjQRRjgn+x/cAaNSK9JRMsxB2vlBL0NqUMggOC3/t8xAulkX2Oz8vuTV06PC9IRyl sey0Ve5nzE7NUVWp+rrNfxM+fYAQfBoqtFGSi0hrFvRAuzQLhC6jmCek0hz06fF75Wto VbVTEearJco9HJUKdoewMRR9iq53NLQxbUdFnxl5pmfNooEvYvlYf22jbtWAPLWNlpQD P1f4BwZWm7MB4if5CNxtsEXDFYa+ryq8OZG2yNmEVEt+uvhbWZ9Nbnz/m1w8813QD/rM RnQ4ekDO9D40fwydnxLHe1rYkj8nJFTZZfz2LNY1qjOMBb//buOuckiMjZCOSlkrBp0K 1MCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708107845; x=1708712645; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=cVDKfJQ+tCuSdXhY6TiavPFjaspEvu7Ad4BGUjaCUAE=; b=dPZsbdg3RcrjqswWaLFvZVMo9AjsbdU9WAhiP5KtzzBd4KCtddkpQVi3VgEIcTjHd9 GVBW0svlHt780Frj8QaNVeINBcQYn9wzkDha1Rvx58O46hC56KtLMw26DYAOisrhPgnq AAPa+KKmFpTixs1DG0D5PRQthYPlJNI3ekdwjq6MJlvU5CeIl/iAKhela3wGBFunL0v/ VZjI8tvCo6dcrEMHN/qLeX1hLTuUAw5XK5w959Tird93I8i7fAz7NOdNSe3kW2bTdOrd sPeUMdZrs8wvbbLo4IwFhYnNbpQVNZF/QuKUfzEvefMjZBYqo1HgPsbbRuZW8hgBVjLr Zr1g== X-Forwarded-Encrypted: i=1; AJvYcCWX5A3hrEJnTwL90NLLpO6ggIVAsbOslYdre15l70hv3GX5bm0LEVpKcGPVPAqJ9hHv94RD/ToByzxAg5Pxk/D0llDRySgPPqRgSJWw/+GZwvzu3008j1TwfbHJYQurtUITBr+1 X-Gm-Message-State: AOJu0YwRggaiRU4qm3P5ISQ/w6skovBIZ9bai3nSXmvVY/KCRaXJb8y5 827lPLlUkVRKB9teE4/FXBgWsZo1aCYBCSqxPWoiVQuQre/ZJDnh X-Google-Smtp-Source: AGHT+IHz8SLuj0HeZjrA59fbpHxEZQx4CNdTDrSGjiJcc/KB/eOtXjEXu1w0IDtU5LUHJY6IknyBAg== X-Received: by 2002:a2e:7202:0:b0:2d0:e730:b7d5 with SMTP id n2-20020a2e7202000000b002d0e730b7d5mr3975909ljc.1.1708107844735; Fri, 16 Feb 2024 10:24:04 -0800 (PST) Received: from mobilestation ([178.176.56.174]) by smtp.gmail.com with ESMTPSA id t25-20020a2e9d19000000b002d100bd4cdbsm40573lji.45.2024.02.16.10.24.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 16 Feb 2024 10:24:04 -0800 (PST) Date: Fri, 16 Feb 2024 21:24:01 +0300 From: Serge Semin To: Florian Fainelli , Jesper Nilsson Cc: Alexandre Torgue , Jose Abreu , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel@axis.com Subject: Re: [PATCH] net: stmmac: mmc_core: Assign, don't add interrupt registers Message-ID: <53fctveh4pl3c5wys37c2wcpbsxr7tggw3d3y5eudgrbvr2vdl@fbqc2meg5yv3> References: <20240216-stmmac_stats-v1-1-7065fa4613f8@axis.com> <61bdd802-abe4-4544-8e48-9493a6bb99c8@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <61bdd802-abe4-4544-8e48-9493a6bb99c8@gmail.com> On Fri, Feb 16, 2024 at 09:13:51AM -0800, Florian Fainelli wrote: > On 2/16/24 07:24, Jesper Nilsson wrote: > > The MMC IPC interrupt status and interrupt mask registers are of > > little use as Ethernet statistics, but incrementing counters > > based on the current interrupt and interrupt mask registers > > makes them worse than useless. > > > > For example, if the interrupt mask is set to 0x08420842, > > the current code will increment by that amount each iteration, > > leading to the following sequence of nonsense: > > > > mmc_rx_ipc_intr_mask: 969816526 > > mmc_rx_ipc_intr_mask: 1108361744 > > > > Change the increment to a straight assignment to make the > > statistics at least nominally useful. > > > > Signed-off-by: Jesper Nilsson > > --- > > drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c > > index 6a7c1d325c46..6051a22b3cec 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c > > @@ -280,8 +280,8 @@ static void dwmac_mmc_read(void __iomem *mmcaddr, struct stmmac_counters *mmc) > > mmc->mmc_rx_vlan_frames_gb += readl(mmcaddr + MMC_RX_VLAN_FRAMES_GB); > > mmc->mmc_rx_watchdog_error += readl(mmcaddr + MMC_RX_WATCHDOG_ERROR); > > /* IPC */ > > - mmc->mmc_rx_ipc_intr_mask += readl(mmcaddr + MMC_RX_IPC_INTR_MASK); > > - mmc->mmc_rx_ipc_intr += readl(mmcaddr + MMC_RX_IPC_INTR); > > + mmc->mmc_rx_ipc_intr_mask = readl(mmcaddr + MMC_RX_IPC_INTR_MASK); > > + mmc->mmc_rx_ipc_intr = readl(mmcaddr + MMC_RX_IPC_INTR); > > So in premise I agree with the patch, that incrementing those is not the > right way to go about them. However these registers are currently provided > as part of the statistics set, but they should instead be accessed via the > register dumping method. > > In either case you will get at best a snapshot of those two registers at any > given time and I suppose this can help diagnose a stuck RX condition, but > not much more than that. Could you please clarify why do those CSRs state need to be exposed in the statistics anyway? Who would need such information really? Wouldn't that be better to just drop the stmmac_counters::{mmc_rx_ipc_intr_mask,mmc_rx_ipc_intr} fields? Is it because of the statistics nodes are a kind of kernel ABI? Even in that case I don't see much reason to support something that has been absolutely useless so far seeing the nodes currently returning basically some random values. -Serge(y) > -- > Florian > > 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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5D75EC48260 for ; Fri, 16 Feb 2024 18:24:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=V/7nxfVqLwVN5yqctxBVtrknSJiL+ILtFUgtmGUs6ho=; b=RilN9LlMm+Ko76 FmrCLxpOuZ8Ca03JVD16oZaGXJR4CBPeJ4Ux5lvfrqSnlXHYNH1jVKe9T7fKUxgbTa2FKw/4KGaPN woz4opRFRYG6BqIOb49sd1kumEI9zJ7rrKC1EKEmNhxyU5EfGHFWXr3MFC8Rc7cpMMcG/EgqJaQyA SDmHC5kiJPwnDoVV9w0OmH4kpHHNUTEuOHXUxfcsRNnvX0IVaz+UI4USPxv18e8Ogm1s6rnKcEZbZ SbkHQkr0SIktIBuchTClh169AspHofN435PiaP32mbRpHc6+lM8lslClmNnBcJgFajQpLBSI6Baw9 LCckkqBTw0aHDmvUMcpA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rb2sb-00000003Jd7-3K7K; Fri, 16 Feb 2024 18:24:09 +0000 Received: from mail-lj1-x234.google.com ([2a00:1450:4864:20::234]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rb2sZ-00000003JcF-3Kks for linux-arm-kernel@lists.infradead.org; Fri, 16 Feb 2024 18:24:09 +0000 Received: by mail-lj1-x234.google.com with SMTP id 38308e7fff4ca-2d094bc2244so31124221fa.1 for ; Fri, 16 Feb 2024 10:24:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708107845; x=1708712645; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=cVDKfJQ+tCuSdXhY6TiavPFjaspEvu7Ad4BGUjaCUAE=; b=moNB1dXATZQ9hOXyxPd3rGRI1Y+5Fe/Vj5/gqHjWzuRk3tLy+sSq+5H1VVz0Xi0vkT ZjCDXR4T1tsNS8sXHSJsSNkzhEPVLrKBdP5/hbGneZ/YTAr5ijT9tb6gkT2s4mv5fBZJ 0DTOzVLWHDdWe/jt2Zxr6crR9BIN1/ANvxfIfX1236fI+BmVD2gxm+ilN/5e88P3iJQh Sz78h5ZOPC1vu934kDw/dRE20VVt1F9FneIuzefJtvJ9m5F9ZYMSVNfvZ1fH3YirKUPH teYPNOA4a1km/HKShKU3D9K/AAx6B3ARaoJjwMch3/PLNIy8SqyA2MLo2w43+VVRBAJD FvFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708107845; x=1708712645; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=cVDKfJQ+tCuSdXhY6TiavPFjaspEvu7Ad4BGUjaCUAE=; b=kl04Tay3GuaxpDDVhPoHRx6yx8MQpitN/OpIwJnrLX3HNaD48TVu29xo3ioXWHfXdg z0XEOOZKkFtTwFvlOHd3VHxOEYpspAur6m6WbWHWMelIuPrzNOtKwlwLGApPC/iEXPgw xzHgrCLjXkAMn/h4gWpHpF+J/c5u4orQpz9HucChH0t2uNSF9hNlLQYj0BJEXlZRscB3 qBn5iFS+6XvMPlx2Pc0e+4rUlF/JHxtgAhpIM95YswVAX3vGK4fAfOsPhFlpPdVVOoW1 mlnlftvz1j2rDvZlsJqegjIzi7LtkrZOuF6Ul24HTT0JLqd0y89oY1jGWVVSzPvwNpFA 0M/g== X-Forwarded-Encrypted: i=1; AJvYcCXiYTJCfv4/3FPjJt8tNo16z887FIN9com4vWAXQynWP9EBpwMztzF4D6VFbXADT8peS0wrqCtXmrRQLV5Go3BLdsdK6C9pw6e+EdJIzTxru9MaiDE= X-Gm-Message-State: AOJu0YzKq3Y3p54fSywU9sItqCYLRUXe5rR/t/VDP0GskbpqBhgYBoU9 gxHmOQg1ksw6+HbkAPrENIagmV7gkuRJp1qnRo+wHxiYwiRXmE1Y X-Google-Smtp-Source: AGHT+IHz8SLuj0HeZjrA59fbpHxEZQx4CNdTDrSGjiJcc/KB/eOtXjEXu1w0IDtU5LUHJY6IknyBAg== X-Received: by 2002:a2e:7202:0:b0:2d0:e730:b7d5 with SMTP id n2-20020a2e7202000000b002d0e730b7d5mr3975909ljc.1.1708107844735; Fri, 16 Feb 2024 10:24:04 -0800 (PST) Received: from mobilestation ([178.176.56.174]) by smtp.gmail.com with ESMTPSA id t25-20020a2e9d19000000b002d100bd4cdbsm40573lji.45.2024.02.16.10.24.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 16 Feb 2024 10:24:04 -0800 (PST) Date: Fri, 16 Feb 2024 21:24:01 +0300 From: Serge Semin To: Florian Fainelli , Jesper Nilsson Cc: Alexandre Torgue , Jose Abreu , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel@axis.com Subject: Re: [PATCH] net: stmmac: mmc_core: Assign, don't add interrupt registers Message-ID: <53fctveh4pl3c5wys37c2wcpbsxr7tggw3d3y5eudgrbvr2vdl@fbqc2meg5yv3> References: <20240216-stmmac_stats-v1-1-7065fa4613f8@axis.com> <61bdd802-abe4-4544-8e48-9493a6bb99c8@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <61bdd802-abe4-4544-8e48-9493a6bb99c8@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240216_102407_858990_C492DD10 X-CRM114-Status: GOOD ( 28.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Feb 16, 2024 at 09:13:51AM -0800, Florian Fainelli wrote: > On 2/16/24 07:24, Jesper Nilsson wrote: > > The MMC IPC interrupt status and interrupt mask registers are of > > little use as Ethernet statistics, but incrementing counters > > based on the current interrupt and interrupt mask registers > > makes them worse than useless. > > > > For example, if the interrupt mask is set to 0x08420842, > > the current code will increment by that amount each iteration, > > leading to the following sequence of nonsense: > > > > mmc_rx_ipc_intr_mask: 969816526 > > mmc_rx_ipc_intr_mask: 1108361744 > > > > Change the increment to a straight assignment to make the > > statistics at least nominally useful. > > > > Signed-off-by: Jesper Nilsson > > --- > > drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c > > index 6a7c1d325c46..6051a22b3cec 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c > > @@ -280,8 +280,8 @@ static void dwmac_mmc_read(void __iomem *mmcaddr, struct stmmac_counters *mmc) > > mmc->mmc_rx_vlan_frames_gb += readl(mmcaddr + MMC_RX_VLAN_FRAMES_GB); > > mmc->mmc_rx_watchdog_error += readl(mmcaddr + MMC_RX_WATCHDOG_ERROR); > > /* IPC */ > > - mmc->mmc_rx_ipc_intr_mask += readl(mmcaddr + MMC_RX_IPC_INTR_MASK); > > - mmc->mmc_rx_ipc_intr += readl(mmcaddr + MMC_RX_IPC_INTR); > > + mmc->mmc_rx_ipc_intr_mask = readl(mmcaddr + MMC_RX_IPC_INTR_MASK); > > + mmc->mmc_rx_ipc_intr = readl(mmcaddr + MMC_RX_IPC_INTR); > > So in premise I agree with the patch, that incrementing those is not the > right way to go about them. However these registers are currently provided > as part of the statistics set, but they should instead be accessed via the > register dumping method. > > In either case you will get at best a snapshot of those two registers at any > given time and I suppose this can help diagnose a stuck RX condition, but > not much more than that. Could you please clarify why do those CSRs state need to be exposed in the statistics anyway? Who would need such information really? Wouldn't that be better to just drop the stmmac_counters::{mmc_rx_ipc_intr_mask,mmc_rx_ipc_intr} fields? Is it because of the statistics nodes are a kind of kernel ABI? Even in that case I don't see much reason to support something that has been absolutely useless so far seeing the nodes currently returning basically some random values. -Serge(y) > -- > Florian > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel