On Mon, May 25, 2020 at 06:16:18PM -0500, Jeremy Linton wrote: > Hi, > > On 5/25/20 5:01 PM, Russell King - ARM Linux admin wrote: > > On Mon, May 25, 2020 at 04:51:16PM -0500, Jeremy Linton wrote: > > > So, my goals here have been to first, not break anything, and then do a > > > slightly better job finding phy's that are (mostly?) responding correctly > > > to > > > the 802.3 spec. So we can say "if you hardware is ACPI conformant, and you > > > have IEEE conformant phy's you should be ok". So, for your example phy, I > > > guess the immediate answer is "use DT" or "find a conformant phy", or even > > > "abstract it in the firmware and use a mailbox interface". > > > > You haven't understood. The PHY does conform for most of the MMDs, > > but there are a number that do not conform. > > > > Maybe I should clarify. This set is still terminating the search for a valid > MMD device list on the first MMD that responds. It then probes the same ID > registers of the flagged MMDs as before. What has changed is that we search > higher into the MMD address space for a device list. So previously if a > device didn't respond to MMD0-8 it was ignored. Now it needs to fail all of > 0-31 to be ignored. Similarly for the ID's, if we find what appears to be a > valid MMD device list, then we will probe not only the original 1-8 MMDs for > IDs, but 1-31 MMDs for IDs.
Clarification is not required; I understand what you're doing, but you are not understanding my points. For the 88x3310, your change means that the list of IDs for this PHY will not only 0x002b09aX, 0x01410daX (the official IDs), but also 0x00000000 and 0xfffe0000 from MMD 30 and 31 respectively, which are not real IDs. That's two incorrect IDs that should actually not be there. Here's what the first few registers from MMD 30 and 31 look like on this PHY: MMD30: Addr Data 0000 0000 0000 0000 0000 0000 0000 0000 0000 0008 0000 0000 0000 0000 0000 0000 0000 0000 0010 0000 0000 0000 0000 0000 0000 0000 0000 MMD31: 0000 fffe 0000 fffe 0000 fffe 0000 fffe 0000 0008 fffe 0000 fffe 0000 fffe 0000 fffe 0000 0010 fffe 0000 fffe 0000 fffe 0000 fffe 0000 We've got away with it so far on several counts: 1. The code doesn't probe that high for IDs. 2. We have no driver that may match those IDs. You're taking away (1), so now all it takes is for condition (2) to be broken, and we can end up with a regression if another driver appears that may match using those. So, I would suggest that you avoid reading IDs from MMD 30/31, or maybe only read the ID from MMDs > 8 if register 8 indicates that there is a device present at that MMD. That would be compliant with IEEE 802.3, preserve our existing behaviour, while allowing us to expand the IDs that we probe for to have a better chance of only detecting truely valid IDs. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
