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 52399CCD18E for ; Wed, 15 Oct 2025 12:52:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=T/PeCLteuTZCtCAousugNZqWbsmGG2vzpEufPHA/fY8=; b=2hShL+C/hWSkhrtVFN8HKtO3oP u3EjH9P33LfXZi8vlZAK6E0AajBFJ3IEt/9XpIMrWmtQWt16W3QvDKVYyRnW/zxR7YEgf16zpNVsn nFgvizTC1Ot+wrnH8Xz/2cwJYgrdvj0oMJiTWpPWNe/vAIpXoRJsqyfidCRw2CEanR1IuZtZYJyeH 7+FCUzc7GCe9V1Bu2eUZ5jv8FDNWkfgl3CMN7q/+RgPr3akGS2W6Ay8wmRgy8IQw5MSnUJZnmNgYA LwGYf3N6zSoEsuRoK+Md3LB1PkMHkooaMBiLCd+n5aMa5JpfOcqb0cy09bnCeCH2lYU2e8p10Y1+Z qEV+NdnQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1v90zZ-00000001b8k-3EhB; Wed, 15 Oct 2025 12:52:33 +0000 Received: from tor.source.kernel.org ([172.105.4.254]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1v90zX-00000001b8Z-48Mv; Wed, 15 Oct 2025 12:52:32 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 34EF2625B1; Wed, 15 Oct 2025 12:52:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3F712C4CEF8; Wed, 15 Oct 2025 12:52:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1760532750; bh=4DK8NsHpOSDw78JlpzEJiBRJK0MDuJa00sse3sDOyDs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=t8o3nVisN7MqQBJeLDOFcbTRf67htnjR8DcPwRhAq4aev4tsHgIkoMlwuWCeCa59R jlqIcNRknLKlRb10gzt5joFkL1eEy7qb+mt/Q3pSm6LISI5M1QcmNxEwCRuQhkjFVy GjM39Goe0J/dWh2Lj3mWdO8BnpWZB64sG2Dhtmlcf+AtuYFim9Tm5oXfgMHB3oJU4G a0cGFDrFaJdrgVy5ES42Knl7f2I61yFf6tVgGI5i1bD8NaBUtayoom8W8zWj5doLaW 9hWYFmzDkaKeDlv5b3NJmcOqRRuKGz2S8nNWATw5VeH3PKNyK2f7GDL6cTTEwJFi4G zEuQ9KR/cNgKw== Date: Wed, 15 Oct 2025 13:52:26 +0100 From: Simon Horman To: Lorenzo Bianconi Cc: Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Philipp Zabel , linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, netdev@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH net-next 03/12] net: airoha: Add airoha_ppe_get_num_stats_entries() and airoha_ppe_get_num_total_stats_entries() Message-ID: References: <20251015-an7583-eth-support-v1-0-064855f05923@kernel.org> <20251015-an7583-eth-support-v1-3-064855f05923@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20251015-an7583-eth-support-v1-3-064855f05923@kernel.org> 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Oct 15, 2025 at 09:15:03AM +0200, Lorenzo Bianconi wrote: > Introduce airoha_ppe_get_num_stats_entries and > airoha_ppe_get_num_total_stats_entries routines in order to make the > code more readable controlling if CONFIG_NET_AIROHA_FLOW_STATS is > enabled or disabled. > Modify airoha_ppe_foe_get_flow_stats_index routine signature relying on > airoha_ppe_get_num_total_stats_entries(). > > Signed-off-by: Lorenzo Bianconi > --- > drivers/net/ethernet/airoha/airoha_eth.h | 10 +-- > drivers/net/ethernet/airoha/airoha_ppe.c | 103 +++++++++++++++++++++++++------ > 2 files changed, 86 insertions(+), 27 deletions(-) > > diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h > index 4330b672d99e1e190efa5ad75d13fb35e77d070e..1f7e34a5f457ca2200e9c81dd05dc03cd7c5eb77 100644 > --- a/drivers/net/ethernet/airoha/airoha_eth.h > +++ b/drivers/net/ethernet/airoha/airoha_eth.h > @@ -50,15 +50,9 @@ > > #define PPE_NUM 2 > #define PPE1_SRAM_NUM_ENTRIES (8 * 1024) > -#define PPE_SRAM_NUM_ENTRIES (2 * PPE1_SRAM_NUM_ENTRIES) > -#ifdef CONFIG_NET_AIROHA_FLOW_STATS > +#define PPE_SRAM_NUM_ENTRIES (PPE_NUM * PPE1_SRAM_NUM_ENTRIES) > #define PPE1_STATS_NUM_ENTRIES (4 * 1024) > -#else > -#define PPE1_STATS_NUM_ENTRIES 0 > -#endif /* CONFIG_NET_AIROHA_FLOW_STATS */ > -#define PPE_STATS_NUM_ENTRIES (2 * PPE1_STATS_NUM_ENTRIES) > -#define PPE1_SRAM_NUM_DATA_ENTRIES (PPE1_SRAM_NUM_ENTRIES - PPE1_STATS_NUM_ENTRIES) > -#define PPE_SRAM_NUM_DATA_ENTRIES (2 * PPE1_SRAM_NUM_DATA_ENTRIES) > +#define PPE_STATS_NUM_ENTRIES (PPE_NUM * PPE1_STATS_NUM_ENTRIES) > #define PPE_DRAM_NUM_ENTRIES (16 * 1024) > #define PPE_NUM_ENTRIES (PPE_SRAM_NUM_ENTRIES + PPE_DRAM_NUM_ENTRIES) > #define PPE_HASH_MASK (PPE_NUM_ENTRIES - 1) > diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c > index 8d1dceadce0becb2b1ce656d64ab77bd3c2f914a..303d31e1da4b723023ee0cc1ca5f6038c16966cd 100644 > --- a/drivers/net/ethernet/airoha/airoha_ppe.c > +++ b/drivers/net/ethernet/airoha/airoha_ppe.c > @@ -32,6 +32,30 @@ static const struct rhashtable_params airoha_l2_flow_table_params = { > .automatic_shrinking = true, > }; > > +static int airoha_ppe_get_num_stats_entries(struct airoha_ppe *ppe, > + u32 *num_stats) > +{ > +#ifdef CONFIG_NET_AIROHA_FLOW_STATS > + *num_stats = PPE1_STATS_NUM_ENTRIES; > + return 0; > +#else > + return -EOPNOTSUPP; > +#endif /* CONFIG_NET_AIROHA_FLOW_STATS */ > +} Hi Lorenzo, I think that in general using IS_ENABLED is preferred over #ifdef in cases where the former can be used. For one thing it improves compile coverage. That does seem applicable here, so I'm wondering if we can do something like the following. (Compile tested only!) static int airoha_ppe_get_num_stats_entries(struct airoha_ppe *ppe, u32 *num_stats) { if (!IS_ENABLED(CONFIG_NET_AIROHA_FLOW_STATS)) return -EOPNOTSUPP; *num_stats = PPE1_STATS_NUM_ENTRIES; return 0; } Also, very subjectively, I might have returned num_stats as a positive return value. I'm assuming it's value will never overflow an int. Likewise elsewhere. But that's just my idea. Feel free to stick with the current scheme. ...