linux-kernel - Re: [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250605181810.GB2946252-robh@kernel.org>
Date: Thu, 5 Jun 2025 13:18:10 -0500
From: Rob Herring <robh@...nel.org>
To: Andrew Lunn <andrew@...n.ch>
Cc: george.moussalem@...look.com, Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Florian Fainelli <f.fainelli@...il.com>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konradybcio@...nel.org>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018
 Internal PHY support

On Mon, Jun 02, 2025 at 02:41:56PM +0200, Andrew Lunn wrote:
> > +	/* PHY DAC values are optional and only set in a PHY to PHY link architecture */
> > +	if (priv->set_short_cable_dac) {
> > +		/* setting MDAC (Multi-level Digital-to-Analog Converter) in MMD1 */
> > +		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MDAC,
> > +			       IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_MMD1_MDAC_VAL);
> > +
> > +		/* setting EDAC (Error-detection and Correction) in debug register */
> > +		at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_EDAC,
> > +				      IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_DEBUG_EDAC_VAL);
> > +	}
> 
> In the binding you say:
> 
> 
> +            If not set, it is assumed the MDI output pins of this PHY are directly
> +            connected to an RJ45 connector and default DAC values will be used.
> 
> So shouldn't there be an else clause here setting these two values to
> their default, undoing what the bootloader might of done etc.
> 
> Or you can change the binding, and say something like:
> 
> +            If not set, DAC values are not modified.
> 
> We often need a tristate in DT, set something, unset something, leave
> it as it is. But that does not exist in DT in an easy form :-(

I'm happy to define that and have thought about it. That does up the 
minimum version of dtschema required, but we pretty much expect people 
to use the latest anyways.

Rob