Messages in this thread | | | From | Jianheng Zhang <> | Subject | RE: [PATCH] net: stmmac: fix FPE events losing | Date | Wed, 15 Nov 2023 16:00:42 +0000 |
| |
> -----Original Message----- > From: Russell King <linux@armlinux.org.uk> > Sent: Wednesday, November 15, 2023 10:26 PM > To: Serge Semin <fancer.lancer@gmail.com> > Cc: Jianheng Zhang <jianheng@synopsys.com>; Alexandre Torgue <alexandre.torgue@foss.st.com>; Jose > Abreu <joabreu@synopsys.com>; David S. Miller <davem@davemloft.net>; Eric Dumazet > <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; > Maxime Coquelin <mcoquelin.stm32@gmail.com>; Tan Tee Min <tee.min.tan@intel.com>; Ong Boon > Leong <boon.leong.ong@intel.com>; Voon Weifeng <weifeng.voon@intel.com>; Mohammad Athari Bin > Ismail <mohammad.athari.ismail@intel.com>; netdev@vger.kernel.org; > linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH] net: stmmac: fix FPE events losing > > On Tue, Nov 14, 2023 at 02:59:57PM +0300, Serge Semin wrote: > > On Tue, Nov 14, 2023 at 11:07:34AM +0000, Jianheng Zhang wrote: > > > The 32-bit access of register MAC_FPE_CTRL_STS may clear the FPE status > > > bits unexpectedly. Use 8-bit access for MAC_FPE_CTRL_STS control bits to > > > avoid unexpected access of MAC_FPE_CTRL_STS status bits that can reduce > > > the FPE handshake retries. > > > > > > The bit[19:17] of register MAC_FPE_CTRL_STS are status register bits. > > > Those bits are clear on read (or write of 1 when RCWE bit in > > > MAC_CSR_SW_Ctrl register is set). Using 32-bit access for > > > MAC_FPE_CTRL_STS control bits makes side effects that clear the status > > > bits. Then the stmmac interrupt handler missing FPE event status and > > > leads to FPE handshake failure and retries. > > > > > > The bit[7:0] of register MAC_FPE_CTRL_STS are control bits or reserved > > > that have no access side effects, so can use 8-bit access for > > > MAC_FPE_CTRL_STS control bits. > > > > > > Fixes: 5a5586112b92 ("net: stmmac: support FPE link partner hand-shaking procedure") > > > Signed-off-by: jianheng <jianheng@synopsys.com> > > > --- > > > drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 12 ++++++------ > > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c > b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c > > > index e95d35f..7333995 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c > > > @@ -716,11 +716,11 @@ void dwmac5_fpe_configure(void __iomem *ioaddr, u32 num_txq, u32 > num_rxq, > > > u32 value; > > > > > > if (!enable) { > > > > > - value = readl(ioaddr + MAC_FPE_CTRL_STS); > > > + value = readb(ioaddr + MAC_FPE_CTRL_STS); > > > > Note this may break the platforms which don't support non-32 MMIOs for > > some devices. None of the currently supported glue-drivers explicitly > > state they have such peculiarity, but at the same time the STMMAC-core > > driver at the present state uses the dword IO ops only. For instance > > the PCIe subsystem has the special accessors for such cases: > > pci_generic_config_read32() > > pci_generic_config_write32() > > which at the very least are utilized on the Tegra and Loongson > > platforms to access the host CSR spaces. These platforms are also > > equipped with the DW MACs. The problem might be irrelevant for all the > > currently supported DW MAC controllers implementations though, but > > still it worth to draw an attention to the problem possibility and in > > order to prevent it before ahead it would be better to just avoid > > using the byte-/word- IOs if it's possible. > > Yes, this exists for configuration accesses, and is damn annoying > because the read-modify-write of these can end up clearing PCI > status register bits that are W1C. > > I've never heard of that problem with MMIO though. > > -- > RMK's Patch system: > https://urldefense.com/v3/__https://www.armlinux.org.uk/developer/patches/__;!!A4F2R9G_pg!ZnsQc > 7LKbdCiIj_nLBkt44MRSL8W8YD4WvMmPNkHFlDMrjrVqEVOVA1-P-9BVE3Oa5bC1ymKXLNV5C9mJ_rfFtLW > Fg$ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! Thanks for all your mention about the potential risks of using readb and suggestions, I'll consider a new solution and update the patch.
Best Regards, Jianheng
| |