changes v4:
- use fallthrough;
- use EOPNOTSUPP instead of ENOTSUPP
- drop flags variable in dsa_slave_phy_connect patch
- extend description for the "net: phy: micrel: apply resume errat"
patch
- fix "use consistent alignments" patch
changes v3:
- remove RFC tag
changes v2:
- use generic MII_* defines where possible
- rework phylink validate
- remove phylink get state function
- reorder cabletest patches to make PHY flag patch in the right order
- fix MDI-X detection
This patches provide support for cable testing on the ksz886x switches.
Since it has one special port, we needed to add phylink with validation
and extra quirk for the PHY to signal, that one port will not provide
valid cable testing reports.
Michael Grzeschik (2):
net: phy: micrel: move phy reg offsets to common header
net: dsa: microchip: ksz8795: add phylink support
Oleksij Rempel (7):
net: phy: micrel: use consistent alignments
net: phy: micrel: apply resume errata workaround for ksz8873 and
ksz8863
net: phy/dsa micrel/ksz886x add MDI-X support
net: phy: micrel: ksz8081 add MDI-X support
net: dsa: microchip: ksz8795: add LINK_MD register support
net: dsa: dsa_slave_phy_connect(): extend phy's flags with port
specific phy flags
net: phy: micrel: ksz886x/ksz8081: add cabletest support
drivers/net/dsa/microchip/ksz8795.c | 214 ++++++++----
drivers/net/dsa/microchip/ksz8795_reg.h | 67 +---
drivers/net/ethernet/micrel/ksz884x.c | 105 +-----
drivers/net/phy/micrel.c | 423 ++++++++++++++++++++++--
include/linux/micrel_phy.h | 16 +
net/dsa/slave.c | 4 +
6 files changed, 593 insertions(+), 236 deletions(-)
--
2.29.2
@@ -55,11 +55,17 @@/* PHY Control 1 */#define MII_KSZPHY_CTRL_1 0x1e+#define KSZ8081_CTRL1_MDIX_STAT BIT(4)/* PHY Control 2 / PHY Control (if no PHY Control 1) */#define MII_KSZPHY_CTRL_2 0x1f#define MII_KSZPHY_CTRL MII_KSZPHY_CTRL_2/* bitmap of PHY register to set interrupt mode */+#define KSZ8081_CTRL2_HP_MDIX BIT(15)+#define KSZ8081_CTRL2_MDI_MDI_X_SELECT BIT(14)+#define KSZ8081_CTRL2_DISABLE_AUTO_MDIX BIT(13)+#define KSZ8081_CTRL2_FORCE_LINK BIT(11)+#define KSZ8081_CTRL2_POWER_SAVING BIT(10)#define KSZPHY_CTRL_INT_ACTIVE_HIGH BIT(9)#define KSZPHY_RMII_REF_CLK_SEL BIT(7)
@@ -422,6 +428,87 @@ static int ksz8081_config_init(struct phy_device *phydev)returnkszphy_config_init(phydev);}+staticintksz8081_config_mdix(structphy_device*phydev,u8ctrl)+{+u16val;++switch(ctrl){+caseETH_TP_MDI:+val=KSZ8081_CTRL2_DISABLE_AUTO_MDIX;+break;+caseETH_TP_MDI_X:+val=KSZ8081_CTRL2_DISABLE_AUTO_MDIX|+KSZ8081_CTRL2_MDI_MDI_X_SELECT;+break;+caseETH_TP_MDI_AUTO:+val=0;+break;+default:+return0;+}++returnphy_modify(phydev,MII_KSZPHY_CTRL_2,+KSZ8081_CTRL2_HP_MDIX|+KSZ8081_CTRL2_MDI_MDI_X_SELECT|+KSZ8081_CTRL2_DISABLE_AUTO_MDIX,+KSZ8081_CTRL2_HP_MDIX|val);+}++staticintksz8081_config_aneg(structphy_device*phydev)+{+intret;++ret=genphy_config_aneg(phydev);+if(ret)+returnret;++/* The MDI-X configuration is automatically changed by the PHY after+*switchingfromautonegofftoon.So,takeMDI-Xconfigurationunder+*owncontrolandsetitafterautonegconfigurationwasdone.+*/+returnksz8081_config_mdix(phydev,phydev->mdix_ctrl);+}++staticintksz8081_mdix_update(structphy_device*phydev)+{+intret;++ret=phy_read(phydev,MII_KSZPHY_CTRL_2);+if(ret<0)+returnret;++if(ret&KSZ8081_CTRL2_DISABLE_AUTO_MDIX){+if(ret&KSZ8081_CTRL2_MDI_MDI_X_SELECT)+phydev->mdix_ctrl=ETH_TP_MDI_X;+else+phydev->mdix_ctrl=ETH_TP_MDI;+}else{+phydev->mdix_ctrl=ETH_TP_MDI_AUTO;+}++ret=phy_read(phydev,MII_KSZPHY_CTRL_1);+if(ret<0)+returnret;++if(ret&KSZ8081_CTRL1_MDIX_STAT)+phydev->mdix=ETH_TP_MDI;+else+phydev->mdix=ETH_TP_MDI_X;++return0;+}++staticintksz8081_read_status(structphy_device*phydev)+{+intret;++ret=ksz8081_mdix_update(phydev);+if(ret<0)+returnret;++returngenphy_read_status(phydev);+}+staticintksz8061_config_init(structphy_device*phydev){intret;
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(-)
@@ -1048,6 +1048,92 @@ static int ksz8873mll_config_aneg(struct phy_device *phydev)return0;}+staticintksz886x_config_mdix(structphy_device*phydev,u8ctrl)+{+u16val;++switch(ctrl){+caseETH_TP_MDI:+val=KSZ886X_BMCR_DISABLE_AUTO_MDIX;+break;+caseETH_TP_MDI_X:+/* Note: The naming of the bit KSZ886X_BMCR_FORCE_MDI is bit+*counterintuitive,the"-X"in"1 = Force MDI"inthedata+*sheetseemstobemissing:+*1=ForceMDI(sic!)(transmitonRX+/RX-pins)+*0=Normaloperation(transmitonTX+/TX-pins)+*/+val=KSZ886X_BMCR_DISABLE_AUTO_MDIX|KSZ886X_BMCR_FORCE_MDI;+break;+caseETH_TP_MDI_AUTO:+val=0;+break;+default:+return0;+}++returnphy_modify(phydev,MII_BMCR,+KSZ886X_BMCR_HP_MDIX|KSZ886X_BMCR_FORCE_MDI|+KSZ886X_BMCR_DISABLE_AUTO_MDIX,+KSZ886X_BMCR_HP_MDIX|val);+}++staticintksz886x_config_aneg(structphy_device*phydev)+{+intret;++ret=genphy_config_aneg(phydev);+if(ret)+returnret;++/* The MDI-X configuration is automatically changed by the PHY after+*switchingfromautonegofftoon.So,takeMDI-Xconfigurationunder+*owncontrolandsetitafterautonegconfigurationwasdone.+*/+returnksz886x_config_mdix(phydev,phydev->mdix_ctrl);+}++staticintksz886x_mdix_update(structphy_device*phydev)+{+intret;++ret=phy_read(phydev,MII_BMCR);+if(ret<0)+returnret;++if(ret&KSZ886X_BMCR_DISABLE_AUTO_MDIX){+if(ret&KSZ886X_BMCR_FORCE_MDI)+phydev->mdix_ctrl=ETH_TP_MDI_X;+else+phydev->mdix_ctrl=ETH_TP_MDI;+}else{+phydev->mdix_ctrl=ETH_TP_MDI_AUTO;+}++ret=phy_read(phydev,MII_KSZPHY_CTRL);+if(ret<0)+returnret;++/* Same reverse logic as KSZ886X_BMCR_FORCE_MDI */+if(ret&KSZ886X_CTRL_MDIX_STAT)+phydev->mdix=ETH_TP_MDI_X;+else+phydev->mdix=ETH_TP_MDI;++return0;+}++staticintksz886x_read_status(structphy_device*phydev)+{+intret;++ret=ksz886x_mdix_update(phydev);+if(ret<0)+returnret;++returngenphy_read_status(phydev);+}+staticintksz886x_resume(structphy_device*phydev){intret;
@@ -1420,6 +1506,8 @@ static struct phy_driver ksphy_driver[] = {.name="Micrel KSZ8851 Ethernet MAC or KSZ886X Switch",/* PHY_BASIC_FEATURES */.config_init=kszphy_config_init,+.config_aneg=ksz886x_config_aneg,+.read_status=ksz886x_read_status,.suspend=genphy_suspend,.resume=ksz886x_resume,},{
This patch extends the flags of the phy that's being connected with the
port specific flags of the switch port.
This is needed to handle a port specific erratum of the KSZ8873 switch,
which is added in a later patch.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
net/dsa/slave.c | 4 ++++
1 file changed, 4 insertions(+)
From: Michael Grzeschik <redacted>
This patch adds the phylink support to the ksz8795 driver to provide
configuration exceptions on quirky KSZ8863 and KSZ8873 ports.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/dsa/microchip/ksz8795.c | 55 +++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
@@ -1420,11 +1421,65 @@ static int ksz8_setup(struct dsa_switch *ds)return0;}+staticvoidksz8_validate(structdsa_switch*ds,intport,+unsignedlong*supported,+structphylink_link_state*state)+{+__ETHTOOL_DECLARE_LINK_MODE_MASK(mask)={0,};+structksz_device*dev=ds->priv;++if(port==dev->cpu_port){+if(state->interface!=PHY_INTERFACE_MODE_RMII&&+state->interface!=PHY_INTERFACE_MODE_MII&&+state->interface!=PHY_INTERFACE_MODE_NA)+gotounsupported;+}else{+if(state->interface!=PHY_INTERFACE_MODE_INTERNAL&&+state->interface!=PHY_INTERFACE_MODE_NA)+gotounsupported;+}++/* Allow all the expected bits */+phylink_set_port_modes(mask);+phylink_set(mask,Autoneg);++/* Silicon Errata Sheet (DS80000830A):+*"Port 1 does not respond to received flow control PAUSE frames"+*So,disablePausesupporton"Port 1"(port==0)forallksz88x3+*switches.+*/+if(!ksz_is_ksz88x3(dev)||port)+phylink_set(mask,Pause);++/* Asym pause is not supported on KSZ8863 and KSZ8873 */+if(!ksz_is_ksz88x3(dev))+phylink_set(mask,Asym_Pause);++/* 10M and 100M are only supported */+phylink_set(mask,10baseT_Half);+phylink_set(mask,10baseT_Full);+phylink_set(mask,100baseT_Half);+phylink_set(mask,100baseT_Full);++bitmap_and(supported,supported,mask,+__ETHTOOL_LINK_MODE_MASK_NBITS);+bitmap_and(state->advertising,state->advertising,mask,+__ETHTOOL_LINK_MODE_MASK_NBITS);++return;++unsupported:+bitmap_zero(supported,__ETHTOOL_LINK_MODE_MASK_NBITS);+dev_err(ds->dev,"Unsupported interface: %s, port: %d\n",+phy_modes(state->interface),port);+}+staticconststructdsa_switch_opsksz8_switch_ops={.get_tag_protocol=ksz8_get_tag_protocol,.setup=ksz8_setup,.phy_read=ksz_phy_read16,.phy_write=ksz_phy_write16,+.phylink_validate=ksz8_validate,.phylink_mac_link_down=ksz_mac_link_down,.port_enable=ksz_enable_port,.get_strings=ksz8_get_strings,
This patch support for cable test for the ksz886x switches and the
ksz8081 PHY.
The patch was tested on a KSZ8873RLL switch with following results:
- port 1:
- provides invalid values, thus return -ENOTSUPP
(Errata: DS80000830A: "LinkMD does not work on Port 1",
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)
- port 2:
- can detect distance
- can detect open on each wire of pair A (wire 1 and 2)
- can detect open only on one wire of pair B (only wire 3)
- can detect short between wires of a pair (wires 1 + 2 or 3 + 6)
- short between pairs is detected as open.
For example short between wires 2 + 3 is detected as open.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/dsa/microchip/ksz8795.c | 13 ++
drivers/net/phy/micrel.c | 180 ++++++++++++++++++++++++++++
include/linux/micrel_phy.h | 1 +
3 files changed, 194 insertions(+)
@@ -53,6 +54,18 @@#define KSZPHY_INTCS_STATUS (KSZPHY_INTCS_LINK_DOWN_STATUS |\KSZPHY_INTCS_LINK_UP_STATUS)+/* LinkMD Control/Status */+#define KSZ8081_LMD 0x1d+#define KSZ8081_LMD_ENABLE_TEST BIT(15)+#define KSZ8081_LMD_STAT_NORMAL 0+#define KSZ8081_LMD_STAT_OPEN 1+#define KSZ8081_LMD_STAT_SHORT 2+#define KSZ8081_LMD_STAT_FAIL 3+#define KSZ8081_LMD_STAT_MASK GENMASK(14, 13)+/* Short cable (<10 meter) has been detected by LinkMD */+#define KSZ8081_LMD_SHORT_INDICATOR BIT(12)+#define KSZ8081_LMD_DELTA_TIME_MASK GENMASK(8, 0)+/* PHY Control 1 */#define MII_KSZPHY_CTRL_1 0x1e#define KSZ8081_CTRL1_MDIX_STAT BIT(4)
@@ -1386,6 +1399,167 @@ static int kszphy_probe(struct phy_device *phydev)return0;}+staticintksz886x_cable_test_start(structphy_device*phydev)+{+if(phydev->dev_flags&MICREL_KSZ8_P1_ERRATA)+return-EOPNOTSUPP;++/* If autoneg is enabled, we won't be able to test cross pair+*short.Inthiscase,thePHYwill"detect"alinkand+*confusetheinternalstatemachine-disableautoneghere.+*Ifautonegisdisabled,weshouldsetthespeedto10mbit.+*/+returnphy_clear_bits(phydev,MII_BMCR,BMCR_ANENABLE|BMCR_SPEED100);+}++staticintksz886x_cable_test_result_trans(u16status)+{+switch(FIELD_GET(KSZ8081_LMD_STAT_MASK,status)){+caseKSZ8081_LMD_STAT_NORMAL:+returnETHTOOL_A_CABLE_RESULT_CODE_OK;+caseKSZ8081_LMD_STAT_SHORT:+returnETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;+caseKSZ8081_LMD_STAT_OPEN:+returnETHTOOL_A_CABLE_RESULT_CODE_OPEN;+caseKSZ8081_LMD_STAT_FAIL:+fallthrough;+default:+returnETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;+}+}++staticboolksz886x_cable_test_failed(u16status)+{+returnFIELD_GET(KSZ8081_LMD_STAT_MASK,status)==+KSZ8081_LMD_STAT_FAIL;+}++staticboolksz886x_cable_test_fault_length_valid(u16status)+{+switch(FIELD_GET(KSZ8081_LMD_STAT_MASK,status)){+caseKSZ8081_LMD_STAT_OPEN:+fallthrough;+caseKSZ8081_LMD_STAT_SHORT:+returntrue;+}+returnfalse;+}++staticintksz886x_cable_test_fault_length(u16status)+{+intdt;++/* According to the data sheet the distance to the fault is+*DELTA_TIME*0.4meters.+*/+dt=FIELD_GET(KSZ8081_LMD_DELTA_TIME_MASK,status);++return(dt*400)/10;+}++staticintksz886x_cable_test_wait_for_completion(structphy_device*phydev)+{+intval,ret;++ret=phy_read_poll_timeout(phydev,KSZ8081_LMD,val,+!(val&KSZ8081_LMD_ENABLE_TEST),+30000,100000,true);++returnret<0?ret:0;+}++staticintksz886x_cable_test_one_pair(structphy_device*phydev,intpair)+{+staticconstintethtool_pair[]={+ETHTOOL_A_CABLE_PAIR_A,+ETHTOOL_A_CABLE_PAIR_B,+};+intret,val,mdix;++/* There is no way to choice the pair, like we do one ksz9031.+*WecanworkaroundthislimitationbyusingtheMDI-Xfunctionality.+*/+if(pair==0)+mdix=ETH_TP_MDI;+else+mdix=ETH_TP_MDI_X;++switch(phydev->phy_id&MICREL_PHY_ID_MASK){+casePHY_ID_KSZ8081:+ret=ksz8081_config_mdix(phydev,mdix);+break;+casePHY_ID_KSZ886X:+ret=ksz886x_config_mdix(phydev,mdix);+break;+default:+ret=-ENODEV;+}++if(ret)+returnret;++/* Now we are ready to fire. This command will send a 100ns pulse+*tothepair.+*/+ret=phy_write(phydev,KSZ8081_LMD,KSZ8081_LMD_ENABLE_TEST);+if(ret)+returnret;++ret=ksz886x_cable_test_wait_for_completion(phydev);+if(ret)+returnret;++val=phy_read(phydev,KSZ8081_LMD);+if(val<0)+returnval;++if(ksz886x_cable_test_failed(val))+return-EAGAIN;++ret=ethnl_cable_test_result(phydev,ethtool_pair[pair],+ksz886x_cable_test_result_trans(val));+if(ret)+returnret;++if(!ksz886x_cable_test_fault_length_valid(val))+return0;++returnethnl_cable_test_fault_length(phydev,ethtool_pair[pair],+ksz886x_cable_test_fault_length(val));+}++staticintksz886x_cable_test_get_status(structphy_device*phydev,+bool*finished)+{+unsignedlongpair_mask=0x3;+intretries=20;+intpair,ret;++*finished=false;++/* Try harder if link partner is active */+while(pair_mask&&retries--){+for_each_set_bit(pair,&pair_mask,4){+ret=ksz886x_cable_test_one_pair(phydev,pair);+if(ret==-EAGAIN)+continue;+if(ret<0)+returnret;+clear_bit(pair,&pair_mask);+}+/* If link partner is in autonegotiation mode it will send 2ms+*ofFLPswithatleast6msofsilence.+*Add2mssleeptohavebetterchancestohitthissilence.+*/+if(pair_mask)+msleep(2);+}++*finished=true;++return0;+}+staticstructphy_driverksphy_driver[]={{.phy_id=PHY_ID_KS8737,
From: Michael Grzeschik <redacted>
Some micrel devices share the same PHY register defines. This patch
moves them to one common header so other drivers can reuse them.
And reuse generic MII_* defines where possible.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/dsa/microchip/ksz8795.c | 119 ++++++++++++------------
drivers/net/dsa/microchip/ksz8795_reg.h | 62 ------------
drivers/net/ethernet/micrel/ksz884x.c | 105 +++------------------
include/linux/micrel_phy.h | 13 +++
4 files changed, 88 insertions(+), 211 deletions(-)
This patch changes the alignments to one space between "#define" and the
macro.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/phy/micrel.c | 44 ++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 22 deletions(-)
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-06-11 19:11:04
On Fri, Jun 11, 2021 at 09:15:19AM +0200, Oleksij Rempel wrote:
From: Michael Grzeschik <redacted>
Some micrel devices share the same PHY register defines. This patch
moves them to one common header so other drivers can reuse them.
And reuse generic MII_* defines where possible.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-06-11 19:13:00
On Fri, Jun 11, 2021 at 09:15:20AM +0200, Oleksij Rempel wrote:
From: Michael Grzeschik <redacted>
This patch adds the phylink support to the ksz8795 driver to provide
configuration exceptions on quirky KSZ8863 and KSZ8873 ports.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
but it would be a good idea for Russell to take a look too.
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-06-11 19:21:16
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.
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-06-11 19:24:38
On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote:
This patch extends the flags of the phy that's being connected with the
port specific flags of the switch port.
This is needed to handle a port specific erratum of the KSZ8873 switch,
which is added in a later patch.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
What happens differently between having this patch and not having it?
On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote:
quoted
This patch extends the flags of the phy that's being connected with the
port specific flags of the switch port.
This is needed to handle a port specific erratum of the KSZ8873 switch,
which is added in a later patch.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
What happens differently between having this patch and not having it?
The current get_phy_flags() is only processed when we connect to a PHY
via a designed phy-handle property via phylink_of_phy_connect((, but if
we fallback on the internal MDIO bus created by a switch and take the
dsa_slave_phy_connect() path then we would not be processing that flag
and using it at PHY connection time. Oleksij, your proposed patch fails
to check that dsa_switch_ops::get_phy_flags is actually non-NULL, how
about this approach instead where we only fetch the flags once, and we
deal with an option get_phy_flags callback too:
@@ -1804,7 +1806,7 @@ static int dsa_slave_phy_setup(struct net_device
*slave_dev)
/* We could not connect to a designated PHY or SFP, so
try to
* use the switch internal MDIO bus instead
*/
- ret = dsa_slave_phy_connect(slave_dev, dp->index);
+ ret = dsa_slave_phy_connect(slave_dev, dp->index,
phy_flags);
if (ret) {
netdev_err(slave_dev,
"failed to connect to port %d: %d\n",
--
Florian
From: Michael Grzeschik <redacted>
This patch adds the phylink support to the ksz8795 driver to provide
configuration exceptions on quirky KSZ8863 and KSZ8873 ports.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
From: Michael Grzeschik <redacted>
Some micrel devices share the same PHY register defines. This patch
moves them to one common header so other drivers can reuse them.
And reuse generic MII_* defines where possible.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
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:
quoted
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
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-06-12 15:14:49
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:
quoted
On Fri, Jun 11, 2021 at 09:15:22AM +0200, Oleksij Rempel wrote:
quoted
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?
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-06-12 18:24:03
+static int ksz886x_cable_test_get_status(struct phy_device *phydev,
+ bool *finished)
+{
+ unsigned long pair_mask = 0x3;
+ int retries = 20;
+ int pair, ret;
+
+ *finished = false;
+
+ /* Try harder if link partner is active */
+ while (pair_mask && retries--) {
+ for_each_set_bit(pair, &pair_mask, 4) {
+ ret = ksz886x_cable_test_one_pair(phydev, pair);
+ if (ret == -EAGAIN)
+ continue;
+ if (ret < 0)
+ return ret;
+ clear_bit(pair, &pair_mask);
+ }
+ /* If link partner is in autonegotiation mode it will send 2ms
+ * of FLPs with at least 6ms of silence.
+ * Add 2ms sleep to have better chances to hit this silence.
+ */
+ if (pair_mask)
+ msleep(2);
+ }
+
+ *finished = true;
+
+ return 0;
If ksz886x_cable_test_one_pair() returns -EAGAIN 20x and it gives up,
you end up returning 0. Maybe it would be better to return ret?
Andrew
On Fri, Jun 11, 2021 at 10:24:17PM +0300, Vladimir Oltean wrote:
On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote:
quoted
This patch extends the flags of the phy that's being connected with the
port specific flags of the switch port.
This is needed to handle a port specific erratum of the KSZ8873 switch,
which is added in a later patch.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
What happens differently between having this patch and not having it?
Without this patch, the PHY driver will not get the phyflag provided by the
DSA driver.
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, Jun 11, 2021 at 04:26:33PM -0700, Florian Fainelli wrote:
On 6/11/2021 12:24 PM, Vladimir Oltean wrote:
quoted
On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote:
quoted
This patch extends the flags of the phy that's being connected with the
port specific flags of the switch port.
This is needed to handle a port specific erratum of the KSZ8873 switch,
which is added in a later patch.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
What happens differently between having this patch and not having it?
The current get_phy_flags() is only processed when we connect to a PHY
via a designed phy-handle property via phylink_of_phy_connect((, but if
we fallback on the internal MDIO bus created by a switch and take the
dsa_slave_phy_connect() path then we would not be processing that flag
and using it at PHY connection time. Oleksij, your proposed patch fails
to check that dsa_switch_ops::get_phy_flags is actually non-NULL, how
about this approach instead where we only fetch the flags once, and we
deal with an option get_phy_flags callback too:
@@ -1804,7 +1806,7 @@ static int dsa_slave_phy_setup(struct net_device
*slave_dev)
/* We could not connect to a designated PHY or SFP, so
try to
* use the switch internal MDIO bus instead
*/
- ret = dsa_slave_phy_connect(slave_dev, dp->index);
+ ret = dsa_slave_phy_connect(slave_dev, dp->index,
phy_flags);
if (ret) {
netdev_err(slave_dev,
"failed to connect to port %d: %d\n",
--
Florian
On Sat, Jun 12, 2021 at 08:23:53PM +0200, Andrew Lunn wrote:
quoted
+static int ksz886x_cable_test_get_status(struct phy_device *phydev,
+ bool *finished)
+{
+ unsigned long pair_mask = 0x3;
+ int retries = 20;
+ int pair, ret;
+
+ *finished = false;
+
+ /* Try harder if link partner is active */
+ while (pair_mask && retries--) {
+ for_each_set_bit(pair, &pair_mask, 4) {
+ ret = ksz886x_cable_test_one_pair(phydev, pair);
+ if (ret == -EAGAIN)
+ continue;
+ if (ret < 0)
+ return ret;
+ clear_bit(pair, &pair_mask);
+ }
+ /* If link partner is in autonegotiation mode it will send 2ms
+ * of FLPs with at least 6ms of silence.
+ * Add 2ms sleep to have better chances to hit this silence.
+ */
+ if (pair_mask)
+ msleep(2);
+ }
+
+ *finished = true;
+
+ return 0;
If ksz886x_cable_test_one_pair() returns -EAGAIN 20x and it gives up,
you end up returning 0. Maybe it would be better to return ret?
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:
quoted
On Fri, Jun 11, 2021 at 10:20:10PM +0300, Vladimir Oltean wrote:
quoted
On Fri, Jun 11, 2021 at 09:15:22AM +0200, Oleksij Rempel wrote:
quoted
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.