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]

Date:   Sun, 24 Feb 2019 21:29:52 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Heiner Kallweit <hkallweit1@...il.com>
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

> 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.

    Andrew