[PATCH v2 2/2] net: phy: air_en8811h: use standard rx-polarity/tx-polarity properties

[Lucien.Jheng]

Vladimir Oltean 於 2026/3/10 下午 04:28 寫道:
> On Sun, Mar 08, 2026 at 10:06:14PM +0800, Lucien.Jheng wrote:
>> diff --git a/drivers/net/phy/airoha/air_en8811.c b/drivers/net/phy/airoha/air_en8811.c
>> index 1a628ede82b..0ec11ba5231 100644
>> --- a/drivers/net/phy/airoha/air_en8811.c
>> +++ b/drivers/net/phy/airoha/air_en8811.c
>> @@ -23,6 +23,7 @@
>>   #include <linux/compat.h>
>>   #include <dm/device_compat.h>
>>   #include <u-boot/crc.h>
>> +#include <linux/phy/phy-common-props.h>
>>
>>   #define EN8811H_PHY_ID		0x03a2a411
>>
>> @@ -660,11 +661,82 @@ static int air_leds_init(struct phy_device *phydev, int num, u16 dur, int mode)
>>   	return 0;
>>   }
>>
>> -static int en8811h_config(struct phy_device *phydev)
>> +static int en8811h_config_polarity(struct phy_device *phydev)
>>   {
>> -	struct en8811h_priv *priv = phydev->priv;
>>   	ofnode node = phy_get_ofnode(phydev);
>> +	unsigned int rx_pol, tx_pol;
>>   	u32 pbus_value = 0;
>> +	int ret;
>> +	bool use_legacy = false;
>> +
>> +	if (!ofnode_valid(node))
>> +		return 0;
>> +
>> +	/* Try standard rx-polarity property first */
>> +	ret = phy_get_manual_rx_polarity(node,
>> +					 phy_interface_strings[phydev->interface],
>> +					 &rx_pol);
>> +	if (ret == 0) {
>> +		dev_info(phydev->dev, "EN8811H: RX polarity = %s\n",
>> +			 rx_pol == PHY_POL_INVERT ? "inverted" : "normal");
>> +		if (rx_pol == PHY_POL_INVERT)
>> +			pbus_value |= EN8811H_POLARITY_RX_REVERSE;
>> +		else
>> +			pbus_value &= ~EN8811H_POLARITY_RX_REVERSE;
>> +	} else if (ret == -ENOENT) {
>> +		/* Fall back to legacy airoha,pnswap-rx property */
>> +		use_legacy = true;
>> +		dev_info(phydev->dev, "EN8811H: Legacy RX polarity\n");
>> +
>> +		if (ofnode_read_bool(node, "airoha,pnswap-rx"))
>> +			pbus_value |= EN8811H_POLARITY_RX_REVERSE;
>> +		else
>> +			pbus_value &= ~EN8811H_POLARITY_RX_REVERSE;
>> +	} else {
>> +		dev_err(phydev->dev, "EN8811H: Error reading rx-polarity: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Try standard tx-polarity property first */
>> +	ret = phy_get_manual_tx_polarity(node,
>> +					 phy_interface_strings[phydev->interface],
>> +					 &tx_pol);
>> +	if (ret == 0) {
>> +		dev_info(phydev->dev, "EN8811H: TX polarity = %s\n",
>> +			 tx_pol == PHY_POL_INVERT ? "inverted" : "normal");
>> +		if (tx_pol == PHY_POL_INVERT)
>> +			pbus_value &= ~EN8811H_POLARITY_TX_NORMAL;
>> +		else
>> +			pbus_value |= EN8811H_POLARITY_TX_NORMAL;
>> +	} else if (ret == -ENOENT) {
>> +		/* Fall back to legacy airoha,pnswap-tx property */
>> +		if (!use_legacy)
>> +			dev_err(phydev->dev, "EN8811H: Mixing standard and legacy polarity properties\n");
> Mixing should be perfectly legal. Otherwise there's no way of converting
> an old scheme to the new one without breaking backwards compatibility.
>
> The way in which this is done in Linux is by reading the legacy
> properties first (with a fallback on PHY_POL_NORMAL if absent) and then
> using them as "default" argument for phy_get_(rx|tx)_polarity(). The
> phy_get_manual_(rx|tx)_polarity() are intended for the use case where
> you have no legacy property to support.
>
> I believe doing the same way here would simplify your logic quite a bit.
> No need to look at the specific "ret" value, and no need to duplicate
> the pbus_value modification.

Thank you for your comment.

I think I misinterpreted the code section.

I will re-examine the Linux implementation and refine my code accordingly.

>> +
>> +		dev_info(phydev->dev, "EN8811H: Legacy TX polarity\n");
>> +
>> +		if (ofnode_read_bool(node, "airoha,pnswap-tx"))
>> +			pbus_value &= ~EN8811H_POLARITY_TX_NORMAL;
>> +		else
>> +			pbus_value |= EN8811H_POLARITY_TX_NORMAL;
>> +	} else {
>> +		dev_err(phydev->dev, "EN8811H: Error reading tx-polarity: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Apply polarity configuration to hardware */
>> +	ret = air_buckpbus_reg_modify(phydev, EN8811H_POLARITY,
>> +				      EN8811H_POLARITY_RX_REVERSE |
>> +				      EN8811H_POLARITY_TX_NORMAL, pbus_value);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int en8811h_config(struct phy_device *phydev)
>> +{
>> +	struct en8811h_priv *priv = phydev->priv;
>>   	int ret = 0;
>>
>>   	/* If restart happened in .probe(), no need to restart now */
>> @@ -695,19 +767,8 @@ static int en8811h_config(struct phy_device *phydev)
>>   	if (ret < 0)
>>   		return ret;
>>
>> -	/* Serdes polarity */
>> -	pbus_value = 0;
>> -	if (ofnode_read_bool(node, "airoha,pnswap-rx"))
>> -		pbus_value |=  EN8811H_POLARITY_RX_REVERSE;
>> -	else
>> -		pbus_value &= ~EN8811H_POLARITY_RX_REVERSE;
>> -	if (ofnode_read_bool(node, "airoha,pnswap-tx"))
>> -		pbus_value &= ~EN8811H_POLARITY_TX_NORMAL;
>> -	else
>> -		pbus_value |=  EN8811H_POLARITY_TX_NORMAL;
>> -	ret = air_buckpbus_reg_modify(phydev, EN8811H_POLARITY,
>> -				      EN8811H_POLARITY_RX_REVERSE |
>> -				      EN8811H_POLARITY_TX_NORMAL, pbus_value);
>> +	/* Configure Serdes polarity from device tree */
>> +	ret = en8811h_config_polarity(phydev);
>>   	if (ret < 0)
>>   		return ret;
>>
>> --
>> 2.34.1
>>