Received-SPF: pass (google.com: domain of linux-kernel+bounces-51182-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <65be6997.7b0a0220.3acf.de81@mx.google.com>
Date: Sat, 3 Feb 2024 17:28:03 +0100
From: Christian Marangi <ansuelsmth@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	Frank Rowand <frowand.list@gmail.com>,
	Robert Marko <robert.marko@sartura.hr>, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [net-next PATCH v5 7/9] net: phy: qcom: add support for QCA807x
 PHY Family
References: <20240201151747.7524-1-ansuelsmth@gmail.com>
 <20240201151747.7524-8-ansuelsmth@gmail.com>
 <a530f40c-b8fd-4da1-b4df-f80ab05f0394@lunn.ch>
 <65bd29f9.050a0220.a175d.3d5f@mx.google.com>
 <c3447c2b-4b43-4c09-b89e-d6a44971b923@lunn.ch>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c3447c2b-4b43-4c09-b89e-d6a44971b923@lunn.ch>

On Sat, Feb 03, 2024 at 05:25:23PM +0100, Andrew Lunn wrote:
> On Fri, Feb 02, 2024 at 06:44:22PM +0100, Christian Marangi wrote:
> > On Fri, Feb 02, 2024 at 02:35:11AM +0100, Andrew Lunn wrote:
> > > > +static int qca807x_read_fiber_status(struct phy_device *phydev)
> > > > +{
> > > > +	int ss, err, lpa, old_link = phydev->link;
> > > > +
> > > > +	/* Update the link, but return if there was an error */
> > > > +	err = genphy_update_link(phydev);
> > > > +	if (err)
> > > > +		return err;
> > > > +
> > > > +	/* why bother the PHY if nothing can have changed */
> > > > +	if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link)
> > > > +		return 0;
> > > > +
> > > > +	phydev->speed = SPEED_UNKNOWN;
> > > > +	phydev->duplex = DUPLEX_UNKNOWN;
> > > > +	phydev->pause = 0;
> > > > +	phydev->asym_pause = 0;
> > > > +
> > > > +	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
> > > > +		lpa = phy_read(phydev, MII_LPA);
> > > > +		if (lpa < 0)
> > > > +			return lpa;
> > > > +
> > > > +		linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> > > > +				 phydev->lp_advertising, lpa & LPA_LPACK);
> > > > +		linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> > > > +				 phydev->lp_advertising, lpa & LPA_1000XFULL);
> > > > +		linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> > > > +				 phydev->lp_advertising, lpa & LPA_1000XPAUSE);
> > > > +		linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> > > > +				 phydev->lp_advertising,
> > > > +				 lpa & LPA_1000XPAUSE_ASYM);
> > > > +
> > > > +		phy_resolve_aneg_linkmode(phydev);
> > > > +	}
> > > 
> > > This looks a lot like genphy_c37_read_status(). Can it be used?
> > >
> > 
> > Yes but I had to expand genphy_c37_read_status. Hope it will be OK.
> 
> You can expand it, but please keep to what is defined within 802.3. We
> don't want any vendor extensions in this common code. Vendor things
> should be kept in the vendor driver. So you can call
> genphy_c37_read_status() and then do any vendor specific fixups
> needed.
>

Sure the expansion is just adding a bool signal if the link has changed
or not (to make it possible to exit early and skip the additional vendor
call...) I didn't add anything to the c37 function ifself.

Anyway of from this, the revision is ready, just need to understand if
Rob is ok with absolute or relative address for PHYs in the PHY package
node.

-- 
	Ansuel