All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Marangi <ansuelsmth@gmail.com>
To: "Pavel Machek" <pavel@ucw.cz>, "Lee Jones" <lee@kernel.org>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Christian Marangi" <ansuelsmth@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Daniel Golle" <daniel@makrotopia.org>,
	"Li Zetao" <lizetao1@huawei.com>,
	linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Marek Behún" <kabel@kernel.org>
Subject: [PATCH] leds: trigger: netdev: display only supported link speed attribute
Date: Sat,  9 Dec 2023 16:07:24 +0100	[thread overview]
Message-ID: <20231209150724.25565-1-ansuelsmth@gmail.com> (raw)

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 added.

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 add only the supported link speed
modes attribute.

This only apply to the link speed modes and every other mode is still
provided by default.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/leds/trigger/ledtrig-netdev.c | 56 +++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index 09e75fd9f2bc..ce84808e231c 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -61,6 +61,8 @@ struct led_netdev_data {
 	bool hw_control;
 };
 
+static int add_link_speed_attr(struct led_netdev_data *trigger_data);
+
 static void set_baseline_state(struct led_netdev_data *trigger_data)
 {
 	int current_brightness;
@@ -262,8 +264,10 @@ static int set_device_name(struct led_netdev_data *trigger_data,
 	trigger_data->carrier_link_up = false;
 	trigger_data->link_speed = SPEED_UNKNOWN;
 	trigger_data->duplex = DUPLEX_UNKNOWN;
-	if (trigger_data->net_dev)
+	if (trigger_data->net_dev) {
 		get_device_state(trigger_data);
+		add_link_speed_attr(trigger_data);
+	}
 
 	trigger_data->last_activity = 0;
 
@@ -396,6 +400,50 @@ DEFINE_NETDEV_TRIGGER(full_duplex, TRIGGER_NETDEV_FULL_DUPLEX);
 DEFINE_NETDEV_TRIGGER(tx, TRIGGER_NETDEV_TX);
 DEFINE_NETDEV_TRIGGER(rx, TRIGGER_NETDEV_RX);
 
+static int add_link_speed_attr(struct led_netdev_data *trigger_data)
+{
+	struct led_classdev *led_cdev = trigger_data->led_cdev;
+	struct device *dev = led_cdev->dev;
+	struct ethtool_link_ksettings cmd;
+	int ret;
+
+	/* First remove any entry previously added */
+	device_remove_file(dev, &dev_attr_link_10);
+	device_remove_file(dev, &dev_attr_link_100);
+	device_remove_file(dev, &dev_attr_link_1000);
+	device_remove_file(dev, &dev_attr_link_2500);
+	device_remove_file(dev, &dev_attr_link_5000);
+	device_remove_file(dev, &dev_attr_link_10000);
+
+	ret = __ethtool_get_link_ksettings(trigger_data->net_dev, &cmd);
+	if (ret)
+		return ret;
+
+	/* Add only supported entry */
+	if (test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, cmd.link_modes.supported) ||
+	    test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, cmd.link_modes.supported))
+		device_create_file(dev, &dev_attr_link_10);
+
+	if (test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, cmd.link_modes.supported) ||
+	    test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, cmd.link_modes.supported))
+		device_create_file(dev, &dev_attr_link_100);
+
+	if (test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, cmd.link_modes.supported) ||
+	    test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, cmd.link_modes.supported))
+		device_create_file(dev, &dev_attr_link_1000);
+
+	if (test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, cmd.link_modes.supported))
+		device_create_file(dev, &dev_attr_link_2500);
+
+	if (test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, cmd.link_modes.supported))
+		device_create_file(dev, &dev_attr_link_5000);
+
+	if (test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, cmd.link_modes.supported))
+		device_create_file(dev, &dev_attr_link_10000);
+
+	return 0;
+}
+
 static ssize_t interval_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
@@ -446,12 +494,6 @@ static DEVICE_ATTR_RO(offloaded);
 static struct attribute *netdev_trig_attrs[] = {
 	&dev_attr_device_name.attr,
 	&dev_attr_link.attr,
-	&dev_attr_link_10.attr,
-	&dev_attr_link_100.attr,
-	&dev_attr_link_1000.attr,
-	&dev_attr_link_2500.attr,
-	&dev_attr_link_5000.attr,
-	&dev_attr_link_10000.attr,
 	&dev_attr_full_duplex.attr,
 	&dev_attr_half_duplex.attr,
 	&dev_attr_rx.attr,
-- 
2.40.1


             reply	other threads:[~2023-12-09 15:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-09 15:07 Christian Marangi [this message]
2023-12-13 10:06 ` [PATCH] leds: trigger: netdev: display only supported link speed attribute Andrew Lunn
2023-12-13 12:05 ` Marek Behún
2023-12-13 12:07   ` Christian Marangi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231209150724.25565-1-ansuelsmth@gmail.com \
    --to=ansuelsmth@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=daniel@makrotopia.org \
    --cc=davem@davemloft.net \
    --cc=kabel@kernel.org \
    --cc=kuba@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=lizetao1@huawei.com \
    --cc=pavel@ucw.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.