2024-04-17 14:07:26

by Gregor Herburger

[permalink] [raw]
Subject: [PATCH v2] net: phy: marvell-88q2xxx: add support for Rev B1 and B2

Different revisions of the Marvell 88q2xxx phy needs different init
sequences.

Add init sequence for Rev B1 and Rev B2. Rev B2 init sequence skips one
register write.

Signed-off-by: Gregor Herburger <[email protected]>
---
Hi,

as discussed when adding support for Marvell 88Q2220 Revision B0 [1],
newer revisions need different init sequences. So add support for Rev B1
and B2 with this patch.

[1] https://lore.kernel.org/netdev/20240216205302.GC3873@debian/

Best regards
Gregor
---
Changes in v2:
- Add helper function to write phy mmd sequences
- Link to v1: https://lore.kernel.org/r/20240403-mv88q222x-revb1-b2-init-v1-1-48b855464c37@ew.tq-group.com
---
drivers/net/phy/marvell-88q2xxx.c | 119 +++++++++++++++++++++++++++++++++-----
1 file changed, 103 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index 6b4bd9883304..bbbb3efb7877 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -12,6 +12,8 @@
#include <linux/hwmon.h>

#define PHY_ID_88Q2220_REVB0 (MARVELL_PHY_ID_88Q2220 | 0x1)
+#define PHY_ID_88Q2220_REVB1 (MARVELL_PHY_ID_88Q2220 | 0x2)
+#define PHY_ID_88Q2220_REVB2 (MARVELL_PHY_ID_88Q2220 | 0x3)

#define MDIO_MMD_AN_MV_STAT 32769
#define MDIO_MMD_AN_MV_STAT_ANEG 0x0100
@@ -129,6 +131,49 @@ static const struct mmd_val mv88q222x_revb0_init_seq1[] = {
{ MDIO_MMD_PCS, 0xfe05, 0x755c },
};

+static const struct mmd_val mv88q222x_revb1_init_seq0[] = {
+ { MDIO_MMD_PCS, 0xFFE4, 0x0007 },
+ { MDIO_MMD_AN, MDIO_AN_T1_CTRL, 0x0 },
+ { MDIO_MMD_PCS, 0xFFE3, 0x7000 },
+ { MDIO_MMD_PMAPMD, MDIO_CTRL1, 0x0840 },
+};
+
+static const struct mmd_val mv88q222x_revb2_init_seq0[] = {
+ { MDIO_MMD_PCS, 0xFFE4, 0x0007 },
+ { MDIO_MMD_AN, MDIO_AN_T1_CTRL, 0x0 },
+ { MDIO_MMD_PMAPMD, MDIO_CTRL1, 0x0840 },
+};
+
+static const struct mmd_val mv88q222x_revb1_revb2_init_seq1[] = {
+ { MDIO_MMD_PCS, 0xFE07, 0x125A },
+ { MDIO_MMD_PCS, 0xFE09, 0x1288 },
+ { MDIO_MMD_PCS, 0xFE08, 0x2588 },
+ { MDIO_MMD_PCS, 0xFE72, 0x042C },
+ { MDIO_MMD_PCS, 0xFFE4, 0x0071 },
+ { MDIO_MMD_PCS, 0xFFE4, 0x0001 },
+ { MDIO_MMD_PCS, 0xFE1B, 0x0048 },
+ { MDIO_MMD_PMAPMD, 0x0000, 0x0000 },
+ { MDIO_MMD_PCS, 0x0000, 0x0000 },
+ { MDIO_MMD_PCS, 0xFFDB, 0xFC10 },
+ { MDIO_MMD_PCS, 0xFE1B, 0x58 },
+ { MDIO_MMD_PCS, 0xFCAD, 0x030C },
+ { MDIO_MMD_PCS, 0x8032, 0x6001 },
+ { MDIO_MMD_PCS, 0xFDFF, 0x05A5 },
+ { MDIO_MMD_PCS, 0xFDEC, 0xDBAF },
+ { MDIO_MMD_PCS, 0xFCAB, 0x1054 },
+ { MDIO_MMD_PCS, 0xFCAC, 0x1483 },
+ { MDIO_MMD_PCS, 0x8033, 0xC801 },
+ { MDIO_MMD_AN, 0x8032, 0x2020 },
+ { MDIO_MMD_AN, 0x8031, 0xA28 },
+ { MDIO_MMD_AN, 0x8031, 0xC28 },
+ { MDIO_MMD_PCS, 0xFBBA, 0x0CB2 },
+ { MDIO_MMD_PCS, 0xFBBB, 0x0C4A },
+ { MDIO_MMD_PCS, 0xFE5F, 0xE8 },
+ { MDIO_MMD_PCS, 0xFE05, 0x755C },
+ { MDIO_MMD_PCS, 0xFA20, 0x002A },
+ { MDIO_MMD_PCS, 0xFE11, 0x1105 },
+};
+
static int mv88q2xxx_soft_reset(struct phy_device *phydev)
{
int ret;
@@ -687,31 +732,72 @@ static int mv88q222x_soft_reset(struct phy_device *phydev)
return 0;
}

-static int mv88q222x_revb0_config_init(struct phy_device *phydev)
+static int mv88q222x_write_mmd_vals(struct phy_device *phydev,
+ const struct mmd_val *vals, size_t len)
{
- int ret, i;
+ int ret;

- for (i = 0; i < ARRAY_SIZE(mv88q222x_revb0_init_seq0); i++) {
- ret = phy_write_mmd(phydev, mv88q222x_revb0_init_seq0[i].devad,
- mv88q222x_revb0_init_seq0[i].regnum,
- mv88q222x_revb0_init_seq0[i].val);
+ for (; len; vals++, len--) {
+ ret = phy_write_mmd(phydev, vals->devad, vals->regnum,
+ vals->val);
if (ret < 0)
return ret;
}

+ return 0;
+}
+
+static int mv88q222x_revb0_config_init(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb0_init_seq0,
+ ARRAY_SIZE(mv88q222x_revb0_init_seq0));
+ if (ret < 0)
+ return ret;
+
usleep_range(5000, 10000);

- for (i = 0; i < ARRAY_SIZE(mv88q222x_revb0_init_seq1); i++) {
- ret = phy_write_mmd(phydev, mv88q222x_revb0_init_seq1[i].devad,
- mv88q222x_revb0_init_seq1[i].regnum,
- mv88q222x_revb0_init_seq1[i].val);
- if (ret < 0)
- return ret;
- }
+ ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb0_init_seq1,
+ ARRAY_SIZE(mv88q222x_revb0_init_seq1));
+ if (ret < 0)
+ return ret;

return mv88q2xxx_config_init(phydev);
}

+static int mv88q222x_revb1_revb2_config_init(struct phy_device *phydev)
+{
+ bool is_rev_b1 = phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD] == PHY_ID_88Q2220_REVB1;
+ int ret;
+
+ if (is_rev_b1)
+ ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb1_init_seq0,
+ ARRAY_SIZE(mv88q222x_revb1_init_seq0));
+ else
+ ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb2_init_seq0,
+ ARRAY_SIZE(mv88q222x_revb2_init_seq0));
+ if (ret < 0)
+ return ret;
+
+ usleep_range(3000, 5000);
+
+ ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb1_revb2_init_seq1,
+ ARRAY_SIZE(mv88q222x_revb1_revb2_init_seq1));
+ if (ret < 0)
+ return ret;
+
+ return mv88q2xxx_config_init(phydev);
+}
+
+static int mv88q222x_config_init(struct phy_device *phydev)
+{
+ if (phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD] == PHY_ID_88Q2220_REVB0)
+ return mv88q222x_revb0_config_init(phydev);
+ else
+ return mv88q222x_revb1_revb2_config_init(phydev);
+}
+
static int mv88q222x_cable_test_start(struct phy_device *phydev)
{
int ret;
@@ -810,14 +896,15 @@ static struct phy_driver mv88q2xxx_driver[] = {
.get_sqi_max = mv88q2xxx_get_sqi_max,
},
{
- PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
+ .phy_id = MARVELL_PHY_ID_88Q2220,
+ .phy_id_mask = MARVELL_PHY_ID_MASK,
.name = "mv88q2220",
.flags = PHY_POLL_CABLE_TEST,
.probe = mv88q2xxx_probe,
.get_features = mv88q2xxx_get_features,
.config_aneg = mv88q2xxx_config_aneg,
.aneg_done = genphy_c45_aneg_done,
- .config_init = mv88q222x_revb0_config_init,
+ .config_init = mv88q222x_config_init,
.read_status = mv88q2xxx_read_status,
.soft_reset = mv88q222x_soft_reset,
.config_intr = mv88q2xxx_config_intr,
@@ -836,7 +923,7 @@ module_phy_driver(mv88q2xxx_driver);

static struct mdio_device_id __maybe_unused mv88q2xxx_tbl[] = {
{ MARVELL_PHY_ID_88Q2110, MARVELL_PHY_ID_MASK },
- { PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0), },
+ { MARVELL_PHY_ID_88Q2220, MARVELL_PHY_ID_MASK },
{ /*sentinel*/ }
};
MODULE_DEVICE_TABLE(mdio, mv88q2xxx_tbl);

---
base-commit: 1fdad13606e104ff103ca19d2d660830cb36d43e
change-id: 20240403-mv88q222x-revb1-b2-init-9961d2cf1d27

Best regards,
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/



2024-04-18 12:32:16

by Dimitri Fedrau

[permalink] [raw]
Subject: Re: [PATCH v2] net: phy: marvell-88q2xxx: add support for Rev B1 and B2

Am Wed, Apr 17, 2024 at 04:06:36PM +0200 schrieb Gregor Herburger:
> Different revisions of the Marvell 88q2xxx phy needs different init
> sequences.
>
> Add init sequence for Rev B1 and Rev B2. Rev B2 init sequence skips one
> register write.
>
> Signed-off-by: Gregor Herburger <[email protected]>
> ---
> Hi,
>
> as discussed when adding support for Marvell 88Q2220 Revision B0 [1],
> newer revisions need different init sequences. So add support for Rev B1
> and B2 with this patch.
>
> [1] https://lore.kernel.org/netdev/20240216205302.GC3873@debian/
>
> Best regards
> Gregor
> ---
> Changes in v2:
> - Add helper function to write phy mmd sequences
> - Link to v1: https://lore.kernel.org/r/20240403-mv88q222x-revb1-b2-init-v1-1-48b855464c37@ew.tq-group.com
> ---
> drivers/net/phy/marvell-88q2xxx.c | 119 +++++++++++++++++++++++++++++++++-----
> 1 file changed, 103 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> index 6b4bd9883304..bbbb3efb7877 100644
> --- a/drivers/net/phy/marvell-88q2xxx.c
> +++ b/drivers/net/phy/marvell-88q2xxx.c
> @@ -12,6 +12,8 @@
> #include <linux/hwmon.h>
>
> #define PHY_ID_88Q2220_REVB0 (MARVELL_PHY_ID_88Q2220 | 0x1)
> +#define PHY_ID_88Q2220_REVB1 (MARVELL_PHY_ID_88Q2220 | 0x2)
> +#define PHY_ID_88Q2220_REVB2 (MARVELL_PHY_ID_88Q2220 | 0x3)
>
> #define MDIO_MMD_AN_MV_STAT 32769
> #define MDIO_MMD_AN_MV_STAT_ANEG 0x0100
> @@ -129,6 +131,49 @@ static const struct mmd_val mv88q222x_revb0_init_seq1[] = {
> { MDIO_MMD_PCS, 0xfe05, 0x755c },
> };
>
> +static const struct mmd_val mv88q222x_revb1_init_seq0[] = {
> + { MDIO_MMD_PCS, 0xFFE4, 0x0007 },
> + { MDIO_MMD_AN, MDIO_AN_T1_CTRL, 0x0 },
> + { MDIO_MMD_PCS, 0xFFE3, 0x7000 },
> + { MDIO_MMD_PMAPMD, MDIO_CTRL1, 0x0840 },
> +};
> +
> +static const struct mmd_val mv88q222x_revb2_init_seq0[] = {
> + { MDIO_MMD_PCS, 0xFFE4, 0x0007 },
> + { MDIO_MMD_AN, MDIO_AN_T1_CTRL, 0x0 },
> + { MDIO_MMD_PMAPMD, MDIO_CTRL1, 0x0840 },
> +};
> +
> +static const struct mmd_val mv88q222x_revb1_revb2_init_seq1[] = {
> + { MDIO_MMD_PCS, 0xFE07, 0x125A },
> + { MDIO_MMD_PCS, 0xFE09, 0x1288 },
> + { MDIO_MMD_PCS, 0xFE08, 0x2588 },
> + { MDIO_MMD_PCS, 0xFE72, 0x042C },
> + { MDIO_MMD_PCS, 0xFFE4, 0x0071 },
> + { MDIO_MMD_PCS, 0xFFE4, 0x0001 },
> + { MDIO_MMD_PCS, 0xFE1B, 0x0048 },
> + { MDIO_MMD_PMAPMD, 0x0000, 0x0000 },
> + { MDIO_MMD_PCS, 0x0000, 0x0000 },
> + { MDIO_MMD_PCS, 0xFFDB, 0xFC10 },
> + { MDIO_MMD_PCS, 0xFE1B, 0x58 },
> + { MDIO_MMD_PCS, 0xFCAD, 0x030C },
> + { MDIO_MMD_PCS, 0x8032, 0x6001 },
> + { MDIO_MMD_PCS, 0xFDFF, 0x05A5 },
> + { MDIO_MMD_PCS, 0xFDEC, 0xDBAF },
> + { MDIO_MMD_PCS, 0xFCAB, 0x1054 },
> + { MDIO_MMD_PCS, 0xFCAC, 0x1483 },
> + { MDIO_MMD_PCS, 0x8033, 0xC801 },
> + { MDIO_MMD_AN, 0x8032, 0x2020 },
> + { MDIO_MMD_AN, 0x8031, 0xA28 },
> + { MDIO_MMD_AN, 0x8031, 0xC28 },
> + { MDIO_MMD_PCS, 0xFBBA, 0x0CB2 },
> + { MDIO_MMD_PCS, 0xFBBB, 0x0C4A },
> + { MDIO_MMD_PCS, 0xFE5F, 0xE8 },
> + { MDIO_MMD_PCS, 0xFE05, 0x755C },
> + { MDIO_MMD_PCS, 0xFA20, 0x002A },
> + { MDIO_MMD_PCS, 0xFE11, 0x1105 },
> +};
> +
nit: use small letters for hex values.

> static int mv88q2xxx_soft_reset(struct phy_device *phydev)
> {
> int ret;
> @@ -687,31 +732,72 @@ static int mv88q222x_soft_reset(struct phy_device *phydev)
> return 0;
> }
>
> -static int mv88q222x_revb0_config_init(struct phy_device *phydev)
> +static int mv88q222x_write_mmd_vals(struct phy_device *phydev,
> + const struct mmd_val *vals, size_t len)
> {
> - int ret, i;
> + int ret;
>
> - for (i = 0; i < ARRAY_SIZE(mv88q222x_revb0_init_seq0); i++) {
> - ret = phy_write_mmd(phydev, mv88q222x_revb0_init_seq0[i].devad,
> - mv88q222x_revb0_init_seq0[i].regnum,
> - mv88q222x_revb0_init_seq0[i].val);
> + for (; len; vals++, len--) {
> + ret = phy_write_mmd(phydev, vals->devad, vals->regnum,
> + vals->val);
> if (ret < 0)
> return ret;
> }
>
> + return 0;
> +}
> +
> +static int mv88q222x_revb0_config_init(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb0_init_seq0,
> + ARRAY_SIZE(mv88q222x_revb0_init_seq0));
> + if (ret < 0)
> + return ret;
> +
> usleep_range(5000, 10000);
>
> - for (i = 0; i < ARRAY_SIZE(mv88q222x_revb0_init_seq1); i++) {
> - ret = phy_write_mmd(phydev, mv88q222x_revb0_init_seq1[i].devad,
> - mv88q222x_revb0_init_seq1[i].regnum,
> - mv88q222x_revb0_init_seq1[i].val);
> - if (ret < 0)
> - return ret;
> - }
> + ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb0_init_seq1,
> + ARRAY_SIZE(mv88q222x_revb0_init_seq1));
> + if (ret < 0)
> + return ret;
>
> return mv88q2xxx_config_init(phydev);
> }
>
> +static int mv88q222x_revb1_revb2_config_init(struct phy_device *phydev)
> +{
> + bool is_rev_b1 = phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD] == PHY_ID_88Q2220_REVB1;
> + int ret;
> +
> + if (is_rev_b1)
> + ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb1_init_seq0,
> + ARRAY_SIZE(mv88q222x_revb1_init_seq0));
> + else
> + ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb2_init_seq0,
> + ARRAY_SIZE(mv88q222x_revb2_init_seq0));
> + if (ret < 0)
> + return ret;
> +
> + usleep_range(3000, 5000);
> +
> + ret = mv88q222x_write_mmd_vals(phydev, mv88q222x_revb1_revb2_init_seq1,
> + ARRAY_SIZE(mv88q222x_revb1_revb2_init_seq1));
> + if (ret < 0)
> + return ret;
> +
> + return mv88q2xxx_config_init(phydev);
> +}
> +
> +static int mv88q222x_config_init(struct phy_device *phydev)
> +{
> + if (phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD] == PHY_ID_88Q2220_REVB0)
> + return mv88q222x_revb0_config_init(phydev);
> + else
> + return mv88q222x_revb1_revb2_config_init(phydev);
> +}
> +
> static int mv88q222x_cable_test_start(struct phy_device *phydev)
> {
> int ret;
> @@ -810,14 +896,15 @@ static struct phy_driver mv88q2xxx_driver[] = {
> .get_sqi_max = mv88q2xxx_get_sqi_max,
> },
> {
> - PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
> + .phy_id = MARVELL_PHY_ID_88Q2220,
> + .phy_id_mask = MARVELL_PHY_ID_MASK,
> .name = "mv88q2220",
> .flags = PHY_POLL_CABLE_TEST,
> .probe = mv88q2xxx_probe,
> .get_features = mv88q2xxx_get_features,
> .config_aneg = mv88q2xxx_config_aneg,
> .aneg_done = genphy_c45_aneg_done,
> - .config_init = mv88q222x_revb0_config_init,
> + .config_init = mv88q222x_config_init,
> .read_status = mv88q2xxx_read_status,
> .soft_reset = mv88q222x_soft_reset,
> .config_intr = mv88q2xxx_config_intr,
> @@ -836,7 +923,7 @@ module_phy_driver(mv88q2xxx_driver);
>
> static struct mdio_device_id __maybe_unused mv88q2xxx_tbl[] = {
> { MARVELL_PHY_ID_88Q2110, MARVELL_PHY_ID_MASK },
> - { PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0), },
> + { MARVELL_PHY_ID_88Q2220, MARVELL_PHY_ID_MASK },
> { /*sentinel*/ }
> };
> MODULE_DEVICE_TABLE(mdio, mv88q2xxx_tbl);
>
Hi Gregor,

tested it for rev. B0 and it works as expected.

Tested-by: Dimitri Fedrau <[email protected]>

Best regards,
Dimitri Fedrau

> ---
> base-commit: 1fdad13606e104ff103ca19d2d660830cb36d43e
> change-id: 20240403-mv88q222x-revb1-b2-init-9961d2cf1d27
>
> Best regards,
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> https://www.tq-group.com/
>

2024-04-22 06:24:37

by Gregor Herburger

[permalink] [raw]
Subject: Re: Re: [PATCH v2] net: phy: marvell-88q2xxx: add support for Rev B1 and B2

On Thu, Apr 18, 2024 at 02:31:55PM +0200, Dimitri Fedrau wrote:
> > +static const struct mmd_val mv88q222x_revb1_revb2_init_seq1[] = {
> > + { MDIO_MMD_PCS, 0xFE07, 0x125A },
> > + { MDIO_MMD_PCS, 0xFE09, 0x1288 },
> > + { MDIO_MMD_PCS, 0xFE08, 0x2588 },
> > + { MDIO_MMD_PCS, 0xFE72, 0x042C },
> > + { MDIO_MMD_PCS, 0xFFE4, 0x0071 },
> > + { MDIO_MMD_PCS, 0xFFE4, 0x0001 },
> > + { MDIO_MMD_PCS, 0xFE1B, 0x0048 },
> > + { MDIO_MMD_PMAPMD, 0x0000, 0x0000 },
> > + { MDIO_MMD_PCS, 0x0000, 0x0000 },
> > + { MDIO_MMD_PCS, 0xFFDB, 0xFC10 },
> > + { MDIO_MMD_PCS, 0xFE1B, 0x58 },
> > + { MDIO_MMD_PCS, 0xFCAD, 0x030C },
> > + { MDIO_MMD_PCS, 0x8032, 0x6001 },
> > + { MDIO_MMD_PCS, 0xFDFF, 0x05A5 },
> > + { MDIO_MMD_PCS, 0xFDEC, 0xDBAF },
> > + { MDIO_MMD_PCS, 0xFCAB, 0x1054 },
> > + { MDIO_MMD_PCS, 0xFCAC, 0x1483 },
> > + { MDIO_MMD_PCS, 0x8033, 0xC801 },
> > + { MDIO_MMD_AN, 0x8032, 0x2020 },
> > + { MDIO_MMD_AN, 0x8031, 0xA28 },
> > + { MDIO_MMD_AN, 0x8031, 0xC28 },
> > + { MDIO_MMD_PCS, 0xFBBA, 0x0CB2 },
> > + { MDIO_MMD_PCS, 0xFBBB, 0x0C4A },
> > + { MDIO_MMD_PCS, 0xFE5F, 0xE8 },
> > + { MDIO_MMD_PCS, 0xFE05, 0x755C },
> > + { MDIO_MMD_PCS, 0xFA20, 0x002A },
> > + { MDIO_MMD_PCS, 0xFE11, 0x1105 },
> > +};
> > +
> nit: use small letters for hex values.
>

Ok, will update in next version.

> Hi Gregor,
>
> tested it for rev. B0 and it works as expected.
>
> Tested-by: Dimitri Fedrau <[email protected]>
>
> Best regards,
> Dimitri Fedrau

Hi Dimitri,

thanks for testing!