From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 537F8A57 for ; Sat, 19 Aug 2023 14:41:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B5515C433C9; Sat, 19 Aug 2023 14:41:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1692456074; bh=s/6tz1ntVu9WHcizXMkOFm+PqgGopb/sQCUQJgN/UCw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RspyVU2C22Gq4717Y75wZ72mw3oilJhDHzF06iNK7vIzIfhkYMC9p3Dl2ictYYcGa Pj8PGuyGFy+ibtb5heSsWapbk8AbmxSkJ/Fb25XVQm55KTGpkxvLPHyOwNeie2tePw rNelyoxgP6jWnHQTvbhRRuyGSALKNez1mGv3j1bmC6xO0dfxZe3YOmraEBSqzLGaoe UVJga9KYMG5ebcm3/1WhLNmrUBt4GplifCR1HcJrkbK8VnZ84veRbtuyXqi1rESuDx Dxu/jLbp8ezbIPP154mbSd6uPmD60Mnbv9OQeXKGBSL4dQqbN592FuHAdql9b58/9g 3FaXaeINNMAsQ== Date: Sat, 19 Aug 2023 16:41:09 +0200 From: Simon Horman To: Daniel Golle Cc: Felix Fietkau , John Crispin , Sean Wang , Mark Lee , Lorenzo Bianconi , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Matthias Brugger , AngeloGioacchino Del Regno , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org Subject: Re: [PATCH net] net: ethernet: mtk_eth_soc: add reset bits for MT7988 Message-ID: References: Precedence: bulk X-Mailing-List: netdev@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: On Fri, Aug 18, 2023 at 07:15:24PM +0100, Daniel Golle wrote: > Add bits needed to reset the frame engine on MT7988. > > Fixes: 445eb6448ed3 ("net: ethernet: mtk_eth_soc: add basic support for MT7988 SoC") > Signed-off-by: Daniel Golle > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 76 +++++++++++++++------ > drivers/net/ethernet/mediatek/mtk_eth_soc.h | 11 ++- > 2 files changed, 64 insertions(+), 23 deletions(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > index fe05c90202699..2482f47313085 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > @@ -3613,19 +3613,34 @@ static void mtk_hw_reset(struct mtk_eth *eth) > { > u32 val; > > - if (mtk_is_netsys_v2_or_greater(eth)) { > + if (mtk_is_netsys_v2_or_greater(eth)) > regmap_write(eth->ethsys, ETHSYS_FE_RST_CHK_IDLE_EN, 0); > + > + if (mtk_is_netsys_v3_or_greater(eth)) { > + val = RSTCTRL_PPE0_V3; > + > + if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1)) > + val |= RSTCTRL_PPE1_V3; > + > + if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE2)) > + val |= RSTCTRL_PPE2; > + > + val |= RSTCTRL_WDMA0 | RSTCTRL_WDMA1 | RSTCTRL_WDMA2; > + } else if (mtk_is_netsys_v2_or_greater(eth)) { > val = RSTCTRL_PPE0_V2; > + > + if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1)) > + val |= RSTCTRL_PPE1; > } else { > val = RSTCTRL_PPE0; > } > > - if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1)) > - val |= RSTCTRL_PPE1; > - > ethsys_reset(eth, RSTCTRL_ETH | RSTCTRL_FE | val); > > - if (mtk_is_netsys_v2_or_greater(eth)) > + if (mtk_is_netsys_v3_or_greater(eth)) > + regmap_write(eth->ethsys, ETHSYS_FE_RST_CHK_IDLE_EN, > + 0x6f8ff); > + else if (mtk_is_netsys_v2_or_greater(eth)) > regmap_write(eth->ethsys, ETHSYS_FE_RST_CHK_IDLE_EN, > 0x3ffffff); > } > @@ -3651,13 +3666,21 @@ static void mtk_hw_warm_reset(struct mtk_eth *eth) > return; > } > > - if (mtk_is_netsys_v2_or_greater(eth)) > + if (mtk_is_netsys_v3_or_greater(eth)) { > + rst_mask = RSTCTRL_ETH | RSTCTRL_PPE0_V3; > + if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1)) > + rst_mask |= RSTCTRL_PPE1_V3; > + if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE2)) > + rst_mask |= RSTCTRL_PPE2; > + > + rst_mask |= RSTCTRL_WDMA0 | RSTCTRL_WDMA1 | RSTCTRL_WDMA2; > + } else if (mtk_is_netsys_v2_or_greater(eth)) { > rst_mask = RSTCTRL_ETH | RSTCTRL_PPE0_V2; > - else > + if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1)) > + rst_mask |= RSTCTRL_PPE1; > + } else { > rst_mask = RSTCTRL_ETH | RSTCTRL_PPE0; > - > - if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1)) > - rst_mask |= RSTCTRL_PPE1; > + } > > regmap_update_bits(eth->ethsys, ETHSYS_RSTCTRL, rst_mask, rst_mask); > Hi Daniel, The bits set by the code in the above two hunks seem both complex and similar. At the risk of suggesting excessive complexity, I do wonder if they can be consolidated somehow. Maybe the approach you have taken is best as a fix for net. But a follow-up could be considered for net-next. Just an idea. ...