| 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: <8570eb47-d446-9fcb-1033-682114c9ca59@roeck-us.net>
Date: Tue, 17 Mar 2020 10:18:33 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Tim Harvey <tharvey@...eworks.com>,
Lee Jones <lee.jones@...aro.org>,
Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
Rob Herring <robh+dt@...nel.org>,
Frank Rowand <frowand.list@...il.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Robert Jones <rjones@...eworks.com>
Subject: Re: [PATCH v6 3/3] hwmon: add Gateworks System Controller support
On 3/17/20 10:00 AM, Tim Harvey wrote:
> The Gateworks System Controller has a hwmon sub-component that exposes
> up to 16 ADC's, some of which are temperature sensors, others which are
> voltage inputs. The ADC configuration (register mapping and name) is
> configured via device-tree and varies board to board.
>
> Cc: Guenter Roeck <linux@...ck-us.net>
> Signed-off-by: Tim Harvey <tharvey@...eworks.com>
> ---
> v6:
> - fix size of info field
> - improve pwm output control documentation
> - include unit suffix in divider and offset
> - change adc subnode name to gsc-adc
> - change fan base to fan subnode
> - change adc type to mode
> - fix voltage offset scaling
>
> v5:
> - fix various checkpatch issues
> - correct gsc-hwmon.rst in MAINTAINERS
> - encorporate Gunter's feedback:
> - switch to SENSOR_DEVICE_ATTR_{RW,RO}
> - use tmp value to avoid excessive pointer deference
> - simplify shift operation
> - scale voffset once
> - simplify is_visible function
> - remove empty line at end of file
>
> v4:
> - adjust for uV offset from device-tree
> - remove unnecessary optional write function
> - remove register range check
> - change dev_err prints to use gsc dev
> - hard-code resolution/scaling for raw adcs
> - describe units of ADC resolution
> - move to using pwm<n>_auto_point<m>_{pwm,temp} for FAN PWM
> - ensure space before/after operators
> - remove unnecessary parens
> - remove more debugging
> - add default case and comment for type_voltage
> - remove unnecessary index bounds checks for channel
> - remove unnecessary clearing of struct fields
> - added Documentation/hwmon/gsc-hwmon.rst
>
> v3:
> - add voltage_raw input type and supporting fields
> - add channel validation to is_visible function
> - remove unnecessary channel validation from read/write functions
>
> v2:
> - change license comment style
> - remove DEBUG
> - simplify regmap_bulk_read err check
> - remove break after returns in switch statement
> - fix fan setpoint buffer address
> - remove unnecessary parens
> - consistently use struct device *dev pointer
> - change license/comment block
> - add validation for hwmon child node props
> - move parsing of of to own function
> - use strlcpy to ensure null termination
> - fix static array sizes and removed unnecessary initializers
> - dynamically allocate channels
> - fix fan input label
> - support platform data
> - fixed whitespace issues
> ---
> Documentation/hwmon/gsc-hwmon.rst | 53 +++++
> Documentation/hwmon/index.rst | 1 +
> MAINTAINERS | 3 +
> drivers/hwmon/Kconfig | 9 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/gsc-hwmon.c | 372 ++++++++++++++++++++++++++++++++
> include/linux/platform_data/gsc_hwmon.h | 44 ++++
> 7 files changed, 483 insertions(+)
> create mode 100644 Documentation/hwmon/gsc-hwmon.rst
> create mode 100644 drivers/hwmon/gsc-hwmon.c
> create mode 100644 include/linux/platform_data/gsc_hwmon.h
>
> diff --git a/Documentation/hwmon/gsc-hwmon.rst b/Documentation/hwmon/gsc-hwmon.rst
> new file mode 100644
> index 00000000..ffac392
> --- /dev/null
> +++ b/Documentation/hwmon/gsc-hwmon.rst
> @@ -0,0 +1,53 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver gsc-hwmon
> +=======================
> +
> +Supported chips: Gateworks GSC
> +Datasheet: http://trac.gateworks.com/wiki/gsc
> +Author: Tim Harvey <tharvey@...eworks.com>
> +
> +Description:
> +------------
> +
> +This driver supports hardware monitoring for the temperature sensor,
> +various ADC's connected to the GSC, and optional FAN controller available
> +on some boards.
> +
> +
> +Voltage Monitoring
> +------------------
> +
> +The voltage inputs are scaled either internally or by the driver depending
> +on the GSC version and firmware. The values returned by the driver do not need
> +further scaling. The voltage input labels provide the voltage rail name:
> +
> +inX_input Measured voltage (mV).
> +inX_label Name of voltage rail.
> +
> +
> +Temperature Monitoring
> +----------------------
> +
> +Temperatures are measured with 12-bit or 10-bit resolution and are scaled
> +either internally or by the driver depending on the GSC version and firmware.
> +The values returned by the driver reflect millidegree Celcius:
> +
> +tempX_input Measured temperature.
> +tempX_label Name of temperature input.
> +
> +
> +PWM Output Control
> +------------------
> +
> +The GSC features 1 PWM output that operates in automatic mode where the
> +PWM value will be scalled depending on 6 temperature boundaries.
> +The tempeature boundaries are read-write and in millidegree Celcius and the
> +read-only PWM values range from 0 (off) to 255 (full speed).
> +Fan speed will be set to minimum (off) when the temperature sensor reads
> +less than pwm1_auto_point1_temp and maximum when the temperature sensor
> +equals or exceeds pwm1_auto_point6_temp.
> +
> +pwm1_auto_point[1-6]_pwm PWM value.
> +pwm1_auto_point[1-6]_temp Temperature boundary.
> +
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 43cc605..a4fab69 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -58,6 +58,7 @@ Hardware Monitoring Kernel Drivers
> ftsteutates
> g760a
> g762
> + gsc-hwmon
> gl518sm
> hih6130
> ibmaem
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bb79b60..3f15542 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6846,6 +6846,9 @@ S: Maintained
> F: Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml
> F: drivers/mfd/gateworks-gsc.c
> F: include/linux/mfd/gsc.h
> +F: Documentation/hwmon/gsc-hwmon.rst
> +F: drivers/hwmon/gsc-hwmon.c
> +F: include/linux/platform_data/gsc_hwmon.h
>
> GCC PLUGINS
> M: Kees Cook <keescook@...omium.org>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 23dfe84..99dae13 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -494,6 +494,15 @@ config SENSORS_F75375S
> This driver can also be built as a module. If so, the module
> will be called f75375s.
>
> +config SENSORS_GSC
> + tristate "Gateworks System Controller ADC"
> + depends on MFD_GATEWORKS_GSC
> + help
> + Support for the Gateworks System Controller A/D converters.
> +
> + To compile this driver as a module, choose M here:
> + the module will be called gsc-hwmon.
> +
> config SENSORS_MC13783_ADC
> tristate "Freescale MC13783/MC13892 ADC"
> depends on MFD_MC13XXX
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 6db5db9..259cba7 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -71,6 +71,7 @@ obj-$(CONFIG_SENSORS_G760A) += g760a.o
> obj-$(CONFIG_SENSORS_G762) += g762.o
> obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
> obj-$(CONFIG_SENSORS_GL520SM) += gl520sm.o
> +obj-$(CONFIG_SENSORS_GSC) += gsc-hwmon.o
> obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
> obj-$(CONFIG_SENSORS_HIH6130) += hih6130.o
> obj-$(CONFIG_SENSORS_ULTRA45) += ultra45_env.o
> diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
> new file mode 100644
> index 00000000..c498786
> --- /dev/null
> +++ b/drivers/hwmon/gsc-hwmon.c
> @@ -0,0 +1,372 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Gateworks System Controller Hardware Monitor module
> + *
> + * Copyright (C) 2020 Gateworks Corporation
> + */
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/mfd/gsc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include <linux/platform_data/gsc_hwmon.h>
> +
> +#define GSC_HWMON_MAX_TEMP_CH 16
> +#define GSC_HWMON_MAX_IN_CH 16
> +
> +#define GSC_HWMON_RESOLUTION 12
> +#define GSC_HWMON_VREF 2500
> +
> +struct gsc_hwmon_data {
> + struct gsc_dev *gsc;
> + struct device *dev;
What is 'dev' used for ? I don't think it is necessary.
Guenter
Powered by blists - more mailing lists