2024-03-26 14:16:55

by Pawel Dembicki

[permalink] [raw]
Subject: [PATCH net-next 2/2] net: phy: marvell: implement cable-test for 88E308X/88E609X family

This commit implements VCT in 88E308X/88E609X Family.

It require two workarounds with some magic configuration.
Regular use require only one register configuration. But Open Circuit
require second workaround.
It cause implementation two phases for fault length measuring.

Fast Ethernet PHY have implemented very simple version of VCT. It's
complitley different than vct5 or vct7.

Signed-off-by: Pawel Dembicki <[email protected]>
---
drivers/net/phy/marvell.c | 252 ++++++++++++++++++++++++++++++++++++++
1 file changed, 252 insertions(+)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index fae7eb57ee2c..38316d641d31 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -279,6 +279,23 @@
#define MII_VCT7_CTRL_METERS BIT(10)
#define MII_VCT7_CTRL_CENTIMETERS 0

+#define MII_VCT_TXPINS 0x1A
+#define MII_VCT_RXPINS 0x1B
+#define MII_VCT_TXPINS_ENVCT BIT(15)
+#define MII_VCT_TXRXPINS_VCTTST GENMASK(14, 13)
+#define MII_VCT_TXRXPINS_VCTTST_SHIFT 13
+#define MII_VCT_TXRXPINS_VCTTST_OK 0
+#define MII_VCT_TXRXPINS_VCTTST_SHORT 1
+#define MII_VCT_TXRXPINS_VCTTST_OPEN 2
+#define MII_VCT_TXRXPINS_VCTTST_FAIL 3
+#define MII_VCT_TXRXPINS_AMPRFLN GENMASK(12, 8)
+#define MII_VCT_TXRXPINS_AMPRFLN_SHIFT 8
+#define MII_VCT_TXRXPINS_DISTRFLN GENMASK(7, 0)
+#define MII_VCT_TXRXPINS_DISTRFLN_MAX 0xff
+
+#define M88E3082_PAIR_A BIT(0)
+#define M88E3082_PAIR_B BIT(1)
+
#define LPA_PAUSE_FIBER 0x180
#define LPA_PAUSE_ASYM_FIBER 0x100

@@ -301,6 +318,12 @@ static struct marvell_hw_stat marvell_hw_stats[] = {
{ "phy_receive_errors_fiber", 1, 21, 16},
};

+enum {
+ M88E3082_VCT_OFF,
+ M88E3082_VCT_PHASE1,
+ M88E3082_VCT_PHASE2,
+};
+
struct marvell_priv {
u64 stats[ARRAY_SIZE(marvell_hw_stats)];
char *hwmon_name;
@@ -310,6 +333,8 @@ struct marvell_priv {
u32 last;
u32 step;
s8 pair;
+ u8 vct_phase;
+ u16 m88e3082_vct_reg_backup;
};

static int marvell_read_page(struct phy_device *phydev)
@@ -2417,6 +2442,231 @@ static int marvell_vct7_cable_test_get_status(struct phy_device *phydev,
return 0;
}

+static int m88e3082_vct_cable_test_start(struct phy_device *phydev)
+{
+ struct marvell_priv *priv = phydev->priv;
+ int ret;
+
+ /* It needs some magic workaround described in VCT manual for this PHY.
+ */
+
+ switch (priv->vct_phase) {
+ case M88E3082_VCT_OFF:
+ ret = phy_write(phydev, 29, 0x0003);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 30, 0x6440);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_read(phydev, MII_BMCR);
+ if (ret < 0)
+ return ret;
+ priv->m88e3082_vct_reg_backup = ret;
+
+ ret = phy_write(phydev, MII_BMCR, 0xa100);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, MII_VCT_TXPINS, MII_VCT_TXPINS_ENVCT);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 29, 0x0003);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 30, 0x0);
+ if (ret < 0)
+ return ret;
+
+ priv->vct_phase = M88E3082_VCT_PHASE1;
+ priv->pair = 0;
+
+ break;
+
+ case M88E3082_VCT_PHASE1:
+ ret = phy_write(phydev, 29, 0x0003);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 30, 0x6440);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 29, 0x000a);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 30, 0x0002);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_read(phydev, MII_BMCR);
+ if (ret < 0)
+ return ret;
+ priv->m88e3082_vct_reg_backup = ret;
+
+ ret = phy_write(phydev, MII_BMCR, 0xa100);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, MII_VCT_TXPINS, MII_VCT_TXPINS_ENVCT);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 29, 0x0003);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 30, 0x0);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 29, 0x000a);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write(phydev, 30, 0x0);
+ if (ret < 0)
+ return ret;
+
+ priv->vct_phase = M88E3082_VCT_PHASE2;
+ break;
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int m88e3082_vct_cable_test_report_trans(int result, u8 distance)
+{
+ switch (result) {
+ case MII_VCT_TXRXPINS_VCTTST_OK:
+ if (distance == MII_VCT_TXRXPINS_DISTRFLN_MAX)
+ return ETHTOOL_A_CABLE_RESULT_CODE_OK;
+ /* Impedance mismatch */
+ return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
+ case MII_VCT_TXRXPINS_VCTTST_SHORT:
+ return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
+ case MII_VCT_TXRXPINS_VCTTST_OPEN:
+ return ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
+ default:
+ return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
+ }
+}
+
+u32 m88e3082_vct_distrfln_2_cm(u8 distrfln)
+{
+ if (distrfln < 24)
+ return 0;
+
+ /* Original function for meters: y = 0.7861x - 18.862 */
+ return (7861 * distrfln - 188620) / 100;
+}
+
+static int m88e3082_vct_cable_test_get_status(struct phy_device *phydev,
+ bool *finished)
+{
+ u8 tx_vcttst_res, rx_vcttst_res, tx_distrfln, rx_distrfln;
+ struct marvell_priv *priv = phydev->priv;
+ int ret, tx_result, rx_result;
+ bool done_phase = true;
+
+ *finished = false;
+
+ ret = phy_read(phydev, MII_VCT_TXPINS);
+ if (ret < 0)
+ return ret;
+ else if (ret & MII_VCT_TXPINS_ENVCT)
+ return 0;
+
+ tx_distrfln = ret & MII_VCT_TXRXPINS_DISTRFLN;
+ tx_vcttst_res = (ret & MII_VCT_TXRXPINS_VCTTST) >>
+ MII_VCT_TXRXPINS_VCTTST_SHIFT;
+
+ ret = phy_read(phydev, MII_VCT_RXPINS);
+ if (ret < 0)
+ return ret;
+
+ rx_distrfln = ret & MII_VCT_TXRXPINS_DISTRFLN;
+ rx_vcttst_res = (ret & MII_VCT_TXRXPINS_VCTTST) >>
+ MII_VCT_TXRXPINS_VCTTST_SHIFT;
+
+ *finished = true;
+
+ switch (priv->vct_phase) {
+ case M88E3082_VCT_PHASE1:
+ tx_result = m88e3082_vct_cable_test_report_trans(tx_vcttst_res,
+ tx_distrfln);
+ rx_result = m88e3082_vct_cable_test_report_trans(rx_vcttst_res,
+ rx_distrfln);
+
+ ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A,
+ tx_result);
+ ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_B,
+ rx_result);
+
+ if (tx_vcttst_res == MII_VCT_TXRXPINS_VCTTST_OPEN) {
+ done_phase = false;
+ priv->pair |= M88E3082_PAIR_A;
+ } else if (tx_distrfln < MII_VCT_TXRXPINS_DISTRFLN_MAX) {
+ u8 pair = ETHTOOL_A_CABLE_PAIR_A;
+ u32 cm = m88e3082_vct_distrfln_2_cm(tx_distrfln);
+
+ ethnl_cable_test_fault_length(phydev, pair, cm);
+ }
+
+ if (rx_vcttst_res == MII_VCT_TXRXPINS_VCTTST_OPEN) {
+ done_phase = false;
+ priv->pair |= M88E3082_PAIR_B;
+ } else if (rx_distrfln < MII_VCT_TXRXPINS_DISTRFLN_MAX) {
+ u8 pair = ETHTOOL_A_CABLE_PAIR_B;
+ u32 cm = m88e3082_vct_distrfln_2_cm(rx_distrfln);
+
+ ethnl_cable_test_fault_length(phydev, pair, cm);
+ }
+
+ break;
+ case M88E3082_VCT_PHASE2:
+ if (priv->pair & M88E3082_PAIR_A &&
+ tx_vcttst_res == MII_VCT_TXRXPINS_VCTTST_OPEN &&
+ tx_distrfln < MII_VCT_TXRXPINS_DISTRFLN_MAX) {
+ u8 pair = ETHTOOL_A_CABLE_PAIR_A;
+ u32 cm = m88e3082_vct_distrfln_2_cm(tx_distrfln);
+
+ ethnl_cable_test_fault_length(phydev, pair, cm);
+ }
+ if (priv->pair & M88E3082_PAIR_B &&
+ rx_vcttst_res == MII_VCT_TXRXPINS_VCTTST_OPEN &&
+ rx_distrfln < MII_VCT_TXRXPINS_DISTRFLN_MAX) {
+ u8 pair = ETHTOOL_A_CABLE_PAIR_B;
+ u32 cm = m88e3082_vct_distrfln_2_cm(rx_distrfln);
+
+ ethnl_cable_test_fault_length(phydev, pair, cm);
+ }
+
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = phy_write(phydev, MII_BMCR, priv->m88e3082_vct_reg_backup);
+ if (ret < 0)
+ return ret;
+
+ if (!done_phase) {
+ *finished = false;
+ return m88e3082_vct_cable_test_start(phydev);
+ }
+ if (*finished)
+ priv->vct_phase = M88E3082_VCT_OFF;
+ return 0;
+}
+
#ifdef CONFIG_HWMON
struct marvell_hwmon_ops {
int (*config)(struct phy_device *phydev);
@@ -3300,6 +3550,8 @@ static struct phy_driver marvell_drivers[] = {
.read_status = marvell_read_status,
.resume = genphy_resume,
.suspend = genphy_suspend,
+ .cable_test_start = m88e3082_vct_cable_test_start,
+ .cable_test_get_status = m88e3082_vct_cable_test_get_status,
},
{
.phy_id = MARVELL_PHY_ID_88E1112,
--
2.25.1



2024-03-26 14:39:48

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next 2/2] net: phy: marvell: implement cable-test for 88E308X/88E609X family

> + ret = phy_write(phydev, MII_BMCR, 0xa100);

BMCR_RESET | BMCR_ANENABLE | BMCR_FULLDPLX

You should check i have that correct, but no need for magic values
with this register.

> +static int m88e3082_vct_cable_test_report_trans(int result, u8 distance)
> +{
> + switch (result) {
> + case MII_VCT_TXRXPINS_VCTTST_OK:
> + if (distance == MII_VCT_TXRXPINS_DISTRFLN_MAX)
> + return ETHTOOL_A_CABLE_RESULT_CODE_OK;
> + /* Impedance mismatch */
> + return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;

It is possible to add more results code, if you think Impedance
mismatch is useful.

> + ret = phy_write(phydev, MII_BMCR, priv->m88e3082_vct_reg_backup);
> + if (ret < 0)
> + return ret;

I'm not sure this is required. When the cable test is finished, the
PHY state machine is moved to PHY_UP. That will cause
phy_config_aneg() to be called which should set BMCR back to the
correct value.

Andrew

2024-03-27 15:46:46

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH net-next 2/2] net: phy: marvell: implement cable-test for 88E308X/88E609X family

On Tue, Mar 26, 2024 at 03:12:36PM +0100, Pawel Dembicki wrote:
> This commit implements VCT in 88E308X/88E609X Family.
>
> It require two workarounds with some magic configuration.
> Regular use require only one register configuration. But Open Circuit
> require second workaround.
> It cause implementation two phases for fault length measuring.
>
> Fast Ethernet PHY have implemented very simple version of VCT. It's
> complitley different than vct5 or vct7.
>
> Signed-off-by: Pawel Dembicki <[email protected]>
> ---
> drivers/net/phy/marvell.c | 252 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 252 insertions(+)
>
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c

..

> +u32 m88e3082_vct_distrfln_2_cm(u8 distrfln)

nit: This function seems to only be used in this file.
If so it should be static.

> +{
> + if (distrfln < 24)
> + return 0;
> +
> + /* Original function for meters: y = 0.7861x - 18.862 */
> + return (7861 * distrfln - 188620) / 100;
> +}

..