[PATCH v2 net] r8169: remove redundant hwmon support

Subsystems: 8169 10/100/1000 gigabit ethernet driver, networking drivers, the rest

STALE589d

6 messages, 3 authors, 2025-01-14 · open the first message on its own page

[PATCH v2 net] r8169: remove redundant hwmon support

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: 2025-01-09 22:43:15

The temperature sensor is actually part of the integrated PHY and available
also on the standalone versions of the PHY. Therefore hwmon support will
be added to the Realtek PHY driver and can be removed here.

Fixes: 1ffcc8d41306 ("r8169: add support for the temperature sensor being available from RTL8125B")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- rebase patch on net
---
 drivers/net/ethernet/realtek/r8169_main.c | 44 -----------------------
 1 file changed, 44 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 739707a7b..8a3959bb2 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -16,7 +16,6 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/ethtool.h>
-#include <linux/hwmon.h>
 #include <linux/phy.h>
 #include <linux/if_vlan.h>
 #include <linux/in.h>
@@ -5347,43 +5346,6 @@ static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
 	return false;
 }
 
-static umode_t r8169_hwmon_is_visible(const void *drvdata,
-				      enum hwmon_sensor_types type,
-				      u32 attr, int channel)
-{
-	return 0444;
-}
-
-static int r8169_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
-			    u32 attr, int channel, long *val)
-{
-	struct rtl8169_private *tp = dev_get_drvdata(dev);
-	int val_raw;
-
-	val_raw = phy_read_paged(tp->phydev, 0xbd8, 0x12) & 0x3ff;
-	if (val_raw >= 512)
-		val_raw -= 1024;
-
-	*val = 1000 * val_raw / 2;
-
-	return 0;
-}
-
-static const struct hwmon_ops r8169_hwmon_ops = {
-	.is_visible =  r8169_hwmon_is_visible,
-	.read = r8169_hwmon_read,
-};
-
-static const struct hwmon_channel_info * const r8169_hwmon_info[] = {
-	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
-	NULL
-};
-
-static const struct hwmon_chip_info r8169_hwmon_chip_info = {
-	.ops = &r8169_hwmon_ops,
-	.info = r8169_hwmon_info,
-};
-
 static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct rtl8169_private *tp;
@@ -5563,12 +5525,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		return rc;
 
-	/* The temperature sensor is available from RTl8125B */
-	if (IS_REACHABLE(CONFIG_HWMON) && tp->mac_version >= RTL_GIGA_MAC_VER_63)
-		/* ignore errors */
-		devm_hwmon_device_register_with_info(&pdev->dev, "nic_temp", tp,
-						     &r8169_hwmon_chip_info,
-						     NULL);
 	rc = register_netdev(dev);
 	if (rc)
 		return rc;
-- 
2.47.1


Re: [PATCH v2 net] r8169: remove redundant hwmon support

From: Jacob Keller <jacob.e.keller@intel.com>
Date: 2025-01-10 01:12:51


On 1/9/2025 2:43 PM, Heiner Kallweit wrote:
The temperature sensor is actually part of the integrated PHY and available
also on the standalone versions of the PHY. Therefore hwmon support will
be added to the Realtek PHY driver and can be removed here.

Fixes: 1ffcc8d41306 ("r8169: add support for the temperature sensor being available from RTL8125B")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- rebase patch on net
In the future, please avoid re-sending within 24 hours:

https://docs.kernel.org/process/maintainer-netdev.html#resending-after-review

Thanks,
Jake

Re: [PATCH v2 net] r8169: remove redundant hwmon support

From: Jacob Keller <jacob.e.keller@intel.com>
Date: 2025-01-10 01:14:43


On 1/9/2025 2:43 PM, Heiner Kallweit wrote:
The temperature sensor is actually part of the integrated PHY and available
also on the standalone versions of the PHY. Therefore hwmon support will
be added to the Realtek PHY driver and can be removed here.

Fixes: 1ffcc8d41306 ("r8169: add support for the temperature sensor being available from RTL8125B")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
Can you explain what user-facing issues this fixes? Do we get conflicts
when multiple hwmon sensors are registered? I'm not sure this counts as
a 'net' fix, unless we can identify the user-facing behavior that is
problematic?

Re: [PATCH v2 net] r8169: remove redundant hwmon support

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: 2025-01-10 06:45:40

On 10.01.2025 02:14, Jacob Keller wrote:

On 1/9/2025 2:43 PM, Heiner Kallweit wrote:
quoted
The temperature sensor is actually part of the integrated PHY and available
also on the standalone versions of the PHY. Therefore hwmon support will
be added to the Realtek PHY driver and can be removed here.

Fixes: 1ffcc8d41306 ("r8169: add support for the temperature sensor being available from RTL8125B")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
Can you explain what user-facing issues this fixes? Do we get conflicts
when multiple hwmon sensors are registered? I'm not sure this counts as
a 'net' fix, unless we can identify the user-facing behavior that is
problematic?
The same sensor would be exposed twice, by the MAC and the PHY driver.
Support in the MAC driver was added for 6.13, so it's still in the rc
version only and not yet part of a released kernel version.

Re: [PATCH v2 net] r8169: remove redundant hwmon support

From: Jacob Keller <jacob.e.keller@intel.com>
Date: 2025-01-11 00:53:31


On 1/9/2025 10:45 PM, Heiner Kallweit wrote:
On 10.01.2025 02:14, Jacob Keller wrote:
quoted

On 1/9/2025 2:43 PM, Heiner Kallweit wrote:
quoted
The temperature sensor is actually part of the integrated PHY and available
also on the standalone versions of the PHY. Therefore hwmon support will
be added to the Realtek PHY driver and can be removed here.

Fixes: 1ffcc8d41306 ("r8169: add support for the temperature sensor being available from RTL8125B")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
Can you explain what user-facing issues this fixes? Do we get conflicts
when multiple hwmon sensors are registered? I'm not sure this counts as
a 'net' fix, unless we can identify the user-facing behavior that is
problematic?
The same sensor would be exposed twice, by the MAC and the PHY driver.
Support in the MAC driver was added for 6.13, so it's still in the rc
version only and not yet part of a released kernel version.
I guess I'm failing to understand what the user repercussions of having
multiple sensors would be?

I suppose that it is confusing and could lead to issues when userspace
is checking or controlling the sensor, and fixing this via net before it
makes it into a full release is good.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH v2 net] r8169: remove redundant hwmon support

From: patchwork-bot+netdevbpf@kernel.org
Date: 2025-01-14 03:30:13

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski [off-list ref]:

On Thu, 9 Jan 2025 23:43:12 +0100 you wrote:
The temperature sensor is actually part of the integrated PHY and available
also on the standalone versions of the PHY. Therefore hwmon support will
be added to the Realtek PHY driver and can be removed here.

Fixes: 1ffcc8d41306 ("r8169: add support for the temperature sensor being available from RTL8125B")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

[...]
Here is the summary with links:
  - [v2,net] r8169: remove redundant hwmon support
    https://git.kernel.org/netdev/net/c/1f691a1fc4be

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help