Re: [PATCH net-next 1/3] net: phy: dp83867: Add SQI support

[João Rodrigues]

On Thu, 13 Jun 2024 19:13:27 +0200
Andrew Lunn <andrew@xxxxxxx> wrote:

> On Thu, Jun 13, 2024 at 04:51:51PM +0200, João Rodrigues wrote:
> > Don't report SQI values for 10 ethernet, since the datasheet
> > says MSE values are only valid for 100/1000 ethernet  
> 
> The commit message could be better. Something like:
> 
> Don't report the SQI value when the link speed is 10Mbps, since the
> datasheet says MSE values are only valid for 100/1000 links.
> 

Thank you, I will use your wording on the next version.

> > +static int dp83867_get_sqi(struct phy_device *phydev)
> > +{
> > +	u16 mse_val;
> > +	int sqi;
> > +	int ret;
> > +
> > +	if (phydev->speed == SPEED_10)
> > +		return -EOPNOTSUPP;  
> 
> What does the datasheet say about MSE where there is no link at all?
> Maybe you need to expand this test to include SPEED_UNKNOWN?
> 
The datasheet does not have any information regarding this register
(or the related 0x265, 0x2A5 and 0x2E5, for the other pairs).
The information from these registers come from the "DP83867
Troubleshooting Guide (Rev. B)".

I will add the additional check for SPEED_UNKNOWN in the next
version.

João