| 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
| ||
|
Message-ID: <609715ff-973a-484d-9267-ff80be7e36f3@roeck-us.net>
Date: Mon, 11 Nov 2024 11:31:14 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Thomas Richard <thomas.richard@...tlin.com>,
Jean Delvare <jdelvare@...e.com>, Lee Jones <lee@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org,
thomas.petazzoni@...tlin.com, blake.vermeer@...sight.com
Subject: Re: [PATCH v2 1/2] hwmon: Add Congatec Board Controller monitoring
driver
On 11/8/24 07:08, Thomas Richard wrote:
> Add support for the Congatec Board Controller. This controller exposes
> temperature, voltage, current and fan sensors.
>
> The available sensors list cannot be predicted. Some sensors can be
> present or not, depending the system.
> The driver has an internal list of all possible sensors, for all Congatec
> boards. The Board Controller gives to the driver its sensors list, and
> their status (active or not).
>
> Signed-off-by: Thomas Richard <thomas.richard@...tlin.com>
> ---
> MAINTAINERS | 1 +
> drivers/hwmon/Kconfig | 9 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/cgbc-hwmon.c | 314 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 325 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3507df3381b1..5e96646593b1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5784,6 +5784,7 @@ CONGATEC BOARD CONTROLLER MFD DRIVER
> M: Thomas Richard <thomas.richard@...tlin.com>
> S: Maintained
> F: drivers/gpio/gpio-cgbc.c
> +F: drivers/hwmon/cgbc-hwmon.c
> F: drivers/i2c/busses/i2c-cgbc.c
> F: drivers/mfd/cgbc-core.c
> F: drivers/watchdog/cgbc_wdt.c
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index cfb4e9314c62..c7b6e93aeb9b 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -463,6 +463,15 @@ config SENSORS_BT1_PVT_ALARMS
> the data conversion will be periodically performed and the data will be
> saved in the internal driver cache.
>
> +config SENSORS_CGBC
> + tristate "Congatec Board Controller Sensors"
> + depends on MFD_CGBC
> + help
> + Enables sensors support for the Congatec Board Controller.
> +
> + This driver can also be built as a module. If so, the module will be
> + called cgbc-hwmon.
> +
> config SENSORS_CHIPCAP2
> tristate "Amphenol ChipCap 2 relative humidity and temperature sensor"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index b827b92f2a78..318da6d8f752 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -59,6 +59,7 @@ obj-$(CONFIG_SENSORS_ASUS_ROG_RYUJIN) += asus_rog_ryujin.o
> obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
> obj-$(CONFIG_SENSORS_AXI_FAN_CONTROL) += axi-fan-control.o
> obj-$(CONFIG_SENSORS_BT1_PVT) += bt1-pvt.o
> +obj-$(CONFIG_SENSORS_CGBC) += cgbc-hwmon.o
> obj-$(CONFIG_SENSORS_CHIPCAP2) += chipcap2.o
> obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
> obj-$(CONFIG_SENSORS_CORSAIR_CPRO) += corsair-cpro.o
> diff --git a/drivers/hwmon/cgbc-hwmon.c b/drivers/hwmon/cgbc-hwmon.c
> new file mode 100644
> index 000000000000..6352d3f38516
> --- /dev/null
> +++ b/drivers/hwmon/cgbc-hwmon.c
> @@ -0,0 +1,314 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * cgbc-hwmon - Congatec Board Controller hardware monitoring driver
> + *
> + * Copyright (C) 2024 Thomas Richard <thomas.richard@...tlin.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/hwmon.h>
> +#include <linux/mfd/cgbc.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#define CGBC_HWMON_CMD_SENSOR 0x77
> +#define CGBC_HWMON_CMD_SENSOR_DATA_SIZE 0x05
> +
> +#define CGBC_HWMON_TYPE_MASK GENMASK(6, 5)
> +#define CGBC_HWMON_ID_MASK GENMASK(4, 0)
> +#define CGBC_HWMON_ACTIVE_BIT BIT(7)
> +
> +struct cgbc_hwmon_sensor {
> + enum hwmon_sensor_types type;
> + bool active;
> + unsigned int index;
> + unsigned int channel;
> + const char *label;
> +};
> +
> +struct cgbc_hwmon_data {
> + struct cgbc_device_data *cgbc;
> + unsigned int nb_sensors;
> + struct cgbc_hwmon_sensor *sensors;
> +};
> +
> +enum cgbc_sensor_types {
> + CGBC_HWMON_TYPE_TEMP = 1,
> + CGBC_HWMON_TYPE_IN,
> + CGBC_HWMON_TYPE_FAN
> +};
> +
> +static const char * const cgbc_hwmon_labels_temp[] = {
> + "CPU Temperature",
> + "Box Temperature",
> + "Ambient Temperature",
> + "Board Temperature",
> + "Carrier Temperature",
> + "Chipset Temperature",
> + "Video Temperature",
> + "Other Temperature",
> + "TOPDIM Temperature",
> + "BOTTOMDIM Temperature",
> +};
> +
> +static const struct {
> + enum hwmon_sensor_types type;
> + const char *label;
> +} cgbc_hwmon_labels_in[] = {
> + { hwmon_in, "CPU Voltage" },
> + { hwmon_in, "DC Runtime Voltage" },
> + { hwmon_in, "DC Standby Voltage" },
> + { hwmon_in, "CMOS Battery Voltage" },
> + { hwmon_in, "Battery Voltage" },
> + { hwmon_in, "AC Voltage" },
> + { hwmon_in, "Other Voltage" },
> + { hwmon_in, "5V Voltage" },
> + { hwmon_in, "5V Standby Voltage" },
> + { hwmon_in, "3V3 Voltage" },
> + { hwmon_in, "3V3 Standby Voltage" },
> + { hwmon_in, "VCore A Voltage" },
> + { hwmon_in, "VCore B Voltage" },
> + { hwmon_in, "12V Voltage" },
> + { hwmon_curr, "DC Current" },
> + { hwmon_curr, "5V Current" },
> + { hwmon_curr, "12V Current" },
> +};
> +
> +#define cgbc_hwmon_compute_curr_channel(channel) \
> + ({ \
> + int i, cnt = 0; \
> + for (i = 0; i < ARRAY_SIZE(cgbc_hwmon_labels_in); i++) { \
> + if (cgbc_hwmon_labels_in[i].type == hwmon_in) \
> + cnt++; \
> + } \
> + (channel) + cnt; \
> + })
> +
From Documentation/hwmon/submitting-patches.rst:
* Avoid calculations in macros and macro-generated functions. While such macros
may save a line or so in the source, it obfuscates the code and makes code
review more difficult. It may also result in code which is more complicated
than necessary. Use inline functions or just regular functions instead.
I don't understand why this would be needed anyway. The current channel
index is well known, so adding or subtracting a constant should do it.
Guenter
Powered by blists - more mailing lists