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.

Reply via email to