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

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>,
	"David S. Miller" <davem@davemloft.net>,
	Florian Fainelli <florian@openwrt.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	netdev <netdev@vger.kernel.org>,
	devicetree-discuss <devicetree-discuss@lists.ozlabs.org>,
	Lior Amsalem <alior@marvell.com>,
	Gregory Clement <gregory.clement@free-electrons.com>,
	Ezequiel Garcia <ezequiel.garcia@free-electrons.com>,
	Mark Rutland <mark.rutland@arm.com>
Subject: Re: [RFC PATCH 1/3] of: provide a binding for the 'fixed-link' property
Date: Tue, 30 Jul 2013 11:07:05 +0200	[thread overview]
Message-ID: <20130730110705.3262bc3d@skate> (raw)
In-Reply-To: <CACxGe6td6uM0v5w92xPVCaVH4tP4AJ-h4SnN-mFoz5gSSosnQw@mail.gmail.com>

Dear Grant Likely,

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

> > +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?

Thanks,

Thomas Petazzoni
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com