diff mbox series

[net-next] r8169: add support for LED's on RTL8168/RTL8101

Message ID bbae1576-63b2-4375-bfe6-f5fa255253ee@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] r8169: add support for LED's on RTL8168/RTL8101 | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1115 this patch: 1115
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 1142 this patch: 1142
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1142 this patch: 1142
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 86 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Heiner Kallweit Dec. 7, 2023, 2:34 p.m. UTC
This adds support for the LED's on most chip versions. Excluded are
the old non-PCIe versions and RTL8125. RTL8125 has a different LED
register layout, support for it will follow later.

LED's can be controlled from userspace using the netdev LED trigger.

Tested on RTL8168h.

Note: The driver can't know which LED's are actually physically
wired. Therefore not every LED device may represent a physically
available LED.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/Makefile     |   3 +
 drivers/net/ethernet/realtek/r8169.h      |   7 +
 drivers/net/ethernet/realtek/r8169_leds.c | 156 ++++++++++++++++++++++
 drivers/net/ethernet/realtek/r8169_main.c |  51 +++++++
 4 files changed, 217 insertions(+)
 create mode 100644 drivers/net/ethernet/realtek/r8169_leds.c

Comments

Heiner Kallweit Dec. 8, 2023, 5:38 p.m. UTC | #1
On 07.12.2023 15:34, Heiner Kallweit wrote:
> This adds support for the LED's on most chip versions. Excluded are
> the old non-PCIe versions and RTL8125. RTL8125 has a different LED
> register layout, support for it will follow later.
> 
> LED's can be controlled from userspace using the netdev LED trigger.
> 
> Tested on RTL8168h.
> 
> Note: The driver can't know which LED's are actually physically
> wired. Therefore not every LED device may represent a physically
> available LED.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---

Will send a v2, setting default_trigger is missing.
Andrew Lunn Dec. 11, 2023, 4:05 p.m. UTC | #2
On Thu, Dec 07, 2023 at 03:34:08PM +0100, Heiner Kallweit wrote:
> This adds support for the LED's on most chip versions. Excluded are
> the old non-PCIe versions and RTL8125. RTL8125 has a different LED
> register layout, support for it will follow later.
> 
> LED's can be controlled from userspace using the netdev LED trigger.

Since you cannot implement set_brightness, the hardware only supports
offload, it probably makes sense to add Kconfig to enable the building
of the netdev LED trigger. It seems pointless just having plain LEDs
which can then usable.

	Andrew
Heiner Kallweit Dec. 16, 2023, 12:28 p.m. UTC | #3
On 11.12.2023 17:05, Andrew Lunn wrote:
> On Thu, Dec 07, 2023 at 03:34:08PM +0100, Heiner Kallweit wrote:
>> This adds support for the LED's on most chip versions. Excluded are
>> the old non-PCIe versions and RTL8125. RTL8125 has a different LED
>> register layout, support for it will follow later.
>>
>> LED's can be controlled from userspace using the netdev LED trigger.
> 
> Since you cannot implement set_brightness, the hardware only supports
> offload, it probably makes sense to add Kconfig to enable the building
> of the netdev LED trigger. It seems pointless just having plain LEDs
> which can then usable.
> 
Right, therefore I create the LED devices only if
CONFIG_LEDS_TRIGGER_NETDEV is enabled:

#if IS_REACHABLE(CONFIG_LEDS_CLASS) && IS_ENABLED(CONFIG_LEDS_TRIGGER_NETDEV)
	[..]
	rtl8168_init_leds(dev);
#endif

Is this what you're referring to?

Using Kconfig select maybe problematic because it doesn't consider
dependencies. So we may have to use:
imply LEDS_TRIGGER_NETDEV
imply LEDS_TRIGGERS

> 	Andrew
Heiner
Andrew Lunn Dec. 16, 2023, 12:53 p.m. UTC | #4
On Sat, Dec 16, 2023 at 01:28:48PM +0100, Heiner Kallweit wrote:
> On 11.12.2023 17:05, Andrew Lunn wrote:
> > On Thu, Dec 07, 2023 at 03:34:08PM +0100, Heiner Kallweit wrote:
> >> This adds support for the LED's on most chip versions. Excluded are
> >> the old non-PCIe versions and RTL8125. RTL8125 has a different LED
> >> register layout, support for it will follow later.
> >>
> >> LED's can be controlled from userspace using the netdev LED trigger.
> > 
> > Since you cannot implement set_brightness, the hardware only supports
> > offload, it probably makes sense to add Kconfig to enable the building
> > of the netdev LED trigger. It seems pointless just having plain LEDs
> > which can then usable.
> > 
> Right, therefore I create the LED devices only if
> CONFIG_LEDS_TRIGGER_NETDEV is enabled:
> 
> #if IS_REACHABLE(CONFIG_LEDS_CLASS) && IS_ENABLED(CONFIG_LEDS_TRIGGER_NETDEV)
> 	[..]
> 	rtl8168_init_leds(dev);
> #endif
> 
> Is this what you're referring to?

No, but that is a good alternative. I'm happy with this.

    Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/realtek/Makefile b/drivers/net/ethernet/realtek/Makefile
index 2e1d78b10..adff9ebfb 100644
--- a/drivers/net/ethernet/realtek/Makefile
+++ b/drivers/net/ethernet/realtek/Makefile
@@ -7,4 +7,7 @@  obj-$(CONFIG_8139CP) += 8139cp.o
 obj-$(CONFIG_8139TOO) += 8139too.o
 obj-$(CONFIG_ATP) += atp.o
 r8169-objs += r8169_main.o r8169_firmware.o r8169_phy_config.o
+ifdef CONFIG_LEDS_TRIGGER_NETDEV
+r8169-objs += r8169_leds.o
+endif
 obj-$(CONFIG_R8169) += r8169.o
diff --git a/drivers/net/ethernet/realtek/r8169.h b/drivers/net/ethernet/realtek/r8169.h
index 55ef8251f..81567fcf3 100644
--- a/drivers/net/ethernet/realtek/r8169.h
+++ b/drivers/net/ethernet/realtek/r8169.h
@@ -8,6 +8,7 @@ 
  * See MAINTAINERS file for support contact information.
  */
 
+#include <linux/netdevice.h>
 #include <linux/types.h>
 #include <linux/phy.h>
 
@@ -77,3 +78,9 @@  u16 rtl8168h_2_get_adc_bias_ioffset(struct rtl8169_private *tp);
 u8 rtl8168d_efuse_read(struct rtl8169_private *tp, int reg_addr);
 void r8169_hw_phy_config(struct rtl8169_private *tp, struct phy_device *phydev,
 			 enum mac_version ver);
+
+void r8169_get_led_name(struct rtl8169_private *tp, int idx,
+			char *buf, int buf_len);
+int rtl8168_get_led_mode(struct rtl8169_private *tp);
+int rtl8168_led_mod_ctrl(struct rtl8169_private *tp, u16 mask, u16 val);
+void rtl8168_init_leds(struct net_device *ndev);
diff --git a/drivers/net/ethernet/realtek/r8169_leds.c b/drivers/net/ethernet/realtek/r8169_leds.c
new file mode 100644
index 000000000..5e741ed59
--- /dev/null
+++ b/drivers/net/ethernet/realtek/r8169_leds.c
@@ -0,0 +1,156 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/* r8169_leds.c: Realtek 8169/8168/8101/8125 ethernet driver.
+ *
+ * Copyright (c) 2023 Heiner Kallweit <hkallweit1@gmail.com>
+ *
+ * See MAINTAINERS file for support contact information.
+ */
+
+#include <linux/leds.h>
+#include <linux/netdevice.h>
+#include <uapi/linux/uleds.h>
+
+#include "r8169.h"
+
+#define RTL8168_LED_CTRL_OPTION2	BIT(15)
+#define RTL8168_LED_CTRL_ACT		BIT(3)
+#define RTL8168_LED_CTRL_LINK_1000	BIT(2)
+#define RTL8168_LED_CTRL_LINK_100	BIT(1)
+#define RTL8168_LED_CTRL_LINK_10	BIT(0)
+
+#define RTL8168_NUM_LEDS		3
+
+#define RTL8168_SUPPORTED_MODES \
+	(BIT(TRIGGER_NETDEV_LINK_1000) | BIT(TRIGGER_NETDEV_LINK_100) | \
+	 BIT(TRIGGER_NETDEV_LINK_10) | BIT(TRIGGER_NETDEV_RX) | \
+	 BIT(TRIGGER_NETDEV_TX))
+
+struct r8169_led_classdev {
+	struct led_classdev led;
+	struct net_device *ndev;
+	int index;
+};
+
+#define lcdev_to_r8169_ldev(lcdev) container_of(lcdev, struct r8169_led_classdev, led)
+
+static int rtl8168_led_hw_control_is_supported(struct led_classdev *led_cdev,
+					       unsigned long flags)
+{
+	struct r8169_led_classdev *ldev = lcdev_to_r8169_ldev(led_cdev);
+	struct rtl8169_private *tp = netdev_priv(ldev->ndev);
+	int shift = ldev->index * 4;
+	bool rx, tx;
+
+	if (flags & ~RTL8168_SUPPORTED_MODES)
+		goto nosupp;
+
+	rx = flags & BIT(TRIGGER_NETDEV_RX);
+	tx = flags & BIT(TRIGGER_NETDEV_TX);
+	if (rx != tx)
+		goto nosupp;
+
+	return 0;
+
+nosupp:
+	/* Switch LED off to indicate that mode isn't supported */
+	rtl8168_led_mod_ctrl(tp, 0x000f << shift, 0);
+	return -EOPNOTSUPP;
+}
+
+static int rtl8168_led_hw_control_set(struct led_classdev *led_cdev,
+				      unsigned long flags)
+{
+	struct r8169_led_classdev *ldev = lcdev_to_r8169_ldev(led_cdev);
+	struct rtl8169_private *tp = netdev_priv(ldev->ndev);
+	int shift = ldev->index * 4;
+	u16 mode = 0;
+
+	if (flags & BIT(TRIGGER_NETDEV_LINK_10))
+		mode |= RTL8168_LED_CTRL_LINK_10;
+	if (flags & BIT(TRIGGER_NETDEV_LINK_100))
+		mode |= RTL8168_LED_CTRL_LINK_100;
+	if (flags & BIT(TRIGGER_NETDEV_LINK_1000))
+		mode |= RTL8168_LED_CTRL_LINK_1000;
+	if (flags & BIT(TRIGGER_NETDEV_TX))
+		mode |= RTL8168_LED_CTRL_ACT;
+
+	return rtl8168_led_mod_ctrl(tp, 0x000f << shift, mode << shift);
+}
+
+static int rtl8168_led_hw_control_get(struct led_classdev *led_cdev,
+				      unsigned long *flags)
+{
+	struct r8169_led_classdev *ldev = lcdev_to_r8169_ldev(led_cdev);
+	struct rtl8169_private *tp = netdev_priv(ldev->ndev);
+	int shift = ldev->index * 4;
+	int mode;
+
+	mode = rtl8168_get_led_mode(tp);
+	if (mode < 0)
+		return mode;
+
+	if (mode & RTL8168_LED_CTRL_OPTION2) {
+		rtl8168_led_mod_ctrl(tp, RTL8168_LED_CTRL_OPTION2, 0);
+		netdev_notice(ldev->ndev, "Deactivating unsupported Option2 LED mode\n");
+	}
+
+	mode = (mode >> shift) & 0x000f;
+
+	if (mode & RTL8168_LED_CTRL_ACT)
+		*flags |= BIT(TRIGGER_NETDEV_TX) | BIT(TRIGGER_NETDEV_RX);
+
+	if (mode & RTL8168_LED_CTRL_LINK_10)
+		*flags |= BIT(TRIGGER_NETDEV_LINK_10);
+	if (mode & RTL8168_LED_CTRL_LINK_100)
+		*flags |= BIT(TRIGGER_NETDEV_LINK_100);
+	if (mode & RTL8168_LED_CTRL_LINK_1000)
+		*flags |= BIT(TRIGGER_NETDEV_LINK_1000);
+
+	return 0;
+}
+
+static struct device *
+	r8169_led_hw_control_get_device(struct led_classdev *led_cdev)
+{
+	struct r8169_led_classdev *ldev = lcdev_to_r8169_ldev(led_cdev);
+
+	return &ldev->ndev->dev;
+}
+
+static void rtl8168_setup_ldev(struct r8169_led_classdev *ldev,
+			       struct net_device *ndev, int index)
+{
+	struct rtl8169_private *tp = netdev_priv(ndev);
+	struct led_classdev *led_cdev = &ldev->led;
+	char led_name[LED_MAX_NAME_SIZE];
+
+	ldev->ndev = ndev;
+	ldev->index = index;
+
+	r8169_get_led_name(tp, index, led_name, LED_MAX_NAME_SIZE);
+	led_cdev->name = led_name;
+	led_cdev->hw_control_trigger = "netdev";
+	led_cdev->flags |= LED_RETAIN_AT_SHUTDOWN;
+	led_cdev->hw_control_is_supported = rtl8168_led_hw_control_is_supported;
+	led_cdev->hw_control_set = rtl8168_led_hw_control_set;
+	led_cdev->hw_control_get = rtl8168_led_hw_control_get;
+	led_cdev->hw_control_get_device = r8169_led_hw_control_get_device;
+
+	/* ignore errors */
+	devm_led_classdev_register(&ndev->dev, led_cdev);
+}
+
+void rtl8168_init_leds(struct net_device *ndev)
+{
+	/* bind resource mgmt to netdev */
+	struct device *dev = &ndev->dev;
+	struct r8169_led_classdev *leds;
+	int i;
+
+	leds = devm_kcalloc(dev, RTL8168_NUM_LEDS, sizeof(*leds), GFP_KERNEL);
+	if (!leds)
+		return;
+
+	for (i = 0; i < RTL8168_NUM_LEDS; i++)
+		rtl8168_setup_ldev(leds + i, ndev, i);
+}
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index c07f82ca9..406cfa9b3 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -285,6 +285,7 @@  enum rtl8168_8101_registers {
 };
 
 enum rtl8168_registers {
+	LED_CTRL		= 0x18,
 	LED_FREQ		= 0x1a,
 	EEE_LED			= 0x1b,
 	ERIDR			= 0x70,
@@ -616,6 +617,7 @@  struct rtl8169_private {
 
 	raw_spinlock_t config25_lock;
 	raw_spinlock_t mac_ocp_lock;
+	struct mutex led_lock;	/* serialize LED ctrl RMW access */
 
 	raw_spinlock_t cfg9346_usage_lock;
 	int cfg9346_usage_count;
@@ -788,6 +790,48 @@  static const struct rtl_cond name = {			\
 							\
 static bool name ## _check(struct rtl8169_private *tp)
 
+int rtl8168_led_mod_ctrl(struct rtl8169_private *tp, u16 mask, u16 val)
+{
+	struct device *dev = tp_to_dev(tp);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		return ret;
+
+	mutex_lock(&tp->led_lock);
+	RTL_W16(tp, LED_CTRL, (RTL_R16(tp, LED_CTRL) & ~mask) | val);
+	mutex_unlock(&tp->led_lock);
+
+	pm_runtime_put_sync(dev);
+
+	return 0;
+}
+
+int rtl8168_get_led_mode(struct rtl8169_private *tp)
+{
+	struct device *dev = tp_to_dev(tp);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		return ret;
+
+	ret = RTL_R16(tp, LED_CTRL);
+
+	pm_runtime_put_sync(dev);
+
+	return ret;
+}
+
+void r8169_get_led_name(struct rtl8169_private *tp, int idx,
+			char *buf, int buf_len)
+{
+	snprintf(buf, buf_len, "r8169-%x%x-led%d",
+		 pci_domain_nr(tp->pci_dev->bus),
+		 pci_dev_id(tp->pci_dev), idx);
+}
+
 static void r8168fp_adjust_ocp_cmd(struct rtl8169_private *tp, u32 *cmd, int type)
 {
 	/* based on RTL8168FP_OOBMAC_BASE in vendor driver */
@@ -5141,6 +5185,7 @@  static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	raw_spin_lock_init(&tp->cfg9346_usage_lock);
 	raw_spin_lock_init(&tp->config25_lock);
 	raw_spin_lock_init(&tp->mac_ocp_lock);
+	mutex_init(&tp->led_lock);
 
 	dev->tstats = devm_netdev_alloc_pcpu_stats(&pdev->dev,
 						   struct pcpu_sw_netstats);
@@ -5297,6 +5342,12 @@  static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		return rc;
 
+#if IS_REACHABLE(CONFIG_LEDS_CLASS) && IS_ENABLED(CONFIG_LEDS_TRIGGER_NETDEV)
+	if (tp->mac_version > RTL_GIGA_MAC_VER_06 &&
+	    tp->mac_version < RTL_GIGA_MAC_VER_61)
+		rtl8168_init_leds(dev);
+#endif
+
 	netdev_info(dev, "%s, %pM, XID %03x, IRQ %d\n",
 		    rtl_chip_infos[chipset].name, dev->dev_addr, xid, tp->irq);