netdev - Re: [PATCH 4/6] of: add API for changing parameters of fixed link

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]

Date:	Tue, 31 Mar 2015 20:11:31 +0300
From:	Stas Sergeev <stsp@...t.ru>
To:	Florian Fainelli <f.fainelli@...il.com>
CC:	netdev <netdev@...r.kernel.org>,
	Linux kernel <linux-kernel@...r.kernel.org>,
	Stas Sergeev <stsp@...rs.sourceforge.net>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <robh+dt@...nel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH 4/6] of: add API for changing parameters of fixed link

30.03.2015 19:06, Florian Fainelli пишет:
> 2015-03-30 7:39 GMT-07:00 Stas Sergeev <stsp@...t.ru>:
>> 27.03.2015 20:15, Florian Fainelli пишет:
>>> I think your concerns are valid, but I don't think there is going to be
>>> any problem with the approach I suggested because there is a contract
>>> that the fixed PHYs and regular PHYs need to
>> Hello Florian.
>>
>> As promised, today I tried to resurrect my first implementation
>> and do things as you suggested: install the link_update callback
>> for mvneta privately.
>> I feel SGMII setup is very common and deserves the separate API,
>> not the per-driver handling, but in any case, I'd like to show
>> the implementation first, then discuss.
>>
>> Unfortunately, it didn't work quite right.
>> The problem is that mvneta calls phy_disconnect() on .ndo_stop
>> callback. After that, phy->attached_dev becomes NULL, and so the
>> link_update callback gets called with net_dev==NULL! And crashs.
>> Of course I can easily work around that, but IMHO its a bug -
>> the one that actually gets fixed by the patches I posted previously.
> 
> I actually submitted some patches a while ago that allow you to
> unregister the fixed_link_update callback before in case you need to,
> precisely for that. Since this is specific to dealing with a fixed
> PHY, it is the driver responsibility to know that is has registered a
> fixed_link_update callback and then unregister it by passing a NULL
> callback as the new callback.
Today I posted the patch that does exactly that.
But I already see the problem:
---
[   10.535424] mvneta f1030000.ethernet eth1: Link is Up - Unsupported (update phy.c)/Half - flow control off
---

Seems like if I register a callback after of_phy_connect()
(and unregister before disconnect), there is a small window
between connect and setting the callback. If someone at that
time will query phy, he'll get an undefined status (speed).
And before of_phy_connect() the phy_device pointer is not known
to register callback earlier. Of course I can see the possible
work-arounds, but I wonder if something better can be done
than using of_phy_find_device() and related magic right before
of_phy_connect() just for that.
--
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