From: Tim Harvey <tharvey@gateworks.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Lee Jones <lee.jones@linaro.org>,
Jean Delvare <jdelvare@suse.com>,
Linux HWMON List <linux-hwmon@vger.kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Frank Rowand <frowand.list@gmail.com>,
Device Tree Mailing List <devicetree@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>,
Robert Jones <rjones@gateworks.com>
Subject: Re: [PATCH v6 3/3] hwmon: add Gateworks System Controller support
Date: Tue, 17 Mar 2020 10:44:06 -0700 [thread overview]
Message-ID: <CAJ+vNU0oc17q-Ajgh29hAZRNeTa+CeRfKjz22-33XqHOdYwQyA@mail.gmail.com> (raw)
In-Reply-To: <8570eb47-d446-9fcb-1033-682114c9ca59@roeck-us.net>
On Tue, Mar 17, 2020 at 10:18 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> 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@roeck-us.net>
> > Signed-off-by: Tim Harvey <tharvey@gateworks.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@gateworks.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@chromium.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.
>
Agreed - will remove.
Thanks!
Tim
prev parent reply other threads:[~2020-03-17 17:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-17 17:00 [PATCH v6 0/3] Add support for the Gateworks System Controller Tim Harvey
2020-03-17 17:00 ` [PATCH v6 1/3] dt-bindings: mfd: Add Gateworks System Controller bindings Tim Harvey
2020-03-20 3:15 ` Rob Herring
2020-03-17 17:00 ` [PATCH v6 2/3] mfd: add Gateworks System Controller core driver Tim Harvey
2020-03-17 17:00 ` [PATCH v6 3/3] hwmon: add Gateworks System Controller support Tim Harvey
2020-03-17 17:03 ` Randy Dunlap
2020-03-17 17:07 ` Tim Harvey
2020-03-17 17:18 ` Guenter Roeck
2020-03-17 17:44 ` Tim Harvey [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJ+vNU0oc17q-Ajgh29hAZRNeTa+CeRfKjz22-33XqHOdYwQyA@mail.gmail.com \
--to=tharvey@gateworks.com \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=jdelvare@suse.com \
--cc=lee.jones@linaro.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=rjones@gateworks.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).