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 64E83D2FFE1 for ; Fri, 18 Oct 2024 09:25:50 +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=jlTM9V5DRsPe4eqW9RQavt8mzladFi9MCIQh4OOLQfA=; b=piy+KcgXbPbPuYjv5qlhC/Tbcw DTJtt9D2s4+WjZhgr8K4xUv57xVSowSncCfJaCoMthi85Gq3plP+KEv1lOJJ1AGfGICNnr+CczFib fV2LgWWvzIsNne0NX+5kZzZyKxcRCGE6s0Fy1lLbQf4Rg+iWO+nc5uSj3GCW8mmLHkgSVsYQfXEM8 LWsJsBncqVsBgGVmni/r3fhHWl/e6uot3i4LNeXuKcy+AC5eDJ+yOzQDFwmk22rqCDFdprbrZqRz8 vckuWboabl/TdcEaJYj5dU+420yTxguFOKzVi02IACAXT5H6E6I8MSukm2f8BESIsxoMj8I3+U2cv 7lK2JVDA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t1jEq-00000000Bj8-3cnR; Fri, 18 Oct 2024 09:25:40 +0000 Received: from mail-ej1-x62b.google.com ([2a00:1450:4864:20::62b]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t1jB1-00000000Aln-38hw for linux-arm-kernel@lists.infradead.org; Fri, 18 Oct 2024 09:21:45 +0000 Received: by mail-ej1-x62b.google.com with SMTP id a640c23a62f3a-a9a19bf0a1dso30697366b.1 for ; Fri, 18 Oct 2024 02:21:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729243302; x=1729848102; 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=jlTM9V5DRsPe4eqW9RQavt8mzladFi9MCIQh4OOLQfA=; b=EvAzO30tZ7PfSo08+WSvirW+pzhYQWMP9+2wtyuZuTyN0a9oG8S5bPkrsAGuVIcg/7 5NnZ40lt92Gtes9OFY2O3B6msqqs0I41v9PgE9NyrwvmVvcz6KEQ0+nWRbaI9YhhWULc o0+IYxF6vvMtAL/luZ9vZS1LQixFfw6WybqFUYziURPkJ1vP2FVmlsnKb8EaFUfj5oCq dWLn3mx9qVgMqT8Vgb8wL/p8fEtpgxwJ76M+weslN1V7PpEN2aEeHybdTtTCxA5R0+MB +7yV27aY2EgVzlZbUbXxKWBiBn3I8NsfUr/CQsfiQdV53NVkr+gN1GANrg4mpV1eVqRh JVKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729243302; x=1729848102; 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=jlTM9V5DRsPe4eqW9RQavt8mzladFi9MCIQh4OOLQfA=; b=nUPcfKVEee0OFRDRAC+ODHyLdcBI48O+mIo6yZVYHPn/UguKNK8f3+jjbp07tMh8VY y25MjI4DcI4HdXf2mFPw7+VspQVbPgICodZ8/spvevdthiZ9sahT5GNkvwGJNvcPjQ/s jN9WXlrpU8CcQzIqRiPUeTySJjCh5/+QR0CwN/9l8M6uWm+okVhXbzM71uj/UDdo/AZL 9ep5pNu4GxjT2iWR3UnWRUeGbGT5YPKxTXMsQDWUYR4NjU4aQ4cGwb6EwDReLJKAefUI bPFmYp51aI6k7Tmj73/XD6Y6x6rTK3MtTtuluzWHKq4S1V7ffPO88mtD7j/xLEgfqObO DO8Q== X-Forwarded-Encrypted: i=1; AJvYcCXrS1x3dWf6dHbmGnfSiLK/IyvOYuNoR22zJzo0qTQSqHyCEJzH+zUjmXsqMRnr3BxqMzIKwns7kalRQXcUCA8E@lists.infradead.org X-Gm-Message-State: AOJu0YwUDvnVb4mL/9wP/6oB7e3V/YOPnYLY4+WtPItQK3K1MJrM2z1q HFn4VqhpORIdlXaTkMLRaTg3FlrkoMhI+TkQMRLmZdBDVF8fAXEY X-Google-Smtp-Source: AGHT+IFDFLCQA1phkwKSMbXUM2S3mB35TEAVZuUbCEytPdAHoYv1Sm/TfjB25QyeBIZIHMBL+yOy2g== X-Received: by 2002:a17:907:3d8e:b0:a9a:2523:b4fa with SMTP id a640c23a62f3a-a9a69a76177mr64789866b.4.1729243301360; Fri, 18 Oct 2024 02:21:41 -0700 (PDT) Received: from skbuf ([188.25.134.29]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9a68bf7227sm67420166b.181.2024.10.18.02.21.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Oct 2024 02:21:40 -0700 (PDT) Date: Fri, 18 Oct 2024 12:21:38 +0300 From: Vladimir Oltean To: Furong Xu <0x1207@gmail.com> Cc: netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Andrew Lunn , Simon Horman , Serge Semin , Alexandre Torgue , Jose Abreu , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , xfr@outlook.com Subject: Re: [PATCH net-next v2 1/8] net: stmmac: Introduce separate files for FPE implementation Message-ID: <20241018092138.xo2zoadx2jng27rp@skbuf> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241018_022143_813557_D3947941 X-CRM114-Status: GOOD ( 21.35 ) 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 Fri, Oct 18, 2024 at 02:39:07PM +0800, Furong Xu wrote: > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.h > new file mode 100644 > index 000000000000..d4d46a07d6a7 > --- /dev/null > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.h > @@ -0,0 +1,54 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (C) 2024 Furong Xu <0x1207@gmail.com> > + * stmmac FPE(802.3 Qbu) handling > + */ > +#include "stmmac.h" > + > +#define STMMAC_FPE_MM_MAX_VERIFY_RETRIES 3 > +#define STMMAC_FPE_MM_MAX_VERIFY_TIME_MS 128 > + > +#define MAC_FPE_CTRL_STS 0x00000234 > +#define TRSP BIT(19) > +#define TVER BIT(18) > +#define RRSP BIT(17) > +#define RVER BIT(16) > +#define SRSP BIT(2) > +#define SVER BIT(1) > +#define EFPE BIT(0) > + > +#define MTL_FPE_CTRL_STS 0x00000c90 > +/* Preemption Classification */ > +#define DWMAC5_PREEMPTION_CLASS GENMASK(15, 8) > +/* Additional Fragment Size of preempted frames */ > +#define DWMAC5_ADD_FRAG_SZ GENMASK(1, 0) > + > +#define XGMAC_FPE_CTRL_STS 0x00000280 > +#define XGMAC_EFPE BIT(0) > + > +/* FPE link-partner hand-shaking mPacket type */ > +enum stmmac_mpacket_type { > + MPACKET_VERIFY = 0, > + MPACKET_RESPONSE = 1, > +}; > + > +void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up); > +void stmmac_fpe_event_status(struct stmmac_priv *priv, int status); > +void stmmac_fpe_init(struct stmmac_priv *priv); > +void stmmac_fpe_apply(struct stmmac_priv *priv); > + > +void dwmac5_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg, > + u32 num_txq, u32 num_rxq, > + bool tx_enable, bool pmac_enable); > +void dwmac5_fpe_send_mpacket(void __iomem *ioaddr, > + struct stmmac_fpe_cfg *cfg, > + enum stmmac_mpacket_type type); > +int dwmac5_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev); > +int dwmac5_fpe_get_add_frag_size(const void __iomem *ioaddr); > +void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr, u32 add_frag_size); > +int dwmac5_fpe_map_preemption_class(struct net_device *ndev, > + struct netlink_ext_ack *extack, u32 pclass); > + > +void dwxgmac3_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg, > + u32 num_txq, u32 num_rxq, > + bool tx_enable, bool pmac_enable); 2 comments about this header: - it would be good if it contained a "#pragma once" equivalent, in case other headers include it (#ifndef STMMAC_FPE_H #define STMMAC_FPE_H). - it is good practice to only keep inside the header those definitions which are truly exported by stmmac_fpe.c towards external callers. That means that the #defines which are only used within stmmac_fpe.c shouldn't be here, but inside that C file. - You could remove the dependency upon the giant #include "stmmac.h" and just provide those definitions necessary for this header to be self-consistent. By self-consistent I mean that a dummy C file containing just #include "stmmac_fpe.h" should compile without errors. diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile index 7e46dca90628..3fe0a555aa9a 100644 --- a/drivers/net/ethernet/stmicro/stmmac/Makefile +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile @@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \ mmc_core.o stmmac_hwtstamp.o stmmac_ptp.o dwmac4_descs.o \ dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \ stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \ - stmmac_xdp.o stmmac_est.o stmmac_fpe.o \ + stmmac_xdp.o stmmac_est.o stmmac_fpe.o dummy.o \ $(stmmac-y) stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o diff --git a/drivers/net/ethernet/stmicro/stmmac/dummy.c b/drivers/net/ethernet/stmicro/stmmac/dummy.c new file mode 100644 index 000000000000..fc976afd5e40 --- /dev/null +++ b/drivers/net/ethernet/stmicro/stmmac/dummy.c @@ -0,0 +1 @@ +#include "stmmac_fpe.h" The current stmmac_fpe.h as you've posted it does pass the dummy test, but I think it can be further minimized. Having headers larger than they need to be will increase build time, little by little. Well, those were 3 comments :(