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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGVrzcZC9G2ny5rtojbzxmPu8-6EKQ_3io-+fxi61LLCt4Hk=w@mail.gmail.com>
Date:	Tue, 30 Jul 2013 11:05:04 +0100
From:	Florian Fainelli <florian@...nwrt.org>
To:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
Cc:	Grant Likely <grant.likely@...retlab.ca>,
	Rob Herring <rob.herring@...xeda.com>,
	"David S. Miller" <davem@...emloft.net>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	netdev <netdev@...r.kernel.org>,
	devicetree-discuss <devicetree-discuss@...ts.ozlabs.org>,
	Lior Amsalem <alior@...vell.com>,
	Gregory Clement <gregory.clement@...e-electrons.com>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	Mark Rutland <mark.rutland@....com>
Subject: Re: [RFC PATCH 1/3] of: provide a binding for the 'fixed-link' property

Hello Thomas,

2013/7/30 Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>:
> 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
--
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