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=-2.4 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 B3CD1C433E0 for ; Wed, 17 Jun 2020 11:22:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 85EFD208B3 for ; Wed, 17 Jun 2020 11:22:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=intenta.de header.i=@intenta.de header.b="OYwsva40" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726591AbgFQLV7 (ORCPT ); Wed, 17 Jun 2020 07:21:59 -0400 Received: from mail.intenta.de ([178.249.25.132]:42895 "EHLO mail.intenta.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725894AbgFQLV6 (ORCPT ); Wed, 17 Jun 2020 07:21:58 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=intenta.de; s=dkim1; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:CC:To:From:Date; bh=fMj+rxRoQetuNQLrf/xuaRctE+S79uoQja9i6d5hznI=; b=OYwsva40BBclxE9twZZr9EjpDYzj6P0j9O2emYLldjN/j1Kq3ROTcv4DvJZZ/hoFf9+3VA1KM10Mtcfwrmrk0rpp59RT8llH+Oj5+ipIoCZ0DruJEHMUKwwKOqlulmAf2SwptXCTKjbMNUwglOxYrjGYBlrB4gCbnotfTY6yCVgpG5FBW/DfNPNG9ffk+HS0zO9gVZMDDPHyMweQZj0YQBFp8zGikc70luDbage7G4zKrc8wi29y1s59SZnOqApIiAK1h16/Thti4ee3V8C8Dcz5ve6UkqzHawHQS8HVbFrtLFrs8jK9XBJxTYLxTdUWqNG9jiQdjUNyHOOEABbQ1g==; Date: Wed, 17 Jun 2020 13:21:53 +0200 From: Helmut Grohne To: Russell King - ARM Linux admin CC: Nicolas Ferre , "David S. Miller" , Jakub Kicinski , Palmer Dabbelt , Paul Walmsley , "netdev@vger.kernel.org" Subject: Re: [PATCH] net: macb: reject unsupported rgmii delays Message-ID: <20200617112153.GB28783@laureti-dev> References: <20200616074955.GA9092@laureti-dev> <20200617105518.GO1551@shell.armlinux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20200617105518.GO1551@shell.armlinux.org.uk> User-Agent: Mutt/1.10.1 (2018-07-13) X-ClientProxiedBy: ICSMA002.intenta.de (10.10.16.48) To ICSMA002.intenta.de (10.10.16.48) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, Jun 17, 2020 at 12:55:18PM +0200, Russell King - ARM Linux admin wrote: > The individual RGMII delay modes are more about what the PHY itself is > asked to do with respect to inserting delays, so I don't think your > patch makes sense. This seems to be the same aspect that Vladimir Oltean remarked. I agree that the relevant hunk should be dropped. > > --- a/drivers/net/ethernet/cadence/macb_main.c > > +++ b/drivers/net/ethernet/cadence/macb_main.c > > @@ -514,7 +514,7 @@ static void macb_validate(struct phylink_config *config, > > state->interface != PHY_INTERFACE_MODE_RMII && > > state->interface != PHY_INTERFACE_MODE_GMII && > > state->interface != PHY_INTERFACE_MODE_SGMII && > > - !phy_interface_mode_is_rgmii(state->interface)) { > > + state->interface != PHY_INTERFACE_MODE_RGMII_ID) { > > Here you reject everything except PHY_INTERFACE_MODE_RGMII_ID. > > > bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS); > > return; > > } > > @@ -694,6 +694,13 @@ static int macb_phylink_connect(struct macb *bp) > > struct phy_device *phydev; > > int ret; > > > > + if (of_phy_is_fixed_link(dn) && > > + phy_interface_mode_is_rgmii(bp->phy_interface) && > > + bp->phy_interface != PHY_INTERFACE_MODE_RGMII) { > > but here you reject everything except PHY_INTERFACE_MODE_RGMII. These > can't both be right. If you start with PHY_INTERFACE_MODE_RGMII, and > have a fixed link, you'll have PHY_INTERFACE_MODE_RGMII passed into > the validate function, which will then fail. For a fixed-link, the validation function is never called. Therefore, it cannot reject PHY_INTERFACE_MODE_RGMII. It works in practice. However, the consensus is to not reject that mode in the validation function. Helmut