Commit aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts
by default") tries to disable MMC interrupts to avoid a storm of
unhandled interrupts, but leaves the FPE(Frame Preemption) MMC
interrupts enabled.
Now we mask FPE TX and RX interrupts to disable all MMC interrupts.
Fixes: aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts by default")
Signed-off-by: Furong Xu <[email protected]>
---
Changes in v2:
- Update commit message, thanks Wojciech and Andrew.
---
drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
index ea4910ae0921..cdd7fbde2bfa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
@@ -177,8 +177,10 @@
#define MMC_XGMAC_RX_DISCARD_OCT_GB 0x1b4
#define MMC_XGMAC_RX_ALIGN_ERR_PKT 0x1bc
+#define MMC_XGMAC_FPE_TX_INTR_MASK 0x204
#define MMC_XGMAC_TX_FPE_FRAG 0x208
#define MMC_XGMAC_TX_HOLD_REQ 0x20c
+#define MMC_XGMAC_FPE_RX_INTR_MASK 0x224
#define MMC_XGMAC_RX_PKT_ASSEMBLY_ERR 0x228
#define MMC_XGMAC_RX_PKT_SMD_ERR 0x22c
#define MMC_XGMAC_RX_PKT_ASSEMBLY_OK 0x230
@@ -352,6 +354,8 @@ static void dwxgmac_mmc_intr_all_mask(void __iomem *mmcaddr)
{
writel(0x0, mmcaddr + MMC_RX_INTR_MASK);
writel(0x0, mmcaddr + MMC_TX_INTR_MASK);
+ writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_FPE_TX_INTR_MASK);
+ writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_FPE_RX_INTR_MASK);
writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_RX_IPC_INTR_MASK);
}
--
2.34.1
On Fri, Nov 24, 2023 at 09:54:33AM +0800, Furong Xu wrote:
> Commit aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts
> by default") tries to disable MMC interrupts to avoid a storm of
> unhandled interrupts, but leaves the FPE(Frame Preemption) MMC
> interrupts enabled.
Now commit message is lacking something like "FPE MMC interrupts can cause the
same problem", but I think it is good enough.
Reviewed-by: Larysa Zaremba <[email protected]>
> Now we mask FPE TX and RX interrupts to disable all MMC interrupts.
>
> Fixes: aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts by default")
> Signed-off-by: Furong Xu <[email protected]>
> ---
> Changes in v2:
> - Update commit message, thanks Wojciech and Andrew.
> ---
> drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
> index ea4910ae0921..cdd7fbde2bfa 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
> @@ -177,8 +177,10 @@
> #define MMC_XGMAC_RX_DISCARD_OCT_GB 0x1b4
> #define MMC_XGMAC_RX_ALIGN_ERR_PKT 0x1bc
>
> +#define MMC_XGMAC_FPE_TX_INTR_MASK 0x204
> #define MMC_XGMAC_TX_FPE_FRAG 0x208
> #define MMC_XGMAC_TX_HOLD_REQ 0x20c
> +#define MMC_XGMAC_FPE_RX_INTR_MASK 0x224
> #define MMC_XGMAC_RX_PKT_ASSEMBLY_ERR 0x228
> #define MMC_XGMAC_RX_PKT_SMD_ERR 0x22c
> #define MMC_XGMAC_RX_PKT_ASSEMBLY_OK 0x230
> @@ -352,6 +354,8 @@ static void dwxgmac_mmc_intr_all_mask(void __iomem *mmcaddr)
> {
> writel(0x0, mmcaddr + MMC_RX_INTR_MASK);
> writel(0x0, mmcaddr + MMC_TX_INTR_MASK);
> + writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_FPE_TX_INTR_MASK);
> + writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_FPE_RX_INTR_MASK);
> writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_RX_IPC_INTR_MASK);
> }
>
> --
> 2.34.1
>
>
On Fri, Nov 24, 2023 at 09:54:33AM +0800, Furong Xu wrote:
> Commit aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts
> by default") tries to disable MMC interrupts to avoid a storm of
> unhandled interrupts, but leaves the FPE(Frame Preemption) MMC
> interrupts enabled.
> Now we mask FPE TX and RX interrupts to disable all MMC interrupts.
>
> Fixes: aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts by default")
> Signed-off-by: Furong Xu <[email protected]>
> ---
> Changes in v2:
> - Update commit message, thanks Wojciech and Andrew.
> ---
> drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
> index ea4910ae0921..cdd7fbde2bfa 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
> @@ -177,8 +177,10 @@
> #define MMC_XGMAC_RX_DISCARD_OCT_GB 0x1b4
> #define MMC_XGMAC_RX_ALIGN_ERR_PKT 0x1bc
>
> +#define MMC_XGMAC_FPE_TX_INTR_MASK 0x204
> #define MMC_XGMAC_TX_FPE_FRAG 0x208
> #define MMC_XGMAC_TX_HOLD_REQ 0x20c
> +#define MMC_XGMAC_FPE_RX_INTR_MASK 0x224
Could you please preserve the local implicit naming convention of
having the Tx_ and RX_ prefixes being placed before the rest of
CSR-specific name part:
#define MMC_XGMAC_TX_FPE_INTR_MASK
instead of
#define MMC_XGMAC_FPE_TX_INTR_MASK
and
#define MMC_XGMAC_RX_FPE_INTR_MASK
instead of
#define MMC_XGMAC_FPE_RX_INTR_MASK
Your macros will then look similar to MMC_XGMAC_TX_*, MMC_XGMAC_RX_*
and finally MMC_XGMAC_RX_IPC_INTR_MASK macros.
-Serge(y)
> #define MMC_XGMAC_RX_PKT_ASSEMBLY_ERR 0x228
> #define MMC_XGMAC_RX_PKT_SMD_ERR 0x22c
> #define MMC_XGMAC_RX_PKT_ASSEMBLY_OK 0x230
> @@ -352,6 +354,8 @@ static void dwxgmac_mmc_intr_all_mask(void __iomem *mmcaddr)
> {
> writel(0x0, mmcaddr + MMC_RX_INTR_MASK);
> writel(0x0, mmcaddr + MMC_TX_INTR_MASK);
> + writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_FPE_TX_INTR_MASK);
> + writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_FPE_RX_INTR_MASK);
> writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_RX_IPC_INTR_MASK);
> }
>
> --
> 2.34.1
>
>
On Fri, 24 Nov 2023 20:03:43 +0300
Serge Semin <[email protected]> wrote:
> On Fri, Nov 24, 2023 at 09:54:33AM +0800, Furong Xu wrote:
> > Commit aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts
> > by default") tries to disable MMC interrupts to avoid a storm of
> > unhandled interrupts, but leaves the FPE(Frame Preemption) MMC
> > interrupts enabled.
> > Now we mask FPE TX and RX interrupts to disable all MMC interrupts.
> >
> > Fixes: aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts by default")
> > Signed-off-by: Furong Xu <[email protected]>
> > ---
> > Changes in v2:
> > - Update commit message, thanks Wojciech and Andrew.
> > ---
> > drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
> > index ea4910ae0921..cdd7fbde2bfa 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
> > @@ -177,8 +177,10 @@
> > #define MMC_XGMAC_RX_DISCARD_OCT_GB 0x1b4
> > #define MMC_XGMAC_RX_ALIGN_ERR_PKT 0x1bc
> >
>
> > +#define MMC_XGMAC_FPE_TX_INTR_MASK 0x204
> > #define MMC_XGMAC_TX_FPE_FRAG 0x208
> > #define MMC_XGMAC_TX_HOLD_REQ 0x20c
> > +#define MMC_XGMAC_FPE_RX_INTR_MASK 0x224
>
> Could you please preserve the local implicit naming convention of
> having the Tx_ and RX_ prefixes being placed before the rest of
> CSR-specific name part:
> #define MMC_XGMAC_TX_FPE_INTR_MASK
> instead of
> #define MMC_XGMAC_FPE_TX_INTR_MASK
> and
> #define MMC_XGMAC_RX_FPE_INTR_MASK
> instead of
> #define MMC_XGMAC_FPE_RX_INTR_MASK
>
> Your macros will then look similar to MMC_XGMAC_TX_*, MMC_XGMAC_RX_*
> and finally MMC_XGMAC_RX_IPC_INTR_MASK macros.
>
> -Serge(y)
>
Hi Serge,
Thanks for your advice, I copied these register names from Synopsys Databook, and
forgot to preserve the local implicit naming convention, I will send a new patch.
> > #define MMC_XGMAC_RX_PKT_ASSEMBLY_ERR 0x228
> > #define MMC_XGMAC_RX_PKT_SMD_ERR 0x22c
> > #define MMC_XGMAC_RX_PKT_ASSEMBLY_OK 0x230
> > @@ -352,6 +354,8 @@ static void dwxgmac_mmc_intr_all_mask(void __iomem *mmcaddr)
> > {
> > writel(0x0, mmcaddr + MMC_RX_INTR_MASK);
> > writel(0x0, mmcaddr + MMC_TX_INTR_MASK);
> > + writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_FPE_TX_INTR_MASK);
> > + writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_FPE_RX_INTR_MASK);
> > writel(MMC_DEFAULT_MASK, mmcaddr + MMC_XGMAC_RX_IPC_INTR_MASK);
> > }
> >
> > --
> > 2.34.1
> >
> >