2023-11-23 09:38:10

by Furong Xu

[permalink] [raw]
Subject: [PATCH net v1] net: stmmac: xgmac: Disable FPE MMC interrupts

Commit aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts
by default") leaves the FPE(Frame Preemption) MMC interrupts enabled.
Now we disable FPE TX and RX interrupts too.

Fixes: aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts by default")
Signed-off-by: Furong Xu <[email protected]>
---
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


2023-11-23 10:17:44

by Wojciech Drewek

[permalink] [raw]
Subject: Re: [PATCH net v1] net: stmmac: xgmac: Disable FPE MMC interrupts



On 23.11.2023 10:35, Furong Xu wrote:
> Commit aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts
> by default") leaves the FPE(Frame Preemption) MMC interrupts enabled.
> Now we disable FPE TX and RX interrupts too.

Hi,
Thanks for the patch, one question:
Why do we have to disable them?

>
> Fixes: aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts by default")
> Signed-off-by: Furong Xu <[email protected]>
> ---
> 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);
> }
>

2023-11-23 10:24:28

by Furong Xu

[permalink] [raw]
Subject: Re: [PATCH net v1] net: stmmac: xgmac: Disable FPE MMC interrupts

On Thu, 23 Nov 2023 11:17:17 +0100
Wojciech Drewek <[email protected]> wrote:

> On 23.11.2023 10:35, Furong Xu wrote:
> > Commit aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts
> > by default") leaves the FPE(Frame Preemption) MMC interrupts enabled.
> > Now we disable FPE TX and RX interrupts too.
>
> Hi,
> Thanks for the patch, one question:
> Why do we have to disable them?
>

The original commit aeb18dd07692 by Jose Abreu says:

MMC interrupts were being enabled, which is not what we want because it
will lead to a storm of interrupts that are not handled at all. Fix it
by disabling all MMC interrupts for XGMAC.

> >
> > Fixes: aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts by default")
> > Signed-off-by: Furong Xu <[email protected]>
> > ---
> > 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);
> > }
> >

2023-11-23 10:34:58

by Wojciech Drewek

[permalink] [raw]
Subject: Re: [PATCH net v1] net: stmmac: xgmac: Disable FPE MMC interrupts



On 23.11.2023 11:24, Furong Xu wrote:
> On Thu, 23 Nov 2023 11:17:17 +0100
> Wojciech Drewek <[email protected]> wrote:
>
>> On 23.11.2023 10:35, Furong Xu wrote:
>>> Commit aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts
>>> by default") leaves the FPE(Frame Preemption) MMC interrupts enabled.
>>> Now we disable FPE TX and RX interrupts too.
>>
>> Hi,
>> Thanks for the patch, one question:
>> Why do we have to disable them?
>>
>
> The original commit aeb18dd07692 by Jose Abreu says:
>
> MMC interrupts were being enabled, which is not what we want because it
> will lead to a storm of interrupts that are not handled at all. Fix it
> by disabling all MMC interrupts for XGMAC.

I would add this information the commit message.

>
>>>
>>> Fixes: aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts by default")
>>> Signed-off-by: Furong Xu <[email protected]>
>>> ---
>>> 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);
>>> }
>>>
>

2023-11-23 18:58:44

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net v1] net: stmmac: xgmac: Disable FPE MMC interrupts

On Thu, Nov 23, 2023 at 05:35:38PM +0800, Furong Xu wrote:
> Commit aeb18dd07692 ("net: stmmac: xgmac: Disable MMC interrupts
> by default") leaves the FPE(Frame Preemption) MMC interrupts enabled.
> Now we disable FPE TX and RX interrupts too.

Just adding to Wojciech reply. We should be able to see 'What' a patch
does by reading the actual change. What is not always obvious is
'Why?' The commit message should be about the 'Why?", and less about
the 'What'.

Andrew