Date:	Wed, 5 Mar 2014 10:24:14 +0100
From:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
To:	Florian Fainelli <f.fainelli@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Lior Amsalem <alior@...vell.com>,
	Gregory Clement <gregory.clement@...e-electrons.com>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Mark Rutland <mark.rutland@....com>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Christian Gmeiner <christian.gmeiner@...il.com>,
	Grant Likely <grant.likely@...aro.org>
Subject: Re: [PATCHv3 3/4] of: provide a binding for fixed link PHYs

Dear Florian Fainelli,

On Tue, 4 Mar 2014 12:58:41 -0800, Florian Fainelli wrote:


> What I like about this new binding is that we could place the
> 'fixed-link' related properties in e.g: a SPI slave node, and have the
> Ethernet MAC be pointed at it by a phandle to tell it: look this is
> your PHY, it might not be one you could address on a MDIO bus, so I
> have been providing additional properties to help you with the link
> configuration.
> 
> One thing that needs to be addressed in this patch is how to deal with
> the existing 5-digit fixed-link, something that sounds fairly easy and
> which would not require changing the callers of of_phy_connect_fixed()
> is to do the following:

I am not sure to understand "would not require changing the callers of
of_phy_connect_fixed()". This function is precisely introduced by the
patch set, so how would we need to "change the callers" ? Maybe you're
making a confusion with the existing of_phy_connect_fixed_link(), which
is used by network drivers to create a PHY using the old-style
fixed-link = <5 digits> binding ?

> - of_phy_is_fixed_link() needs to look for *all* required compatible
> properties of the new binding to give an accurate verdict on the
> nature of the PHY (to avoid false positives as mentioned in PATCH 4),

Hum?

The false positive problem only exists if you want to automatically
instantiate the fixed PHYs, as I proposed in a patch as a reply to my
series. And checking for *all* required properties does not make the
problem better: you could very well have other nodes in the tree that
have a "fixed-link" and a "speed" property, for example.

> and it also needs to look for the 5-digit fixed-link property and
> ensure the property is 5-digits long if existing

I don't understand how this could work. The of_phy_is_fixed_link()
function is meant to take as argument a Device Tree node that describes
a fixed PHY, using the new proposed DT binding for fixed PHYs.

The old 'fixed-link' binding has the fixed-link property as part of the
Ethernet node itself.

So I don't really see how a sane function could check both.

> - of_phy_register_fixed_link() needs to also parse the old 5-digit
> fixed-link property, most likely just copy-pasting what
> arch/powerpc/sysdev/fsl_soc.c::of_add_fixed_phys does with the
> property endian-swapping (as this code is for PowerPC)
> 
> Then we can deal with how to make that semi-automatic for the new
> binding users to make it smoother to use a regular or "fixed PHY"
> device.

I still don't understand. With the old binding, the "fixed-link"
property is within some random Ethernet node, and there is no way for
us to find out whether a given node having a "fixed-link" property
corresponds to a fixed PHY, or something completely unrelated.

So to conclude, I'm sorry, but I didn't understand at all what you
meant to say here, so I'm completely puzzled about what your
suggestions are.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists