netdev - Re: [net-next] net: phy: marvell-88q2xxx: Enable temperature sensor for mv88q211x

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9fe63634-9961-451a-b98d-0a9df1eef8b4@redhat.com>
Date: Fri, 21 Mar 2025 21:37:16 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Niklas Söderlund
 <niklas.soderlund+renesas@...natech.se>,
 Heiner Kallweit <hkallweit1@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Russell King <linux@...linux.org.uk>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Dimitri Fedrau <dima.fedrau@...il.com>,
 netdev@...r.kernel.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [net-next] net: phy: marvell-88q2xxx: Enable temperature sensor
 for mv88q211x

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@gmail.com/

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