Hi, On 3/16/25 1:02 PM, Niklas Söderlund wrote: > On 2025-03-16 12:47:55 +0100, Heiner Kallweit wrote: >> On 16.03.2025 12:20, Niklas Söderlund wrote: >>> The temperature sensor enabled for mv88q222x devices also functions for >>> mv88q211x based devices. Unify the two devices probe functions to enable >>> the sensors for all devices supported by this driver. >>> >>> The same oddity as for mv88q222x devices exists, the PHY must be up for >>> a correct temperature reading to be reported. >>> >> In this case, wouldn't it make sense to extend mv88q2xxx_hwmon_is_visible() >> and hide the temp_input attribute if PHY is down? >> Whatever down here means in detail: Link down? In power-down mode? > > These are good suggestions, this issue is being worked on [1]. I just > wanted to highlight that this entablement behaves the same as the > current models that support the temperature sensor and log how this was > tested on mv88q211x. > > 1. https://lore.kernel.org/all/20250220-marvell-88q2xxx-hwmon-enable-at-probe-v2-0-78b2838a62da@xxxxxxxxx/ My take is that you should at least clarify in the commit message the state required for a correct reading - e.g. link up vs power-up. Thanks, Paolo