From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933334AbbGJVOb (ORCPT ); Fri, 10 Jul 2015 17:14:31 -0400 Received: from smtp3.mail.ru ([94.100.179.58]:42932 "EHLO smtp3.mail.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754354AbbGJVOY (ORCPT ); Fri, 10 Jul 2015 17:14:24 -0400 Subject: Re: [PATCH 1/3] fixed_phy: handle link-down case To: Florian Fainelli , netdev References: <559FF511.5080102@list.ru> <559FF5A3.2030607@list.ru> <55A02E90.1090602@gmail.com> Cc: Linux kernel , Sebastien Rannou , Arnaud Ebalard , Stas Sergeev From: Stas Sergeev Message-ID: <55A035A2.2040404@list.ru> Date: Sat, 11 Jul 2015 00:14:10 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <55A02E90.1090602@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed 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 10.07.2015 23:44, Florian Fainelli пишет: > On 10/07/15 09:41, Stas Sergeev wrote: >> Currently fixed_phy driver recognizes only the link-up state. >> This simple patch adds an implementation of link-down state. >> The actual change is 1-line, the rest is an indentation. > It is not clear to me how this is useful, if you have a link_update > callback manipulating the link state, the fixed PHY driver returns > appropriate MII_BMSR values and always re-initializes everything. It returns the appropriate values only for link status (when its down), but it still returns speed, duplex etc as if the link is up. I had hard times finding the relevant specs, but from what I have googled, when link is down, the speed/duplex/etc status fields should _also_ be zero, which is what my patch does. What is more important is that fixed_phy_add() would return -EINVAL if you didn't specify speed while the link is down. This is an absolute must-fix, or I will have to add an arbitrary speed value again, on which you already objected. > Is this meant to be some sort of optimization? If so, you could just > avoid the re-intendation completely and do a goto instead? Oh, c'mon... Adding goto just to keep the _patch_ smaller? (not smaller code, just a smaller patch) Well, this is certainly something that can be done, feel free to request that explicitly and I'll release v3 next week.