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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B8C2AC54FB3 for ; Thu, 29 May 2025 09:42:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:To:Subject:CC:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=zC8cmXQe4aeYEc1DYyfOF86iqEoX3EyOXWUE3yN7yUA=; b=Fz6tsdAsvX9GG6k4/ILlyqXD62 TNXk5Kx3KWz2JCW5qNxNv1fg58XLuyKFd7a+PMnxH3C1TtIUmR3AkNtw0LwwVw0EcVIy9yqDNodU5 as8w2iLfk1lFvTdi1TeQKQ6taN4QH6rb7HuH1Jz3fea+oNje04vwKxO1/xea+ufJ5Su0Y/HVYBRfN klB5F8vlSY8BN8RLiWnnjVdMLmgTuHFqRw5RsYGonom6UhFzOT2ASGuxPDwltkg9ue3ZTWtJC+jw/ lbE0vRDP8gPMZSkNqCYG4sCoO596szUjLR7Ol+S/XVXvfH4FuNXib7c6UTvM5vNdFFPGrion6JFHs gRe2zEaQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uKZmd-0000000FRAP-3odB; Thu, 29 May 2025 09:42:43 +0000 Received: from szxga06-in.huawei.com ([45.249.212.32]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uKZgX-0000000FQIh-3X8F for linux-arm-kernel@lists.infradead.org; Thu, 29 May 2025 09:36:27 +0000 Received: from mail.maildlp.com (unknown [172.19.88.234]) by szxga06-in.huawei.com (SkyGuard) with ESMTP id 4b7Lqg0hnZz27hd7; Thu, 29 May 2025 17:37:03 +0800 (CST) Received: from kwepemk100013.china.huawei.com (unknown [7.202.194.61]) by mail.maildlp.com (Postfix) with ESMTPS id E769E1401F0; Thu, 29 May 2025 17:36:12 +0800 (CST) Received: from [10.67.120.192] (10.67.120.192) by kwepemk100013.china.huawei.com (7.202.194.61) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Thu, 29 May 2025 17:36:11 +0800 Message-ID: <5bbf1201-e803-40ea-a081-c25919f5e89d@huawei.com> Date: Thu, 29 May 2025 17:36:11 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird CC: , , , , , Christophe Leroy , Herve Codina , Florian Fainelli , Vladimir Oltean , =?UTF-8?Q?K=C3=B6ry_Maincent?= , Oleksij Rempel , Simon Horman , Romain Gantois , "shenjian15@huawei.com" Subject: Re: [PATCH net-next v5 07/13] net: phy: phy_caps: Allow looking-up link caps based on speed and duplex To: Maxime Chevallier , , Andrew Lunn , Jakub Kicinski , Eric Dumazet , Paolo Abeni , Russell King , Heiner Kallweit References: <20250307173611.129125-1-maxime.chevallier@bootlin.com> <20250307173611.129125-8-maxime.chevallier@bootlin.com> From: Jijie Shao In-Reply-To: <20250307173611.129125-8-maxime.chevallier@bootlin.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.120.192] X-ClientProxiedBy: kwepems200001.china.huawei.com (7.221.188.67) To kwepemk100013.china.huawei.com (7.202.194.61) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250529_023626_192694_B65F8435 X-CRM114-Status: GOOD ( 35.87 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org on 2025/3/8 1:36, Maxime Chevallier wrote: > As the link_caps array is efficient for lookups, > implement a function for speed/duplex lookups that matches a given > mask. This replicates to some extent the phy_lookup_settings() > behaviour, matching full link_capabilities instead of a single linkmode. > > phy.c's phy_santize_settings() and phylink's > phylink_ethtool_ksettings_set() performs such lookup using the > phy_settings table, but are only interested in the actual speed/duplex > that were matched, rathet than the individual linkmode. > > Similar to phy_lookup_settings(), the newly introduced phy_caps_lookup() > will run through the link_caps[] array by descending speed/duplex order. > > If the link_capabilities for a given tuple intersects the > passed linkmodes, we consider that a match. > > Similar to phy_lookup_settings(), we also allow passing an 'exact' > boolean, allowing non-exact match. Here, we MUST always match the > linkmodes mask, but we allow matching on lower speed settings. > > Signed-off-by: Maxime Chevallier > --- > drivers/net/phy/phy-caps.h | 4 ++++ > drivers/net/phy/phy.c | 32 ++++++-------------------- > drivers/net/phy/phy_caps.c | 47 ++++++++++++++++++++++++++++++++++++++ > drivers/net/phy/phylink.c | 17 +++++++------- > 4 files changed, 67 insertions(+), 33 deletions(-) > > diff --git a/drivers/net/phy/phy-caps.h b/drivers/net/phy/phy-caps.h > index 8833798f141f..aef4b54a8429 100644 > --- a/drivers/net/phy/phy-caps.h > +++ b/drivers/net/phy/phy-caps.h > @@ -51,4 +51,8 @@ phy_caps_lookup_by_linkmode(const unsigned long *linkmodes); > const struct link_capabilities * > phy_caps_lookup_by_linkmode_rev(const unsigned long *linkmodes, bool fdx_only); > > +const struct link_capabilities * > +phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported, > + bool exact); > + > #endif /* __PHY_CAPS_H */ > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index 8df37d221fba..562acde89224 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -213,25 +213,6 @@ int phy_aneg_done(struct phy_device *phydev) > } > EXPORT_SYMBOL(phy_aneg_done); > > -/** > - * phy_find_valid - find a PHY setting that matches the requested parameters > - * @speed: desired speed > - * @duplex: desired duplex > - * @supported: mask of supported link modes > - * > - * Locate a supported phy setting that is, in priority order: > - * - an exact match for the specified speed and duplex mode > - * - a match for the specified speed, or slower speed > - * - the slowest supported speed > - * Returns the matched phy_setting entry, or %NULL if no supported phy > - * settings were found. > - */ > -static const struct phy_setting * > -phy_find_valid(int speed, int duplex, unsigned long *supported) > -{ > - return phy_lookup_setting(speed, duplex, supported, false); > -} > - > /** > * phy_supported_speeds - return all speeds currently supported by a phy device > * @phy: The phy device to return supported speeds of. > @@ -274,13 +255,14 @@ EXPORT_SYMBOL(phy_check_valid); > */ > static void phy_sanitize_settings(struct phy_device *phydev) > { > - const struct phy_setting *setting; > + const struct link_capabilities *c; > + > + c = phy_caps_lookup(phydev->speed, phydev->duplex, phydev->supported, > + false); > > - setting = phy_find_valid(phydev->speed, phydev->duplex, > - phydev->supported); > - if (setting) { > - phydev->speed = setting->speed; > - phydev->duplex = setting->duplex; > + if (c) { > + phydev->speed = c->speed; > + phydev->duplex = c->duplex; > } else { > /* We failed to find anything (no supported speeds?) */ > phydev->speed = SPEED_UNKNOWN; > diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c > index c39f38c12ef2..0366feee2912 100644 > --- a/drivers/net/phy/phy_caps.c > +++ b/drivers/net/phy/phy_caps.c > @@ -170,6 +170,53 @@ phy_caps_lookup_by_linkmode_rev(const unsigned long *linkmodes, bool fdx_only) > return NULL; > } > > +/** > + * phy_caps_lookup() - Lookup capabilities by speed/duplex that matches a mask > + * @speed: Speed to match > + * @duplex: Duplex to match > + * @supported: Mask of linkmodes to match > + * @exact: Perform an exact match or not. > + * > + * Lookup a link_capabilities entry that intersect the supported linkmodes mask, > + * and that matches the passed speed and duplex. > + * > + * When @exact is set, an exact match is performed on speed and duplex, meaning > + * that if the linkmodes for the given speed and duplex intersect the supported > + * mask, this capability is returned, otherwise we don't have a match and return > + * NULL. > + * > + * When @exact is not set, we return either an exact match, or matching capabilities > + * at lower speed, or the lowest matching speed, or NULL. > + * > + * Returns: a matched link_capabilities according to the above process, NULL > + * otherwise. > + */ > +const struct link_capabilities * > +phy_caps_lookup(int speed, unsigned int duplex, const unsigned long *supported, > + bool exact) > +{ > + const struct link_capabilities *lcap, *last = NULL; > + > + for_each_link_caps_desc_speed(lcap) { > + if (linkmode_intersects(lcap->linkmodes, supported)) { > + last = lcap; > + /* exact match on speed and duplex*/ > + if (lcap->speed == speed && lcap->duplex == duplex) { > + return lcap; > + } else if (!exact) { > + if (lcap->speed <= speed) > + return lcap; > + } > + } > + } > + > + if (!exact) > + return last; > + > + return NULL; > +} > +EXPORT_SYMBOL_GPL(phy_caps_lookup); > + > /** > * phy_caps_linkmode_max_speed() - Clamp a linkmodes set to a max speed > * @max_speed: Speed limit for the linkmode set > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > index a3f64b6d2d34..cf9f019382ad 100644 > --- a/drivers/net/phy/phylink.c > +++ b/drivers/net/phy/phylink.c > @@ -20,6 +20,7 @@ > #include > #include > > +#include "phy-caps.h" > #include "sfp.h" > #include "swphy.h" > > @@ -2852,8 +2853,8 @@ int phylink_ethtool_ksettings_set(struct phylink *pl, > const struct ethtool_link_ksettings *kset) > { > __ETHTOOL_DECLARE_LINK_MODE_MASK(support); > + const struct link_capabilities *c; > struct phylink_link_state config; > - const struct phy_setting *s; > > ASSERT_RTNL(); > > @@ -2896,23 +2897,23 @@ int phylink_ethtool_ksettings_set(struct phylink *pl, > /* Autonegotiation disabled, select a suitable speed and > * duplex. > */ > - s = phy_lookup_setting(kset->base.speed, kset->base.duplex, > - pl->supported, false); > - if (!s) > + c = phy_caps_lookup(kset->base.speed, kset->base.duplex, > + pl->supported, false); > + if (!c) > return -EINVAL; Hi Maxime, fc81e257d19f ("net: phy: phy_caps: Allow looking-up link caps based on speed and duplex") might have different behavior than the modification. My case is set 10M Half with disable autoneg both sides and I expect it is link in 10M Half. But now, it is link in 10M Full，which is not what I expect. I used followed command and trace how phy worked. ethtool -s eth1 autoneg off speed 10 duplex half The log is showed as followed: ethtool-13127 [067] 6164.771853: phy_ethtool_ksettings set: (phy_ethtool ksettings set+0x0/0x200) duplex=0 speed=10 kworker/u322:2-11096 [070] 6164.771853: _phy_start_aneq: ( _phy_start_aneg+0x0/0xb8) duplex=0 speed=10 kworker/u322:2-11096 [070] 6164.771854: phy_caps_lookup: (phy_caps_lookup+0x0/0xf0) duplex=0 speed=10 kworker/u322:2-11096 [070] 6164.771855: phy_config_aneg: (phy_config_aneg+0x0/0x70) duplex=1 speed=10 kworker/u322:2-11096 [070] 6164.771856: genphy_config_aneg: (__genphy_config_aneg+0X0/0X270) duplex=1 speed=10 I also try to fixed it and it works. Do you have any idea about it. diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 0e762fc3a529..2986c41c42a8 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -258,7 +258,7 @@ static void phy_sanitize_settings(struct phy_device *phydev) const struct link_capabilities *c; c = phy_caps_lookup(phydev->speed, phydev->duplex, phydev->supported, - false); + true); if (c) { phydev->speed = c->speed; > > /* If we have a fixed link, refuse to change link parameters. > * If the link parameters match, accept them but do nothing. > */ > if (pl->req_link_an_mode == MLO_AN_FIXED) { > - if (s->speed != pl->link_config.speed || > - s->duplex != pl->link_config.duplex) > + if (c->speed != pl->link_config.speed || > + c->duplex != pl->link_config.duplex) > return -EINVAL; > return 0; > } > > - config.speed = s->speed; > - config.duplex = s->duplex; > + config.speed = c->speed; > + config.duplex = c->duplex; > break; > > case AUTONEG_ENABLE: