diff mbox series

[net-next,v4,4/9] net: phy: micrel: apply resume errata workaround for ksz8873 and ksz8863

Message ID 20210611071527.9333-5-o.rempel@pengutronix.de (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series provide cable test support for the ksz886x switch | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 1 maintainers not CCed: hkallweit1@gmail.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 34 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Oleksij Rempel June 11, 2021, 7:15 a.m. UTC
The ksz8873 and ksz8863 switches are affected by following errata:

| "Receiver error in 100BASE-TX mode following Soft Power Down"
|
| Some KSZ8873 devices may exhibit receiver errors after transitioning
| from Soft Power Down mode to Normal mode, as controlled by register 195
| (0xC3) bits [1:0]. When exiting Soft Power Down mode, the receiver
| blocks may not start up properly, causing the PHY to miss data and
| exhibit erratic behavior. The problem may appear on either port 1 or
| port 2, or both ports. The problem occurs only for 100BASE-TX, not
| 10BASE-T.
|
| END USER IMPLICATIONS
| When the failure occurs, the following symptoms are seen on the affected
| port(s):
| - The port is able to link
| - LED0 blinks, even when there is no traffic
| - The MIB counters indicate receive errors (Rx Fragments, Rx Symbol
|   Errors, Rx CRC Errors, Rx Alignment Errors)
| - Only a small fraction of packets is correctly received and forwarded
|   through the switch. Most packets are dropped due to receive errors.
|
| The failing condition cannot be corrected by the following:
| - Removing and reconnecting the cable
| - Hardware reset
| - Software Reset and PCS Reset bits in register 67 (0x43)
|
| Work around:
| The problem can be corrected by setting and then clearing the Port Power
| Down bits (registers 29 (0x1D) and 45 (0x2D), bit 3). This must be done
| separately for each affected port after returning from Soft Power Down
| Mode to Normal Mode. The following procedure will ensure no further
| issues due to this erratum. To enter Soft Power Down Mode, set register
| 195 (0xC3), bits [1:0] = 10.
|
| To exit Soft Power Down Mode, follow these steps:
| 1. Set register 195 (0xC3), bits [1:0] = 00 // Exit soft power down mode
| 2. Wait 1ms minimum
| 3. Set register 29 (0x1D), bit [3] = 1 // Enter PHY port 1 power down mode
| 4. Set register 29 (0x1D), bit [3] = 0 // Exit PHY port 1 power down mode
| 5. Set register 45 (0x2D), bit [3] = 1 // Enter PHY port 2 power down mode
| 6. Set register 45 (0x2D), bit [3] = 0 // Exit PHY port 2 power down mode

This patch implements steps 2...6 of the suggested workaround. During
(initial) switch power up, step 1 is executed by the dsa/ksz8795
driver's probe function.

Note: In this workaround we toggle the MII_BMCR register's BMCR_PDOWN
bit, this is translated to the actual register and bit (as mentioned in
the arratum) by the ksz8_r_phy()/ksz8_w_phy() functions.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/micrel.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Vladimir Oltean June 11, 2021, 7:20 p.m. UTC | #1
On Fri, Jun 11, 2021 at 09:15:22AM +0200, Oleksij Rempel wrote:
> The ksz8873 and ksz8863 switches are affected by following errata:
> 
> | "Receiver error in 100BASE-TX mode following Soft Power Down"
> |
> | Some KSZ8873 devices may exhibit receiver errors after transitioning
> | from Soft Power Down mode to Normal mode, as controlled by register 195
> | (0xC3) bits [1:0]. When exiting Soft Power Down mode, the receiver
> | blocks may not start up properly, causing the PHY to miss data and
> | exhibit erratic behavior. The problem may appear on either port 1 or
> | port 2, or both ports. The problem occurs only for 100BASE-TX, not
> | 10BASE-T.
> |
> | END USER IMPLICATIONS
> | When the failure occurs, the following symptoms are seen on the affected
> | port(s):
> | - The port is able to link
> | - LED0 blinks, even when there is no traffic
> | - The MIB counters indicate receive errors (Rx Fragments, Rx Symbol
> |   Errors, Rx CRC Errors, Rx Alignment Errors)
> | - Only a small fraction of packets is correctly received and forwarded
> |   through the switch. Most packets are dropped due to receive errors.
> |
> | The failing condition cannot be corrected by the following:
> | - Removing and reconnecting the cable
> | - Hardware reset
> | - Software Reset and PCS Reset bits in register 67 (0x43)
> |
> | Work around:
> | The problem can be corrected by setting and then clearing the Port Power
> | Down bits (registers 29 (0x1D) and 45 (0x2D), bit 3). This must be done
> | separately for each affected port after returning from Soft Power Down
> | Mode to Normal Mode. The following procedure will ensure no further
> | issues due to this erratum. To enter Soft Power Down Mode, set register
> | 195 (0xC3), bits [1:0] = 10.
> |
> | To exit Soft Power Down Mode, follow these steps:
> | 1. Set register 195 (0xC3), bits [1:0] = 00 // Exit soft power down mode
> | 2. Wait 1ms minimum
> | 3. Set register 29 (0x1D), bit [3] = 1 // Enter PHY port 1 power down mode
> | 4. Set register 29 (0x1D), bit [3] = 0 // Exit PHY port 1 power down mode
> | 5. Set register 45 (0x2D), bit [3] = 1 // Enter PHY port 2 power down mode
> | 6. Set register 45 (0x2D), bit [3] = 0 // Exit PHY port 2 power down mode
> 
> This patch implements steps 2...6 of the suggested workaround. During
> (initial) switch power up, step 1 is executed by the dsa/ksz8795
> driver's probe function.
> 
> Note: In this workaround we toggle the MII_BMCR register's BMCR_PDOWN
> bit, this is translated to the actual register and bit (as mentioned in
> the arratum) by the ksz8_r_phy()/ksz8_w_phy() functions.

s/arratum/erratum/

Also, the commit message is still missing this piece of information you
gave in the previous thread:

| this issue was seen  at some early point of development (back in 2019)
| reproducible on system start. Where switch was in some default state or
| on a state configured by the bootloader. I didn't tried to reproduce it
| now.

Years from now, some poor souls might struggle to understand why this
patch was done this way. If it is indeed the case that the issue is only
seen during the handover between bootloader and kernel, there is really
no reason to implement the ERR workaround in phy_resume instead of doing
it once at probe time.

> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
Oleksij Rempel June 12, 2021, 4:26 a.m. UTC | #2
On Fri, Jun 11, 2021 at 10:20:10PM +0300, Vladimir Oltean wrote:
> On Fri, Jun 11, 2021 at 09:15:22AM +0200, Oleksij Rempel wrote:
> > The ksz8873 and ksz8863 switches are affected by following errata:
> > 
> > | "Receiver error in 100BASE-TX mode following Soft Power Down"
> > |
> > | Some KSZ8873 devices may exhibit receiver errors after transitioning
> > | from Soft Power Down mode to Normal mode, as controlled by register 195
> > | (0xC3) bits [1:0]. When exiting Soft Power Down mode, the receiver
> > | blocks may not start up properly, causing the PHY to miss data and
> > | exhibit erratic behavior. The problem may appear on either port 1 or
> > | port 2, or both ports. The problem occurs only for 100BASE-TX, not
> > | 10BASE-T.
> > |
> > | END USER IMPLICATIONS
> > | When the failure occurs, the following symptoms are seen on the affected
> > | port(s):
> > | - The port is able to link
> > | - LED0 blinks, even when there is no traffic
> > | - The MIB counters indicate receive errors (Rx Fragments, Rx Symbol
> > |   Errors, Rx CRC Errors, Rx Alignment Errors)
> > | - Only a small fraction of packets is correctly received and forwarded
> > |   through the switch. Most packets are dropped due to receive errors.
> > |
> > | The failing condition cannot be corrected by the following:
> > | - Removing and reconnecting the cable
> > | - Hardware reset
> > | - Software Reset and PCS Reset bits in register 67 (0x43)
> > |
> > | Work around:
> > | The problem can be corrected by setting and then clearing the Port Power
> > | Down bits (registers 29 (0x1D) and 45 (0x2D), bit 3). This must be done
> > | separately for each affected port after returning from Soft Power Down
> > | Mode to Normal Mode. The following procedure will ensure no further
> > | issues due to this erratum. To enter Soft Power Down Mode, set register
> > | 195 (0xC3), bits [1:0] = 10.
> > |
> > | To exit Soft Power Down Mode, follow these steps:
> > | 1. Set register 195 (0xC3), bits [1:0] = 00 // Exit soft power down mode
> > | 2. Wait 1ms minimum
> > | 3. Set register 29 (0x1D), bit [3] = 1 // Enter PHY port 1 power down mode
> > | 4. Set register 29 (0x1D), bit [3] = 0 // Exit PHY port 1 power down mode
> > | 5. Set register 45 (0x2D), bit [3] = 1 // Enter PHY port 2 power down mode
> > | 6. Set register 45 (0x2D), bit [3] = 0 // Exit PHY port 2 power down mode
> > 
> > This patch implements steps 2...6 of the suggested workaround. During
> > (initial) switch power up, step 1 is executed by the dsa/ksz8795
> > driver's probe function.
> > 
> > Note: In this workaround we toggle the MII_BMCR register's BMCR_PDOWN
> > bit, this is translated to the actual register and bit (as mentioned in
> > the arratum) by the ksz8_r_phy()/ksz8_w_phy() functions.
> 
> s/arratum/erratum/
> 
> Also, the commit message is still missing this piece of information you
> gave in the previous thread:
> 
> | this issue was seen  at some early point of development (back in 2019)
> | reproducible on system start. Where switch was in some default state or
> | on a state configured by the bootloader. I didn't tried to reproduce it
> | now.
> 
> Years from now, some poor souls might struggle to understand why this
> patch was done this way. If it is indeed the case that the issue is only
> seen during the handover between bootloader and kernel, there is really
> no reason to implement the ERR workaround in phy_resume instead of doing
> it once at probe time.

Ok, i'll drop this patch for now.

Regards,
Oleksij
Vladimir Oltean June 12, 2021, 3:13 p.m. UTC | #3
On Sat, Jun 12, 2021 at 06:26:39AM +0200, Oleksij Rempel wrote:
> On Fri, Jun 11, 2021 at 10:20:10PM +0300, Vladimir Oltean wrote:
> > On Fri, Jun 11, 2021 at 09:15:22AM +0200, Oleksij Rempel wrote:
> > > The ksz8873 and ksz8863 switches are affected by following errata:
> > > 
> > > | "Receiver error in 100BASE-TX mode following Soft Power Down"
> > > |
> > > | Some KSZ8873 devices may exhibit receiver errors after transitioning
> > > | from Soft Power Down mode to Normal mode, as controlled by register 195
> > > | (0xC3) bits [1:0]. When exiting Soft Power Down mode, the receiver
> > > | blocks may not start up properly, causing the PHY to miss data and
> > > | exhibit erratic behavior. The problem may appear on either port 1 or
> > > | port 2, or both ports. The problem occurs only for 100BASE-TX, not
> > > | 10BASE-T.
> > > |
> > > | END USER IMPLICATIONS
> > > | When the failure occurs, the following symptoms are seen on the affected
> > > | port(s):
> > > | - The port is able to link
> > > | - LED0 blinks, even when there is no traffic
> > > | - The MIB counters indicate receive errors (Rx Fragments, Rx Symbol
> > > |   Errors, Rx CRC Errors, Rx Alignment Errors)
> > > | - Only a small fraction of packets is correctly received and forwarded
> > > |   through the switch. Most packets are dropped due to receive errors.
> > > |
> > > | The failing condition cannot be corrected by the following:
> > > | - Removing and reconnecting the cable
> > > | - Hardware reset
> > > | - Software Reset and PCS Reset bits in register 67 (0x43)
> > > |
> > > | Work around:
> > > | The problem can be corrected by setting and then clearing the Port Power
> > > | Down bits (registers 29 (0x1D) and 45 (0x2D), bit 3). This must be done
> > > | separately for each affected port after returning from Soft Power Down
> > > | Mode to Normal Mode. The following procedure will ensure no further
> > > | issues due to this erratum. To enter Soft Power Down Mode, set register
> > > | 195 (0xC3), bits [1:0] = 10.
> > > |
> > > | To exit Soft Power Down Mode, follow these steps:
> > > | 1. Set register 195 (0xC3), bits [1:0] = 00 // Exit soft power down mode
> > > | 2. Wait 1ms minimum
> > > | 3. Set register 29 (0x1D), bit [3] = 1 // Enter PHY port 1 power down mode
> > > | 4. Set register 29 (0x1D), bit [3] = 0 // Exit PHY port 1 power down mode
> > > | 5. Set register 45 (0x2D), bit [3] = 1 // Enter PHY port 2 power down mode
> > > | 6. Set register 45 (0x2D), bit [3] = 0 // Exit PHY port 2 power down mode
> > > 
> > > This patch implements steps 2...6 of the suggested workaround. During
> > > (initial) switch power up, step 1 is executed by the dsa/ksz8795
> > > driver's probe function.
> > > 
> > > Note: In this workaround we toggle the MII_BMCR register's BMCR_PDOWN
> > > bit, this is translated to the actual register and bit (as mentioned in
> > > the arratum) by the ksz8_r_phy()/ksz8_w_phy() functions.
> > 
> > s/arratum/erratum/
> > 
> > Also, the commit message is still missing this piece of information you
> > gave in the previous thread:
> > 
> > | this issue was seen  at some early point of development (back in 2019)
> > | reproducible on system start. Where switch was in some default state or
> > | on a state configured by the bootloader. I didn't tried to reproduce it
> > | now.
> > 
> > Years from now, some poor souls might struggle to understand why this
> > patch was done this way. If it is indeed the case that the issue is only
> > seen during the handover between bootloader and kernel, there is really
> > no reason to implement the ERR workaround in phy_resume instead of doing
> > it once at probe time.
> 
> Ok, i'll drop this patch for now.

I mean, you don't have to drop it, you just have to provide a competent
explanation for how the patch addresses the ERR as described by Microchip.
Do you still have a board with this switch?
Oleksij Rempel June 14, 2021, 6:55 a.m. UTC | #4
On Sat, Jun 12, 2021 at 06:13:30PM +0300, Vladimir Oltean wrote:
> On Sat, Jun 12, 2021 at 06:26:39AM +0200, Oleksij Rempel wrote:
> > On Fri, Jun 11, 2021 at 10:20:10PM +0300, Vladimir Oltean wrote:
> > > On Fri, Jun 11, 2021 at 09:15:22AM +0200, Oleksij Rempel wrote:
> > > > The ksz8873 and ksz8863 switches are affected by following errata:
> > > > 
> > > > | "Receiver error in 100BASE-TX mode following Soft Power Down"
> > > > |
> > > > | Some KSZ8873 devices may exhibit receiver errors after transitioning
> > > > | from Soft Power Down mode to Normal mode, as controlled by register 195
> > > > | (0xC3) bits [1:0]. When exiting Soft Power Down mode, the receiver
> > > > | blocks may not start up properly, causing the PHY to miss data and
> > > > | exhibit erratic behavior. The problem may appear on either port 1 or
> > > > | port 2, or both ports. The problem occurs only for 100BASE-TX, not
> > > > | 10BASE-T.
> > > > |
> > > > | END USER IMPLICATIONS
> > > > | When the failure occurs, the following symptoms are seen on the affected
> > > > | port(s):
> > > > | - The port is able to link
> > > > | - LED0 blinks, even when there is no traffic
> > > > | - The MIB counters indicate receive errors (Rx Fragments, Rx Symbol
> > > > |   Errors, Rx CRC Errors, Rx Alignment Errors)
> > > > | - Only a small fraction of packets is correctly received and forwarded
> > > > |   through the switch. Most packets are dropped due to receive errors.
> > > > |
> > > > | The failing condition cannot be corrected by the following:
> > > > | - Removing and reconnecting the cable
> > > > | - Hardware reset
> > > > | - Software Reset and PCS Reset bits in register 67 (0x43)
> > > > |
> > > > | Work around:
> > > > | The problem can be corrected by setting and then clearing the Port Power
> > > > | Down bits (registers 29 (0x1D) and 45 (0x2D), bit 3). This must be done
> > > > | separately for each affected port after returning from Soft Power Down
> > > > | Mode to Normal Mode. The following procedure will ensure no further
> > > > | issues due to this erratum. To enter Soft Power Down Mode, set register
> > > > | 195 (0xC3), bits [1:0] = 10.
> > > > |
> > > > | To exit Soft Power Down Mode, follow these steps:
> > > > | 1. Set register 195 (0xC3), bits [1:0] = 00 // Exit soft power down mode
> > > > | 2. Wait 1ms minimum
> > > > | 3. Set register 29 (0x1D), bit [3] = 1 // Enter PHY port 1 power down mode
> > > > | 4. Set register 29 (0x1D), bit [3] = 0 // Exit PHY port 1 power down mode
> > > > | 5. Set register 45 (0x2D), bit [3] = 1 // Enter PHY port 2 power down mode
> > > > | 6. Set register 45 (0x2D), bit [3] = 0 // Exit PHY port 2 power down mode
> > > > 
> > > > This patch implements steps 2...6 of the suggested workaround. During
> > > > (initial) switch power up, step 1 is executed by the dsa/ksz8795
> > > > driver's probe function.
> > > > 
> > > > Note: In this workaround we toggle the MII_BMCR register's BMCR_PDOWN
> > > > bit, this is translated to the actual register and bit (as mentioned in
> > > > the arratum) by the ksz8_r_phy()/ksz8_w_phy() functions.
> > > 
> > > s/arratum/erratum/
> > > 
> > > Also, the commit message is still missing this piece of information you
> > > gave in the previous thread:
> > > 
> > > | this issue was seen  at some early point of development (back in 2019)
> > > | reproducible on system start. Where switch was in some default state or
> > > | on a state configured by the bootloader. I didn't tried to reproduce it
> > > | now.
> > > 
> > > Years from now, some poor souls might struggle to understand why this
> > > patch was done this way. If it is indeed the case that the issue is only
> > > seen during the handover between bootloader and kernel, there is really
> > > no reason to implement the ERR workaround in phy_resume instead of doing
> > > it once at probe time.
> > 
> > Ok, i'll drop this patch for now.
> 
> I mean, you don't have to drop it,

Right now it blocks other patches, so it is easier for me to send it
separately.

> you just have to provide a competent
> explanation for how the patch addresses the ERR as described by Microchip.

Sorry fail to formulate it competent enough. Can you please suggest a
needed description. 

> Do you still have a board with this switch?

Yes.

Regards,
Oleksij
diff mbox series

Patch

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 77640b990977..e462e718d68e 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1048,6 +1048,26 @@  static int ksz8873mll_config_aneg(struct phy_device *phydev)
 	return 0;
 }
 
+static int ksz886x_resume(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Apply errata workaround for KSZ8863 and KSZ8873:
+	 * Receiver error in 100BASE-TX mode following Soft Power Down
+	 *
+	 * When exiting Soft Power Down mode, the receiver blocks may not start
+	 * up properly, causing the PHY to miss data and exhibit erratic
+	 * behavior.
+	 */
+	usleep_range(1000, 2000);
+
+	ret = phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN);
+	if (ret)
+		return ret;
+
+	return phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN);
+}
+
 static int kszphy_get_sset_count(struct phy_device *phydev)
 {
 	return ARRAY_SIZE(kszphy_hw_stats);
@@ -1401,7 +1421,7 @@  static struct phy_driver ksphy_driver[] = {
 	/* PHY_BASIC_FEATURES */
 	.config_init	= kszphy_config_init,
 	.suspend	= genphy_suspend,
-	.resume		= genphy_resume,
+	.resume		= ksz886x_resume,
 }, {
 	.name		= "Micrel KSZ87XX Switch",
 	/* PHY_BASIC_FEATURES */