Re: [RFC PATCH 1/3] of: provide a binding for the 'fixed-link' property

On Tue, Jul 30, 2013 at 10:07:05AM +0100, Thomas Petazzoni wrote:

Dear Grant Likely,

On Tue, 23 Jul 2013 12:39:52 +0100, Grant Likely wrote:

quoted

quoted

+Such a fixed link situation is described within an Ethernet device
+Device Tree node using a 'fixed-link' property, composed of 5
+elements:
+
+ 1. A fake PHY ID, which must be unique accross all fixed-link PHYs in
+    the system.

That's just loony!  :)  Regardless of existing code doing this, it is
absolutely ridiculous to have it in the driver. The kernel should
handle generating a phy id transparently. I'd rather mark this field
as reserved in the binding and change the code to not care about it
anymore.

In fact, this value is used for two things:

 * As the PHY address on the fake "fixed" MDIO bus.

 * As the PHY identifier, as reported by the MII registers PHYS_ID1
   (0x2) and PHYS_ID2 (0x3).

I think this doesn't make sense, because the two things are completely
unrelated. Ideally, we'd like the PHY identifier for fixed PHYs to be
something fixed, identical for all fixed PHYs. The problem is finding
an OUI and device number that is available for that, but maybe we can
ask the OpenMoko people to allocate one (see
http://wiki.openmoko.org/wiki/OUI).

Then, the PHY address could be generated dynamically. This would
require:

 * Adding a fixed_phy_create() function that internally uses
   fixed_phy_add(), but before that creates an unique PHY address for
   this newly created PHY. Those unique addresses will be generated by
   incrementing a global number of fixed PHYs, up to PHY_MAX_ADDR,
   which is the maximum number of fixed PHYs that can anyway be
   registered on the fixed MDIO bus.

   fixed_phy_create() would return this PHY address (positive) on
   success, or a negative error code on failure.

 * Change of_phy_register_fixed_link() to call fixed_phy_create()
   instead of fixed_phy_add() and make it return the PHY address
   allocated by fixed_phy_create().

 * Add a of_phy_connect_fixed_link_direct() that is similar to
   of_phy_connect_fixed_link() but takes an additional PHY address as
   argument and uses that to generate the 'bus_id' used to find the
   phy_device.

Grant, Mark, Florian, do you have other proposals?

The above sounds good to me.

Thanks,
Mark.