From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [RFC PATCH 1/3] of: provide a binding for the 'fixed-link' property Date: Tue, 30 Jul 2013 11:05:04 +0100 Message-ID: References: <1373902450-11857-1-git-send-email-thomas.petazzoni@free-electrons.com> <1373902450-11857-2-git-send-email-thomas.petazzoni@free-electrons.com> <20130730110705.3262bc3d@skate> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: Grant Likely , Rob Herring , "David S. Miller" , "linux-arm-kernel@lists.infradead.org" , netdev , devicetree-discuss , Lior Amsalem , Gregory Clement , Ezequiel Garcia , Mark Rutland To: Thomas Petazzoni Return-path: Received: from mail-pb0-f44.google.com ([209.85.160.44]:43239 "EHLO mail-pb0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758992Ab3G3KFp (ORCPT ); Tue, 30 Jul 2013 06:05:45 -0400 Received: by mail-pb0-f44.google.com with SMTP id wy7so5786662pbc.31 for ; Tue, 30 Jul 2013 03:05:44 -0700 (PDT) In-Reply-To: <20130730110705.3262bc3d@skate> Sender: netdev-owner@vger.kernel.org List-ID: Hello Thomas, 2013/7/30 Thomas Petazzoni : > 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). Right, so I would start with disambiguating the two and just forget about PHYS_ID1 and PHYS_ID2 for the moment since they probably do not need to be per-PHY configurable. > > 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). Well that would be ideal indeed, but I am wondering if we cannot just go with some kind of magic value here anyway, regardless of allocations since this is not a real hardware device. How about the Linux Foundation? Is not that the same problem as with gadget USB devices which should have some sort of real MAC address for instance? > > 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. Even though these are purely software PHY implementations, I believe that some sort of predictability would be welcome, so I would just use the phy_id argument passed to fixed_phy_add() as the address on the fixed MDIO bus like it is today. > > 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? To sum up, let's just forget about the misuse of phy_id to fill in PHYS_ID1 and PHYS_ID2 registers and keep the existing code with a clear note that phy_id means the "PHY address on the fixed MDIO bus". -- Florian