Received-SPF: pass (google.com: domain of linux-kernel+bounces-56913-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 7 Feb 2024 19:35:47 +0100
From: Andrew Lunn <andrew@lunn.ch>
To: Catalin Popescu <catalin.popescu@leica-geosystems.com>
Cc: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	conor+dt@kernel.org, afd@ti.com, hkallweit1@gmail.com,
	linux@armlinux.org.uk, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	bsp-development.geo@leica-geosystems.com, m.felsch@pengutronix.de
Subject: Re: [PATCH v2 2/2] net: phy: dp83826: support TX data voltage tuning
Message-ID: <4dc382bd-3477-45cb-8044-fc5c2c7251f4@lunn.ch>
References: <20240207175845.764775-1-catalin.popescu@leica-geosystems.com>
 <20240207175845.764775-2-catalin.popescu@leica-geosystems.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240207175845.764775-2-catalin.popescu@leica-geosystems.com>

> +static int dp83826_config_init(struct phy_device *phydev)
> +{
> +	struct dp83822_private *dp83822 = phydev->priv;
> +	u16 val, mask;
> +	int ret;
> +
> +	if (dp83822->cfg_dac_minus != DP83826_CFG_DAC_MINUS_DEFAULT) {
> +		val = FIELD_PREP(DP83826_VOD_CFG1_MINUS_MDI_MASK, dp83822->cfg_dac_minus) |
> +		      FIELD_PREP(DP83826_VOD_CFG1_MINUS_MDIX_MASK,
> +				 FIELD_GET(DP83826_CFG_DAC_MINUS_MDIX_5_TO_4,
> +					   dp83822->cfg_dac_minus));
> +		mask = DP83826_VOD_CFG1_MINUS_MDIX_MASK | DP83826_VOD_CFG1_MINUS_MDI_MASK;
> +		ret = phy_modify_mmd(phydev, DP83822_DEVADDR, MII_DP83826_VOD_CFG1, mask, val);
> +		if (ret)
> +			return ret;
> +
> +		val = FIELD_PREP(DP83826_VOD_CFG2_MINUS_MDIX_MASK,
> +				 FIELD_GET(DP83826_CFG_DAC_MINUS_MDIX_3_TO_0,
> +					   dp83822->cfg_dac_minus));
> +		mask = DP83826_VOD_CFG2_MINUS_MDIX_MASK;
> +		ret = phy_modify_mmd(phydev, DP83822_DEVADDR, MII_DP83826_VOD_CFG2, mask, val);
> +		if (ret)
> +			return ret;
> +	}

I could be reading this wrong, but it looks like
DP83826_CFG_DAC_MINUS_DEFAULT actually means leave the value
unchanged? Is there anything guaranteeing it does in fact have the
default value in the hardware?

	Andrew