All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: dp83822: disable false carrier interrupt
@ 2022-06-17 13:46 Enguerrand de Ribaucourt
  2022-06-17 17:55 ` Andrew Lunn
  0 siblings, 1 reply; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-17 13:46 UTC (permalink / raw)
  To: andrew, davem
  Cc: netdev, linux-kernel, linux, hkallweit1, Enguerrand de Ribaucourt

When unplugging an Ethernet cable, false carrier events were produced by
the PHY at a very high rate. Once the false carrier counter full, an
interrupt was triggered every few clock cycles until the cable was
replugged. This resulted in approximately 10k/s interrupts.

Since the false carrier counter (FCSCR) is never used, we can safely
disable this interrupt.

In addition to improving performance, this also solved MDIO read
timeouts I was randomly encountering with an i.MX8 fec MAC because of
the interrupt flood. The interrupt count and MDIO timeout fix were
tested on a v5.4.110 kernel.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
---
 drivers/net/phy/dp83822.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index e6ad3a494d32..95ef507053a6 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -230,7 +230,6 @@ static int dp83822_config_intr(struct phy_device *phydev)
 			return misr_status;
 
 		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
-				DP83822_FALSE_CARRIER_HF_INT_EN |
 				DP83822_LINK_STAT_INT_EN |
 				DP83822_ENERGY_DET_INT_EN |
 				DP83822_LINK_QUAL_INT_EN);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] net: dp83822: disable false carrier interrupt
  2022-06-17 13:46 [PATCH] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
@ 2022-06-17 17:55 ` Andrew Lunn
  2022-06-23  8:51   ` Enguerrand de Ribaucourt
                     ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Andrew Lunn @ 2022-06-17 17:55 UTC (permalink / raw)
  To: Enguerrand de Ribaucourt; +Cc: davem, netdev, linux-kernel, linux, hkallweit1

On Fri, Jun 17, 2022 at 03:46:11PM +0200, Enguerrand de Ribaucourt wrote:
> When unplugging an Ethernet cable, false carrier events were produced by
> the PHY at a very high rate. Once the false carrier counter full, an
> interrupt was triggered every few clock cycles until the cable was
> replugged. This resulted in approximately 10k/s interrupts.
> 
> Since the false carrier counter (FCSCR) is never used, we can safely
> disable this interrupt.
> 
> In addition to improving performance, this also solved MDIO read
> timeouts I was randomly encountering with an i.MX8 fec MAC because of
> the interrupt flood. The interrupt count and MDIO timeout fix were
> tested on a v5.4.110 kernel.
> 
> Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
> ---
>  drivers/net/phy/dp83822.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index e6ad3a494d32..95ef507053a6 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -230,7 +230,6 @@ static int dp83822_config_intr(struct phy_device *phydev)
>  			return misr_status;
>  
>  		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
> -				DP83822_FALSE_CARRIER_HF_INT_EN |

Does the same problem exist for RX_ERR_HF_INT ? That appears to be
that the RX error counter has reached half full. I don't see anything
using register 0x15.

      Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] net: dp83822: disable false carrier interrupt
  2022-06-17 17:55 ` Andrew Lunn
@ 2022-06-23  8:51   ` Enguerrand de Ribaucourt
  2022-06-23  8:51   ` [PATCH 1/2] " Enguerrand de Ribaucourt
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23  8:51 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: davem, netdev, linux-kernel, linux, hkallweit1

----- Original Message -----
> From: "Andrew Lunn" <andrew@lunn.ch>
> To: "Enguerrand de Ribaucourt" <enguerrand.de-ribaucourt@savoirfairelinux.com>
> Cc: davem@davemloft.net, "netdev" <netdev@vger.kernel.org>, linux-kernel@vger.kernel.org, "linux"
> <linux@armlinux.org.uk>, "hkallweit1" <hkallweit1@gmail.com>
> Sent: Friday, June 17, 2022 7:55:54 PM
> Subject: Re: [PATCH] net: dp83822: disable false carrier interrupt

> On Fri, Jun 17, 2022 at 03:46:11PM +0200, Enguerrand de Ribaucourt wrote:
> > When unplugging an Ethernet cable, false carrier events were produced by
> > the PHY at a very high rate. Once the false carrier counter full, an
> > interrupt was triggered every few clock cycles until the cable was
> > replugged. This resulted in approximately 10k/s interrupts.

> > Since the false carrier counter (FCSCR) is never used, we can safely
> > disable this interrupt.

> > In addition to improving performance, this also solved MDIO read
> > timeouts I was randomly encountering with an i.MX8 fec MAC because of
> > the interrupt flood. The interrupt count and MDIO timeout fix were
> > tested on a v5.4.110 kernel.

>> Signed-off-by: Enguerrand de Ribaucourt
> > <enguerrand.de-ribaucourt@savoirfairelinux.com>
> > ---
> > drivers/net/phy/dp83822.c | 1 -
> > 1 file changed, 1 deletion(-)

> > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > index e6ad3a494d32..95ef507053a6 100644
> > --- a/drivers/net/phy/dp83822.c
> > +++ b/drivers/net/phy/dp83822.c
> > @@ -230,7 +230,6 @@ static int dp83822_config_intr(struct phy_device *phydev)
> > return misr_status;

> > misr_status |= (DP83822_RX_ERR_HF_INT_EN |
> > - DP83822_FALSE_CARRIER_HF_INT_EN |

> Does the same problem exist for RX_ERR_HF_INT ? That appears to be
> that the RX error counter has reached half full. I don't see anything
> using register 0x15.

> Andrew

I can produce RX errors using improper ethtool speed settings, which can be seen
in the statistics, but they do not increment register 0x15. However, RX errors due 
to cable disconnection do increase it. After the counter is half full (0x7fff, 
the datasheet is wrong), interrupts that we don't need were indeed generated.

I measured a ~3k/s interrupt flood which also kept going even if I stopped the 
RX errors transfer so we should definitively disable RX_ERR_HF_INT as well.

I'll send a separate patch for RX_ERR_HF_INT_EN to have clear commmit messages.

Thanks,
Enguerrand

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2] net: dp83822: disable false carrier interrupt
  2022-06-17 17:55 ` Andrew Lunn
  2022-06-23  8:51   ` Enguerrand de Ribaucourt
@ 2022-06-23  8:51   ` Enguerrand de Ribaucourt
  2022-06-23 12:42     ` Andrew Lunn
  2022-06-23  8:51   ` [PATCH 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23  8:51 UTC (permalink / raw)
  To: andrew
  Cc: davem, netdev, linux-kernel, linux, hkallweit1, Enguerrand de Ribaucourt

When unplugging an Ethernet cable, false carrier events were produced by
the PHY at a very high rate. Once the false carrier counter full, an
interrupt was triggered every few clock cycles until the cable was
replugged. This resulted in approximately 10k/s interrupts.

Since the false carrier counter (FCSCR) is never used, we can safely
disable this interrupt.

In addition to improving performance, this also solved MDIO read
timeouts I was randomly encountering with an i.MX8 fec MAC because of
the interrupt flood. The interrupt count and MDIO timeout fix were
tested on a v5.4.110 kernel.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/dp83822.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index e6ad3a494d32..95ef507053a6 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -230,7 +230,6 @@ static int dp83822_config_intr(struct phy_device *phydev)
 			return misr_status;

 		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
-				DP83822_FALSE_CARRIER_HF_INT_EN |
 				DP83822_LINK_STAT_INT_EN |
 				DP83822_ENERGY_DET_INT_EN |
 				DP83822_LINK_QUAL_INT_EN);
--
2.25.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] net: dp83822: disable rx error interrupt
  2022-06-17 17:55 ` Andrew Lunn
  2022-06-23  8:51   ` Enguerrand de Ribaucourt
  2022-06-23  8:51   ` [PATCH 1/2] " Enguerrand de Ribaucourt
@ 2022-06-23  8:51   ` Enguerrand de Ribaucourt
  2022-06-23 13:06   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23  8:51 UTC (permalink / raw)
  To: andrew
  Cc: davem, netdev, linux-kernel, linux, hkallweit1, Enguerrand de Ribaucourt

Some RX errors, notably when disconnecting the cable, increase the RCSR
register. Once half full (0x7fff), an interrupt flood is generated. I
measured ~3k/s interrupts even after the RX errors transfer was
stopped.

Since we don't read and clear the RCSR register, we should disable this
interrupt.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
---
 drivers/net/phy/dp83822.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 95ef507053a6..8549e0e356c9 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -229,8 +229,7 @@ static int dp83822_config_intr(struct phy_device *phydev)
 		if (misr_status < 0)
 			return misr_status;
 
-		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
-				DP83822_LINK_STAT_INT_EN |
+		misr_status |= (DP83822_LINK_STAT_INT_EN |
 				DP83822_ENERGY_DET_INT_EN |
 				DP83822_LINK_QUAL_INT_EN);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] net: dp83822: disable false carrier interrupt
  2022-06-23  8:51   ` [PATCH 1/2] " Enguerrand de Ribaucourt
@ 2022-06-23 12:42     ` Andrew Lunn
  2022-06-23 13:16       ` Enguerrand de Ribaucourt
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Lunn @ 2022-06-23 12:42 UTC (permalink / raw)
  To: Enguerrand de Ribaucourt; +Cc: davem, netdev, linux-kernel, linux, hkallweit1

On Thu, Jun 23, 2022 at 10:51:25AM +0200, Enguerrand de Ribaucourt wrote:
> When unplugging an Ethernet cable, false carrier events were produced by
> the PHY at a very high rate. Once the false carrier counter full, an
> interrupt was triggered every few clock cycles until the cable was
> replugged. This resulted in approximately 10k/s interrupts.
> 
> Since the false carrier counter (FCSCR) is never used, we can safely
> disable this interrupt.
> 
> In addition to improving performance, this also solved MDIO read
> timeouts I was randomly encountering with an i.MX8 fec MAC because of
> the interrupt flood. The interrupt count and MDIO timeout fix were
> tested on a v5.4.110 kernel.

Since this is version 2, you should add v2 into the subject line. See
the submitting patches document in the kernel documentation.

Also, with patch sets, please include a patch 0/X which describes the
big picture.

This is also a bug fix, you are stopping an interrupt storm. So please
include a Fixes: tag indicating where the issue was introduced.

The code itself looks good, it is just getting the processes right.

	Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 0/2] net: dp83822: fix interrupt floods
  2022-06-17 17:55 ` Andrew Lunn
                     ` (2 preceding siblings ...)
  2022-06-23  8:51   ` [PATCH 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
@ 2022-06-23 13:06   ` Enguerrand de Ribaucourt
  2022-06-23 13:06     ` [PATCH v2 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
  2022-06-23 13:06     ` [PATCH v2 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
  2022-06-23 13:14   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
  2022-06-23 13:46   ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
  5 siblings, 2 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:06 UTC (permalink / raw)
  To: andrew; +Cc: davem, netdev, linux-kernel, linux, hkallweit1

The false carrier and RX error counters, once half full, produce interrupt
floods. Since we do not use these counters, these interrupts should be disabled.

In-Reply-To: YqzAKguRaxr74oXh@lunn.ch
Fixes: 87461f7a58ab694e638ac52afa543b427751a9d0



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/2] net: dp83822: disable false carrier interrupt
  2022-06-23 13:06   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
@ 2022-06-23 13:06     ` Enguerrand de Ribaucourt
  2022-06-23 13:06     ` [PATCH v2 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
  1 sibling, 0 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:06 UTC (permalink / raw)
  To: andrew
  Cc: davem, netdev, linux-kernel, linux, hkallweit1, Enguerrand de Ribaucourt

When unplugging an Ethernet cable, false carrier events were produced by
the PHY at a very high rate. Once the false carrier counter full, an
interrupt was triggered every few clock cycles until the cable was
replugged. This resulted in approximately 10k/s interrupts.

Since the false carrier counter (FCSCR) is never used, we can safely
disable this interrupt.

In addition to improving performance, this also solved MDIO read
timeouts I was randomly encountering with an i.MX8 fec MAC because of
the interrupt flood. The interrupt count and MDIO timeout fix were
tested on a v5.4.110 kernel.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/dp83822.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index e6ad3a494d32..95ef507053a6 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -230,7 +230,6 @@ static int dp83822_config_intr(struct phy_device *phydev)
 			return misr_status;

 		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
-				DP83822_FALSE_CARRIER_HF_INT_EN |
 				DP83822_LINK_STAT_INT_EN |
 				DP83822_ENERGY_DET_INT_EN |
 				DP83822_LINK_QUAL_INT_EN);
--
2.25.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/2] net: dp83822: disable rx error interrupt
  2022-06-23 13:06   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
  2022-06-23 13:06     ` [PATCH v2 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
@ 2022-06-23 13:06     ` Enguerrand de Ribaucourt
  1 sibling, 0 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:06 UTC (permalink / raw)
  To: andrew
  Cc: davem, netdev, linux-kernel, linux, hkallweit1, Enguerrand de Ribaucourt

Some RX errors, notably when disconnecting the cable, increase the RCSR
register. Once half full (0x7fff), an interrupt flood is generated. I
measured ~3k/s interrupts even after the RX errors transfer was
stopped.

Since we don't read and clear the RCSR register, we should disable this
interrupt.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/dp83822.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 95ef507053a6..8549e0e356c9 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -229,8 +229,7 @@ static int dp83822_config_intr(struct phy_device *phydev)
 		if (misr_status < 0)
 			return misr_status;

-		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
-				DP83822_LINK_STAT_INT_EN |
+		misr_status |= (DP83822_LINK_STAT_INT_EN |
 				DP83822_ENERGY_DET_INT_EN |
 				DP83822_LINK_QUAL_INT_EN);

--
2.25.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 0/2] net: dp83822: fix interrupt floods
  2022-06-17 17:55 ` Andrew Lunn
                     ` (3 preceding siblings ...)
  2022-06-23 13:06   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
@ 2022-06-23 13:14   ` Enguerrand de Ribaucourt
  2022-06-23 13:14     ` [PATCH v2 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
  2022-06-23 13:14     ` [PATCH v2 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
  2022-06-23 13:46   ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
  5 siblings, 2 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:14 UTC (permalink / raw)
  To: andrew; +Cc: davem, netdev, linux-kernel, linux, hkallweit1

The false carrier and RX error counters, once half full, produce interrupt
floods. Since we do not use these counters, these interrupts should be disabled.

In-Reply-To: YqzAKguRaxr74oXh@lunn.ch
Fixes: 87461f7a58ab694e638ac52afa543b427751a9d0



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/2] net: dp83822: disable false carrier interrupt
  2022-06-23 13:14   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
@ 2022-06-23 13:14     ` Enguerrand de Ribaucourt
  2022-06-23 13:14     ` [PATCH v2 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
  1 sibling, 0 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:14 UTC (permalink / raw)
  To: andrew
  Cc: davem, netdev, linux-kernel, linux, hkallweit1, Enguerrand de Ribaucourt

When unplugging an Ethernet cable, false carrier events were produced by
the PHY at a very high rate. Once the false carrier counter full, an
interrupt was triggered every few clock cycles until the cable was
replugged. This resulted in approximately 10k/s interrupts.

Since the false carrier counter (FCSCR) is never used, we can safely
disable this interrupt.

In addition to improving performance, this also solved MDIO read
timeouts I was randomly encountering with an i.MX8 fec MAC because of
the interrupt flood. The interrupt count and MDIO timeout fix were
tested on a v5.4.110 kernel.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Fixes: 87461f7a58ab694e638ac52afa543b427751a9d0
---
 drivers/net/phy/dp83822.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index e6ad3a494d32..95ef507053a6 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -230,7 +230,6 @@ static int dp83822_config_intr(struct phy_device *phydev)
 			return misr_status;
 
 		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
-				DP83822_FALSE_CARRIER_HF_INT_EN |
 				DP83822_LINK_STAT_INT_EN |
 				DP83822_ENERGY_DET_INT_EN |
 				DP83822_LINK_QUAL_INT_EN);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/2] net: dp83822: disable rx error interrupt
  2022-06-23 13:14   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
  2022-06-23 13:14     ` [PATCH v2 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
@ 2022-06-23 13:14     ` Enguerrand de Ribaucourt
  1 sibling, 0 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:14 UTC (permalink / raw)
  To: andrew
  Cc: davem, netdev, linux-kernel, linux, hkallweit1, Enguerrand de Ribaucourt

Some RX errors, notably when disconnecting the cable, increase the RCSR
register. Once half full (0x7fff), an interrupt flood is generated. I
measured ~3k/s interrupts even after the RX errors transfer was
stopped.

Since we don't read and clear the RCSR register, we should disable this
interrupt.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Fixes: 87461f7a58ab694e638ac52afa543b427751a9d0
---
 drivers/net/phy/dp83822.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 95ef507053a6..8549e0e356c9 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -229,8 +229,7 @@ static int dp83822_config_intr(struct phy_device *phydev)
 		if (misr_status < 0)
 			return misr_status;
 
-		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
-				DP83822_LINK_STAT_INT_EN |
+		misr_status |= (DP83822_LINK_STAT_INT_EN |
 				DP83822_ENERGY_DET_INT_EN |
 				DP83822_LINK_QUAL_INT_EN);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] net: dp83822: disable false carrier interrupt
  2022-06-23 12:42     ` Andrew Lunn
@ 2022-06-23 13:16       ` Enguerrand de Ribaucourt
  0 siblings, 0 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:16 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: davem, netdev, linux-kernel, linux, hkallweit1

----- Original Message -----
> From: "Andrew Lunn" <andrew@lunn.ch>
> To: "Enguerrand de Ribaucourt" <enguerrand.de-ribaucourt@savoirfairelinux.com>
> Cc: "davem" <davem@davemloft.net>, "netdev" <netdev@vger.kernel.org>, "linux-kernel" <linux-kernel@vger.kernel.org>,
> "linux" <linux@armlinux.org.uk>, "hkallweit1" <hkallweit1@gmail.com>
> Sent: Thursday, June 23, 2022 2:42:54 PM
> Subject: Re: [PATCH 1/2] net: dp83822: disable false carrier interrupt

> On Thu, Jun 23, 2022 at 10:51:25AM +0200, Enguerrand de Ribaucourt wrote:
> > When unplugging an Ethernet cable, false carrier events were produced by
> > the PHY at a very high rate. Once the false carrier counter full, an
> > interrupt was triggered every few clock cycles until the cable was
> > replugged. This resulted in approximately 10k/s interrupts.

> > Since the false carrier counter (FCSCR) is never used, we can safely
> > disable this interrupt.

> > In addition to improving performance, this also solved MDIO read
> > timeouts I was randomly encountering with an i.MX8 fec MAC because of
> > the interrupt flood. The interrupt count and MDIO timeout fix were
> > tested on a v5.4.110 kernel.

> Since this is version 2, you should add v2 into the subject line. See
> the submitting patches document in the kernel documentation.

> Also, with patch sets, please include a patch 0/X which describes the
> big picture.

> This is also a bug fix, you are stopping an interrupt storm. So please
> include a Fixes: tag indicating where the issue was introduced.

> The code itself looks good, it is just getting the processes right.

> Andrew

Sorry, I'm still not familiar with the process. I resubmitted the patches
with your recommendations.

Thank you very much for your advice.
Enguerrand

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 0/2] net: dp83822: fix interrupt floods
  2022-06-17 17:55 ` Andrew Lunn
                     ` (4 preceding siblings ...)
  2022-06-23 13:14   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
@ 2022-06-23 13:46   ` Enguerrand de Ribaucourt
  2022-06-23 13:46     ` [PATCH v3 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
                       ` (3 more replies)
  5 siblings, 4 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:46 UTC (permalink / raw)
  To: andrew; +Cc: davem, netdev, linux-kernel, linux, hkallweit1

The false carrier and RX error counters, once half full, produce interrupt
floods. Since we do not use these counters, these interrupts should be disabled.

v2: added Fixes: and patchset description 0/2
v3: Fixed Fixes: commit format

In-Reply-To: YqzAKguRaxr74oXh@lunn.ch
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Fixes: 87461f7a58ab ("net: phy: DP83822 initial driver submission")


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/2] net: dp83822: disable false carrier interrupt
  2022-06-23 13:46   ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
@ 2022-06-23 13:46     ` Enguerrand de Ribaucourt
  2022-06-23 18:27       ` Andrew Lunn
  2022-06-23 13:46     ` [PATCH v3 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:46 UTC (permalink / raw)
  To: andrew
  Cc: davem, netdev, linux-kernel, linux, hkallweit1, Enguerrand de Ribaucourt

When unplugging an Ethernet cable, false carrier events were produced by
the PHY at a very high rate. Once the false carrier counter full, an
interrupt was triggered every few clock cycles until the cable was
replugged. This resulted in approximately 10k/s interrupts.

Since the false carrier counter (FCSCR) is never used, we can safely
disable this interrupt.

In addition to improving performance, this also solved MDIO read
timeouts I was randomly encountering with an i.MX8 fec MAC because of
the interrupt flood. The interrupt count and MDIO timeout fix were
tested on a v5.4.110 kernel.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Fixes: 87461f7a58ab ("net: phy: DP83822 initial driver submission")
---
 drivers/net/phy/dp83822.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index e6ad3a494d32..95ef507053a6 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -230,7 +230,6 @@ static int dp83822_config_intr(struct phy_device *phydev)
 			return misr_status;
 
 		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
-				DP83822_FALSE_CARRIER_HF_INT_EN |
 				DP83822_LINK_STAT_INT_EN |
 				DP83822_ENERGY_DET_INT_EN |
 				DP83822_LINK_QUAL_INT_EN);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/2] net: dp83822: disable rx error interrupt
  2022-06-23 13:46   ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
  2022-06-23 13:46     ` [PATCH v3 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
@ 2022-06-23 13:46     ` Enguerrand de Ribaucourt
  2022-06-23 18:24     ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Andrew Lunn
  2022-06-24 23:50     ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 19+ messages in thread
From: Enguerrand de Ribaucourt @ 2022-06-23 13:46 UTC (permalink / raw)
  To: andrew
  Cc: davem, netdev, linux-kernel, linux, hkallweit1, Enguerrand de Ribaucourt

Some RX errors, notably when disconnecting the cable, increase the RCSR
register. Once half full (0x7fff), an interrupt flood is generated. I
measured ~3k/s interrupts even after the RX errors transfer was
stopped.

Since we don't read and clear the RCSR register, we should disable this
interrupt.

Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Fixes: 87461f7a58ab ("net: phy: DP83822 initial driver submission")
---
 drivers/net/phy/dp83822.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 95ef507053a6..8549e0e356c9 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -229,8 +229,7 @@ static int dp83822_config_intr(struct phy_device *phydev)
 		if (misr_status < 0)
 			return misr_status;
 
-		misr_status |= (DP83822_RX_ERR_HF_INT_EN |
-				DP83822_LINK_STAT_INT_EN |
+		misr_status |= (DP83822_LINK_STAT_INT_EN |
 				DP83822_ENERGY_DET_INT_EN |
 				DP83822_LINK_QUAL_INT_EN);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 0/2] net: dp83822: fix interrupt floods
  2022-06-23 13:46   ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
  2022-06-23 13:46     ` [PATCH v3 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
  2022-06-23 13:46     ` [PATCH v3 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
@ 2022-06-23 18:24     ` Andrew Lunn
  2022-06-24 23:50     ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 19+ messages in thread
From: Andrew Lunn @ 2022-06-23 18:24 UTC (permalink / raw)
  To: Enguerrand de Ribaucourt; +Cc: davem, netdev, linux-kernel, linux, hkallweit1

On Thu, Jun 23, 2022 at 03:46:43PM +0200, Enguerrand de Ribaucourt wrote:
> The false carrier and RX error counters, once half full, produce interrupt
> floods. Since we do not use these counters, these interrupts should be disabled.
> 
> v2: added Fixes: and patchset description 0/2
> v3: Fixed Fixes: commit format
> 
> In-Reply-To: YqzAKguRaxr74oXh@lunn.ch
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Fixes: 87461f7a58ab ("net: phy: DP83822 initial driver submission")

It is maybe only part of the netdev FAQ, but it states there, don't
post new versions of the patches in less than 24 hours. You need to
give people time to review the changes and make comments.

It is correct to append a Reviewed-by, but it should only be to the
patch which was actually reviewed. So please add my Reviewed-by to
just patch 1/2. The Fixes should also be on each individual patch, not
the 0/X patch, since that often gets lost in the noise.

    Andrew



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/2] net: dp83822: disable false carrier interrupt
  2022-06-23 13:46     ` [PATCH v3 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
@ 2022-06-23 18:27       ` Andrew Lunn
  0 siblings, 0 replies; 19+ messages in thread
From: Andrew Lunn @ 2022-06-23 18:27 UTC (permalink / raw)
  To: Enguerrand de Ribaucourt; +Cc: davem, netdev, linux-kernel, linux, hkallweit1

On Thu, Jun 23, 2022 at 03:46:44PM +0200, Enguerrand de Ribaucourt wrote:
> When unplugging an Ethernet cable, false carrier events were produced by
> the PHY at a very high rate. Once the false carrier counter full, an
> interrupt was triggered every few clock cycles until the cable was
> replugged. This resulted in approximately 10k/s interrupts.
> 
> Since the false carrier counter (FCSCR) is never used, we can safely
> disable this interrupt.
> 
> In addition to improving performance, this also solved MDIO read
> timeouts I was randomly encountering with an i.MX8 fec MAC because of
> the interrupt flood. The interrupt count and MDIO timeout fix were
> tested on a v5.4.110 kernel.
> 
> Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Fixes: 87461f7a58ab ("net: phy: DP83822 initial driver submission")

For future reference, you should put these in the opposite order. Your
Signed-off-by should come last. Fixes generally comes first.

No need to resend for this patchset.

   Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 0/2] net: dp83822: fix interrupt floods
  2022-06-23 13:46   ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
                       ` (2 preceding siblings ...)
  2022-06-23 18:24     ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Andrew Lunn
@ 2022-06-24 23:50     ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 19+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-06-24 23:50 UTC (permalink / raw)
  To: Enguerrand de Ribaucourt
  Cc: andrew, davem, netdev, linux-kernel, linux, hkallweit1

Hello:

This series was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 23 Jun 2022 15:46:43 +0200 you wrote:
> The false carrier and RX error counters, once half full, produce interrupt
> floods. Since we do not use these counters, these interrupts should be disabled.
> 
> v2: added Fixes: and patchset description 0/2
> v3: Fixed Fixes: commit format
> 
> In-Reply-To: YqzAKguRaxr74oXh@lunn.ch
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Fixes: 87461f7a58ab ("net: phy: DP83822 initial driver submission")

Here is the summary with links:
  - [v3,1/2] net: dp83822: disable false carrier interrupt
    https://git.kernel.org/netdev/net/c/c96614eeab66
  - [v3,2/2] net: dp83822: disable rx error interrupt
    https://git.kernel.org/netdev/net/c/0e597e2affb9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-06-24 23:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17 13:46 [PATCH] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
2022-06-17 17:55 ` Andrew Lunn
2022-06-23  8:51   ` Enguerrand de Ribaucourt
2022-06-23  8:51   ` [PATCH 1/2] " Enguerrand de Ribaucourt
2022-06-23 12:42     ` Andrew Lunn
2022-06-23 13:16       ` Enguerrand de Ribaucourt
2022-06-23  8:51   ` [PATCH 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
2022-06-23 13:06   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
2022-06-23 13:06     ` [PATCH v2 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
2022-06-23 13:06     ` [PATCH v2 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
2022-06-23 13:14   ` [PATCH v2 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
2022-06-23 13:14     ` [PATCH v2 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
2022-06-23 13:14     ` [PATCH v2 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
2022-06-23 13:46   ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Enguerrand de Ribaucourt
2022-06-23 13:46     ` [PATCH v3 1/2] net: dp83822: disable false carrier interrupt Enguerrand de Ribaucourt
2022-06-23 18:27       ` Andrew Lunn
2022-06-23 13:46     ` [PATCH v3 2/2] net: dp83822: disable rx error interrupt Enguerrand de Ribaucourt
2022-06-23 18:24     ` [PATCH v3 0/2] net: dp83822: fix interrupt floods Andrew Lunn
2022-06-24 23:50     ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.