Message ID | 20240123085037.939471-1-0x1207@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-34944-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:2553:b0:103:945f:af90 with SMTP id p19csp200606dyi; Tue, 23 Jan 2024 00:51:20 -0800 (PST) X-Google-Smtp-Source: AGHT+IGhH1yZZ5UUTqs5LWghG+mQLvoXIWQI14NG4R9SvPpkvg0bYtah7KPfqhWZQC1eifQiZSOd X-Received: by 2002:a81:9112:0:b0:5ff:83be:cf1 with SMTP id i18-20020a819112000000b005ff83be0cf1mr5041183ywg.12.1705999880388; Tue, 23 Jan 2024 00:51:20 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1705999880; cv=pass; d=google.com; s=arc-20160816; b=Qwehzl/Mnpw21OWNu8gxN7CmAgCizm3Mdpp5OmGnU9BWzArbqh/OfwhRcw9/4oT76I IjQ4ptwxqE4t+5u81ULObzeUhyOw5NLJjz3XzUpJAjbh60eqQKQn4eh2AGTBs4L22FJB EovzXjO10e8SrtvEH86WPI3uvi8k/rv2nPE02gtNvOtOcBP4I5zuk33RH3eFA+iJBGSR J6sdWmC60HeteIHxWJ6z1mm71cvFqkv6KVXqBYlCcttWayShynO4S0f/qXkRdBCzRhFN BmMH4olNHAukDH8XaegTJIb7E/q7AZRXE00wAQV0Au0V1mEYtYzbCgQcIC2nwCp+g8kq w61g== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=FNB6nZlr/06kPBlEqaF8a61wrkKCAoA4BnsL6cXaDrQ=; fh=lNbqm66A/wpozhFc+nLwOwrPosFlY20hAnN9jlxMS5I=; b=TfkOQio0gc8NhDi9ZOH0L0hUbEbnIUQuvsBYsOU+bxjih8Jk5Jkt82iYf3ORzbDPv6 QSIRc1HZ5lL0I5dwQQN0cmcISA5AKwQAyxgxnb6DL3kq1CNc+olGiK1a23nMV/V3UVJq Zh0r4TjXaiMa7wMmuE/YmZK1dDY0aZQtogyIAVUS9ZTytx5ZZkRjWjenIL+QbJYSvzqy aNwKmsJFzXTiEQbWAbsJaBjRI26ZFqfxG7NzeRa1YslfIGWmbtEN+9gXjsT8gFby97A4 H4z+vqween3G/dZyMgcTZmYvZWsVB3PG/h3XylWsdahw6mBU4MZlJyLsr5peRl5ZXOQ/ UYTw== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=hDaQTB+K; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-34944-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-34944-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id f3-20020ac859c3000000b0042a09b71debsi7384267qtf.477.2024.01.23.00.51.20 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Jan 2024 00:51:20 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-34944-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=hDaQTB+K; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-34944-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-34944-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 22E421C220CF for <ouuuleilei@gmail.com>; Tue, 23 Jan 2024 08:51:20 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 4CF195813E; Tue, 23 Jan 2024 08:50:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="hDaQTB+K" Received: from mail-ot1-f53.google.com (mail-ot1-f53.google.com [209.85.210.53]) (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 BF3C92B9CD; Tue, 23 Jan 2024 08:50:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705999850; cv=none; b=Nq/Foybri0O27qXNqb1j3I1SvB6Kn1dXbLz4vZmnoPhbbcQCCkSdHwFJ/j2UOKniSCj6HZ3tElW7lq1ZfKFC/xjFccy6fSMIwpyJWH2FQ1HrPcWv6VLAKqRk3sX362EaCGzBJk2D1jlaIayK/YcAJ+qKXUaMFCso6lK7R+JGAlQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705999850; c=relaxed/simple; bh=wLPog+0OdPWTf/vpChuci+4P8FjJPM6rI/8KZQ+nq7w=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=hfNB3gMX3gvEWJbEXSxQsEWaVAHJRTWr71z6GNCc9y5sTvznaCkLYu8sEwXIAi0LqYdpeUdtq8AbRXbAL2GiIhue/nCaDDPPFUMuP0OvOd/U42cgXO8lUZnGA/mnIqE1UiiiJRcwfNS9kQrv9JylL6W6Lq2GmdgRGhaXk1uT8Uc= 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=hDaQTB+K; arc=none smtp.client-ip=209.85.210.53 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 Received: by mail-ot1-f53.google.com with SMTP id 46e09a7af769-6de83f5a004so2792387a34.1; Tue, 23 Jan 2024 00:50:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705999848; x=1706604648; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=FNB6nZlr/06kPBlEqaF8a61wrkKCAoA4BnsL6cXaDrQ=; b=hDaQTB+KL5dQHp1OiEXldk3WKg0d1jvkbCh+eaWcP3c2JAF2k7v3wk0CR5SdISx3aB bzNmKzDSIgLwt5Ik2ka6zd/PnFZS2ugclv2VTbPJUYvQ4R7JWi7awivTSwE+uM396zuN jaZz9W+5E8X4eDgvk3MVqvUwwGK3m9NXp2GDMmx+0fE5RbVFI86lPuRuykKjQa/h4yah CYGE2++fxbpBgQmk8vAjieRVj96k2Zki3STreNGDUGY0gW0fBPiR8XXp7wGbB7fmHrXW lqbEsSDvt/XSwvexyldaF3ArhHx5nJjTtQyeizajiWhQ+thYB1T3c+RCGAgFKwU+JjpU M8PQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705999848; x=1706604648; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=FNB6nZlr/06kPBlEqaF8a61wrkKCAoA4BnsL6cXaDrQ=; b=NLOpQnSkQ/A92wUQqWGUv7EvRnGQ5q99xuLTMLnsYuWkOnJ/k1/JGLYW+WLb9yvTcX 5bHco7quLyktKwS0W1iD8dGnQFmkG4FibD01qrELLLh4rAjIw5gjAv9D2FeL2RdZYZeW MHhqwPDb+TfNvsCU1dSQ32dLgT3tMlDtc3LGF242cJ7GANrEIxOe2SlGIp6YUvm8bZZ6 7a/Wo9TXlw5TPtoEGsd9QehIp0/PZITxKkXuzYo192YNo0L804mLrc6QKXt3wBZ11QDs ZOq/OFnhjR+XwAenkl0Ejq+eghzT/0R0wvf2tUFI7hBHE4ppQpivSznTmg2lUE50Ipvm GVUg== X-Gm-Message-State: AOJu0YxKf3TfD0nmT9Oj5kgBpHByUkrvjDpEtuzjXOoNObUcTtWtfVMr BjXtwvS1u8Fi/tmddQtn62stxLBrsC72WGmzh7SsLiaEimDNYr3Z X-Received: by 2002:a05:6830:1e2d:b0:6dc:8dfb:3a86 with SMTP id t13-20020a0568301e2d00b006dc8dfb3a86mr5969046otr.15.1705999847791; Tue, 23 Jan 2024 00:50:47 -0800 (PST) Received: from localhost.localdomain ([112.65.140.130]) by smtp.googlemail.com with ESMTPSA id w70-20020a638249000000b005cf88b016cesm9626322pgd.72.2024.01.23.00.50.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Jan 2024 00:50:47 -0800 (PST) From: Furong Xu <0x1207@gmail.com> To: "David S. Miller" <davem@davemloft.net>, Alexandre Torgue <alexandre.torgue@foss.st.com>, Jose Abreu <joabreu@synopsys.com>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Joao Pinto <jpinto@synopsys.com>, Simon Horman <horms@kernel.org> Cc: netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, xfr@outlook.com, rock.xu@nio.com, Furong Xu <0x1207@gmail.com> Subject: [PATCH net] net: stmmac: xgmac: fix safety error descriptions Date: Tue, 23 Jan 2024 16:50:37 +0800 Message-Id: <20240123085037.939471-1-0x1207@gmail.com> X-Mailer: git-send-email 2.34.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1788870530503176690 X-GMAIL-MSGID: 1788870530503176690 |
Series |
[net] net: stmmac: xgmac: fix safety error descriptions
|
|
Commit Message
Furong Xu
Jan. 23, 2024, 8:50 a.m. UTC
Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in
XGMAC core") prints safety error descriptions when safety error assert,
but missed some special errors, and mixed correctable errors and
uncorrectable errors together.
This patch complete the error code list and print the type of errors.
Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core")
Signed-off-by: Furong Xu <0x1207@gmail.com>
---
.../ethernet/stmicro/stmmac/dwxgmac2_core.c | 36 +++++++++----------
1 file changed, 18 insertions(+), 18 deletions(-)
Comments
On Tue, Jan 23, 2024 at 04:50:37PM +0800, Furong Xu wrote: > Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in > XGMAC core") prints safety error descriptions when safety error assert, > but missed some special errors, and mixed correctable errors and > uncorrectable errors together. > This patch complete the error code list and print the type of errors. > > Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core") > Signed-off-by: Furong Xu <0x1207@gmail.com> I'm not entirely sure this is a fix rather than an enhancement. But the code change itself looks good to me. Reviewed-by: Simon Horman <horms@kernel.org>
On Tue, Jan 23, 2024 at 04:50:37PM +0800, Furong Xu wrote: > Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in > XGMAC core") prints safety error descriptions when safety error assert, > but missed some special errors, and mixed correctable errors and > uncorrectable errors together. > This patch complete the error code list and print the type of errors. The XGMAC ECC Safety code has likely been just copied from the DW GMAC v5 (DW QoS Eth) part. So this change is partly relevant to that code too. I can't confirm that the special errors support is relevant to the DW QoS Eth too (it likely is though), so what about splitting this patch up into two: 1. Elaborate the errors description for DW GMAC v5 and DW XGMAC. 2. Add new ECC safety errors support. ? On the other hand if we were sure that both DW QoS Eth and XGMAC safety features implementation match the ideal solution would be to refactor out the common code into a dedicated module. -Serge(y) > > Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core") > Signed-off-by: Furong Xu <0x1207@gmail.com> > --- > .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 36 +++++++++---------- > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > index eb48211d9b0e..ad812484059e 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > @@ -748,29 +748,29 @@ static void dwxgmac3_handle_mac_err(struct net_device *ndev, > } > > static const struct dwxgmac3_error_desc dwxgmac3_mtl_errors[32]= { > - { true, "TXCES", "MTL TX Memory Error" }, > + { true, "TXCES", "MTL TX Memory Correctable Error" }, > { true, "TXAMS", "MTL TX Memory Address Mismatch Error" }, > - { true, "TXUES", "MTL TX Memory Error" }, > + { true, "TXUES", "MTL TX Memory Uncorrectable Error" }, > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > - { true, "RXCES", "MTL RX Memory Error" }, > + { true, "RXCES", "MTL RX Memory Correctable Error" }, > { true, "RXAMS", "MTL RX Memory Address Mismatch Error" }, > - { true, "RXUES", "MTL RX Memory Error" }, > + { true, "RXUES", "MTL RX Memory Uncorrectable Error" }, > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > - { true, "ECES", "MTL EST Memory Error" }, > + { true, "ECES", "MTL EST Memory Correctable Error" }, > { true, "EAMS", "MTL EST Memory Address Mismatch Error" }, > - { true, "EUES", "MTL EST Memory Error" }, > + { true, "EUES", "MTL EST Memory Uncorrectable Error" }, > { false, "UNKNOWN", "Unknown Error" }, /* 11 */ > - { true, "RPCES", "MTL RX Parser Memory Error" }, > + { true, "RPCES", "MTL RX Parser Memory Correctable Error" }, > { true, "RPAMS", "MTL RX Parser Memory Address Mismatch Error" }, > - { true, "RPUES", "MTL RX Parser Memory Error" }, > + { true, "RPUES", "MTL RX Parser Memory Uncorrectable Error" }, > { false, "UNKNOWN", "Unknown Error" }, /* 15 */ > - { false, "UNKNOWN", "Unknown Error" }, /* 16 */ > - { false, "UNKNOWN", "Unknown Error" }, /* 17 */ > - { false, "UNKNOWN", "Unknown Error" }, /* 18 */ > + { true, "SCES", "MTL SGF GCL Memory Correctable Error" }, > + { true, "SAMS", "MTL SGF GCL Memory Address Mismatch Error" }, > + { true, "SUES", "MTL SGF GCL Memory Uncorrectable Error" }, > { false, "UNKNOWN", "Unknown Error" }, /* 19 */ > - { false, "UNKNOWN", "Unknown Error" }, /* 20 */ > - { false, "UNKNOWN", "Unknown Error" }, /* 21 */ > - { false, "UNKNOWN", "Unknown Error" }, /* 22 */ > + { true, "RXFCES", "MTL RXF Memory Correctable Error" }, > + { true, "RXFAMS", "MTL RXF Memory Address Mismatch Error" }, > + { true, "RXFUES", "MTL RXF Memory Uncorrectable Error" }, > { false, "UNKNOWN", "Unknown Error" }, /* 23 */ > { false, "UNKNOWN", "Unknown Error" }, /* 24 */ > { false, "UNKNOWN", "Unknown Error" }, /* 25 */ > @@ -796,13 +796,13 @@ static void dwxgmac3_handle_mtl_err(struct net_device *ndev, > } > > static const struct dwxgmac3_error_desc dwxgmac3_dma_errors[32]= { > - { true, "TCES", "DMA TSO Memory Error" }, > + { true, "TCES", "DMA TSO Memory Correctable Error" }, > { true, "TAMS", "DMA TSO Memory Address Mismatch Error" }, > - { true, "TUES", "DMA TSO Memory Error" }, > + { true, "TUES", "DMA TSO Memory Uncorrectable Error" }, > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > - { true, "DCES", "DMA DCACHE Memory Error" }, > + { true, "DCES", "DMA DCACHE Memory Correctable Error" }, > { true, "DAMS", "DMA DCACHE Address Mismatch Error" }, > - { true, "DUES", "DMA DCACHE Memory Error" }, > + { true, "DUES", "DMA DCACHE Memory Uncorrectable Error" }, > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > { false, "UNKNOWN", "Unknown Error" }, /* 8 */ > { false, "UNKNOWN", "Unknown Error" }, /* 9 */ > -- > 2.34.1 > >
On Wed, 24 Jan 2024 17:25:27 +0300 Serge Semin <fancer.lancer@gmail.com> wrote: > On Tue, Jan 23, 2024 at 04:50:37PM +0800, Furong Xu wrote: > > Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in > > XGMAC core") prints safety error descriptions when safety error assert, > > but missed some special errors, and mixed correctable errors and > > uncorrectable errors together. > > This patch complete the error code list and print the type of errors. > > The XGMAC ECC Safety code has likely been just copied from the DW GMAC > v5 (DW QoS Eth) part. So this change is partly relevant to that code too. I > can't confirm that the special errors support is relevant to the DW > QoS Eth too (it likely is though), so what about splitting this patch > up into two: > 1. Elaborate the errors description for DW GMAC v5 and DW XGMAC. > 2. Add new ECC safety errors support. > ? > > On the other hand if we were sure that both DW QoS Eth and XGMAC > safety features implementation match the ideal solution would be to > refactor out the common code into a dedicated module. > > -Serge(y) > Checked XGMAC Version 3.20a and DW QoS Eth Version 5.20a, the safety error code definitions are not identical at all, they do have some differences, about more than 20 bits of status register are different. I think we should just leave them in individual implementations. > > > > Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core") > > Signed-off-by: Furong Xu <0x1207@gmail.com> > > --- > > .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 36 +++++++++---------- > > 1 file changed, 18 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > index eb48211d9b0e..ad812484059e 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > @@ -748,29 +748,29 @@ static void dwxgmac3_handle_mac_err(struct net_device *ndev, > > } > > > > static const struct dwxgmac3_error_desc dwxgmac3_mtl_errors[32]= { > > - { true, "TXCES", "MTL TX Memory Error" }, > > + { true, "TXCES", "MTL TX Memory Correctable Error" }, > > { true, "TXAMS", "MTL TX Memory Address Mismatch Error" }, > > - { true, "TXUES", "MTL TX Memory Error" }, > > + { true, "TXUES", "MTL TX Memory Uncorrectable Error" }, > > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > > - { true, "RXCES", "MTL RX Memory Error" }, > > + { true, "RXCES", "MTL RX Memory Correctable Error" }, > > { true, "RXAMS", "MTL RX Memory Address Mismatch Error" }, > > - { true, "RXUES", "MTL RX Memory Error" }, > > + { true, "RXUES", "MTL RX Memory Uncorrectable Error" }, > > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > > - { true, "ECES", "MTL EST Memory Error" }, > > + { true, "ECES", "MTL EST Memory Correctable Error" }, > > { true, "EAMS", "MTL EST Memory Address Mismatch Error" }, > > - { true, "EUES", "MTL EST Memory Error" }, > > + { true, "EUES", "MTL EST Memory Uncorrectable Error" }, > > { false, "UNKNOWN", "Unknown Error" }, /* 11 */ > > - { true, "RPCES", "MTL RX Parser Memory Error" }, > > + { true, "RPCES", "MTL RX Parser Memory Correctable Error" }, > > { true, "RPAMS", "MTL RX Parser Memory Address Mismatch Error" }, > > - { true, "RPUES", "MTL RX Parser Memory Error" }, > > + { true, "RPUES", "MTL RX Parser Memory Uncorrectable Error" }, > > { false, "UNKNOWN", "Unknown Error" }, /* 15 */ > > - { false, "UNKNOWN", "Unknown Error" }, /* 16 */ > > - { false, "UNKNOWN", "Unknown Error" }, /* 17 */ > > - { false, "UNKNOWN", "Unknown Error" }, /* 18 */ > > + { true, "SCES", "MTL SGF GCL Memory Correctable Error" }, > > + { true, "SAMS", "MTL SGF GCL Memory Address Mismatch Error" }, > > + { true, "SUES", "MTL SGF GCL Memory Uncorrectable Error" }, > > { false, "UNKNOWN", "Unknown Error" }, /* 19 */ > > - { false, "UNKNOWN", "Unknown Error" }, /* 20 */ > > - { false, "UNKNOWN", "Unknown Error" }, /* 21 */ > > - { false, "UNKNOWN", "Unknown Error" }, /* 22 */ > > + { true, "RXFCES", "MTL RXF Memory Correctable Error" }, > > + { true, "RXFAMS", "MTL RXF Memory Address Mismatch Error" }, > > + { true, "RXFUES", "MTL RXF Memory Uncorrectable Error" }, > > { false, "UNKNOWN", "Unknown Error" }, /* 23 */ > > { false, "UNKNOWN", "Unknown Error" }, /* 24 */ > > { false, "UNKNOWN", "Unknown Error" }, /* 25 */ > > @@ -796,13 +796,13 @@ static void dwxgmac3_handle_mtl_err(struct net_device *ndev, > > } > > > > static const struct dwxgmac3_error_desc dwxgmac3_dma_errors[32]= { > > - { true, "TCES", "DMA TSO Memory Error" }, > > + { true, "TCES", "DMA TSO Memory Correctable Error" }, > > { true, "TAMS", "DMA TSO Memory Address Mismatch Error" }, > > - { true, "TUES", "DMA TSO Memory Error" }, > > + { true, "TUES", "DMA TSO Memory Uncorrectable Error" }, > > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > > - { true, "DCES", "DMA DCACHE Memory Error" }, > > + { true, "DCES", "DMA DCACHE Memory Correctable Error" }, > > { true, "DAMS", "DMA DCACHE Address Mismatch Error" }, > > - { true, "DUES", "DMA DCACHE Memory Error" }, > > + { true, "DUES", "DMA DCACHE Memory Uncorrectable Error" }, > > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > > { false, "UNKNOWN", "Unknown Error" }, /* 8 */ > > { false, "UNKNOWN", "Unknown Error" }, /* 9 */ > > -- > > 2.34.1 > > > >
On Thu, 2024-01-25 at 10:34 +0800, Furong Xu wrote: > On Wed, 24 Jan 2024 17:25:27 +0300 > Serge Semin <fancer.lancer@gmail.com> wrote: > > > On Tue, Jan 23, 2024 at 04:50:37PM +0800, Furong Xu wrote: > > > Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in > > > XGMAC core") prints safety error descriptions when safety error assert, > > > but missed some special errors, and mixed correctable errors and > > > uncorrectable errors together. > > > This patch complete the error code list and print the type of errors. > > > > The XGMAC ECC Safety code has likely been just copied from the DW GMAC > > v5 (DW QoS Eth) part. So this change is partly relevant to that code too. I > > can't confirm that the special errors support is relevant to the DW > > QoS Eth too (it likely is though), so what about splitting this patch > > up into two: > > 1. Elaborate the errors description for DW GMAC v5 and DW XGMAC. > > 2. Add new ECC safety errors support. > > ? > > > > On the other hand if we were sure that both DW QoS Eth and XGMAC > > safety features implementation match the ideal solution would be to > > refactor out the common code into a dedicated module. > > > > -Serge(y) > > > > Checked XGMAC Version 3.20a and DW QoS Eth Version 5.20a, the safety error > code definitions are not identical at all, they do have some differences, > about more than 20 bits of status register are different. > I think we should just leave them in individual implementations. @Serge: given the above, would you still advice for splitting this patch into 2? Thanks, Paolo
On Thu, Jan 25, 2024 at 11:09:06AM +0100, Paolo Abeni wrote: > On Thu, 2024-01-25 at 10:34 +0800, Furong Xu wrote: > > On Wed, 24 Jan 2024 17:25:27 +0300 > > Serge Semin <fancer.lancer@gmail.com> wrote: > > > > > On Tue, Jan 23, 2024 at 04:50:37PM +0800, Furong Xu wrote: > > > > Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in > > > > XGMAC core") prints safety error descriptions when safety error assert, > > > > but missed some special errors, and mixed correctable errors and > > > > uncorrectable errors together. > > > > This patch complete the error code list and print the type of errors. > > > > > > The XGMAC ECC Safety code has likely been just copied from the DW GMAC > > > v5 (DW QoS Eth) part. So this change is partly relevant to that code too. I > > > can't confirm that the special errors support is relevant to the DW > > > QoS Eth too (it likely is though), so what about splitting this patch > > > up into two: > > > 1. Elaborate the errors description for DW GMAC v5 and DW XGMAC. > > > 2. Add new ECC safety errors support. > > > ? > > > > > > On the other hand if we were sure that both DW QoS Eth and XGMAC > > > safety features implementation match the ideal solution would be to > > > refactor out the common code into a dedicated module. > > > > > > -Serge(y) > > > > > > > Checked XGMAC Version 3.20a and DW QoS Eth Version 5.20a, the safety error > > code definitions are not identical at all, they do have some differences, > > about more than 20 bits of status register are different. > > I think we should just leave them in individual implementations. > > @Serge: given the above, would you still advice for splitting this > patch into 2? Preliminary I would still in insist on splitting up. I'll double check the patch and the Safety feature implementations in both devices and give more detailed response to Furong in an hour or so. -Serge(y) > > Thanks, > > Paolo >
On Thu, Jan 25, 2024 at 10:34:54AM +0800, Furong Xu wrote: > On Wed, 24 Jan 2024 17:25:27 +0300 > Serge Semin <fancer.lancer@gmail.com> wrote: > > > On Tue, Jan 23, 2024 at 04:50:37PM +0800, Furong Xu wrote: > > > Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in > > > XGMAC core") prints safety error descriptions when safety error assert, > > > but missed some special errors, and mixed correctable errors and > > > uncorrectable errors together. > > > This patch complete the error code list and print the type of errors. > > > > The XGMAC ECC Safety code has likely been just copied from the DW GMAC > > v5 (DW QoS Eth) part. So this change is partly relevant to that code too. I > > can't confirm that the special errors support is relevant to the DW > > QoS Eth too (it likely is though), so what about splitting this patch > > up into two: > > 1. Elaborate the errors description for DW GMAC v5 and DW XGMAC. > > 2. Add new ECC safety errors support. > > ? > > > > On the other hand if we were sure that both DW QoS Eth and XGMAC > > safety features implementation match the ideal solution would be to > > refactor out the common code into a dedicated module. > > > > -Serge(y) > > > > Checked XGMAC Version 3.20a and DW QoS Eth Version 5.20a, the safety error > code definitions are not identical at all, they do have some differences, > about more than 20 bits of status register are different. > I think we should just leave them in individual implementations. For some reason you answered to the last part of my comment and completely ignored the first part which was the main point of my message. Regarding the Safety Feature support implemented in QoS Eth and XGMAC STMMAC modules. You were wrong in using the statement "at all". Except the optional events enable/disable procedure introduced in the commit 5ac712dcdfef ("net: stmmac: enable platform specific safety features"), there aren't many differences: at least the errors handling and report are identical, MTL and DMA error flags match, even MTL and DMA ECC/Safety IRQ flags match. The only difference is in the MTL/MAC DPP (Data Parity Protection) part which can be easily factored out based on the device ID should we attempt to refactor the safety feature code. See the attached html-diff for more details of what match and what is different. Anyway I am not insisting on the refactoring. That was just a proposal, a more preferred alternative to simultaneously patching two parts of the drivers looking very much alike. Such refactoring would improve the code maintainability. The main point of my comment was to extend your patch for DW QoS Eth safety feature implementation too since some of the changes you introduced were useful for it too, and in splitting the patch up since your patch added new flags support which was unrelated change. Thus your patch would turned into the two-patches patchset like this: [Patch 1] would provide an elaborated errors description for both DW QOS Eth (GMAC v5.x) and DW XGMAC. [Patch 2] would introduce the new ECC safety errors support. See my further comments about the respective changes. > > > > > > > Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core") > > > Signed-off-by: Furong Xu <0x1207@gmail.com> > > > --- > > > .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 36 +++++++++---------- > > > 1 file changed, 18 insertions(+), 18 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > index eb48211d9b0e..ad812484059e 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > @@ -748,29 +748,29 @@ static void dwxgmac3_handle_mac_err(struct net_device *ndev, > > > } > > > > > > static const struct dwxgmac3_error_desc dwxgmac3_mtl_errors[32]= { > > > - { true, "TXCES", "MTL TX Memory Error" }, > > > + { true, "TXCES", "MTL TX Memory Correctable Error" }, Applicable for both IP-cores [Patch 1] +QoS, +XGMAC please apply this change to dwmac5.c too. > > > { true, "TXAMS", "MTL TX Memory Address Mismatch Error" }, > > > - { true, "TXUES", "MTL TX Memory Error" }, > > > + { true, "TXUES", "MTL TX Memory Uncorrectable Error" }, [Patch 1] +QoS, +XGMAC ditto > > > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > > > - { true, "RXCES", "MTL RX Memory Error" }, > > > + { true, "RXCES", "MTL RX Memory Correctable Error" }, [Patch 1] +QoS, +XGMAC ditto > > > { true, "RXAMS", "MTL RX Memory Address Mismatch Error" }, > > > - { true, "RXUES", "MTL RX Memory Error" }, > > > + { true, "RXUES", "MTL RX Memory Uncorrectable Error" }, [Patch 1] +QoS, +XGMAC ditto > > > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > > > - { true, "ECES", "MTL EST Memory Error" }, > > > + { true, "ECES", "MTL EST Memory Correctable Error" }, [Patch 1] +QoS, +XGMAC ditto > > > { true, "EAMS", "MTL EST Memory Address Mismatch Error" }, > > > - { true, "EUES", "MTL EST Memory Error" }, > > > + { true, "EUES", "MTL EST Memory Uncorrectable Error" }, [Patch 1] +QoS, +XGMAC ditto > > > { false, "UNKNOWN", "Unknown Error" }, /* 11 */ > > > - { true, "RPCES", "MTL RX Parser Memory Error" }, > > > + { true, "RPCES", "MTL RX Parser Memory Correctable Error" }, [Patch 1] +QoS, +XGMAC ditto > > > { true, "RPAMS", "MTL RX Parser Memory Address Mismatch Error" }, > > > - { true, "RPUES", "MTL RX Parser Memory Error" }, > > > + { true, "RPUES", "MTL RX Parser Memory Uncorrectable Error" }, [Patch 1] +QoS, +XGMAC ditto > > > { false, "UNKNOWN", "Unknown Error" }, /* 15 */ > > > - { false, "UNKNOWN", "Unknown Error" }, /* 16 */ > > > - { false, "UNKNOWN", "Unknown Error" }, /* 17 */ > > > - { false, "UNKNOWN", "Unknown Error" }, /* 18 */ > > > + { true, "SCES", "MTL SGF GCL Memory Correctable Error" }, > > > + { true, "SAMS", "MTL SGF GCL Memory Address Mismatch Error" }, > > > + { true, "SUES", "MTL SGF GCL Memory Uncorrectable Error" }, > > > { false, "UNKNOWN", "Unknown Error" }, /* 19 */ > > > - { false, "UNKNOWN", "Unknown Error" }, /* 20 */ > > > - { false, "UNKNOWN", "Unknown Error" }, /* 21 */ > > > - { false, "UNKNOWN", "Unknown Error" }, /* 22 */ > > > + { true, "RXFCES", "MTL RXF Memory Correctable Error" }, > > > + { true, "RXFAMS", "MTL RXF Memory Address Mismatch Error" }, > > > + { true, "RXFUES", "MTL RXF Memory Uncorrectable Error" }, This introduces the new flags support. Please move this change into a separate patch (Patch 2): [Patch 2] +XGMAC only. My DW QoS Eth v5.10a databook doesn't have these flags defined. If your 5.20a HW-manual have them described then please add them for DW QoS Eth too. > > > { false, "UNKNOWN", "Unknown Error" }, /* 23 */ > > > { false, "UNKNOWN", "Unknown Error" }, /* 24 */ > > > { false, "UNKNOWN", "Unknown Error" }, /* 25 */ > > > @@ -796,13 +796,13 @@ static void dwxgmac3_handle_mtl_err(struct net_device *ndev, > > > } > > > > > > static const struct dwxgmac3_error_desc dwxgmac3_dma_errors[32]= { > > > - { true, "TCES", "DMA TSO Memory Error" }, > > > + { true, "TCES", "DMA TSO Memory Correctable Error" }, Applicable for both IP-cores [Patch 1] +QoS, +XGMAC please apply this change to dwmac5.c too. > > > { true, "TAMS", "DMA TSO Memory Address Mismatch Error" }, > > > - { true, "TUES", "DMA TSO Memory Error" }, > > > + { true, "TUES", "DMA TSO Memory Uncorrectable Error" }, [Patch 1] +QoS, +XGMAC ditto > > > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > > > - { true, "DCES", "DMA DCACHE Memory Error" }, > > > + { true, "DCES", "DMA DCACHE Memory Correctable Error" }, > > > { true, "DAMS", "DMA DCACHE Address Mismatch Error" }, > > > - { true, "DUES", "DMA DCACHE Memory Error" }, > > > + { true, "DUES", "DMA DCACHE Memory Uncorrectable Error" }, AFAICS applicable for XGMAC only [Patch 1] +XGMAC only. Once again, My DW QoS Eth v5.10a databook doesn't have these flags defined. So if your DW QoS Eth 5.20a HW-manual do have them described please add them for DW QoS Eth with the elaborated description in the framework of the Patch 2. -Serge(y) > > > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > > > { false, "UNKNOWN", "Unknown Error" }, /* 8 */ > > > { false, "UNKNOWN", "Unknown Error" }, /* 9 */ > > > -- > > > 2.34.1 > > > > > > >
On Thu, Jan 25, 2024 at 04:48:27PM +0300, Serge Semin wrote: > On Thu, Jan 25, 2024 at 10:34:54AM +0800, Furong Xu wrote: > > On Wed, 24 Jan 2024 17:25:27 +0300 > > Serge Semin <fancer.lancer@gmail.com> wrote: > > > > > On Tue, Jan 23, 2024 at 04:50:37PM +0800, Furong Xu wrote: > > > > Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in > > > > XGMAC core") prints safety error descriptions when safety error assert, > > > > but missed some special errors, and mixed correctable errors and > > > > uncorrectable errors together. > > > > This patch complete the error code list and print the type of errors. > > > > > > The XGMAC ECC Safety code has likely been just copied from the DW GMAC > > > v5 (DW QoS Eth) part. So this change is partly relevant to that code too. I > > > can't confirm that the special errors support is relevant to the DW > > > QoS Eth too (it likely is though), so what about splitting this patch > > > up into two: > > > 1. Elaborate the errors description for DW GMAC v5 and DW XGMAC. > > > 2. Add new ECC safety errors support. > > > ? > > > > > > On the other hand if we were sure that both DW QoS Eth and XGMAC > > > safety features implementation match the ideal solution would be to > > > refactor out the common code into a dedicated module. > > > > > > -Serge(y) > > > > > > > > Checked XGMAC Version 3.20a and DW QoS Eth Version 5.20a, the safety error > > code definitions are not identical at all, they do have some differences, > > about more than 20 bits of status register are different. > > I think we should just leave them in individual implementations. > > For some reason you answered to the last part of my comment and > completely ignored the first part which was the main point of my > message. > > Regarding the Safety Feature support implemented in QoS Eth and XGMAC > STMMAC modules. You were wrong in using the statement "at all". Except > the optional events enable/disable procedure introduced in the commit > 5ac712dcdfef ("net: stmmac: enable platform specific safety > features"), there aren't many differences: at least the errors > handling and report are identical, MTL and DMA error flags match, even > MTL and DMA ECC/Safety IRQ flags match. The only difference is in the > MTL/MAC DPP (Data Parity Protection) part which can be easily factored > out based on the device ID should we attempt to refactor the safety > feature code. See the attached html-diff for more details of what > match and what is different. Grrr, forgot to attach the html-file... -Serge(y) > > Anyway I am not insisting on the refactoring. That was just a > proposal, a more preferred alternative to simultaneously patching two > parts of the drivers looking very much alike. Such refactoring would > improve the code maintainability. The main point of my comment was to > extend your patch for DW QoS Eth safety feature implementation too > since some of the changes you introduced were useful for it too, and > in splitting the patch up since your patch added new flags support > which was unrelated change. Thus your patch would turned into the > two-patches patchset like this: > [Patch 1] would provide an elaborated errors description for both DW > QOS Eth (GMAC v5.x) and DW XGMAC. > [Patch 2] would introduce the new ECC safety errors support. > > See my further comments about the respective changes. > > > > > > > > > > > Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core") > > > > Signed-off-by: Furong Xu <0x1207@gmail.com> > > > > --- > > > > .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 36 +++++++++---------- > > > > 1 file changed, 18 insertions(+), 18 deletions(-) > > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > > index eb48211d9b0e..ad812484059e 100644 > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > > @@ -748,29 +748,29 @@ static void dwxgmac3_handle_mac_err(struct net_device *ndev, > > > > } > > > > > > > > static const struct dwxgmac3_error_desc dwxgmac3_mtl_errors[32]= { > > > > > - { true, "TXCES", "MTL TX Memory Error" }, > > > > + { true, "TXCES", "MTL TX Memory Correctable Error" }, > > Applicable for both IP-cores > [Patch 1] +QoS, +XGMAC > please apply this change to dwmac5.c too. > > > > > { true, "TXAMS", "MTL TX Memory Address Mismatch Error" }, > > > > > - { true, "TXUES", "MTL TX Memory Error" }, > > > > + { true, "TXUES", "MTL TX Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > > > > > - { true, "RXCES", "MTL RX Memory Error" }, > > > > + { true, "RXCES", "MTL RX Memory Correctable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { true, "RXAMS", "MTL RX Memory Address Mismatch Error" }, > > > > > - { true, "RXUES", "MTL RX Memory Error" }, > > > > + { true, "RXUES", "MTL RX Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > > > > > - { true, "ECES", "MTL EST Memory Error" }, > > > > + { true, "ECES", "MTL EST Memory Correctable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { true, "EAMS", "MTL EST Memory Address Mismatch Error" }, > > > > > - { true, "EUES", "MTL EST Memory Error" }, > > > > + { true, "EUES", "MTL EST Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 11 */ > > > > > - { true, "RPCES", "MTL RX Parser Memory Error" }, > > > > + { true, "RPCES", "MTL RX Parser Memory Correctable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { true, "RPAMS", "MTL RX Parser Memory Address Mismatch Error" }, > > > > > - { true, "RPUES", "MTL RX Parser Memory Error" }, > > > > + { true, "RPUES", "MTL RX Parser Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 15 */ > > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 16 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 17 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 18 */ > > > > + { true, "SCES", "MTL SGF GCL Memory Correctable Error" }, > > > > + { true, "SAMS", "MTL SGF GCL Memory Address Mismatch Error" }, > > > > + { true, "SUES", "MTL SGF GCL Memory Uncorrectable Error" }, > > > > { false, "UNKNOWN", "Unknown Error" }, /* 19 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 20 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 21 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 22 */ > > > > + { true, "RXFCES", "MTL RXF Memory Correctable Error" }, > > > > + { true, "RXFAMS", "MTL RXF Memory Address Mismatch Error" }, > > > > + { true, "RXFUES", "MTL RXF Memory Uncorrectable Error" }, > > This introduces the new flags support. Please move this change into a > separate patch (Patch 2): > [Patch 2] +XGMAC only. > > My DW QoS Eth v5.10a databook doesn't have these flags defined. If > your 5.20a HW-manual have them described then please add them for DW > QoS Eth too. > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 23 */ > > > > { false, "UNKNOWN", "Unknown Error" }, /* 24 */ > > > > { false, "UNKNOWN", "Unknown Error" }, /* 25 */ > > > > @@ -796,13 +796,13 @@ static void dwxgmac3_handle_mtl_err(struct net_device *ndev, > > > > } > > > > > > > > static const struct dwxgmac3_error_desc dwxgmac3_dma_errors[32]= { > > > > > - { true, "TCES", "DMA TSO Memory Error" }, > > > > + { true, "TCES", "DMA TSO Memory Correctable Error" }, > > Applicable for both IP-cores > [Patch 1] +QoS, +XGMAC > please apply this change to dwmac5.c too. > > > > > { true, "TAMS", "DMA TSO Memory Address Mismatch Error" }, > > > > > - { true, "TUES", "DMA TSO Memory Error" }, > > > > + { true, "TUES", "DMA TSO Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > > > > > - { true, "DCES", "DMA DCACHE Memory Error" }, > > > > + { true, "DCES", "DMA DCACHE Memory Correctable Error" }, > > > > { true, "DAMS", "DMA DCACHE Address Mismatch Error" }, > > > > - { true, "DUES", "DMA DCACHE Memory Error" }, > > > > + { true, "DUES", "DMA DCACHE Memory Uncorrectable Error" }, > > AFAICS applicable for XGMAC only > [Patch 1] +XGMAC only. > Once again, My DW QoS Eth v5.10a databook doesn't have these flags > defined. So if your DW QoS Eth 5.20a HW-manual do have them described > please add them for DW QoS Eth with the elaborated description in the > framework of the Patch 2. > > -Serge(y) > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > > > > { false, "UNKNOWN", "Unknown Error" }, /* 8 */ > > > > { false, "UNKNOWN", "Unknown Error" }, /* 9 */ > > > > -- > > > > 2.34.1 > > > > > > > > > >
On Thu, 25 Jan 2024 16:48:23 +0300 Serge Semin <fancer.lancer@gmail.com> wrote: > On Thu, Jan 25, 2024 at 10:34:54AM +0800, Furong Xu wrote: > > On Wed, 24 Jan 2024 17:25:27 +0300 > > Serge Semin <fancer.lancer@gmail.com> wrote: > > > > > On Tue, Jan 23, 2024 at 04:50:37PM +0800, Furong Xu wrote: > > > > Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in > > > > XGMAC core") prints safety error descriptions when safety error assert, > > > > but missed some special errors, and mixed correctable errors and > > > > uncorrectable errors together. > > > > This patch complete the error code list and print the type of errors. > > > > > > The XGMAC ECC Safety code has likely been just copied from the DW GMAC > > > v5 (DW QoS Eth) part. So this change is partly relevant to that code too. I > > > can't confirm that the special errors support is relevant to the DW > > > QoS Eth too (it likely is though), so what about splitting this patch > > > up into two: > > > 1. Elaborate the errors description for DW GMAC v5 and DW XGMAC. > > > 2. Add new ECC safety errors support. > > > ? > > > > > > On the other hand if we were sure that both DW QoS Eth and XGMAC > > > safety features implementation match the ideal solution would be to > > > refactor out the common code into a dedicated module. > > > > > > -Serge(y) > > > > > > > > Checked XGMAC Version 3.20a and DW QoS Eth Version 5.20a, the safety error > > code definitions are not identical at all, they do have some differences, > > about more than 20 bits of status register are different. > > I think we should just leave them in individual implementations. > > For some reason you answered to the last part of my comment and > completely ignored the first part which was the main point of my > message. > > Regarding the Safety Feature support implemented in QoS Eth and XGMAC > STMMAC modules. You were wrong in using the statement "at all". Except > the optional events enable/disable procedure introduced in the commit > 5ac712dcdfef ("net: stmmac: enable platform specific safety > features"), there aren't many differences: at least the errors > handling and report are identical, MTL and DMA error flags match, even > MTL and DMA ECC/Safety IRQ flags match. The only difference is in the > MTL/MAC DPP (Data Parity Protection) part which can be easily factored > out based on the device ID should we attempt to refactor the safety > feature code. See the attached html-diff for more details of what > match and what is different. > > Anyway I am not insisting on the refactoring. That was just a > proposal, a more preferred alternative to simultaneously patching two > parts of the drivers looking very much alike. Such refactoring would > improve the code maintainability. The main point of my comment was to > extend your patch for DW QoS Eth safety feature implementation too > since some of the changes you introduced were useful for it too, and > in splitting the patch up since your patch added new flags support > which was unrelated change. Thus your patch would turned into the > two-patches patchset like this: > [Patch 1] would provide an elaborated errors description for both DW > QOS Eth (GMAC v5.x) and DW XGMAC. > [Patch 2] would introduce the new ECC safety errors support. > > See my further comments about the respective changes. > > > > > > > > > > > Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core") > > > > Signed-off-by: Furong Xu <0x1207@gmail.com> > > > > --- > > > > .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 36 +++++++++---------- > > > > 1 file changed, 18 insertions(+), 18 deletions(-) > > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > > index eb48211d9b0e..ad812484059e 100644 > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > > @@ -748,29 +748,29 @@ static void dwxgmac3_handle_mac_err(struct net_device *ndev, > > > > } > > > > > > > > static const struct dwxgmac3_error_desc dwxgmac3_mtl_errors[32]= { > > > > > - { true, "TXCES", "MTL TX Memory Error" }, > > > > + { true, "TXCES", "MTL TX Memory Correctable Error" }, > > Applicable for both IP-cores > [Patch 1] +QoS, +XGMAC > please apply this change to dwmac5.c too. > > > > > { true, "TXAMS", "MTL TX Memory Address Mismatch Error" }, > > > > > - { true, "TXUES", "MTL TX Memory Error" }, > > > > + { true, "TXUES", "MTL TX Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > > > > > - { true, "RXCES", "MTL RX Memory Error" }, > > > > + { true, "RXCES", "MTL RX Memory Correctable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { true, "RXAMS", "MTL RX Memory Address Mismatch Error" }, > > > > > - { true, "RXUES", "MTL RX Memory Error" }, > > > > + { true, "RXUES", "MTL RX Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > > > > > - { true, "ECES", "MTL EST Memory Error" }, > > > > + { true, "ECES", "MTL EST Memory Correctable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { true, "EAMS", "MTL EST Memory Address Mismatch Error" }, > > > > > - { true, "EUES", "MTL EST Memory Error" }, > > > > + { true, "EUES", "MTL EST Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 11 */ > > > > > - { true, "RPCES", "MTL RX Parser Memory Error" }, > > > > + { true, "RPCES", "MTL RX Parser Memory Correctable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { true, "RPAMS", "MTL RX Parser Memory Address Mismatch Error" }, > > > > > - { true, "RPUES", "MTL RX Parser Memory Error" }, > > > > + { true, "RPUES", "MTL RX Parser Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 15 */ > > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 16 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 17 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 18 */ > > > > + { true, "SCES", "MTL SGF GCL Memory Correctable Error" }, > > > > + { true, "SAMS", "MTL SGF GCL Memory Address Mismatch Error" }, > > > > + { true, "SUES", "MTL SGF GCL Memory Uncorrectable Error" }, > > > > { false, "UNKNOWN", "Unknown Error" }, /* 19 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 20 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 21 */ > > > > - { false, "UNKNOWN", "Unknown Error" }, /* 22 */ > > > > + { true, "RXFCES", "MTL RXF Memory Correctable Error" }, > > > > + { true, "RXFAMS", "MTL RXF Memory Address Mismatch Error" }, > > > > + { true, "RXFUES", "MTL RXF Memory Uncorrectable Error" }, > > This introduces the new flags support. Please move this change into a > separate patch (Patch 2): > [Patch 2] +XGMAC only. > > My DW QoS Eth v5.10a databook doesn't have these flags defined. If > your 5.20a HW-manual have them described then please add them for DW > QoS Eth too. > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 23 */ > > > > { false, "UNKNOWN", "Unknown Error" }, /* 24 */ > > > > { false, "UNKNOWN", "Unknown Error" }, /* 25 */ > > > > @@ -796,13 +796,13 @@ static void dwxgmac3_handle_mtl_err(struct net_device *ndev, > > > > } > > > > > > > > static const struct dwxgmac3_error_desc dwxgmac3_dma_errors[32]= { > > > > > - { true, "TCES", "DMA TSO Memory Error" }, > > > > + { true, "TCES", "DMA TSO Memory Correctable Error" }, > > Applicable for both IP-cores > [Patch 1] +QoS, +XGMAC > please apply this change to dwmac5.c too. > > > > > { true, "TAMS", "DMA TSO Memory Address Mismatch Error" }, > > > > > - { true, "TUES", "DMA TSO Memory Error" }, > > > > + { true, "TUES", "DMA TSO Memory Uncorrectable Error" }, > > [Patch 1] +QoS, +XGMAC > ditto > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 3 */ > > > > > - { true, "DCES", "DMA DCACHE Memory Error" }, > > > > + { true, "DCES", "DMA DCACHE Memory Correctable Error" }, > > > > { true, "DAMS", "DMA DCACHE Address Mismatch Error" }, > > > > - { true, "DUES", "DMA DCACHE Memory Error" }, > > > > + { true, "DUES", "DMA DCACHE Memory Uncorrectable Error" }, > > AFAICS applicable for XGMAC only > [Patch 1] +XGMAC only. > Once again, My DW QoS Eth v5.10a databook doesn't have these flags > defined. So if your DW QoS Eth 5.20a HW-manual do have them described > please add them for DW QoS Eth with the elaborated description in the > framework of the Patch 2. > > -Serge(y) > > > > > { false, "UNKNOWN", "Unknown Error" }, /* 7 */ > > > > { false, "UNKNOWN", "Unknown Error" }, /* 8 */ > > > > { false, "UNKNOWN", "Unknown Error" }, /* 9 */ > > > > -- > > > > 2.34.1 > > > > > > > > > > Hi Serge: Thanks for your detailed explanation, new refactoring will be sent to net-next.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index eb48211d9b0e..ad812484059e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -748,29 +748,29 @@ static void dwxgmac3_handle_mac_err(struct net_device *ndev, } static const struct dwxgmac3_error_desc dwxgmac3_mtl_errors[32]= { - { true, "TXCES", "MTL TX Memory Error" }, + { true, "TXCES", "MTL TX Memory Correctable Error" }, { true, "TXAMS", "MTL TX Memory Address Mismatch Error" }, - { true, "TXUES", "MTL TX Memory Error" }, + { true, "TXUES", "MTL TX Memory Uncorrectable Error" }, { false, "UNKNOWN", "Unknown Error" }, /* 3 */ - { true, "RXCES", "MTL RX Memory Error" }, + { true, "RXCES", "MTL RX Memory Correctable Error" }, { true, "RXAMS", "MTL RX Memory Address Mismatch Error" }, - { true, "RXUES", "MTL RX Memory Error" }, + { true, "RXUES", "MTL RX Memory Uncorrectable Error" }, { false, "UNKNOWN", "Unknown Error" }, /* 7 */ - { true, "ECES", "MTL EST Memory Error" }, + { true, "ECES", "MTL EST Memory Correctable Error" }, { true, "EAMS", "MTL EST Memory Address Mismatch Error" }, - { true, "EUES", "MTL EST Memory Error" }, + { true, "EUES", "MTL EST Memory Uncorrectable Error" }, { false, "UNKNOWN", "Unknown Error" }, /* 11 */ - { true, "RPCES", "MTL RX Parser Memory Error" }, + { true, "RPCES", "MTL RX Parser Memory Correctable Error" }, { true, "RPAMS", "MTL RX Parser Memory Address Mismatch Error" }, - { true, "RPUES", "MTL RX Parser Memory Error" }, + { true, "RPUES", "MTL RX Parser Memory Uncorrectable Error" }, { false, "UNKNOWN", "Unknown Error" }, /* 15 */ - { false, "UNKNOWN", "Unknown Error" }, /* 16 */ - { false, "UNKNOWN", "Unknown Error" }, /* 17 */ - { false, "UNKNOWN", "Unknown Error" }, /* 18 */ + { true, "SCES", "MTL SGF GCL Memory Correctable Error" }, + { true, "SAMS", "MTL SGF GCL Memory Address Mismatch Error" }, + { true, "SUES", "MTL SGF GCL Memory Uncorrectable Error" }, { false, "UNKNOWN", "Unknown Error" }, /* 19 */ - { false, "UNKNOWN", "Unknown Error" }, /* 20 */ - { false, "UNKNOWN", "Unknown Error" }, /* 21 */ - { false, "UNKNOWN", "Unknown Error" }, /* 22 */ + { true, "RXFCES", "MTL RXF Memory Correctable Error" }, + { true, "RXFAMS", "MTL RXF Memory Address Mismatch Error" }, + { true, "RXFUES", "MTL RXF Memory Uncorrectable Error" }, { false, "UNKNOWN", "Unknown Error" }, /* 23 */ { false, "UNKNOWN", "Unknown Error" }, /* 24 */ { false, "UNKNOWN", "Unknown Error" }, /* 25 */ @@ -796,13 +796,13 @@ static void dwxgmac3_handle_mtl_err(struct net_device *ndev, } static const struct dwxgmac3_error_desc dwxgmac3_dma_errors[32]= { - { true, "TCES", "DMA TSO Memory Error" }, + { true, "TCES", "DMA TSO Memory Correctable Error" }, { true, "TAMS", "DMA TSO Memory Address Mismatch Error" }, - { true, "TUES", "DMA TSO Memory Error" }, + { true, "TUES", "DMA TSO Memory Uncorrectable Error" }, { false, "UNKNOWN", "Unknown Error" }, /* 3 */ - { true, "DCES", "DMA DCACHE Memory Error" }, + { true, "DCES", "DMA DCACHE Memory Correctable Error" }, { true, "DAMS", "DMA DCACHE Address Mismatch Error" }, - { true, "DUES", "DMA DCACHE Memory Error" }, + { true, "DUES", "DMA DCACHE Memory Uncorrectable Error" }, { false, "UNKNOWN", "Unknown Error" }, /* 7 */ { false, "UNKNOWN", "Unknown Error" }, /* 8 */ { false, "UNKNOWN", "Unknown Error" }, /* 9 */