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

[Andrew Lunn <andrew@lunn.ch>]

> 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@cogentembedded.com>
> + * Author: Andrew Lunn <andrew@lunn.ch>
> + * Author: Heiner Kallweit <hkallweit1@gmail.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