* Re: [PATCH net-next] net: phy: realtek: register hwmon thermal zones
2026-11-03 11:21 [PATCH net-next] net: phy: realtek: register hwmon thermal zones Hleb Shauchenka via B4 Relay
@ 2026-11-03 13:41 ` Andrew Lunn
2026-11-06 21:25 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Lunn @ 2026-11-03 13:41 UTC (permalink / raw)
To: me
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
On Tue, Nov 03, 2026 at 12:21:28PM +0100, Hleb Shauchenka via B4 Relay wrote:
> From: Hleb Shauchenka <me@marleb.org>
>
> The Realtek PHY hwmon device exposes the PHY temperature, but its
> channel info lacks HWMON_C_REGISTER_TZ, so the hwmon core does not
> register the sensor with the thermal framework and device-tree
> thermal zones that reference the PHY cannot bind.
>
> Set the flag, as the Aquantia PHY driver does. This lets the RTL8261N
> sensors on the Zyxel XGS1250-12 B1 drive the chassis fan through
> thermal cooling maps.
>
> Tested on a Zyxel XGS1250-12 B1 with OpenWrt's Linux 6.18.55: the
> three PHY thermal zones register, and an emulated 85 C reading on one
> of them (CONFIG_THERMAL_EMULATION) switches the fan on through the
> existing 80 C trip and off again once cleared.
>
> This patch was prepared with AI assistance (Claude Opus 5.5 and Sol
> 6.1), which also helped investigate the problem and run the tests on
> my switch.
>
> Signed-off-by: Hleb Shauchenka <me@marleb.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net-next] net: phy: realtek: register hwmon thermal zones
2026-11-03 11:21 [PATCH net-next] net: phy: realtek: register hwmon thermal zones Hleb Shauchenka via B4 Relay
2026-11-03 13:41 ` Andrew Lunn
@ 2026-11-06 21:25 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-11-06 21:25 UTC (permalink / raw)
To: me
Cc: Jakub Kicinski, andrew, hkallweit1, linux, davem, edumazet,
pabeni, netdev, linux-kernel
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: phy: realtek: register hwmon thermal zones
This commit adds HWMON_C_REGISTER_TZ to the channel info of the Realtek
PHY hwmon device. With it, the hwmon core registers the PHY temperature
sensor with the thermal framework, so device-tree thermal zones that
reference the PHY can bind.
> Set the flag, as the Aquantia PHY driver does. This lets the RTL8261N
> sensors on the Zyxel XGS1250-12 B1 drive the chassis fan through
> thermal cooling maps.
>
> Tested on a Zyxel XGS1250-12 B1 with OpenWrt's Linux 6.18.55: the
> three PHY thermal zones register, and an emulated 85 C reading on one
> of them (CONFIG_THERMAL_EMULATION) switches the fan on through the
> existing 80 C trip and off again once cleared.
[Severity: Medium]
Does this description hold for the mainline driver?
rtl822x_hwmon_init() has only one caller, rtl822x_probe() in
drivers/net/phy/realtek/realtek_main.c:
if (IS_ENABLED(CONFIG_REALTEK_PHY_HWMON) &&
phydev->phy_id != RTL_GENERIC_PHYID)
return rtl822x_hwmon_init(phydev);
Only four entries use that probe: RTL8221B-VB-CG, RTL8221B-VM-CG,
RTL8251B and Realtek Internal NBASE-T PHY.
The RTL8261C/CE/D_VM entries use rtl8261x_probe(). That probe only
allocates priv and picks a firmware name. It never calls
rtl822x_hwmon_init(). The driver table also has no RTL8261N ID.
An external C45 PHY cannot fall back to the internal NBASE-T entry
either, because rtl_internal_nbaset_match_phy_device() has:
if (phydev->is_c45)
return false;
...
return rtlgen_supports_2_5gbps(phydev) && !rtlgen_supports_mmd(phydev);
So on mainline, no RTL8261 PHY seems to register a hwmon device. The
three thermal zones seen in testing would then come from out-of-tree
OpenWrt PHY support.
Could the commit message name the PHYs that actually gain thermal zone
registration (RTL8221B, RTL8251B and the internal NBASE-T PHYs)? Or
could it say that RTL8261 hwmon support is a missing prerequisite?
> diff --git a/drivers/net/phy/realtek/realtek_hwmon.c b/drivers/net/phy/realtek/realtek_hwmon.c
> index ac96e2d1ebe82..e6235df288c32 100644
> --- a/drivers/net/phy/realtek/realtek_hwmon.c
> +++ b/drivers/net/phy/realtek/realtek_hwmon.c
> @@ -51,6 +51,7 @@ static const struct hwmon_ops rtl822x_hwmon_ops = {
> };
>
> static const struct hwmon_channel_info * const rtl822x_hwmon_info[] = {
> + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
[Severity: Low]
Does this need a DT binding update to go with it?
With HWMON_C_REGISTER_TZ set, hwmon_thermal_add_sensor() calls
devm_thermal_of_zone_register() on the PHY's of_node. Thermal-of finds
zones through thermal-sensors phandles, so the PHY node needs a
#thermal-sensor-cells property.
None of the relevant schemas seem to allow that property:
realtek,rtl9301-mdio.yaml is the RTL930x MDIO binding used by the
board in the commit message. Its ethernet-phy@ children are
$ref: ethernet-phy.yaml# with unevaluatedProperties: false.
realtek,rtl8224-package.yaml restricts its PHY children the same way.
realtek,rtl82xx.yaml, which covers the
ethernet-phy-id001c.c849/c84a/c862 compatibles, ends with
unevaluatedProperties: false.
ethernet-phy.yaml does not evaluate any thermal property.
Would a DTS that adds #thermal-sensor-cells = <0> to an ethernet-phy@N
node under a realtek,rtl9301-mdio bus fail dtbs_check?
The Aquantia and Marvell PHYs have the same gap, but this patch makes
Realtek PHYs new users of it.
[Severity: Medium]
With this flag, rtl822x_hwmon_read() becomes the get_temp backend of a
thermal zone that drives cooling devices. Should it check phy_read_mmd()
for errors now?
drivers/net/phy/realtek/realtek_hwmon.c:rtl822x_hwmon_read() {
...
case hwmon_temp_input:
raw = phy_read_mmd(phydev, MDIO_MMD_VEND2, RTL822X_VND2_TSRR) & 0x3ff;
*val = rtl822x_hwmon_get_temp(raw);
break;
...
return 0;
}
The mask turns a negative errno into a plausible temperature, and the
function returns success. For example, -EIO becomes 0x3fb, which reads
as -2500 millidegrees. -ETIMEDOUT reads as -55000.
hwmon_thermal_get_temp() treats only ret < 0 as a failure:
ret = hwdev->chip->ops->read(tdata->dev, hwmon_temp, hwmon_temp_input,
tdata->index, &t);
if (ret < 0)
return ret;
*temp = t;
If one MDIO read fails while the PHY is hot, the thermal core gets a
sub-zero reading. Could the governor then switch the fan off for at
least one polling interval? If the errno were returned, the thermal
core would keep its previous state.
The hwmon_temp_max path, which reads TSSR and shifts by 6, has the same
unchecked pattern, although thermal does not use it.
The masking itself goes back to commit 33700ca45b7d2e ("net: phy:
realtek: add hwmon support for temp sensor on RTL822x"). This patch is
what connects it to a cooling control loop.
> HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_MAX),
> NULL
> };
^ permalink raw reply [flat|nested] 3+ messages in thread