Messages in this thread | | | Subject | Re: [RFC 01/11] net: phy: Don't report success if devices weren't found | From | Jeremy Linton <> | Date | Sun, 24 May 2020 21:46:55 -0500 |
| |
Hi,
Thanks for taking a look at this.
On 5/23/20 1:20 PM, Russell King - ARM Linux admin wrote: > On Fri, May 22, 2020 at 04:30:49PM -0500, Jeremy Linton wrote: >> C45 devices are to return 0 for registers they haven't >> implemented. This means in theory we can terminate the >> device search loop without finding any MMDs. In that >> case we want to immediately return indicating that >> nothing was found rather than continuing to probe >> and falling into the success state at the bottom. > > This is a little confusing when you read this comment: > > /* If mostly Fs, there is no device there, > * then let's continue to probe more, as some > * 10G PHYs have zero Devices In package, > * e.g. Cortina CS4315/CS4340 PHY. > */ > > Since it appears to be talking about the case of a PHY where *devs will > be zero. However, tracking down the original submission, it seems this > is not the case at all, and the comment is grossly misleading. > > It seems these PHYs only report a valid data in the Devices In Package > registers for devad=0 - it has nothing to do with a zero Devices In > Package.
Yes, this ended up being my understanding of this commit, and is part of my justification for starting the devices search at the reserved address 0 rather than 1.
> > Can I suggest that this comment is fixed while we're changing the code > to explicitly reject this "zero Devices In package" so that it's not > confusing?
Its probably better to kill it in 5/11 with a mention that we are starting at a reserved address?
OTOH, I'm a bit concerned that reading at 0 as the first address will cause problems because the original code was only triggering it after a read returning 0xFFFFFFFF at a valid MMD address. It does simplify/clarify the loop though. If it weren't for this 0 read, I would have tried to avoid some of the additional MMD reserved addresses.
| |