Re: [PATCH net-next] net: phy: realtek: register hwmon thermal zones

[Hleb Shauchenka <me@marleb.org>]

On Fri, Nov 06, 2026, Jakub Kicinski wrote:
 > 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.

Correct. On that board OpenWrt's out-of-tree RTL8261N support calls
rtl822x_hwmon_init(). In mainline the flag reaches the RTL8221B-VB-CG,
RTL8221B-VM-CG and RTL8251B when they have a DT node. The internal
NBASE-T PHYs normally have none, so the hwmon core skips them. I will
reword the commit message in v2.

 > Would a DTS that adds #thermal-sensor-cells = <0> to an ethernet-phy@N
 > node under a realtek,rtl9301-mdio bus fail dtbs_check?

Yes. dt-schema drops the additionalProperties: true of ethernet-phy.yaml,
so nothing evaluates the property. v2 will add #thermal-sensor-cells to
ethernet-phy.yaml, which also covers the Aquantia and Marvell PHYs.

 > If the errno were returned, the thermal core would keep its previous
 > state.

Agreed. v2 will return the phy_read_mmd() error from both reads, as a
separate patch with a Fixes tag for 33700ca45b7d.

On 06/11/2026 22:25, Jakub Kicinski wrote:
> 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
>>   };