All of lore.kernel.org
 help / color / mirror / Atom feed
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 1/3] of: provide a binding for the 'fixed-link' property
Date: Mon, 12 Aug 2013 10:16:49 +0200	[thread overview]
Message-ID: <20130812101649.743c08aa@skate> (raw)
In-Reply-To: <20130812063806.GD2324@pengutronix.de>

Dear Sascha Hauer,

On Mon, 12 Aug 2013 08:38:06 +0200, Sascha Hauer wrote:

> > This patch adds:
> > 
> >  * A documentation for the Device Tree property "fixed-link".
> > 
> >  * A of_phy_register_fixed_link() OF helper, which provided an OF node
> >    that contains a "fixed-link" property, registers the corresponding
> >    fixed PHY.
> > 
> >  * Removes the warning on the of_phy_connect_fixed_link() that says
> >    new drivers should not use it, since Grant Likely indicated that
> >    this "fixed-link" property is indeed the way to go.
> > 
> 
> Any progress with this series?

I am not sure there really was a consensus yet on what the DT binding
looks like. As soon as there is a consensus, I'm definitely willing to
make progress on this series.

> We have more and more boards here with exactly the same problem as
> Thomas has. For reasons stated below I don't like this binding, but
> still it would solve my problem.

Ok.

> > +Example:
> > +
> > +ethernet at 0 {
> > +	...
> > +	fixed-link = <1 1 1000 0 0>;
> > +	...
> > +};
> 
> I must say I don't like this binding at all for two reasons.

As I explained, this binding was chosen for this RFC for two reasons:

 * It's the binding used on PowerPC platforms to represent fixed links.
 * It allows to encode all the informations into a single property,
   which avoids the need for a separate DT node for a "fake PHY", which
   isn't a representation of the hardware.

> First the positional arguments make it impossible to add optional
> arguments to the link.
> 
> Second the other side of the link is most likely a switch. Once this
> switch has its own node in the devicetree it seems like having a phandle
> to the switch here would be better.

So, in other words, what you're suggesting is something like:

	ethernet at 0 {
		reg = <...>;
		interrupt = <...>;
		phy = <&phy0>;
		phy0: phy at 0 {
			fixed-link;
			speed = <1000>;
			full-duplex;
			...
		};
	};

Or something else?

Best regards,

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

WARNING: multiple messages have this Message-ID (diff)
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Grant Likely <grant.likely@secretlab.ca>,
	Rob Herring <rob.herring@calxeda.com>,
	"David S. Miller" <davem@davemloft.net>,
	Florian Fainelli <florian@openwrt.org>,
	Lior Amsalem <alior@marvell.com>,
	netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 1/3] of: provide a binding for the 'fixed-link' property
Date: Mon, 12 Aug 2013 10:16:49 +0200	[thread overview]
Message-ID: <20130812101649.743c08aa@skate> (raw)
In-Reply-To: <20130812063806.GD2324@pengutronix.de>

Dear Sascha Hauer,

On Mon, 12 Aug 2013 08:38:06 +0200, Sascha Hauer wrote:

> > This patch adds:
> > 
> >  * A documentation for the Device Tree property "fixed-link".
> > 
> >  * A of_phy_register_fixed_link() OF helper, which provided an OF node
> >    that contains a "fixed-link" property, registers the corresponding
> >    fixed PHY.
> > 
> >  * Removes the warning on the of_phy_connect_fixed_link() that says
> >    new drivers should not use it, since Grant Likely indicated that
> >    this "fixed-link" property is indeed the way to go.
> > 
> 
> Any progress with this series?

I am not sure there really was a consensus yet on what the DT binding
looks like. As soon as there is a consensus, I'm definitely willing to
make progress on this series.

> We have more and more boards here with exactly the same problem as
> Thomas has. For reasons stated below I don't like this binding, but
> still it would solve my problem.

Ok.

> > +Example:
> > +
> > +ethernet@0 {
> > +	...
> > +	fixed-link = <1 1 1000 0 0>;
> > +	...
> > +};
> 
> I must say I don't like this binding at all for two reasons.

As I explained, this binding was chosen for this RFC for two reasons:

 * It's the binding used on PowerPC platforms to represent fixed links.
 * It allows to encode all the informations into a single property,
   which avoids the need for a separate DT node for a "fake PHY", which
   isn't a representation of the hardware.

> First the positional arguments make it impossible to add optional
> arguments to the link.
> 
> Second the other side of the link is most likely a switch. Once this
> switch has its own node in the devicetree it seems like having a phandle
> to the switch here would be better.

So, in other words, what you're suggesting is something like:

	ethernet@0 {
		reg = <...>;
		interrupt = <...>;
		phy = <&phy0>;
		phy0: phy@0 {
			fixed-link;
			speed = <1000>;
			full-duplex;
			...
		};
	};

Or something else?

Best regards,

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

  reply	other threads:[~2013-08-12  8:16 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-15 15:34 [RFC PATCH 0/3] Add DT support for fixed PHYs Thomas Petazzoni
2013-07-15 15:34 ` Thomas Petazzoni
2013-07-15 15:34 ` [RFC PATCH 1/3] of: provide a binding for the 'fixed-link' property Thomas Petazzoni
2013-07-15 15:34   ` Thomas Petazzoni
2013-07-15 15:50   ` Florian Fainelli
2013-07-15 15:50     ` Florian Fainelli
2013-07-23 11:22   ` Mark Rutland
2013-07-23 11:22     ` Mark Rutland
2013-07-23 11:39     ` Florian Fainelli
2013-07-23 11:39       ` Florian Fainelli
2013-07-30  9:16     ` Thomas Petazzoni
2013-07-30  9:16       ` Thomas Petazzoni
2013-07-30 10:26       ` Florian Fainelli
2013-07-30 10:26         ` Florian Fainelli
2013-07-30 15:28       ` Mark Rutland
2013-07-30 15:28         ` Mark Rutland
2013-07-23 11:39   ` Grant Likely
2013-07-23 11:39     ` Grant Likely
2013-07-30  9:07     ` Thomas Petazzoni
2013-07-30  9:07       ` Thomas Petazzoni
2013-07-30 10:05       ` Florian Fainelli
2013-07-30 10:05         ` Florian Fainelli
2013-07-30 11:23         ` Thomas Petazzoni
2013-07-30 11:23           ` Thomas Petazzoni
2013-07-30 11:43           ` Florian Fainelli
2013-07-30 11:43             ` Florian Fainelli
2013-07-30 15:31       ` Mark Rutland
2013-07-30 15:31         ` Mark Rutland
2013-08-12  6:38   ` Sascha Hauer
2013-08-12  6:38     ` Sascha Hauer
2013-08-12  8:16     ` Thomas Petazzoni [this message]
2013-08-12  8:16       ` Thomas Petazzoni
2013-08-12  8:37       ` Sascha Hauer
2013-08-12  8:37         ` Sascha Hauer
2013-08-21 10:55         ` Christian Gmeiner
2013-08-21 10:55           ` Christian Gmeiner
2013-08-21 11:25           ` Florian Fainelli
2013-08-21 11:25             ` Florian Fainelli
2013-08-21 11:46         ` Florian Fainelli
2013-08-21 11:46           ` Florian Fainelli
2013-07-15 15:34 ` [RFC PATCH 2/3] net: phy: call mdiobus_scan() after adding a fixed PHY Thomas Petazzoni
2013-07-15 15:34   ` Thomas Petazzoni
2013-07-15 15:46   ` Florian Fainelli
2013-07-15 15:46     ` Florian Fainelli
2013-07-15 15:34 ` [RFC PATCH 3/3] net: mvneta: add support for fixed links Thomas Petazzoni
2013-07-15 15:34   ` Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130812101649.743c08aa@skate \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.