netdev - Re: [net-next PATCH 02/13] drivers: net: phy: at803x: add DAC amplitude fix for 8327 phy

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]

Date:   Thu, 7 Oct 2021 01:59:55 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Ansuel Smith <ansuelsmth@...il.com>
Cc:     Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [net-next PATCH 02/13] drivers: net: phy: at803x: add DAC
 amplitude fix for 8327 phy

On Thu, Oct 07, 2021 at 12:35:52AM +0200, Ansuel Smith wrote:
> QCA8327 internal phy require DAC amplitude adjustement set to +6% with
> 100m speed. Also add additional define to report a change of the same
> reg in QCA8337. (different scope it does set 1000m voltage)
> Add link_change_notify function to set the proper amplitude adjustement
> on PHY_RUNNING state and disable on any other state.
> 
> Fixes: c6bcec0d6928 ("net: phy: at803x: add support for qca 8327 A variant internal phy")
> Signed-off-by: Ansuel Smith <ansuelsmth@...il.com>

Since this is a fix, you might want to send it on its own, based on
net.

> +	/* QCA8327 require DAC amplitude adjustment for 100m set to +6%.
> +	 * Disable on init and enable only with 100m speed following
> +	 * qca original source code.
> +	 */
> +	if (phydev->drv->phy_id == QCA8327_A_PHY_ID ||
> +	    phydev->drv->phy_id == QCA8327_B_PHY_ID)
> +		at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> +				      QCA8327_DEBUG_MANU_CTRL_EN, 0);
> +
>  	return 0;
>  }
>  
> +static void qca83xx_link_change_notify(struct phy_device *phydev)
> +{
> +	/* QCA8337 doesn't require DAC Amplitude adjustement */
> +	if (phydev->drv->phy_id == QCA8337_PHY_ID)
> +		return;
> +
> +	/* Set DAC Amplitude adjustment to +6% for 100m on link running */
> +	if (phydev->state == PHY_RUNNING) {
> +		if (phydev->speed == SPEED_100)
> +			at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> +					      QCA8327_DEBUG_MANU_CTRL_EN,
> +					      QCA8327_DEBUG_MANU_CTRL_EN);
> +	} else {
> +		/* Reset DAC Amplitude adjustment */
> +		at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> +				      QCA8327_DEBUG_MANU_CTRL_EN, 0);

Here you don't make it conditional on QCA8327_A_PHY_ID and
QCA8327_B_PHY_ID, where as above you do?

	  Andrew