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 22551D3C526 for ; Thu, 17 Oct 2024 17:36:25 +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=bQhe5YTuIUzDPM9p0+DYkvny8q+P5qebGCGivJy9ZCM=; b=jkI/kVEw2eH9qy9UbjmHSzRlUw Pm/5yOcF35JxgkfxHiJ+AIW/GaFkKpfg5QCfCnCah9Opduum9L+WkUz6o1L8QlTrR3PhvTHZHuf5c 0FCcYasbH43zprSX8QZ0S7Bn8a/hnfKZ9uFjU0OUuI7j9he9iZbATJ2RUDAqQBjzNkzoLLSukGiIA dgw+tLkOcOG7c4ytSvKFiUnJenydyTxZ1VGmZfAPOnEgx3cOE6LwM0VBZ7G2MOyQCeq0n2gRO2w3q o/GTtwkn3vYKyOP/U2U+7oVv/JLOc+n+zVtnXleV3yvqmRBS+W8yxcveqUFVIYHzdeY9Z0dadczTY gOZ/o9xw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t1UQ3-0000000FjVM-0gaK; Thu, 17 Oct 2024 17:36:15 +0000 Received: from mail-wm1-x32b.google.com ([2a00:1450:4864:20::32b]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t1UL5-0000000Fim6-1giB for linux-arm-kernel@lists.infradead.org; Thu, 17 Oct 2024 17:31:08 +0000 Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-4315dfa3e0bso935705e9.0 for ; Thu, 17 Oct 2024 10:31:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729186265; x=1729791065; 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=bQhe5YTuIUzDPM9p0+DYkvny8q+P5qebGCGivJy9ZCM=; b=OBesBw7qvI4TX1w/EkOAWsMLzAGc2kc+cztHK6ArMlOuz61Lrf3pgypI66UAs7A5Xn 6hMWYSpehgamwuCpx59sDq+Eju5BYfKU2WjLfyd7aDNq4+tteVEvNsxP28OHsa52lmEU U6B4qK6SDZi999TbS9mrKdJhiClnf3FJTfrePwp6Nzvj6Xg4P8kcIVYZJ9PMvZhMahVU FyZ/Bka/LkXAa+EvZWeLnMO+H65k8ajFm133dHs+JkomFmW4C/0p+m29/8r5RtldyL+f saCvFbuSGMS3bX6intAeghck9tDUch//l8x92sI2TB4MGrVkGFBYBt68sd/7i9zZ3SHH IS5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729186265; x=1729791065; 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=bQhe5YTuIUzDPM9p0+DYkvny8q+P5qebGCGivJy9ZCM=; b=Osyr1guu0Z5h9BcWSyS8//WGeXwKP3vXFdoPq0C8Px4tlIvuqE53P9mWFOosJfVOJa HU7hXELIFwMWqWIpZkjM+ms/z0SBkVgucZAMlqXGRyPuaXaXD7rnTWXuZLX2QoIK8diL GsfBnjCJFwbklQcHSGatk0wMn0LieMthGE5H/9gvpaVj/ZD2FlPxsFGOIddXm2wpqhrN 6serppzZeUQhihi9vvPWSAFnCRioOBG7gsyKaJn4HqN6i9nB+h32wo81aVZ72dkpsiF2 DlaZbIoZeKXpJKewuOxnrpdt1PR6gb2qAwPzPXdgY7Mv/b/izLuaU8TV26j2/PRgqMWI ESZA== X-Forwarded-Encrypted: i=1; AJvYcCUFtdmpV0TlOkSlE6rRNxyJWvxlZvsxt3mWA/lLijXO/unTBzy/m2os3yX85bI0W4OXIQds1yLukPiG4lv6DA/R@lists.infradead.org X-Gm-Message-State: AOJu0Yx3nAq+m+nNNxP3yBK+MnGmZbT6yy9UJ42RwRBrP0i3FWGjiVYo dCqFRO1/FItLXqukgWCMNGoeedWevSBioeemPtUFxVSkMWNi6At/ X-Google-Smtp-Source: AGHT+IE27Q0RXwZxaGIC0/Z8xDnKDmjt7pfBnbOjjTsf8K19xvnS2BcDyVndGa3dsQPmgSt6uPJamw== X-Received: by 2002:a05:600c:511c:b0:42c:aeee:e603 with SMTP id 5b1f17b1804b1-431585f994dmr14823905e9.7.1729186265288; Thu, 17 Oct 2024 10:31:05 -0700 (PDT) Received: from skbuf ([188.25.134.29]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4316069e12dsm1898315e9.22.2024.10.17.10.31.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Oct 2024 10:31:03 -0700 (PDT) Date: Thu, 17 Oct 2024 20:31:01 +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 , Alexandre Torgue , Jose Abreu , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , xfr@outlook.com Subject: Re: [PATCH net-next v1 5/5] net: stmmac: xgmac: Complete FPE support Message-ID: <20241017173101.oby36jwek7tninsx@skbuf> References: <7b244a9d6550bd856298150fb4c083ca95b41f38.1728980110.git.0x1207@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7b244a9d6550bd856298150fb4c083ca95b41f38.1728980110.git.0x1207@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241017_103107_470541_11699D90 X-CRM114-Status: GOOD ( 20.67 ) 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 Tue, Oct 15, 2024 at 05:09:26PM +0800, Furong Xu wrote: > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c > index 6060a1d702c6..80f12b6e84e6 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c > @@ -160,41 +160,54 @@ void stmmac_fpe_apply(struct stmmac_priv *priv) > } > } > > -static void dwmac5_fpe_configure(void __iomem *ioaddr, > - struct stmmac_fpe_cfg *cfg, > - u32 num_txq, u32 num_rxq, > - bool tx_enable, bool pmac_enable) > +static void common_fpe_configure(void __iomem *ioaddr, > + struct stmmac_fpe_cfg *cfg, u32 rxq, > + bool tx_enable, bool pmac_enable, > + u32 rxq_addr, u32 fprq_mask, u32 fprq_shift, > + u32 mac_fpe_addr, u32 int_en_addr, > + u32 int_en_bit) 11 arguments to a function is a bit too much. Could you introduce a structure with FPE constants per hardware IP, and just pass a pointer to that? > { > u32 value; > > - writel(value, ioaddr + XGMAC_MAC_FPE_CTRL_STS); > - return; > + if (!num_tc) { > + /* Restore default TC:Queue mapping */ > + for (u32 i = 0; i < priv->plat->tx_queues_to_use; i++) { > + val = readl(priv->ioaddr + XGMAC_MTL_TXQ_OPMODE(i)); > + writel(u32_replace_bits(val, i, XGMAC_Q2TCMAP), > + priv->ioaddr + XGMAC_MTL_TXQ_OPMODE(i)); > + } > } > > - value = readl(ioaddr + XGMAC_RXQ_CTRL1); > - value &= ~XGMAC_FPRQ; > - value |= (num_rxq - 1) << XGMAC_FPRQ_SHIFT; > - writel(value, ioaddr + XGMAC_RXQ_CTRL1); > + /* Synopsys Databook: > + * "All Queues within a traffic class are selected in a round robin > + * fashion (when packets are available) when the traffic class is > + * selected by the scheduler for packet transmission. This is true for > + * any of the scheduling algorithms." > + */ > + for (u32 tc = 0; tc < num_tc; tc++) { > + count = ndev->tc_to_txq[tc].count; > + offset = ndev->tc_to_txq[tc].offset; > + > + if (pclass & BIT(tc)) > + preemptible_txqs |= GENMASK(offset + count - 1, offset); > > - value = readl(ioaddr + XGMAC_MAC_FPE_CTRL_STS); > - value |= STMMAC_MAC_FPE_CTRL_STS_EFPE; > - writel(value, ioaddr + XGMAC_MAC_FPE_CTRL_STS); > + for (u32 i = 0; i < count; i++) { > + val = readl(priv->ioaddr + XGMAC_MTL_TXQ_OPMODE(offset + i)); > + writel(u32_replace_bits(val, tc, XGMAC_Q2TCMAP), > + priv->ioaddr + XGMAC_MTL_TXQ_OPMODE(offset + i)); > + } > + } I agree with Simon that this patch is hard to review. The diff looks like a jungle here, the portion with - has nothing to do with the portion with +. Please try to do as suggested, first refactor existing code into the common stuff, then call common stuff from new places. Also try to keep an eye on how things look in git diff, and splitting even further if it gets messy.