netdev - Re: mvneta: SGMII fixed-link not so fixed

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: <55FC684F.2060808@list.ru>
Date:	Fri, 18 Sep 2015 22:38:55 +0300
From:	Stas Sergeev <stsp@...t.ru>
To:	Florian Fainelli <f.fainelli@...il.com>,
	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	andrew@...n.ch, David Miller <davem@...emloft.net>,
	linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org
Subject: Re: mvneta: SGMII fixed-link not so fixed

18.09.2015 20:30, Florian Fainelli пишет:
> On 18/09/15 10:22, Russell King - ARM Linux wrote:
>> On Fri, Sep 18, 2015 at 07:04:09PM +0300, Stas Sergeev wrote:
>>> 18.09.2015 18:43, Russell King - ARM Linux пишет:
>>>> On Fri, Sep 18, 2015 at 05:45:27PM +0300, Stas Sergeev wrote:
>>>>> AFAICS if it has use_inband_status==true,
>>>>> then it went through of_phy_register_fixed_link(dn),
>>>> That's totally incorrect.  The test for setting use_inband_status in
>>>> mvneta is:
>>>>
>>>>          err = of_property_read_string(dn, "managed", &managed);
>>>>          pp->use_inband_status = (err == 0 &&
>>>>                                   strcmp(managed, "in-band-status") == 0);
>>> Arrrr! I was looking at the branch without the last
>>> patch applied, so it occurred to me as
>>>
>>> 	pp->use_inband_status = (phy_mode == PHY_INTERFACE_MODE_SGMII) &&
>>> 		fixed_phy;
>>>
>>> Sorry for that.
>> Yay :)
>>
>>> So we seem to indeed have a nasty regression with the patch
>>> that just went to stable. :( Great news.
>> Yes.
>>
>>> Thanks for you time.
>>>
>>> I still have problems with this part though:
>>>> If there's neither a MDIO PHY nor a fixed-link, then the network driver
>>>> fails to initialise the device.
>>> I think I am looking into the right source this time, seems like
>>> if we don't have both but still have managed="in-band-status", that
>>> should go the fixed-link path and still work... no?
>> If we have no fixed-link and no phy, then you're correct.
>>
>> However, I really don't like the idea of abusing "fixed-link" as a
>> method to generate an ethtool/miitool/miidiag compatible output for
>> this, but I'm willing to let that pass for the moment. :)
> It is not just for that, you get all the goodies from the PHY library
> without modifying your Ethernet MAC driver specifically for it:
> phy_connect, adjust_link and phy_ethtool_{set,get}.
But you still need to modify the MAC driver, and in a bit
nasty way. Eg when you want the AN to work right, you
need to do of_phy_find_device() and feed the current AN
settings to fixed-phy manually, or, after phy_connect(), you'll
run with wrong link state for some time (and there is still
a bit of race anyway, as the link state could change between
of_phy_find_device() and phy_connect()).
So I am not sure what would the best API look like, but
my overall impression was it is currently not the one.
--
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