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

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

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:
> 
> > > +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.