[PATCH net-next 0/2] r8169: extend hwmon support

[PATCH net-next 0/2] r8169: extend hwmon support

[Heiner Kallweit]

This series extends the hwmon support and adds support for the
over-temp threshold.

Heiner Kallweit (2):
  r8169: prepare for extending hwmon support
  r8169: add support for reading over-temp threshold

 drivers/net/ethernet/realtek/r8169_main.c | 30 +++++++++++++++++------
 1 file changed, 23 insertions(+), 7 deletions(-)

-- 
2.47.1

[PATCH net-next 1/2] r8169: prepare for extending hwmon support

[Heiner Kallweit]

This factors out the temperature value calculation and
prepares for adding further temperature attributes.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 5724f650f..38f915956 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5338,17 +5338,28 @@ static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
 	return false;
 }
 
+static int r8169_hwmon_get_temp(int raw)
+{
+	if (raw >= 512)
+		raw -= 1024;
+
+	return 1000 * raw / 2;
+}
+
 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;
+	int raw;
 
-	*val = 1000 * val_raw / 2;
+	switch (attr) {
+	case hwmon_temp_input:
+		raw = phy_read_paged(tp->phydev, 0xbd8, 0x12) & 0x3ff;
+		*val = r8169_hwmon_get_temp(raw);
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	return 0;
 }
-- 
2.47.1

Re: [PATCH net-next 1/2] r8169: prepare for extending hwmon support

[Andrew Lunn]

On Mon, Jan 06, 2025 at 07:01:16PM +0100, Heiner Kallweit wrote:
> This factors out the temperature value calculation and
> prepares for adding further temperature attributes.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 2/2] r8169: add support for reading over-temp threshold

[Heiner Kallweit]

Add support for reading the over-temp threshold. If the chip temperature
exceeds this value, the chip will reduce the speed to 1Gbps (by disabling
2.5G/5G advertisement and triggering a renegotiation).

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 38f915956..fd6ff77e8 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5357,6 +5357,11 @@ static int r8169_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 		raw = phy_read_paged(tp->phydev, 0xbd8, 0x12) & 0x3ff;
 		*val = r8169_hwmon_get_temp(raw);
 		break;
+	case hwmon_temp_max:
+		/* Chip reduces speed to 1G if threshold is exceeded */
+		raw = phy_read_paged(tp->phydev, 0xb54, 0x16) >> 6;
+		*val = r8169_hwmon_get_temp(raw);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -5370,7 +5375,7 @@ static const struct hwmon_ops r8169_hwmon_ops = {
 };
 
 static const struct hwmon_channel_info * const r8169_hwmon_info[] = {
-	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_MAX),
 	NULL
 };
 
-- 
2.47.1

Re: [PATCH net-next 2/2] r8169: add support for reading over-temp threshold

[Andrew Lunn]

On Mon, Jan 06, 2025 at 07:05:13PM +0100, Heiner Kallweit wrote:
> Add support for reading the over-temp threshold. If the chip temperature
> exceeds this value, the chip will reduce the speed to 1Gbps (by disabling
> 2.5G/5G advertisement and triggering a renegotiation).

I'm assuming here that the over-temp threshold always exists when the
temp_in sensors exists? If so:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Does it reduce the speed in the same way as downshift? Can the user
tell it has happened, other than networking is slower?

    Andrew

Re: [PATCH net-next 2/2] r8169: add support for reading over-temp threshold

[Heiner Kallweit]

On 06.01.2025 22:15, Andrew Lunn wrote:
> On Mon, Jan 06, 2025 at 07:05:13PM +0100, Heiner Kallweit wrote:
>> Add support for reading the over-temp threshold. If the chip temperature
>> exceeds this value, the chip will reduce the speed to 1Gbps (by disabling
>> 2.5G/5G advertisement and triggering a renegotiation).
> 
> I'm assuming here that the over-temp threshold always exists when the
> temp_in sensors exists? If so:
> 
Right

> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> Does it reduce the speed in the same way as downshift? Can the user
> tell it has happened, other than networking is slower?
> 
It internally disables 2.5G/5G advertisement and triggers an autoneg.
So you get the usual message on console/dmesg indicating the new speed.
This internal action sets a register bit, and it can also trigger an interrupt.
So it should be possible to check in the link_change_notify() callback
whether an over-temp event occurred. The silent change of the advertisement
may also result in the phylib-cached advertisement being out-of-sync.
So we would have to re-sync it. But I didn't fully test this yet.

This patch only allows to read the over-temp threshold set as power-on default
or by the boot loader. It doesn't change the existing behavior.

>     Andrew

Heiner

Re: [PATCH net-next 2/2] r8169: add support for reading over-temp threshold

[Andrew Lunn]

> > Does it reduce the speed in the same way as downshift? Can the user
> > tell it has happened, other than networking is slower?
> > 
> It internally disables 2.5G/5G advertisement and triggers an autoneg.
> So you get the usual message on console/dmesg indicating the new speed.
> This internal action sets a register bit, and it can also trigger an interrupt.
> So it should be possible to check in the link_change_notify() callback
> whether an over-temp event occurred. The silent change of the advertisement
> may also result in the phylib-cached advertisement being out-of-sync.
> So we would have to re-sync it. But I didn't fully test this yet.
> 
> This patch only allows to read the over-temp threshold set as power-on default
> or by the boot loader. It doesn't change the existing behavior.

Thanks for the details. So it does seem to be different to downshift,
where generally advertised link modes in the registers is not changed,
and the speed indicated in BMSR generally does not indicate the
downshifted speed, you need vendor registers to get the actual
speed. But downshift happens at link up, not latter when the device
starts to overheat.

Does it restore advertisement to 2.5G/5G when it cools down? There is
an interesting policy decision here. Do you want 1s downtime very so
often as it speeds up and slows down, or should it keep at the slower
speed until the user kicks it back up to the higher speed?

	Andrew

Re: [PATCH net-next 2/2] r8169: add support for reading over-temp threshold

[Heiner Kallweit]

On 07.01.2025 00:16, Andrew Lunn wrote:
>>> Does it reduce the speed in the same way as downshift? Can the user
>>> tell it has happened, other than networking is slower?
>>>
>> It internally disables 2.5G/5G advertisement and triggers an autoneg.
>> So you get the usual message on console/dmesg indicating the new speed.
>> This internal action sets a register bit, and it can also trigger an interrupt.
>> So it should be possible to check in the link_change_notify() callback
>> whether an over-temp event occurred. The silent change of the advertisement
>> may also result in the phylib-cached advertisement being out-of-sync.
>> So we would have to re-sync it. But I didn't fully test this yet.
>>
>> This patch only allows to read the over-temp threshold set as power-on default
>> or by the boot loader. It doesn't change the existing behavior.
> 
> Thanks for the details. So it does seem to be different to downshift,
> where generally advertised link modes in the registers is not changed,
> and the speed indicated in BMSR generally does not indicate the
> downshifted speed, you need vendor registers to get the actual
> speed. But downshift happens at link up, not latter when the device
> starts to overheat.
> 
> Does it restore advertisement to 2.5G/5G when it cools down? There is
> an interesting policy decision here. Do you want 1s downtime very so
> often as it speeds up and slows down, or should it keep at the slower
> speed until the user kicks it back up to the higher speed?
> 
I will do more testing and provide an update.

> 	Andrew
> 
Heiner

Re: [PATCH net-next 2/2] r8169: add support for reading over-temp threshold

[Jakub Kicinski]

On Mon, 6 Jan 2025 19:05:13 +0100 Heiner Kallweit wrote:
> Add support for reading the over-temp threshold. If the chip temperature
> exceeds this value, the chip will reduce the speed to 1Gbps (by disabling
> 2.5G/5G advertisement and triggering a renegotiation).

If there is a v2 -- please make sure hwmon folks are CCed.
Looks like get_maintainers doesn't flag it, but since we're charting 
a new territory for networking a broader audience may be quite useful.

Re: [PATCH net-next 2/2] r8169: add support for reading over-temp threshold

[Heiner Kallweit]

On 07.01.2025 00:30, Jakub Kicinski wrote:
> On Mon, 6 Jan 2025 19:05:13 +0100 Heiner Kallweit wrote:
>> Add support for reading the over-temp threshold. If the chip temperature
>> exceeds this value, the chip will reduce the speed to 1Gbps (by disabling
>> 2.5G/5G advertisement and triggering a renegotiation).
> 
> If there is a v2 -- please make sure hwmon folks are CCed.
> Looks like get_maintainers doesn't flag it, but since we're charting 
> a new territory for networking a broader audience may be quite useful.

OK. No v2 is planned, but there may be follow-up patches.

Re: [PATCH net-next 0/2] r8169: extend hwmon support

[Heiner Kallweit]

On 06.01.2025 19:00, Heiner Kallweit wrote:
> This series extends the hwmon support and adds support for the
> over-temp threshold.
> 
> Heiner Kallweit (2):
>   r8169: prepare for extending hwmon support
>   r8169: add support for reading over-temp threshold
> 
>  drivers/net/ethernet/realtek/r8169_main.c | 30 +++++++++++++++++------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
These patches are superseded. The temperature sensor is also available
in the standalone version of the integrated PHY, so I'll move the hwmon
support to the Realtek PHY driver.