From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8C24BC433DB for ; Fri, 22 Jan 2021 22:12:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5C35423B16 for ; Fri, 22 Jan 2021 22:12:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729626AbhAVWMR (ORCPT ); Fri, 22 Jan 2021 17:12:17 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:55214 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729912AbhAVWJT (ORCPT ); Fri, 22 Jan 2021 17:09:19 -0500 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1l34b5-0029Gh-PB; Fri, 22 Jan 2021 23:08:03 +0100 Date: Fri, 22 Jan 2021 23:08:03 +0100 From: Andrew Lunn To: Sergej Bauer Cc: netdev@vger.kernel.org, f.fainelli@gmail.com, "David S. Miller" , Jakub Kicinski , Bryan Whitehead , UNGLinuxDriver@microchip.com, Simon Horman , Mark Einon , Madalin Bucur , Arnd Bergmann , Masahiro Yamada , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] lan743x: add virtual PHY for PHY-less devices Message-ID: References: <20210122214247.6536-1-sbauer@blackbox.su> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210122214247.6536-1-sbauer@blackbox.su> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jan 23, 2021 at 12:42:41AM +0300, Sergej Bauer wrote: > From: sbauer@blackbox.su > > v1->v2: > switch to using of fixed_phy as was suggested by Andrew and Florian > also features-related parts are removed This is not using fixed_phy, at least not in the normal way. Take a look at bgmac_phy_connect_direct() for example. Call fixed_phy_register(), and then phy_connect_direct() with the phydev. End of story. Done. > +int lan743x_set_link_ksettings(struct net_device *netdev, > + const struct ethtool_link_ksettings *cmd) > +{ > + if (!netdev->phydev) > + return -ENETDOWN; > + > + return phy_is_pseudo_fixed_link(netdev->phydev) ? > + lan743x_set_virtual_link_ksettings(netdev, cmd) > + : phy_ethtool_set_link_ksettings(netdev, cmd); > +} There should not be any need to do something different. The whole point of fixed_phy is it looks like a PHY. So calling phy_ethtool_set_link_ksettings() should work, nothing special needed. > @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct lan743x_adapter *adapter) > struct net_device *netdev = adapter->netdev; > > phy_stop(netdev->phydev); > - phy_disconnect(netdev->phydev); > - netdev->phydev = NULL; > + if (phy_is_pseudo_fixed_link(netdev->phydev)) > + lan743x_virtual_phy_disconnect(netdev->phydev); > + else > + phy_disconnect(netdev->phydev); phy_disconnect() should work. You might want to call fixed_phy_unregister() afterwards, so you do not leak memory. > + if (phy_is_pseudo_fixed_link(phydev)) { > + ret = phy_connect_direct(netdev, phydev, > + lan743x_virtual_phy_status_change, > + PHY_INTERFACE_MODE_MII); > + } else { > + ret = phy_connect_direct(netdev, phydev, > + lan743x_phy_link_status_change, There should not be any need for a special link change callback. lan743x_phy_link_status_change() should work fine, the MAC should have no idea it is using a fixed_phy. > + PHY_INTERFACE_MODE_GMII); > + } > + > if (ret) > goto return_error; > } > @@ -1031,6 +1049,15 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter) > /* MAC doesn't support 1000T Half */ > phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT); > > + if (phy_is_pseudo_fixed_link(phydev)) { > + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_TP_BIT); > + linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, > + phydev->supported); > + phy_advertise_supported(phydev); > + } The fixed PHY driver will set these bits depending on the speed it has been configured for. No need to change them. The MAC should also not care if it is TP, AUI, Fibre or smoke signals. Andrew