2024-02-14 13:21:26

by Gregor Herburger

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 05/14] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY

Hi Dimitri,

On Tue, Feb 13, 2024 at 10:39:44PM +0100, Dimitri Fedrau wrote:
> static struct phy_driver mv88q2xxx_driver[] = {
> {
> .phy_id = MARVELL_PHY_ID_88Q2110,
> @@ -255,12 +439,26 @@ static struct phy_driver mv88q2xxx_driver[] = {
> .get_sqi = mv88q2xxx_get_sqi,
> .get_sqi_max = mv88q2xxx_get_sqi_max,
> },
> + {
> + PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),

I tested the series on a 88Q2220 REV B1 (which is id 0x002b0b22). The
driver works fine on this revision.

I understand that in the Marvell API the initialization for Rev B0 and
B1 differ. For B0 some additional init sequence is executed. I did not look
into the details of this sequence. However this patch seems to work on
Rev B1.

Would you consider adding compatibility for Rev B1 and following? I
tested with:
.phy_id = MARVELL_PHY_ID_88Q2220,
.phy_id_mask = MARVELL_PHY_ID_MASK,

Otherwise:

Tested-by: Gregor Herburger <[email protected]>

Best regards
Gregor
--
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-02-15 20:31:00

by Dimitri Fedrau

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 05/14] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY

Am Wed, Feb 14, 2024 at 02:20:37PM +0100 schrieb Gregor Herburger:
> Hi Dimitri,
>
Hi Gregor,

> On Tue, Feb 13, 2024 at 10:39:44PM +0100, Dimitri Fedrau wrote:
> > static struct phy_driver mv88q2xxx_driver[] = {
> > {
> > .phy_id = MARVELL_PHY_ID_88Q2110,
> > @@ -255,12 +439,26 @@ static struct phy_driver mv88q2xxx_driver[] = {
> > .get_sqi = mv88q2xxx_get_sqi,
> > .get_sqi_max = mv88q2xxx_get_sqi_max,
> > },
> > + {
> > + PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
>
> I tested the series on a 88Q2220 REV B1 (which is id 0x002b0b22). The
> driver works fine on this revision.
>
> I understand that in the Marvell API the initialization for Rev B0 and
> B1 differ. For B0 some additional init sequence is executed. I did not look
> into the details of this sequence. However this patch seems to work on
> Rev B1.
>
> Would you consider adding compatibility for Rev B1 and following? I
> tested with:
> .phy_id = MARVELL_PHY_ID_88Q2220,
> .phy_id_mask = MARVELL_PHY_ID_MASK,
>

thanks for testing. I would stick to the exact initialization sequence
provided by the Marvell API. Registers and bits are mostly undocumented
and I think it is safest this way. Besides that it should be relatively
easy to add the support for rev. B1 by just adding the init sequence for
it.

Best regards,
Dimitri Fedrau

2024-02-16 09:18:58

by Gregor Herburger

[permalink] [raw]
Subject: Re: Re: [PATCH v6 net-next 05/14] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY

On Thu, Feb 15, 2024 at 09:24:03PM +0100, Dimitri Fedrau wrote:
> > Hi Dimitri,
> >
> Hi Gregor,
>
> > On Tue, Feb 13, 2024 at 10:39:44PM +0100, Dimitri Fedrau wrote:
> > > static struct phy_driver mv88q2xxx_driver[] = {
> > > {
> > > .phy_id = MARVELL_PHY_ID_88Q2110,
> > > @@ -255,12 +439,26 @@ static struct phy_driver mv88q2xxx_driver[] = {
> > > .get_sqi = mv88q2xxx_get_sqi,
> > > .get_sqi_max = mv88q2xxx_get_sqi_max,
> > > },
> > > + {
> > > + PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
> >
> > I tested the series on a 88Q2220 REV B1 (which is id 0x002b0b22). The
> > driver works fine on this revision.
> >
> > I understand that in the Marvell API the initialization for Rev B0 and
> > B1 differ. For B0 some additional init sequence is executed. I did not look
> > into the details of this sequence. However this patch seems to work on
> > Rev B1.
> >
> > Would you consider adding compatibility for Rev B1 and following? I
> > tested with:
> > .phy_id = MARVELL_PHY_ID_88Q2220,
> > .phy_id_mask = MARVELL_PHY_ID_MASK,
> >
>
> thanks for testing. I would stick to the exact initialization sequence
> provided by the Marvell API. Registers and bits are mostly undocumented
> and I think it is safest this way. Besides that it should be relatively
> easy to add the support for rev. B1 by just adding the init sequence for
> it.

Ok. I will have an closer look at the marvell API and eventually come up
with a patch for Rev. B1.

There is also a Rev.B2 for which I cannot find any init sequence. But
Rev. B1 will no longer be produced so I need a solution for B2
eventually.

Best regards
Gregor

--
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/