From: Mark Rutland <mark.rutland@arm.com>
To: Grant Likely <grant.likely@secretlab.ca>,
Florian Fainelli <florian@openwrt.org>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
"David S. Miller" <davem@davemloft.net>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Lior Amsalem <alior@marvell.com>,
Sascha Hauer <s.hauer@pengutronix.de>,
Christian Gmeiner <christian.gmeiner@gmail.com>,
Ezequiel Garcia <ezequiel.garcia@free-electrons.com>,
Gregory Clement <gregory.clement@free-electrons.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCHv2 3/4] of: provide a binding for fixed link PHYs
Date: Tue, 12 Nov 2013 16:29:14 +0000 [thread overview]
Message-ID: <20131112162914.GE4237@e106331-lin.cambridge.arm.com> (raw)
In-Reply-To: <20131112123746.EAEF0C42283@trevor.secretlab.ca>
Hi Florian, Grant,
On Tue, Nov 12, 2013 at 12:37:46PM +0000, Grant Likely wrote:
> On Fri, 25 Oct 2013 05:40:57 +0100, Florian Fainelli <florian@openwrt.org> wrote:
> > Le mercredi 18 septembre 2013, 18:11:12 Thomas Petazzoni a écrit :
> > > Dear Grant Likely,
> > >
> > > On Tue, 17 Sep 2013 23:29:23 -0500, Grant Likely wrote:
> > > > I understand what you're trying to do here, but it causes a troublesome
> > > > leakage of implementation detail into the binding, making the whole
> > > > thing look very odd. This binding tries to make a fixed link look
> > > > exactly like a real PHY even to the point of including a phandle to the
> > > > phy. But having a phandle to a node which is *always* a direct child of
> > > > the MAC node is redundant and a rather looney. Yes, doing it that way
> > > > makes it easy for of_phy_find_device() to be transparent for fixed link,
> > > > but that should *not* drive bindings, especially when that makes the
> > > > binding really rather weird.
> > > >
> > > > Second, this new binding doesn't provide anything over and above the
> > > > existing fixed-link binding. It may not be pretty, but it is
> > > > estabilshed.
> > >
> > > Have you followed the past discussions about this patch set? Basically
> > > the *only* feedback I received on RFCv1 is that the fixed-link property
> > > sucks, and everybody (including the known Device Tree binding
> > > maintainer Mark Rutland) suggested to not use the fixed-link mechanism.
> > > See http://article.gmane.org/gmane.linux.network/276932, where Mark
> > > said:
> > >
> > > ""
> > > I'm not sure grouping these values together is the best way of handling
> > > this. It's rather opaque, and inflexible for future extension.
> > > ""
> > >
> > > So, please DT maintainers, tell me what you want. I honestly don't care
> > > whether fixed-link or a separate node is chosen. However, I do care
> > > about being dragged around between two solutions just because the
> > > former DT maintainer and the new DT maintainers do not agree.
>
> I've been sleepy on this issue, which limits how much I can push. I'll
> say one more thing on the issue (below) and then leave the decision up
> to Mark. I trust him and he knows what he is doing.
>
> > Since I would like to move forward so I can one day use that binding in a
> > real-life product, I am going to go for Mark's side which happens to be how I
> > want the binding to look like.
> >
> > Do we all agree that the new binding is just way better than the old one? In
> > light of the recent unstable DT ABI discussion, we can still continue parsing
> > the old one for the sake of compatibility.
>
> Regardless of what you want it to look like, does the old binding work
> for your purposes? If yes then use it. The only valid reason for
> creating a new binding is if the old one doesn't work for a specific
> (not theoretical) use case.
I think the issue here was that I am not versed in the history of all of
the existing bindings. While I'm not keen on the existing fixed-link
property and I think it should be done differently were it being created
from scratch today, as Grant has pointed out we're already supporting it
today, and adding a new binding is going to make the code handling it
more complex.
If fixed-link works for your use case today, then use fixed-link.
If we have a valid reason to create a new binding, we should. At the
moment I think the only egregious portion of the binding is the globally
unique fake PHY id, and if that causes issues we should be able to
assign IDs within Linux ignoring the values in the DT, or reorganise
things such that the arbitrary ID doesn't matter.
If there are configurations we need to support that the fixed-link
property cannot encode, then I think we should go ahead with the binding
style that you are proposing today. However, we don't need to go with it
right away, and we can continue to support fixed-link regardless.
I apologise for the lack of consistency here, and I'm sorry that I've
delayed this series for so long.
Thanks,
Mark.
next prev parent reply other threads:[~2013-11-12 16:29 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-06 15:18 [RFC PATCHv2 0/4] Add DT support for fixed PHYs Thomas Petazzoni
2013-09-06 15:18 ` [RFC PATCHv2 1/4] net: phy: decouple PHY id and PHY address in fixed PHY driver Thomas Petazzoni
2013-09-06 15:18 ` [RFC PATCHv2 2/4] net: phy: extend fixed driver with fixed_phy_register() Thomas Petazzoni
2013-09-06 15:18 ` [RFC PATCHv2 3/4] of: provide a binding for fixed link PHYs Thomas Petazzoni
[not found] ` <1378480701-12908-4-git-send-email-thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2013-09-18 4:29 ` Grant Likely
[not found] ` <20130918042923.5D845C42CF7-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2013-09-18 9:21 ` Florian Fainelli
[not found] ` <CAGVrzcbVTenhVC4tQznJFqVpO08ruxLyy1ZiLmw6Bu1=3zbGZg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-09-18 15:00 ` Grant Likely
[not found] ` <20130918150031.D9034C42CDF-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2013-09-19 6:36 ` Sascha Hauer
2013-09-18 16:11 ` Thomas Petazzoni
2013-10-25 4:40 ` Florian Fainelli
2013-11-12 12:37 ` Grant Likely
2013-11-12 16:29 ` Mark Rutland [this message]
2013-11-12 17:40 ` Florian Fainelli
2013-11-12 1:43 ` Florian Fainelli
2013-09-06 15:18 ` [RFC PATCHv2 4/4] net: mvneta: add support for fixed links Thomas Petazzoni
2013-09-06 20:42 ` [RFC PATCHv2 0/4] Add DT support for fixed PHYs Florian Fainelli
2013-09-07 10:27 ` Thomas Petazzoni
2013-09-11 6:42 ` Sascha Hauer
[not found] ` <20130911064248.GI30088-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-09-25 7:12 ` Christian Gmeiner
[not found] ` <CAH9NwWfBGHmZ+HfUndeh18NW+HyZ=c82W=O_4hJSOH-oZuM9jA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-02-10 10:30 ` Christian Gmeiner
2014-02-10 12:09 ` 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=20131112162914.GE4237@e106331-lin.cambridge.arm.com \
--to=mark.rutland@arm.com \
--cc=alior@marvell.com \
--cc=christian.gmeiner@gmail.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=ezequiel.garcia@free-electrons.com \
--cc=florian@openwrt.org \
--cc=grant.likely@secretlab.ca \
--cc=gregory.clement@free-electrons.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=netdev@vger.kernel.org \
--cc=s.hauer@pengutronix.de \
--cc=thomas.petazzoni@free-electrons.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).