netdev - Re: [PATCH net-next 2/2] net: phy: aquantia: add hwmon support

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: <d934bb9a-5edc-6a51-3012-aab9c7d40768@gmail.com>
Date:   Sun, 24 Feb 2019 22:14:46 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        David Miller <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/2] net: phy: aquantia: add hwmon support

On 24.02.2019 21:29, Andrew Lunn wrote:
>> diff --git a/drivers/net/phy/aquantia_hwmon.c b/drivers/net/phy/aquantia_hwmon.c
>> new file mode 100644
>> index 000000000..c0dd695f6
>> --- /dev/null
>> +++ b/drivers/net/phy/aquantia_hwmon.c
>> @@ -0,0 +1,263 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* HWMON driver for Aquantia PHY
>> + *
>> + * Author: Nikita Yushchenko <nikita.yoush@...entembedded.com>
>> + * Author: Andrew Lunn <andrew@...n.ch>
>> + * Author: Heiner Kallweit <hkallweit1@...il.com>
>> + */
>> +
>> +#include <linux/phy.h>
>> +#include <linux/device.h>
>> +#include <linux/ctype.h>
>> +#include <linux/hwmon.h>
>> +
>> +#include "aquantia_hwmon.h"
>> +
>> +/* Vendor specific 1, MDIO_MMD_VEND1 */
>> +#define VEND1_THERMAL_PROV_HIGH_TEMP_FAIL	0xc421
>> +#define VEND1_THERMAL_PROV_LOW_TEMP_FAIL	0xc422
>> +#define VEND1_THERMAL_PROV_HIGH_TEMP_WARN	0xc423
>> +#define VEND1_THERMAL_PROV_LOW_TEMP_WARN	0xc424
>> +#define VEND1_THERMAL_STAT1			0xc820
>> +#define VEND1_THERMAL_STAT2			0xc821
>> +#define VEND1_THERMAL_STAT2_VALID		BIT(0)
>> +#define VEND1_GENERAL_STAT1			0xc830
>> +#define VEND1_GENERAL_STAT1_HIGH_TEMP_FAIL	BIT(14)
>> +#define VEND1_GENERAL_STAT1_LOW_TEMP_FAIL	BIT(13)
>> +#define VEND1_GENERAL_STAT1_HIGH_TEMP_WARN	BIT(12)
>> +#define VEND1_GENERAL_STAT1_LOW_TEMP_WARN	BIT(11)
>> +
>> +struct aqr_priv {
>> +	struct device *hwmon_dev;
>> +	char *hwmon_name;
>> +};
> 
> Hi Heiner
> 
> It seems a bit odd having the driver private structure here. I expect
> with time we are going to need other things in it which are not
> HWMON. e.g many of the statistics counters are clear on read. So we
> need to keep the running totals somewhere.
> 
> I would keep the probe code and the allocation of this structure in
> the main driver file.
> 
I just see that we don't need struct aqr_priv at all for hwmon
because it uses device-managed versions of the relevant functions.
So we can add such a struct when it's actually needed.

>     Andrew
> 
Heiner