From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754093AbbCaRLu (ORCPT ); Tue, 31 Mar 2015 13:11:50 -0400 Received: from smtp8.mail.ru ([94.100.181.96]:54840 "EHLO smtp8.mail.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752679AbbCaRLr (ORCPT ); Tue, 31 Mar 2015 13:11:47 -0400 Message-ID: <551AD543.1010906@list.ru> Date: Tue, 31 Mar 2015 20:11:31 +0300 From: Stas Sergeev User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Florian Fainelli CC: netdev , Linux kernel , Stas Sergeev , Grant Likely , Rob Herring , "devicetree@vger.kernel.org" , Thomas Petazzoni , Andrew Lunn Subject: Re: [PATCH 4/6] of: add API for changing parameters of fixed link References: <55155AFC.4050800@list.ru> <55155D35.1070703@list.ru> <5515803F.3020600@list.ru> <551587D1.5070408@list.ru> <5515904A.1060600@gmail.com> <55196011.7080502@list.ru> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam: Not detected X-Mras: Ok Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 30.03.2015 19:06, Florian Fainelli пишет: > 2015-03-30 7:39 GMT-07:00 Stas Sergeev : >> 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.