Received-SPF: pass (google.com: domain of linux-kernel+bounces-67651-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 15 Feb 2024 21:24:03 +0100
From: Dimitri Fedrau <dima.fedrau@gmail.com>
To: Gregor Herburger <gregor.herburger@ew.tq-group.com>
Cc: Andrew Lunn <andrew@lunn.ch>, Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Stefan Eichenberger <eichest@gmail.com>, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 net-next 05/14] net: phy: marvell-88q2xxx: add driver
 for the Marvell 88Q2220 PHY
Message-ID: <20240215202403.GA3103@debian>
References: <20240213213955.178762-1-dima.fedrau@gmail.com>
 <20240213213955.178762-6-dima.fedrau@gmail.com>
 <Zcy9E4riyFRk8B1P@herburgerg-w2>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Zcy9E4riyFRk8B1P@herburgerg-w2>

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