Re: [net-next PATCH v3 4/5] net: phy: Introduce fwnode_get_phy_id()

[Russell King - ARM Linux admin]

On Mon, May 11, 2020 at 03:59:30PM +0530, Calvin Johnson wrote:
> On Mon, May 11, 2020 at 10:38:49AM +0100, Russell King - ARM Linux admin wrote:
> > On Mon, May 11, 2020 at 01:30:40PM +0530, Calvin Johnson wrote:
> > > On Sat, May 09, 2020 at 01:42:57AM +0200, Andrew Lunn wrote:
> > > > On Fri, May 08, 2020 at 05:48:33PM -0500, Jeremy Linton wrote:
> > > > > Hi,
> > > > > 
> > > > > On 5/8/20 3:27 PM, Andrew Lunn wrote:
> > > > > > > > There is a very small number of devices where the vendor messed up,
> > > > > > > > and did not put valid contents in the ID registers. In such cases, we
> > > > > > > > can read the IDs from device tree. These are then used in exactly the
> > > > > > > > same way as if they were read from the device.
> > > > > > > > 
> > > > > > > 
> > > > > > > Is that the case here?
> > > > > > 
> > > > > > Sorry, I don't understand the question?
> > > > > 
> > > > > I was asking in general, does this machine report the ID's correctly.
> > > > 
> > > > Very likely, it does.
> > > > 
> > > > > The embedded single mac:mdio per nic case seems like the normal case, and
> > > > > most of the existing ACPI described devices are setup that way.
> > > > 
> > > > Somebody in this thread pointed to ACPI patches for the
> > > > MACCHIATOBin. If i remember the hardware correctly, it has 4 Ethernet
> > > > interfaces, and two MDIO bus masters. One of the bus masters can only
> > > > do C22 and the other can only do C45. It is expected that the busses
> > > > are shared, not a nice one to one mapping.
> > > > 
> > > > > But at the same time, that shifts the c22/45 question to the nic
> > > > > driver, where use of a DSD property before instantiating/probing
> > > > > MDIO isn't really a problem if needed.
> > > > 
> > > > This in fact does not help you. The MAC driver has no idea what PHY is
> > > > connected to it. The MAC does not know if it is C22 or C45. It uses
> > > > the phylib abstraction which hides all this. Even if you assume 1:1,
> > > > use phy_find_first(), it will not find a C45 PHY because without
> > > > knowing there is a C45 PHY, we don't scan for it. And we should expect
> > > > C45 PHYs to become more popular in the next few years.
> > > 
> > > Agree.
> > > 
> > > NXP's LX2160ARDB platform currently has the following MDIO-PHY connection.
> > > 
> > > MDIO-1 ==> one 40G PHY, two 1G PHYs(C45), two 10G PHYs(C22)
> > 
> > I'm not entirely sure you have that correct.  The Clause 45 register set
> > as defined by IEEE 802.3 does not define registers for 1G negotiation,
> > unless the PHY either supports Clause 22 accesses, or implements some
> > kind of vendor extension.  For a 1G PHY, this would be wasteful, and
> > likely incompatible with a lot of hardware/software.
> > 
> > Conversely, Clause 22 does not define registers for 10G speeds, except
> > accessing Clause 45 registers indirectly through clause 22 registers,
> > which would also be wasteful.
> > 
> Got your point.
> Let me try to clarify.
> 
> MDIO-1 ==> one 40G PHY, two 1G PHYs(C45), two 10G PHYs(C22)
> MDIO-2 ==> one 25G PHY
> This is the physical connection of MDIO & PHYs on the platform.
> 
> For the c45 PHYs(two 10G), we use compatible "ethernet-phy-ieee802.3-c45"(not
> yet upstreamed).
> For c22 PHYs(two 1G), we don't mention the c45 compatible string and hence the
> access also will be using c22, if I'm not wrong.

You seem to have just repeated the same mistake (it seems to be a direct
copy-n-paste of what you sent in the email I replied to) - and then gone
on to say something different.  Either you're confused or you're not
writing in your email what you intend to.

You first say "MDIO-1 ==> two 1G PHYs(C45)".  You then say lower down
"For C22 PHYs (two 1G)".  Both these statements can't be true.

Similarly, you first say "MDIO-1 ==> two 10G PHYs(C22)".  You then say
lower down "For the c45 PHYs(two 10G)".  Again, both these statements
can't be true.

Given that this discussion in this thread has been about C22 vs C45, I
would have thought accuracy in regard to this point would have been of
the up-most importance.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up