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 (3):
net: phy: micrel: move phy reg offsets to common header
net: dsa: microchip: ksz8795: add phylink support
net: phy: micrel: add patch for erratas on port1
Oleksij Rempel (6):
net: phy: micrel: use consistent indention after define
net: phy: micrel: apply resume errata workaround for ksz8873 and
ksz8863
net: phy: micrel: ksz886x add MDI-X support
net: phy: micrel: ksz8081 add MDI-X support
net: dsa: microchip: ksz8795: add LINK_MD register support
net: phy: micrel: ksz886x/ksz8081: add cabletest support
drivers/net/dsa/microchip/ksz8795.c | 110 +++++++
drivers/net/dsa/microchip/ksz8795_reg.h | 67 +---
drivers/net/ethernet/micrel/ksz884x.c | 70 +---
drivers/net/phy/micrel.c | 416 +++++++++++++++++++++++-
drivers/net/phy/phylink.c | 2 +-
include/linux/micrel_phy.h | 64 ++++
net/dsa/slave.c | 4 +
7 files changed, 586 insertions(+), 147 deletions(-)
--
2.29.2
From: Michael Grzeschik <redacted>
This patch adds the phylink support to the ksz8795 driver, since
phylib is obsolete for dsa drivers.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
@@ -1420,11 +1421,83 @@ static int ksz8_setup(struct dsa_switch *ds)return0;}+staticintksz_get_state(structdsa_switch*ds,intport,+structphylink_link_state*state)+{+structksz_device*dev=ds->priv;+structksz8*ksz8=dev->priv;+constu8*regs=ksz8->regs;+u8speed,link;++ksz_pread8(dev,port,regs[P_LINK_STATUS],&link);+ksz_pread8(dev,port,regs[P_SPEED_STATUS],&speed);++state->link=!!(link&PORT_STAT_LINK_GOOD);+if(state->link){+state->speed=+(speed&PORT_STAT_SPEED_100MBIT)?SPEED_100:SPEED_10;+state->duplex=+(speed&PORT_STAT_FULL_DUPLEX)?DUPLEX_FULL:DUPLEX_HALF;+}++return0;+}++staticvoidksz_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))+gotounsupported;+}elseif(port>dev->port_cnt){+bitmap_zero(supported,__ETHTOOL_LINK_MODE_MASK_NBITS);+dev_err(ds->dev,"Unsupported port: %i\n",port);+return;+}else{+if(state->interface!=PHY_INTERFACE_MODE_INTERNAL)+gotounsupported;+}++/* Allow all the expected bits */+phylink_set_port_modes(mask);+phylink_set(mask,Autoneg);++phylink_set(mask,Pause);+/* Silicon Errata Sheet (DS80000830A): Asym_Pause limit to port 2 */+if(port||!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: %d, port: %d\n",+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=ksz_validate,+.phylink_mac_link_state=ksz_get_state,.phylink_mac_link_down=ksz_mac_link_down,.port_enable=ksz_enable_port,.get_strings=ksz8_get_strings,
@@ -28,6 +28,19 @@#include<linux/clk.h>#include<linux/delay.h>+/* Device specific MII_BMCR (Reg 0) bits */+/* 1 = HP Auto MDI/MDI-X mode, 0 = Microchip Auto MDI/MDI-X mode */+#define KSZ886X_BMCR_HP_MDIX BIT(5)+/* 1 = Force MDI (transmit on RXP/RXM pins), 0 = Normal operation+*(transmitonTXP/TXMpins)+*/+#define KSZ886X_BMCR_FORCE_MDI BIT(4)+/* 1 = Disable auto MDI-X */+#define KSZ886X_BMCR_DISABLE_AUTO_MDIX BIT(3)+#define KSZ886X_BMCR_DISABLE_FAR_END_FAULT BIT(2)+#define KSZ886X_BMCR_DISABLE_TRANSMIT BIT(1)+#define KSZ886X_BMCR_DISABLE_LED BIT(0)+/* Operation Mode Strap Override */#define MII_KSZPHY_OMSO 0x16#define KSZPHY_OMSO_FACTORY_TEST BIT(15)
@@ -62,6 +75,7 @@/* bitmap of PHY register to set interrupt mode */#define KSZPHY_CTRL_INT_ACTIVE_HIGH BIT(9)#define KSZPHY_RMII_REF_CLK_SEL BIT(7)+#define KSZ886X_CTRL_MDIX_STAT BIT(4)/* Write/read to/from extended registers */#define MII_KSZPHY_EXTREG 0x0b
@@ -1048,6 +1062,91 @@ 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;++if(ret&KSZ886X_CTRL_MDIX_STAT)+phydev->mdix=ETH_TP_MDI;+else+phydev->mdix=ETH_TP_MDI_X;++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 +1519,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 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:
- cannot detect any distance
- provides inverted values
(Errata: DS80000830A: "LinkMD does not work on Port 1",
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)
- Reports "short" on open or ok.
- Reports "ok" on short.
- 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.
In order to work around the errata for port 1, the ksz8795 switch driver
should be extended to provide proper device tree support for the related
PHY nodes. So we can set a DT property to mark the port 1 as affected by
the errata.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
- added PHY_POLL_CABLE_TEST to make it work in interrupt mode
---
drivers/net/phy/micrel.c | 177 +++++++++++++++++++++++++++++++++++++++
1 file changed, 177 insertions(+)
@@ -66,6 +67,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)
@@ -1399,6 +1412,164 @@ static int kszphy_probe(struct phy_device *phydev)return0;}+staticintksz886x_cable_test_start(structphy_device*phydev)+{+/* 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:+/* fall through */+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:+/* fall through */+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>
The micrel switch ksz8873 has the following errata [1]:
* Module 1: LinkMD does not work on Port 1
* Module 4: Port 1 does not respond to received flow control PAUSE frames
This patch disables the support on the defective port.
[1] http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf
Signed-off-by: Michael Grzeschik <redacted>
---
drivers/net/dsa/microchip/ksz8795.c | 13 +++++++++++++
drivers/net/phy/micrel.c | 3 +++
drivers/net/phy/phylink.c | 2 +-
include/linux/micrel_phy.h | 1 +
net/dsa/slave.c | 4 ++++
5 files changed, 22 insertions(+), 1 deletion(-)
@@ -1414,6 +1414,9 @@ static int kszphy_probe(struct phy_device *phydev)staticintksz886x_cable_test_start(structphy_device*phydev){+if(phydev->dev_flags&MICREL_KSZ8_P1_ERRATA)+return-ENOTSUPP;+/* If autoneg is enabled, we won't be able to test cross pair*short.Inthiscase,thePHYwill"detect"alinkand*confusetheinternalstatemachine-disableautoneghere.
@@ -68,11 +68,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)#define KSZ886X_CTRL_MDIX_STAT BIT(4)
@@ -436,6 +442,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;
This patch changes the indention to one space between "#define" and the
macro.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/phy/micrel.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
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. The first
step needs to be implemented in the switch driver.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/phy/micrel.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
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.
Signed-off-by: Michael Grzeschik <redacted>
---
drivers/net/dsa/microchip/ksz8795.c | 1 +
drivers/net/dsa/microchip/ksz8795_reg.h | 62 ----------------------
drivers/net/ethernet/micrel/ksz884x.c | 70 +------------------------
include/linux/micrel_phy.h | 63 ++++++++++++++++++++++
4 files changed, 65 insertions(+), 131 deletions(-)
These also look to be pretty standard BMSR defines.
+
+#define PHY_REG_ID_1 2
+#define PHY_REG_ID_2 3
MII_PHYSID1 & MII_PHYSID2
Please remove everything which directly matches the existing defines.
Just add defines for bits which don't follow 802.3 c22.
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-05-05 12:37:48
+/* Device specific MII_BMCR (Reg 0) bits */
+/* 1 = HP Auto MDI/MDI-X mode, 0 = Microchip Auto MDI/MDI-X mode */
+#define KSZ886X_BMCR_HP_MDIX BIT(5)
+/* 1 = Force MDI (transmit on RXP/RXM pins), 0 = Normal operation
+ * (transmit on TXP/TXM pins)
+ */
+#define KSZ886X_BMCR_FORCE_MDI BIT(4)
+/* 1 = Disable auto MDI-X */
+#define KSZ886X_BMCR_DISABLE_AUTO_MDIX BIT(3)
+#define KSZ886X_BMCR_DISABLE_FAR_END_FAULT BIT(2)
+#define KSZ886X_BMCR_DISABLE_TRANSMIT BIT(1)
+#define KSZ886X_BMCR_DISABLE_LED BIT(0)
Do these have the same values as what you added in patch 1?
+static int ksz886x_config_mdix(struct phy_device *phydev, u8 ctrl)
+{
+ u16 val;
+
+ switch (ctrl) {
+ case ETH_TP_MDI:
+ val = KSZ886X_BMCR_DISABLE_AUTO_MDIX;
+ break;
+ case ETH_TP_MDI_X:
+ /* Note: The naming of the bit KSZ886X_BMCR_FORCE_MDI is bit
+ * counter intuitive, the "-X" in "1 = Force MDI" in the data
+ * sheet seems to be missing:
+ * 1 = Force MDI (sic!) (transmit on RX+/RX- pins)
+ * 0 = Normal operation (transmit on TX+/TX- pins)
+ */
+ val = KSZ886X_BMCR_DISABLE_AUTO_MDIX | KSZ886X_BMCR_FORCE_MDI;
+ break;
+ case ETH_TP_MDI_AUTO:
+ val = 0;
+ break;
+ default:
+ return 0;
+ }
+
+ return phy_modify(phydev, MII_BMCR,
+ KSZ886X_BMCR_HP_MDIX | KSZ886X_BMCR_FORCE_MDI |
+ KSZ886X_BMCR_DISABLE_AUTO_MDIX,
+ KSZ886X_BMCR_HP_MDIX | val);
+}
Maybe this will also work for the PHY driver embedded in ksz8795.c?
Maybe as another patchset, see if that PHY driver can be moved out of the DSA driver,
and share some code with this driver?
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-05-05 12:47:28
On Wed, May 05, 2021 at 11:20:24AM +0200, Oleksij Rempel wrote:
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:
- cannot detect any distance
- provides inverted values
(Errata: DS80000830A: "LinkMD does not work on Port 1",
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)
- Reports "short" on open or ok.
- Reports "ok" on short.
Quite broken. Distance is optional, simply don't report it. Status is
harder. Reporting ETHTOOL_A_CABLE_RESULT_CODE_OK should really mean
the cable is O.K. If you cannot tell open from O.K, i would return
ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC.
More later.
Andrew
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-05-06 12:14:12
Hi Oleksij,
On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
quoted hunk
From: Michael Grzeschik <redacted>
This patch adds the phylink support to the ksz8795 driver, since
phylib is obsolete for dsa drivers.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
@@ -1420,11 +1421,83 @@ static int ksz8_setup(struct dsa_switch *ds)return0;}+staticintksz_get_state(structdsa_switch*ds,intport,+structphylink_link_state*state)+{+structksz_device*dev=ds->priv;+structksz8*ksz8=dev->priv;+constu8*regs=ksz8->regs;+u8speed,link;++ksz_pread8(dev,port,regs[P_LINK_STATUS],&link);+ksz_pread8(dev,port,regs[P_SPEED_STATUS],&speed);++state->link=!!(link&PORT_STAT_LINK_GOOD);+if(state->link){+state->speed=+(speed&PORT_STAT_SPEED_100MBIT)?SPEED_100:SPEED_10;+state->duplex=+(speed&PORT_STAT_FULL_DUPLEX)?DUPLEX_FULL:DUPLEX_HALF;+}++return0;+}++staticvoidksz_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))+gotounsupported;+}elseif(port>dev->port_cnt){+bitmap_zero(supported,__ETHTOOL_LINK_MODE_MASK_NBITS);+dev_err(ds->dev,"Unsupported port: %i\n",port);+return;+}else{+if(state->interface!=PHY_INTERFACE_MODE_INTERNAL)+gotounsupported;+}++/* Allow all the expected bits */+phylink_set_port_modes(mask);+phylink_set(mask,Autoneg);++phylink_set(mask,Pause);+/* Silicon Errata Sheet (DS80000830A): Asym_Pause limit to port 2 */+if(port||!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: %d, port: %d\n",+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=ksz_validate,+.phylink_mac_link_state=ksz_get_state,.phylink_mac_link_down=ksz_mac_link_down,.port_enable=ksz_enable_port,.get_strings=ksz8_get_strings,
Hi Vladimir,
On Thu, May 06, 2021 at 03:13:21PM +0300, Vladimir Oltean wrote:
Hi Oleksij,
On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
quoted
From: Michael Grzeschik <redacted>
This patch adds the phylink support to the ksz8795 driver, since
phylib is obsolete for dsa drivers.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
@@ -1420,11 +1421,83 @@ static int ksz8_setup(struct dsa_switch *ds)return0;}+staticintksz_get_state(structdsa_switch*ds,intport,+structphylink_link_state*state)+{+structksz_device*dev=ds->priv;+structksz8*ksz8=dev->priv;+constu8*regs=ksz8->regs;+u8speed,link;++ksz_pread8(dev,port,regs[P_LINK_STATUS],&link);+ksz_pread8(dev,port,regs[P_SPEED_STATUS],&speed);++state->link=!!(link&PORT_STAT_LINK_GOOD);+if(state->link){+state->speed=+(speed&PORT_STAT_SPEED_100MBIT)?SPEED_100:SPEED_10;+state->duplex=+(speed&PORT_STAT_FULL_DUPLEX)?DUPLEX_FULL:DUPLEX_HALF;+}++return0;+}++staticvoidksz_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))+gotounsupported;+}elseif(port>dev->port_cnt){+bitmap_zero(supported,__ETHTOOL_LINK_MODE_MASK_NBITS);+dev_err(ds->dev,"Unsupported port: %i\n",port);+return;+}else{+if(state->interface!=PHY_INTERFACE_MODE_INTERNAL)+gotounsupported;+}++/* Allow all the expected bits */+phylink_set_port_modes(mask);+phylink_set(mask,Autoneg);++phylink_set(mask,Pause);+/* Silicon Errata Sheet (DS80000830A): Asym_Pause limit to port 2 */+if(port||!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: %d, port: %d\n",+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=ksz_validate,+.phylink_mac_link_state=ksz_get_state,.phylink_mac_link_down=ksz_mac_link_down,.port_enable=ksz_enable_port,.get_strings=ksz8_get_strings,
This switch has proper flow control only on one port, see
Module 4: Port 1 does not respond to received flow control PAUSE frames:
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf
With phylink we can disable Pause here, instead of bypassing it to the PHY
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 |
From: Vladimir Oltean <olteanv@gmail.com> Date: 2021-05-06 13:29:04
On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
quoted hunk
From: Michael Grzeschik <redacted>
This patch adds the phylink support to the ksz8795 driver, since
phylib is obsolete for dsa drivers.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
Hi Vladimir,
On Thu, May 06, 2021 at 04:28:55PM +0300, Vladimir Oltean wrote:
On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
quoted
From: Michael Grzeschik <redacted>
This patch adds the phylink support to the ksz8795 driver, since
phylib is obsolete for dsa drivers.
Signed-off-by: Michael Grzeschik <redacted>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
@@ -1420,11 +1421,83 @@ static int ksz8_setup(struct dsa_switch *ds)return0;}+staticintksz_get_state(structdsa_switch*ds,intport,+structphylink_link_state*state)+{+structksz_device*dev=ds->priv;+structksz8*ksz8=dev->priv;+constu8*regs=ksz8->regs;+u8speed,link;++ksz_pread8(dev,port,regs[P_LINK_STATUS],&link);+ksz_pread8(dev,port,regs[P_SPEED_STATUS],&speed);++state->link=!!(link&PORT_STAT_LINK_GOOD);+if(state->link){+state->speed=+(speed&PORT_STAT_SPEED_100MBIT)?SPEED_100:SPEED_10;+state->duplex=+(speed&PORT_STAT_FULL_DUPLEX)?DUPLEX_FULL:DUPLEX_HALF;+}++return0;+}
How does the port know the speed?
PHY and switch control registers are mixed on this switch, so we have
access to the PHY bits directly over switch control registers.
On other hand, we provide proper PHY abstraction and there is no need to
provide this function at all. I'll remove it.
quoted
+
+static void ksz_validate(struct dsa_switch *ds, int port,
+ unsigned long *supported,
+ struct phylink_link_state *state)
Indentation looks odd.
Also, I expect that not all KSZ PHYs to have the same validation
function, so maybe you should call this ksz8_phylink_validate.
On Wed, May 05, 2021 at 02:47:19PM +0200, Andrew Lunn wrote:
On Wed, May 05, 2021 at 11:20:24AM +0200, Oleksij Rempel wrote:
quoted
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:
- cannot detect any distance
- provides inverted values
(Errata: DS80000830A: "LinkMD does not work on Port 1",
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)
- Reports "short" on open or ok.
- Reports "ok" on short.
Quite broken. Distance is optional, simply don't report it. Status is
harder. Reporting ETHTOOL_A_CABLE_RESULT_CODE_OK should really mean
the cable is O.K. If you cannot tell open from O.K, i would return
ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC.
Yes, patch "net: phy: micrel: add patch for erratas on port1" provides
a flag to return -ENOTSUPP on this port.
Is it acceptable way? Should I squash this patches?
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 Wed, May 05, 2021 at 02:37:35PM +0200, Andrew Lunn wrote:
quoted
+/* Device specific MII_BMCR (Reg 0) bits */
+/* 1 = HP Auto MDI/MDI-X mode, 0 = Microchip Auto MDI/MDI-X mode */
+#define KSZ886X_BMCR_HP_MDIX BIT(5)
+/* 1 = Force MDI (transmit on RXP/RXM pins), 0 = Normal operation
+ * (transmit on TXP/TXM pins)
+ */
+#define KSZ886X_BMCR_FORCE_MDI BIT(4)
+/* 1 = Disable auto MDI-X */
+#define KSZ886X_BMCR_DISABLE_AUTO_MDIX BIT(3)
+#define KSZ886X_BMCR_DISABLE_FAR_END_FAULT BIT(2)
+#define KSZ886X_BMCR_DISABLE_TRANSMIT BIT(1)
+#define KSZ886X_BMCR_DISABLE_LED BIT(0)
Do these have the same values as what you added in patch 1?
ACK, i'll move it
quoted
+static int ksz886x_config_mdix(struct phy_device *phydev, u8 ctrl)
+{
+ u16 val;
+
+ switch (ctrl) {
+ case ETH_TP_MDI:
+ val = KSZ886X_BMCR_DISABLE_AUTO_MDIX;
+ break;
+ case ETH_TP_MDI_X:
+ /* Note: The naming of the bit KSZ886X_BMCR_FORCE_MDI is bit
+ * counter intuitive, the "-X" in "1 = Force MDI" in the data
+ * sheet seems to be missing:
+ * 1 = Force MDI (sic!) (transmit on RX+/RX- pins)
+ * 0 = Normal operation (transmit on TX+/TX- pins)
+ */
+ val = KSZ886X_BMCR_DISABLE_AUTO_MDIX | KSZ886X_BMCR_FORCE_MDI;
+ break;
+ case ETH_TP_MDI_AUTO:
+ val = 0;
+ break;
+ default:
+ return 0;
+ }
+
+ return phy_modify(phydev, MII_BMCR,
+ KSZ886X_BMCR_HP_MDIX | KSZ886X_BMCR_FORCE_MDI |
+ KSZ886X_BMCR_DISABLE_AUTO_MDIX,
+ KSZ886X_BMCR_HP_MDIX | val);
+}
Maybe this will also work for the PHY driver embedded in ksz8795.c?
Maybe as another patchset, see if that PHY driver can be moved out of the DSA driver,
and share some code with this driver?
Hm, i'm sure it can be done, but right now i have no access to ksz8795
hardware. I'll keep it in mind.
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: Andrew Lunn <andrew@lunn.ch> Date: 2021-05-10 13:02:38
On Mon, May 10, 2021 at 11:06:56AM +0200, Oleksij Rempel wrote:
On Wed, May 05, 2021 at 02:47:19PM +0200, Andrew Lunn wrote:
quoted
On Wed, May 05, 2021 at 11:20:24AM +0200, Oleksij Rempel wrote:
quoted
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:
- cannot detect any distance
- provides inverted values
(Errata: DS80000830A: "LinkMD does not work on Port 1",
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)
- Reports "short" on open or ok.
- Reports "ok" on short.
Quite broken. Distance is optional, simply don't report it. Status is
harder. Reporting ETHTOOL_A_CABLE_RESULT_CODE_OK should really mean
the cable is O.K. If you cannot tell open from O.K, i would return
ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC.
Yes, patch "net: phy: micrel: add patch for erratas on port1" provides
a flag to return -ENOTSUPP on this port.
Is it acceptable way? Should I squash this patches?
This is O.K. Maybe add a comment that later patches in the series with
handle the errata?
Andrew