Commit 1fc20219 authored by David S. Miller's avatar David S. Miller

Merge branch 'vsc73xx-fix-mdio-and-phy'

Pawel Dembicki says:

====================
net: dsa: vsc73xx: fix MDIO bus access and PHY opera

This series are extracted patches from net-next series [0].

The VSC73xx driver has issues with PHY configuration. This patch series
fixes most of them.

The first patch synchronizes the register configuration routine with the
datasheet recommendations.

Patches 2-3 restore proper communication on the MDIO bus. Currently,
the write value isn't sent to the MDIO register, and without a busy check,
communication with the PHY can be interrupted. This causes the PHY to
receive improper configuration and autonegotiation could fail.

The fourth patch removes the PHY reset blockade, as it is no longer
required.

After fixing the MDIO operations, autonegotiation became possible.
The last patch removes the blockade, which became unnecessary after
the MDIO operations fix.

[0] https://patchwork.kernel.org/project/netdevbpf/list/?series=874739&state=%2A&archive=both
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 9ff2f816 de7a670f
...@@ -40,6 +40,10 @@ ...@@ -40,6 +40,10 @@
#define VSC73XX_BLOCK_ARBITER 0x5 /* Only subblock 0 */ #define VSC73XX_BLOCK_ARBITER 0x5 /* Only subblock 0 */
#define VSC73XX_BLOCK_SYSTEM 0x7 /* Only subblock 0 */ #define VSC73XX_BLOCK_SYSTEM 0x7 /* Only subblock 0 */
/* MII Block subblock */
#define VSC73XX_BLOCK_MII_INTERNAL 0x0 /* Internal MDIO subblock */
#define VSC73XX_BLOCK_MII_EXTERNAL 0x1 /* External MDIO subblock */
#define CPU_PORT 6 /* CPU port */ #define CPU_PORT 6 /* CPU port */
/* MAC Block registers */ /* MAC Block registers */
...@@ -225,6 +229,8 @@ ...@@ -225,6 +229,8 @@
#define VSC73XX_MII_CMD 0x1 #define VSC73XX_MII_CMD 0x1
#define VSC73XX_MII_DATA 0x2 #define VSC73XX_MII_DATA 0x2
#define VSC73XX_MII_STAT_BUSY BIT(3)
/* Arbiter block 5 registers */ /* Arbiter block 5 registers */
#define VSC73XX_ARBEMPTY 0x0c #define VSC73XX_ARBEMPTY 0x0c
#define VSC73XX_ARBDISC 0x0e #define VSC73XX_ARBDISC 0x0e
...@@ -299,6 +305,7 @@ ...@@ -299,6 +305,7 @@
#define IS_739X(a) (IS_7395(a) || IS_7398(a)) #define IS_739X(a) (IS_7395(a) || IS_7398(a))
#define VSC73XX_POLL_SLEEP_US 1000 #define VSC73XX_POLL_SLEEP_US 1000
#define VSC73XX_MDIO_POLL_SLEEP_US 5
#define VSC73XX_POLL_TIMEOUT_US 10000 #define VSC73XX_POLL_TIMEOUT_US 10000
struct vsc73xx_counter { struct vsc73xx_counter {
...@@ -527,6 +534,22 @@ static int vsc73xx_detect(struct vsc73xx *vsc) ...@@ -527,6 +534,22 @@ static int vsc73xx_detect(struct vsc73xx *vsc)
return 0; return 0;
} }
static int vsc73xx_mdio_busy_check(struct vsc73xx *vsc)
{
int ret, err;
u32 val;
ret = read_poll_timeout(vsc73xx_read, err,
err < 0 || !(val & VSC73XX_MII_STAT_BUSY),
VSC73XX_MDIO_POLL_SLEEP_US,
VSC73XX_POLL_TIMEOUT_US, false, vsc,
VSC73XX_BLOCK_MII, VSC73XX_BLOCK_MII_INTERNAL,
VSC73XX_MII_STAT, &val);
if (ret)
return ret;
return err;
}
static int vsc73xx_phy_read(struct dsa_switch *ds, int phy, int regnum) static int vsc73xx_phy_read(struct dsa_switch *ds, int phy, int regnum)
{ {
struct vsc73xx *vsc = ds->priv; struct vsc73xx *vsc = ds->priv;
...@@ -534,12 +557,20 @@ static int vsc73xx_phy_read(struct dsa_switch *ds, int phy, int regnum) ...@@ -534,12 +557,20 @@ static int vsc73xx_phy_read(struct dsa_switch *ds, int phy, int regnum)
u32 val; u32 val;
int ret; int ret;
ret = vsc73xx_mdio_busy_check(vsc);
if (ret)
return ret;
/* Setting bit 26 means "read" */ /* Setting bit 26 means "read" */
cmd = BIT(26) | (phy << 21) | (regnum << 16); cmd = BIT(26) | (phy << 21) | (regnum << 16);
ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, 0, 1, cmd); ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, 0, 1, cmd);
if (ret) if (ret)
return ret; return ret;
msleep(2);
ret = vsc73xx_mdio_busy_check(vsc);
if (ret)
return ret;
ret = vsc73xx_read(vsc, VSC73XX_BLOCK_MII, 0, 2, &val); ret = vsc73xx_read(vsc, VSC73XX_BLOCK_MII, 0, 2, &val);
if (ret) if (ret)
return ret; return ret;
...@@ -563,18 +594,11 @@ static int vsc73xx_phy_write(struct dsa_switch *ds, int phy, int regnum, ...@@ -563,18 +594,11 @@ static int vsc73xx_phy_write(struct dsa_switch *ds, int phy, int regnum,
u32 cmd; u32 cmd;
int ret; int ret;
/* It was found through tedious experiments that this router ret = vsc73xx_mdio_busy_check(vsc);
* chip really hates to have it's PHYs reset. They if (ret)
* never recover if that happens: autonegotiation stops return ret;
* working after a reset. Just filter out this command.
* (Resetting the whole chip is OK.)
*/
if (regnum == 0 && (val & BIT(15))) {
dev_info(vsc->dev, "reset PHY - disallowed\n");
return 0;
}
cmd = (phy << 21) | (regnum << 16); cmd = (phy << 21) | (regnum << 16) | val;
ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, 0, 1, cmd); ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, 0, 1, cmd);
if (ret) if (ret)
return ret; return ret;
...@@ -957,6 +981,11 @@ static void vsc73xx_mac_link_up(struct phylink_config *config, ...@@ -957,6 +981,11 @@ static void vsc73xx_mac_link_up(struct phylink_config *config,
if (duplex == DUPLEX_FULL) if (duplex == DUPLEX_FULL)
val |= VSC73XX_MAC_CFG_FDX; val |= VSC73XX_MAC_CFG_FDX;
else
/* In datasheet description ("Port Mode Procedure" in 5.6.2)
* this bit is configured only for half duplex.
*/
val |= VSC73XX_MAC_CFG_WEXC_DIS;
/* This routine is described in the datasheet (below ARBDISC register /* This routine is described in the datasheet (below ARBDISC register
* description) * description)
...@@ -967,7 +996,6 @@ static void vsc73xx_mac_link_up(struct phylink_config *config, ...@@ -967,7 +996,6 @@ static void vsc73xx_mac_link_up(struct phylink_config *config,
get_random_bytes(&seed, 1); get_random_bytes(&seed, 1);
val |= seed << VSC73XX_MAC_CFG_SEED_OFFSET; val |= seed << VSC73XX_MAC_CFG_SEED_OFFSET;
val |= VSC73XX_MAC_CFG_SEED_LOAD; val |= VSC73XX_MAC_CFG_SEED_LOAD;
val |= VSC73XX_MAC_CFG_WEXC_DIS;
/* Those bits are responsible for MTU only. Kernel takes care about MTU, /* Those bits are responsible for MTU only. Kernel takes care about MTU,
* let's enable +8 bytes frame length unconditionally. * let's enable +8 bytes frame length unconditionally.
......
...@@ -237,16 +237,6 @@ static int vsc739x_config_init(struct phy_device *phydev) ...@@ -237,16 +237,6 @@ static int vsc739x_config_init(struct phy_device *phydev)
return 0; return 0;
} }
static int vsc73xx_config_aneg(struct phy_device *phydev)
{
/* The VSC73xx switches does not like to be instructed to
* do autonegotiation in any way, it prefers that you just go
* with the power-on/reset defaults. Writing some registers will
* just make autonegotiation permanently fail.
*/
return 0;
}
/* This adds a skew for both TX and RX clocks, so the skew should only be /* This adds a skew for both TX and RX clocks, so the skew should only be
* applied to "rgmii-id" interfaces. It may not work as expected * applied to "rgmii-id" interfaces. It may not work as expected
* on "rgmii-txid", "rgmii-rxid" or "rgmii" interfaces. * on "rgmii-txid", "rgmii-rxid" or "rgmii" interfaces.
...@@ -444,7 +434,6 @@ static struct phy_driver vsc82xx_driver[] = { ...@@ -444,7 +434,6 @@ static struct phy_driver vsc82xx_driver[] = {
.phy_id_mask = 0x000ffff0, .phy_id_mask = 0x000ffff0,
/* PHY_GBIT_FEATURES */ /* PHY_GBIT_FEATURES */
.config_init = vsc738x_config_init, .config_init = vsc738x_config_init,
.config_aneg = vsc73xx_config_aneg,
.read_page = vsc73xx_read_page, .read_page = vsc73xx_read_page,
.write_page = vsc73xx_write_page, .write_page = vsc73xx_write_page,
}, { }, {
...@@ -453,7 +442,6 @@ static struct phy_driver vsc82xx_driver[] = { ...@@ -453,7 +442,6 @@ static struct phy_driver vsc82xx_driver[] = {
.phy_id_mask = 0x000ffff0, .phy_id_mask = 0x000ffff0,
/* PHY_GBIT_FEATURES */ /* PHY_GBIT_FEATURES */
.config_init = vsc738x_config_init, .config_init = vsc738x_config_init,
.config_aneg = vsc73xx_config_aneg,
.read_page = vsc73xx_read_page, .read_page = vsc73xx_read_page,
.write_page = vsc73xx_write_page, .write_page = vsc73xx_write_page,
}, { }, {
...@@ -462,7 +450,6 @@ static struct phy_driver vsc82xx_driver[] = { ...@@ -462,7 +450,6 @@ static struct phy_driver vsc82xx_driver[] = {
.phy_id_mask = 0x000ffff0, .phy_id_mask = 0x000ffff0,
/* PHY_GBIT_FEATURES */ /* PHY_GBIT_FEATURES */
.config_init = vsc739x_config_init, .config_init = vsc739x_config_init,
.config_aneg = vsc73xx_config_aneg,
.read_page = vsc73xx_read_page, .read_page = vsc73xx_read_page,
.write_page = vsc73xx_write_page, .write_page = vsc73xx_write_page,
}, { }, {
...@@ -471,7 +458,6 @@ static struct phy_driver vsc82xx_driver[] = { ...@@ -471,7 +458,6 @@ static struct phy_driver vsc82xx_driver[] = {
.phy_id_mask = 0x000ffff0, .phy_id_mask = 0x000ffff0,
/* PHY_GBIT_FEATURES */ /* PHY_GBIT_FEATURES */
.config_init = vsc739x_config_init, .config_init = vsc739x_config_init,
.config_aneg = vsc73xx_config_aneg,
.read_page = vsc73xx_read_page, .read_page = vsc73xx_read_page,
.write_page = vsc73xx_write_page, .write_page = vsc73xx_write_page,
}, { }, {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment