netdev - Re: [PATCH net-next 09/12] mlxsw: core: Extend hwmon interface with fan fault attribute

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <470dd874-a3b8-3885-5077-f2b16d1a63ca@roeck-us.net>
Date:   Wed, 13 Feb 2019 07:02:38 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Andrew Lunn <andrew@...n.ch>, Ido Schimmel <idosch@...lanox.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Jiri Pirko <jiri@...lanox.com>, mlxsw <mlxsw@...lanox.com>,
        Vadim Pasternak <vadimp@...lanox.com>
Subject: Re: [PATCH net-next 09/12] mlxsw: core: Extend hwmon interface with
 fan fault attribute

On 2/13/19 5:53 AM, Andrew Lunn wrote:
> On Wed, Feb 13, 2019 at 11:28:53AM +0000, Ido Schimmel wrote:
>> From: Vadim Pasternak <vadimp@...lanox.com>
>>
>> Add new fan hwmon attribute for exposing fan faults (fault indication is
>> read from Fan Out of Range Event Register).
>>
>> Signed-off-by: Vadim Pasternak <vadimp@...lanox.com>
>> Reviewed-by: Jiri Pirko <jiri@...lanox.com>
>> Signed-off-by: Ido Schimmel <idosch@...lanox.com>
> 
> Hi Ido
> 
> You should include the HWMON maintainer in the Cc: list.
> 
> I would not be too surprised if he says to use
> hwmon_device_register_with_info().
> 

I would ask to do that for new drivers, but this is is not a new driver.
On top of that, I wasn't included in its initial review. Since I wasn't
involved, I have no idea what shape the driver is in, and for sure won't
review it now (to retain my sanity).

Only comment I have is that using the _with_info API and using devm_
functions might simplify the driver a lot. I'll be happy to do a review
if/when that is done.

Guenter