Re: [RFC PATCH 2/2] net: phy: dp83869: fix mii mode when rgmii strap cfg is used

[Siddharth Vadapalli <s-vadapalli@ti.com>]

On 25/04/23 17:48, Andrew Lunn wrote:
> On Tue, Apr 25, 2023 at 11:14:29AM +0530, Siddharth Vadapalli wrote:
>> From: Grygorii Strashko <grygorii.strashko@ti.com>
>>
>> The DP83869 PHY on TI's k3-am642-evm supports both MII and RGMII
>> interfaces and is configured by default to use RGMII interface (strap).
>> However, the board design allows switching dynamically to MII interface
>> for testing purposes by applying different set of pinmuxes.
> 
> Only for testing? So nobody should actually design a board to use MII
> and use software to change the interface from RGMII to MII?
> 
> This does not seem to be a fix, it is a new feature. So please submit
> to net-next, in two weeks time when it opens again.

Sure. I will split this patch from the series and post the v2 of this patch with
the subject "RFC PATCH net-next" for requesting further feedback on this patch
if needed. Following that, I will post it to net-next as a new patch.

> 
>> @@ -692,8 +692,11 @@ static int dp83869_configure_mode(struct phy_device *phydev,
>>  	/* Below init sequence for each operational mode is defined in
>>  	 * section 9.4.8 of the datasheet.
>>  	 */
>> +	phy_ctrl_val = dp83869->mode;
>> +	if (phydev->interface == PHY_INTERFACE_MODE_MII)
>> +		phy_ctrl_val |= DP83869_OP_MODE_MII;
> 
> Should there be some validation here with dp83869->mode?
> 
> DP83869_RGMII_COPPER_ETHERNET, DP83869_RGMII_SGMII_BRIDGE etc don't
> make sense if MII is being used. DP83869_100M_MEDIA_CONVERT and maybe
> DP83869_RGMII_100_BASE seem to be the only valid modes with MII?

The DP83869_OP_MODE_MII macro corresponds to BIT(5) which is the RGMII_MII_SEL
bit in the OP_MODE_DECODE register. If the RGMII_MII_SEL bit is set, MII mode is
selected. If the bit is cleared, which is the default value, RGMII mode is
selected. As pointed out by you, there are modes which aren't valid with MII
mode. However, a mode which isn't valid with RGMII mode (default value of the
RGMII_MII_SEL bit) also exists: DP83869_SGMII_COPPER_ETHERNET. For this reason,
I believe that setting the bit when MII mode is requested shouldn't cause any
issues.

> 
> 	Andrew

-- 
Regards,
Siddharth.