Re: [PATCH 2/3] net: ethernet: ti: am65-cpsw: Add support for QSGMII mode

[Siddharth Vadapalli]

Hello Russell,

On 31/05/22 17:20, Russell King (Oracle) wrote:
> On Tue, May 31, 2022 at 05:00:57PM +0530, Siddharth Vadapalli wrote:
>>  static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode,
>>  				      const struct phylink_link_state *state)
>>  {
>> -	/* Currently not used */
>> +	struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data,
>> +							  phylink_config);
>> +	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
>> +
>> +	if (state->interface == PHY_INTERFACE_MODE_QSGMII)
>> +		writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE,
>> +		       port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
> 
> What about writing this register when the interface mode isn't QSGMII?

In TI's J7200 device, there are two CPSW MACs namely CPSW2G and CPSW5G. While
CPSW5G supports QSGMII mode, CPSW2G does not. The same am65-cpsw-nuss driver is
used to control both CPSW2G and CPSW5G. Thus, the am65_cpsw_nuss_mac_config()
function is called for both CPSW2G and CPSW5G MACs. The SGMII CONTROL Register
is only present for CPSW5G. Thus, the write should only be performed for QSGMII
mode which is supported only by CPSW5G.

Functionally, always writing to the register even if the mode is not QSGMII does
not cause any problems, as long as it is CPSW5G MAC that is being used.

Thinking about it again, I will add a compatible to differentiate CPSW2G ports
from CPSW5G ports in order to make it cleaner, so that the register can always
be written to if the CPSW5G ports are used, irrespective of the interface mode.

Thanks,
Siddharth.