Received: by 2002:a05:7412:b995:b0:f9:9502:5bb8 with SMTP id it21csp27054rdb; Thu, 21 Dec 2023 01:34:08 -0800 (PST) X-Google-Smtp-Source: AGHT+IHZRo3y/w2VQ1Ou5zBm0CIGJkPIkYPtoFmNe8WjyhgG7D+n1OhZqcVc2dbG8vZ4DDug95pW X-Received: by 2002:a17:902:7ed0:b0:1d0:6ffd:cea8 with SMTP id p16-20020a1709027ed000b001d06ffdcea8mr10402146plb.97.1703151248668; Thu, 21 Dec 2023 01:34:08 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1703151248; cv=none; d=google.com; s=arc-20160816; b=dKK/9ZdEOIO9bPqr3LqAFxVWoMlGz/78wrHkHtKUf1xEAEZ7jkrwwp5Ia5jTAf2S9h TA/GcnjJkVe4xPjSvxKz0Bnn0+Xezm4/yi2IwoacuOWzIgyeKrixvxvO/IbMM8RaQzn/ 9P0i+WzrgXCP8WTAuFt15+OeiQ7DXpWgMlYtJqP1GfhxaRFcIwd373JZxVPGaSojclCg RCkqSFo/VZx2EM1r0hS8+kcfI4yMX6S59XX5CRHDCYz8xjMuyOTt8sXO3oLopQD/AB0E YmBbz2P5Z7flvYbbWVEGRSS8+CJ0KEzMdMdW2+38LZEx93M9qIy/9oRlwMeDs48A8GVV NncQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :subject:cc:to:from:date:dkim-signature; bh=C03DISpxfOG6L9a0uUimjLMCoPYxZp6jgmImiTnJzXU=; fh=9MntKrF4XOokBA69GqF+Flah4tGU03whyNMNxmYzl60=; b=qP4G4+SwGgecASSBNSljGGR8e6lOBLA3s24XAFCv7hAYj+6UQwVyCT6YETEgbsSIm1 M4SnwgeklqKq89Sk9RKLvqVfGg8HBKz7WlYtUIgBVu2aiADkKzPlVMN7666I6g7ejXZ5 FNfyxMoaeMOcnvpDKfF0b3QNDf1XHHmk6neaOKyj/aAJyxPGnZiR+8tVQGLqlDiF6beA 4QhJhVTyZpzlcPNT2mnHwmrXie3PuvwhJWQ/WjqxPWpBt6IpUfAtyl5YeKzIY+QF32NZ cT/G7gSop5CUzC2umyNQNhBq9vvivNZmqeWFmMvr68TaXMTWIXgmllHGUXfEmbYRpENw RArQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=KKDXin5u; spf=pass (google.com: domain of linux-kernel+bounces-8094-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-8094-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from sy.mirrors.kernel.org (sy.mirrors.kernel.org. [2604:1380:40f1:3f00::1]) by mx.google.com with ESMTPS id p1-20020a170902e74100b001d076ffeecdsi1241300plf.198.2023.12.21.01.34.08 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Dec 2023 01:34:08 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-8094-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=KKDXin5u; spf=pass (google.com: domain of linux-kernel+bounces-8094-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-8094-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sy.mirrors.kernel.org (Postfix) with ESMTPS id 7E43CB2109E for ; Thu, 21 Dec 2023 09:23:22 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 7ADAA4EB57; Thu, 21 Dec 2023 09:08:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="KKDXin5u" X-Original-To: linux-kernel@vger.kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A45B04EB46; Thu, 21 Dec 2023 09:08:54 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E5FCAC433C8; Thu, 21 Dec 2023 09:08:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1703149734; bh=88oXhSF+rhYZhelTwKokwU9Sbkk5HFBCZH6RBQhsahE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=KKDXin5uKJfHVSo00Yhg3t600TDjhpdItjPoL0EVfoTSpZPXB9Vo972PfXAywWqFH NqaStISp+YaFogwMrYandG+hphmkNidpX9gKCRBqTaMzhNd6MMCRVxIQWbhhmgSyJD Ve0/+c6YAH/0WUmiVDBqALl/a/0jmaqDB4Zxt5lhGcqW0p7eboNb6qeXPdB5W38JPt MlSgyMmk+1s1cWskfjzIFTu1UfMJofdKR+3fn0jNESEdOHj6M+Y8E/9etuGBSEoJ66 ZjqCklbWH3cBtKjJibBpg5GeQyVT5ZxZ+feWO5l6UQlcWK7Y+Vz3HZ5PXTvwizDpeI ztRLBITWJAxIw== Date: Thu, 21 Dec 2023 10:08:48 +0100 From: Marek =?UTF-8?B?QmVow7pu?= To: Christian Marangi Cc: Pavel Machek , Lee Jones , Andrew Lunn , Jakub Kicinski , Daniel Golle , "David S. Miller" , Li Zetao , linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org Subject: Re: [PATCH v5 1/2] leds: trigger: netdev: display only supported link speed attribute Message-ID: <20231221100848.4a7da2df@dellmb> In-Reply-To: <20231220224827.27667-1-ansuelsmth@gmail.com> References: <20231220224827.27667-1-ansuelsmth@gmail.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 20 Dec 2023 23:48:26 +0100 Christian Marangi wrote: > With the addition of more link speed mode to the netdev trigger, it was > pointed out that there may be a problem with bloating the attribute list > with modes that won't ever be supported by the trigger as the attached > device name doesn't support them. > > To clear and address this problem, change the logic where these > additional trigger modes are listed. > > Since the netdev trigger REQUIRE a device name to be set, attach to the > device name change function additional logic to parse the supported link > speed modes using ethtool APIs and show only the supported link speed > modes attribute. > > Link speed attribute are refreshed on device_name set and on > NETDEV_CHANGE events. > > This only apply to the link speed modes and every other mode is still > provided by default. > > Signed-off-by: Christian Marangi > --- > Took a while but I found a way to not use phy_speeds. > > Saddly that is way too specific to PHYs and we can't add PHYLIB as > a dependency for leds. > > I instead found a handy way to keep everything to ethtool, it's a bit > worse optimization wise but does the same work. (the perf penality > is really minimal as we only parse supported speeds and it's difficult > to have a device that have tons of speeds modes) > > Changes v5: > - Add macro to make code less ugly > Changes v4: > - Rework to use an alternative to phy_speeds API > Changes v3: > - Use phy_speeds API to parse the ethtool mask > Changes v2: > - Use is_visibile instead of removing/adding attr > > drivers/leds/trigger/ledtrig-netdev.c | 88 +++++++++++++++++++++++++-- > 1 file changed, 82 insertions(+), 6 deletions(-) > > diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c > index bd68da15c723..f0f946747ff5 100644 > --- a/drivers/leds/trigger/ledtrig-netdev.c > +++ b/drivers/leds/trigger/ledtrig-netdev.c > @@ -18,10 +18,12 @@ > #include > #include > #include > +#include > #include > #include > #include > #include > +#include > #include > #include > #include "../leds.h" > @@ -55,12 +57,15 @@ struct led_netdev_data { > > unsigned long mode; > int link_speed; > + __ETHTOOL_DECLARE_LINK_MODE_MASK(supported_link_modes); > u8 duplex; > > bool carrier_link_up; > bool hw_control; > }; > > +static const struct attribute_group netdev_trig_link_speed_attrs_group; > + > static void set_baseline_state(struct led_netdev_data *trigger_data) > { > int current_brightness; > @@ -208,13 +213,19 @@ static void get_device_state(struct led_netdev_data *trigger_data) > struct ethtool_link_ksettings cmd; > > trigger_data->carrier_link_up = netif_carrier_ok(trigger_data->net_dev); > - if (!trigger_data->carrier_link_up) > + > + if (__ethtool_get_link_ksettings(trigger_data->net_dev, &cmd)) > return; > > - if (!__ethtool_get_link_ksettings(trigger_data->net_dev, &cmd)) { > + if (trigger_data->carrier_link_up) { > trigger_data->link_speed = cmd.base.speed; > trigger_data->duplex = cmd.base.duplex; > } > + > + /* Have a local copy of the link speed supported to not rtnl lock every time ^ to avoid > + * Modes are refreshed on any change event to handle mode changes ^ capital letter but not beginning of sentence If you change these two, you can add my R-b tag to both patches. I am also wondering if this sysfs ABI could be extended for multi-color LEDs. For example: echo green >link_1000 echo yellow >link_100 Or something like that. But that is a completely different discussion.