diff mbox series

[net,v1] net: stmmac: xgmac: Disable FPE MMC interrupts

Message ID 20231123093538.2216633-1-0x1207@gmail.com
State New
Headers
Series [net,v1] net: stmmac: xgmac: Disable FPE MMC interrupts |

Commit Message

Furong Xu Nov. 23, 2023, 9:35 a.m. UTC 
  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 <0x1207@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Wojciech Drewek Nov. 23, 2023, 10:17 a.m. UTC | #1 
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 <0x1207@gmail.com>
> ---
>  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);
>  }
>
Furong Xu Nov. 23, 2023, 10:24 a.m. UTC | #2 
On Thu, 23 Nov 2023 11:17:17 +0100
Wojciech Drewek <wojciech.drewek@intel.com> 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 <0x1207@gmail.com>
> > ---
> >  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);
> >  }
> >
Wojciech Drewek Nov. 23, 2023, 10:34 a.m. UTC | #3 
On 23.11.2023 11:24, Furong Xu wrote:
> On Thu, 23 Nov 2023 11:17:17 +0100
> Wojciech Drewek <wojciech.drewek@intel.com> 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 <0x1207@gmail.com>
>>> ---
>>>  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);
>>>  }
>>>    
>
Andrew Lunn Nov. 23, 2023, 6:57 p.m. UTC | #4 
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
diff mbox series

Patch

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);
 }