netdev - Re: [PATCH 2/3] of_mdio: add new DT property 'managed' to specify the PHY management type

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] [day] [month] [year] [list]

Message-ID: <55A57062.80703@list.ru>
Date:	Tue, 14 Jul 2015 23:26:10 +0300
From:	Stas Sergeev <stsp@...t.ru>
To:	Florian Fainelli <f.fainelli@...il.com>,
	netdev <netdev@...r.kernel.org>
Cc:	Linux kernel <linux-kernel@...r.kernel.org>,
	Sebastien Rannou <mxs@...k.org>,
	Arnaud Ebalard <arno@...isbad.org>,
	Stas Sergeev <stsp@...rs.sourceforge.net>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Grant Likely <grant.likely@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 2/3] of_mdio: add new DT property 'managed' to specify the
 PHY management type

14.07.2015 20:51, Florian Fainelli пишет:
> On 14/07/15 10:13, Stas Sergeev wrote:
>> Currently the PHY management type is selected by the MAC driver arbitrary.
>> The decision is based on the presence of the "fixed-link" node and on a
>> will of the driver's authors.
>> This caused a regression recently, when mvneta driver suddenly started
>> to use the in-band status for auto-negotiation on fixed links.
>> It appears the auto-negotiation may not work when expected by the MAC driver.
>> Sebastien Rannou explains:
>> << Yes, I confirm that my HW does not generate an in-band status. AFAIK, it's
>> a PHY that aggregates 4xSGMIIs to 1xQSGMII ; the MAC side of the PHY (with
>> inband status) is connected to the switch through QSGMII, and in this context
>> we are on the media side of the PHY. >>
>> https://lkml.org/lkml/2015/7/10/206
>>
>> This patch introduces the new string property 'managed' that allows
>> the user to set the management type explicitly.
>> The supported values are:
>> "auto" - default. Uses either MDIO or nothing, depending on the presence
>> of the fixed-link node
>> "mdio" - use MDIO
>> "in-band-status" - use in-band status
>> "none" - use fixed-link description
> I thought we agreed in the last thread that "mdio" was implied whenever
> a proper PHY phandle to a device sitting on MDIO bus is used and that
> "auto" did not make much sense unless we were also describing how to do
> this auto-negotiation completely?
Exactly not:
---
> If we would implement autoneg outside of the fixed-link,
> then its semantic would likely be
> autoneg = "mdio" | "in-band" | "off"

Right, if auto-negotiation was defined outside of fixed-link, that is
indeed how I would also specify this.
---

That's why I implemented it the way you see.
But as you changed your mind, I'll remove "mdio".

> As mentioned below, "mdio" is redundant with finding a "phy-handle", 
> and "auto" is not specific enough to be useful to a consumer of this 
> information. 
I prefer to keep "auto", as otherwise we'll have the default
value (when the option is not specified) never achievable
when the option _is_ specified, which is probably not very
good. But I can remove "none" instead, leaving only
"in-band-status" and "auto". Ok?
Of course one can propose to completely ignore that option
in presence of MDIO, but I wonder why not to allow for
example to opt for in-band status even if you have MDIO?
So if we want this option to play nicely with MDIO, as opposed
to being entirely overridden, then "auto" still makes sense IMO.
--
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