[4/8] hwmon: Add support for Azoteq IQS620AT temperature sensor

Commit Message

Jeff LaBundy Oct. 21, 2019, 4:11 a.m. UTC

This patch adds support for the Azoteq IQS620AT temperature sensor,
capable of reporting its absolute die temperature.

Signed-off-by: Jeff LaBundy <[e-mail]>
---
 drivers/hwmon/Kconfig         | 12 +++++-
 drivers/hwmon/Makefile        |  1 +
 drivers/hwmon/iqs620at-temp.c | 96 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hwmon/iqs620at-temp.c

Comments

Guenter Roeck Oct. 21, 2019, 3:38 p.m. UTC | #1

On Sun, Oct 20, 2019 at 11:11:19PM -0500, Jeff LaBundy wrote:
> This patch adds support for the Azoteq IQS620AT temperature sensor,
> capable of reporting its absolute die temperature.
> 
> Signed-off-by: Jeff LaBundy <[e-mail]>

Seems to me this might be more feasible as iio driver.
Jonathan, what do you think ?

> ---
>  drivers/hwmon/Kconfig         | 12 +++++-
>  drivers/hwmon/Makefile        |  1 +
>  drivers/hwmon/iqs620at-temp.c | 96 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 108 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/hwmon/iqs620at-temp.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 13a6b4a..3e56be6 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -385,6 +385,16 @@ config SENSORS_ATXP1
>  	  This driver can also be built as a module. If so, the module
>  	  will be called atxp1.
>  
> +config SENSORS_IQS620AT
> +	tristate "Azoteq IQS620AT temperature sensor"
> +	depends on MFD_IQS62X
> +	help
> +	  Say Y here if you want to build support for the Azoteq IQS620AT
> +	  temperature sensor.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called iqs620at-temp.
> +
>  config SENSORS_DS620
>  	tristate "Dallas Semiconductor DS620"
>  	depends on I2C
> @@ -1593,7 +1603,7 @@ config SENSORS_ADS7871
>  
>  config SENSORS_AMC6821
>  	tristate "Texas Instruments AMC6821"
> -	depends on I2C 
> +	depends on I2C

No unrelated changes, please, and most definitely no
unrelated whitespace changes.

>  	help
>  	  If you say yes here you get support for the Texas Instruments
>  	  AMC6821 hardware monitoring chips.
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 40c036e..2360add 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
>  obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
>  obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
>  obj-$(CONFIG_SENSORS_INA3221)	+= ina3221.o
> +obj-$(CONFIG_SENSORS_IQS620AT)	+= iqs620at-temp.o
>  obj-$(CONFIG_SENSORS_IT87)	+= it87.o
>  obj-$(CONFIG_SENSORS_JC42)	+= jc42.o
>  obj-$(CONFIG_SENSORS_K8TEMP)	+= k8temp.o
> diff --git a/drivers/hwmon/iqs620at-temp.c b/drivers/hwmon/iqs620at-temp.c
> new file mode 100644
> index 0000000..0af49b6
> --- /dev/null
> +++ b/drivers/hwmon/iqs620at-temp.c
> @@ -0,0 +1,96 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Azoteq IQS620AT Temperature Sensor
> + *
> + * Copyright (C) 2019
> + * Author: Jeff LaBundy <[e-mail]>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/iqs62x.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define IQS620_TEMP_UI_OUT			0x1A
> +
> +static umode_t iqs620_temp_is_visible(const void *drvdata,
> +				      enum hwmon_sensor_types type,
> +				      u32 attr, int channel)
> +{
> +	if (type != hwmon_temp || attr != hwmon_temp_input)
> +		return 0;
> +
> +	return 0444;
> +}
> +
> +static int iqs620_temp_read(struct device *dev, enum hwmon_sensor_types type,
> +			    u32 attr, int channel, long *val)
> +{
> +	struct iqs62x_core *iqs62x = dev_get_drvdata(dev);
> +	int error;
> +	__le16 val_buf;
> +
> +	if (type != hwmon_temp || attr != hwmon_temp_input)
> +		return -EINVAL;

			-EOPNOTSUPP
> +
> +	error = regmap_raw_read(iqs62x->map, IQS620_TEMP_UI_OUT, &val_buf,
> +				sizeof(val_buf));
> +	if (error)
> +		return error;
> +
> +	*val = (le16_to_cpu(val_buf) - 100) * 1000;
> +
> +	return 0;
> +}
> +
> +static const struct hwmon_ops iqs620_temp_ops = {
> +	.is_visible = iqs620_temp_is_visible,
> +	.read = iqs620_temp_read,
> +};
> +
> +static const struct hwmon_channel_info *iqs620_temp_channel_info[] = {
> +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
> +	NULL
> +};
> +
> +static const struct hwmon_chip_info iqs620_temp_chip_info = {
> +	.ops = &iqs620_temp_ops,
> +	.info = iqs620_temp_channel_info,
> +};
> +
> +static int iqs620_temp_probe(struct platform_device *pdev)
> +{
> +	struct iqs62x_core *iqs62x = dev_get_drvdata(pdev->dev.parent);
> +	struct device *hdev;
> +	int error = 0;
> +
> +	hdev = devm_hwmon_device_register_with_info(&pdev->dev,
> +						    iqs62x->dev_desc->dev_name,
> +						    iqs62x,
> +						    &iqs620_temp_chip_info,
> +						    NULL);
> +	if (IS_ERR(hdev)) {
> +		error = PTR_ERR(hdev);
> +		dev_err(&pdev->dev, "Failed to register device: %d\n", error);

Such an error would either be static, caused by bad attributes,
or a bad name, which is already logged, or a memory allocation
failure, which is also already logged. The error message does
therefore not add any value.

> +	}
> +
> +	return error;
> +}
> +
> +static struct platform_driver iqs620_temp_platform_driver = {
> +	.driver = {
> +		.name	= IQS620_DRV_NAME_TEMP,
> +	},
> +	.probe		= iqs620_temp_probe,
> +};
> +module_platform_driver(iqs620_temp_platform_driver);
> +
> +MODULE_AUTHOR("Jeff LaBundy <[e-mail]>");
> +MODULE_DESCRIPTION("Azoteq IQS620AT Temperature Sensor");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:" IQS620_DRV_NAME_TEMP);
> -- 
> 2.7.4
>

Jeff LaBundy Oct. 22, 2019, 2:26 a.m. UTC | #2

Hi Guenter,

Thank you for your prompt review.

On Mon, Oct 21, 2019 at 08:38:25AM -0700, Guenter Roeck wrote:
> On Sun, Oct 20, 2019 at 11:11:19PM -0500, Jeff LaBundy wrote:
> > This patch adds support for the Azoteq IQS620AT temperature sensor,
> > capable of reporting its absolute die temperature.
> > 
> > Signed-off-by: Jeff LaBundy <[e-mail]>
> 
> Seems to me this might be more feasible as iio driver.
> Jonathan, what do you think ?
> 

Interestingly enough, this actually started as an iio driver; however the
"When to Use" slide of [0] made me suspect that conventional devices with
the temperature sensing element integrated on the die belong in hwmon.

I then found the highly similar ad7314, which Jonathan himself appears to
have converted from iio to hwmon. Therefore, I placed this where existing
drivers seemed to match the most, especially since the temperature sensors
in iio generally use IR or a thermocouple.

That being said, I would be happy to move this into iio so long as Jonathan
does not mind, as it would limit the blast radius of this patch series.

> > ---
> >  drivers/hwmon/Kconfig         | 12 +++++-
> >  drivers/hwmon/Makefile        |  1 +
> >  drivers/hwmon/iqs620at-temp.c | 96 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 108 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/hwmon/iqs620at-temp.c
> > 
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 13a6b4a..3e56be6 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -385,6 +385,16 @@ config SENSORS_ATXP1
> >  	  This driver can also be built as a module. If so, the module
> >  	  will be called atxp1.
> >  
> > +config SENSORS_IQS620AT
> > +	tristate "Azoteq IQS620AT temperature sensor"
> > +	depends on MFD_IQS62X
> > +	help
> > +	  Say Y here if you want to build support for the Azoteq IQS620AT
> > +	  temperature sensor.
> > +
> > +	  To compile this driver as a module, choose M here: the module
> > +	  will be called iqs620at-temp.
> > +
> >  config SENSORS_DS620
> >  	tristate "Dallas Semiconductor DS620"
> >  	depends on I2C
> > @@ -1593,7 +1603,7 @@ config SENSORS_ADS7871
> >  
> >  config SENSORS_AMC6821
> >  	tristate "Texas Instruments AMC6821"
> > -	depends on I2C 
> > +	depends on I2C
> 
> No unrelated changes, please, and most definitely no
> unrelated whitespace changes.
> 

Sorry about that; it seems the original file had some trailing whitespace
here and my editor cropped it automatically. Unfortunately I didn't catch
it until after I sent out the series.

> >  	help
> >  	  If you say yes here you get support for the Texas Instruments
> >  	  AMC6821 hardware monitoring chips.
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index 40c036e..2360add 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
> >  obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
> >  obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
> >  obj-$(CONFIG_SENSORS_INA3221)	+= ina3221.o
> > +obj-$(CONFIG_SENSORS_IQS620AT)	+= iqs620at-temp.o
> >  obj-$(CONFIG_SENSORS_IT87)	+= it87.o
> >  obj-$(CONFIG_SENSORS_JC42)	+= jc42.o
> >  obj-$(CONFIG_SENSORS_K8TEMP)	+= k8temp.o
> > diff --git a/drivers/hwmon/iqs620at-temp.c b/drivers/hwmon/iqs620at-temp.c
> > new file mode 100644
> > index 0000000..0af49b6
> > --- /dev/null
> > +++ b/drivers/hwmon/iqs620at-temp.c
> > @@ -0,0 +1,96 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Azoteq IQS620AT Temperature Sensor
> > + *
> > + * Copyright (C) 2019
> > + * Author: Jeff LaBundy <[e-mail]>
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mfd/iqs62x.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +
> > +#define IQS620_TEMP_UI_OUT			0x1A
> > +
> > +static umode_t iqs620_temp_is_visible(const void *drvdata,
> > +				      enum hwmon_sensor_types type,
> > +				      u32 attr, int channel)
> > +{
> > +	if (type != hwmon_temp || attr != hwmon_temp_input)
> > +		return 0;
> > +
> > +	return 0444;
> > +}
> > +
> > +static int iqs620_temp_read(struct device *dev, enum hwmon_sensor_types type,
> > +			    u32 attr, int channel, long *val)
> > +{
> > +	struct iqs62x_core *iqs62x = dev_get_drvdata(dev);
> > +	int error;
> > +	__le16 val_buf;
> > +
> > +	if (type != hwmon_temp || attr != hwmon_temp_input)
> > +		return -EINVAL;
> 
> 			-EOPNOTSUPP

Sure thing; will do.

> > +
> > +	error = regmap_raw_read(iqs62x->map, IQS620_TEMP_UI_OUT, &val_buf,
> > +				sizeof(val_buf));
> > +	if (error)
> > +		return error;
> > +
> > +	*val = (le16_to_cpu(val_buf) - 100) * 1000;
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct hwmon_ops iqs620_temp_ops = {
> > +	.is_visible = iqs620_temp_is_visible,
> > +	.read = iqs620_temp_read,
> > +};
> > +
> > +static const struct hwmon_channel_info *iqs620_temp_channel_info[] = {
> > +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> > +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
> > +	NULL
> > +};
> > +
> > +static const struct hwmon_chip_info iqs620_temp_chip_info = {
> > +	.ops = &iqs620_temp_ops,
> > +	.info = iqs620_temp_channel_info,
> > +};
> > +
> > +static int iqs620_temp_probe(struct platform_device *pdev)
> > +{
> > +	struct iqs62x_core *iqs62x = dev_get_drvdata(pdev->dev.parent);
> > +	struct device *hdev;
> > +	int error = 0;
> > +
> > +	hdev = devm_hwmon_device_register_with_info(&pdev->dev,
> > +						    iqs62x->dev_desc->dev_name,
> > +						    iqs62x,
> > +						    &iqs620_temp_chip_info,
> > +						    NULL);
> > +	if (IS_ERR(hdev)) {
> > +		error = PTR_ERR(hdev);
> > +		dev_err(&pdev->dev, "Failed to register device: %d\n", error);
> 
> Such an error would either be static, caused by bad attributes,
> or a bad name, which is already logged, or a memory allocation
> failure, which is also already logged. The error message does
> therefore not add any value.
> 

Sure thing; I'll remove the error message.

> > +	}
> > +
> > +	return error;
> > +}
> > +
> > +static struct platform_driver iqs620_temp_platform_driver = {
> > +	.driver = {
> > +		.name	= IQS620_DRV_NAME_TEMP,
> > +	},
> > +	.probe		= iqs620_temp_probe,
> > +};
> > +module_platform_driver(iqs620_temp_platform_driver);
> > +
> > +MODULE_AUTHOR("Jeff LaBundy <[e-mail]>");
> > +MODULE_DESCRIPTION("Azoteq IQS620AT Temperature Sensor");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:" IQS620_DRV_NAME_TEMP);
> > -- 
> > 2.7.4
> > 
> 

Kind regards,
Jeff LaBundy

[0]: https://elinux.org/images/b/ba/ELC_2017_-_Industrial_IO_and_You-_Nonsense_Hacks%21.pdf

Guenter Roeck Oct. 22, 2019, 3:22 a.m. UTC | #3

On 10/21/19 7:26 PM, Jeff LaBundy wrote:
> Hi Guenter,
> 
> Thank you for your prompt review.
> 
> On Mon, Oct 21, 2019 at 08:38:25AM -0700, Guenter Roeck wrote:
>> On Sun, Oct 20, 2019 at 11:11:19PM -0500, Jeff LaBundy wrote:
>>> This patch adds support for the Azoteq IQS620AT temperature sensor,
>>> capable of reporting its absolute die temperature.
>>>
>>> Signed-off-by: Jeff LaBundy <[e-mail]>
>>
>> Seems to me this might be more feasible as iio driver.
>> Jonathan, what do you think ?
>>
> 
> Interestingly enough, this actually started as an iio driver; however the
> "When to Use" slide of [0] made me suspect that conventional devices with
> the temperature sensing element integrated on the die belong in hwmon.
> 
> I then found the highly similar ad7314, which Jonathan himself appears to
> have converted from iio to hwmon. Therefore, I placed this where existing
> drivers seemed to match the most, especially since the temperature sensors
> in iio generally use IR or a thermocouple.
> 
> That being said, I would be happy to move this into iio so long as Jonathan
> does not mind, as it would limit the blast radius of this patch series.
> 

I don't recall why the ad7314 driver was moved. With a conversion time of 40uS
it is most definitely not a typical use case for a hwmon sensor.

Anyway, not worth arguing about. Just don't complain later. There is an
iio->hwmon bridge, but no hwmon->iio bridge, so the decision does have some
impact. Specifically, userspace will have to implement both hwmon and iio
access to handle the chip.

Guenter

Jonathan Cameron Oct. 22, 2019, 11:38 a.m. UTC | #4

On Mon, 21 Oct 2019 20:22:44 -0700
Guenter Roeck <[e-mail]> wrote:

> On 10/21/19 7:26 PM, Jeff LaBundy wrote:
> > Hi Guenter,
> > 
> > Thank you for your prompt review.
> > 
> > On Mon, Oct 21, 2019 at 08:38:25AM -0700, Guenter Roeck wrote:  
> >> On Sun, Oct 20, 2019 at 11:11:19PM -0500, Jeff LaBundy wrote:  
> >>> This patch adds support for the Azoteq IQS620AT temperature sensor,
> >>> capable of reporting its absolute die temperature.
> >>>
> >>> Signed-off-by: Jeff LaBundy <[e-mail]>  
> >>
> >> Seems to me this might be more feasible as iio driver.
> >> Jonathan, what do you think ?
> >>  
> > 
> > Interestingly enough, this actually started as an iio driver; however the
> > "When to Use" slide of [0] made me suspect that conventional devices with
> > the temperature sensing element integrated on the die belong in hwmon.
> > 
> > I then found the highly similar ad7314, which Jonathan himself appears to
> > have converted from iio to hwmon. Therefore, I placed this where existing
> > drivers seemed to match the most, especially since the temperature sensors
> > in iio generally use IR or a thermocouple.
> > 
> > That being said, I would be happy to move this into iio so long as Jonathan
> > does not mind, as it would limit the blast radius of this patch series.
> >   
> 
> I don't recall why the ad7314 driver was moved. With a conversion time of 40uS
> it is most definitely not a typical use case for a hwmon sensor.

I'll be honest, I can't remember either ;)
> 
> Anyway, not worth arguing about. Just don't complain later. There is an
> iio->hwmon bridge, but no hwmon->iio bridge, so the decision does have some
> impact. Specifically, userspace will have to implement both hwmon and iio
> access to handle the chip.

So I had a very quick look at one of the data sheets.  The temperature sensor
here is described as: 

"The IQS620(A) provides temperature monitoring capabilities which can be used for temperature
change detection in order to ensure the integrity of other sensing technology".

Superficially this sounds like it's probably inappropriate for any sort
of system temperature monitoring.  It's really just there to allow
for clever compensation algorithms for the other bits on this chip
(much like the temperature sensors we almost always get on a decent
IMU).

Normally we'd just tack an extra channel for the temperature sensor on
to the the the sensor it is integrated with.  This is a bit more
complex though as we have 3 different IIO sensors that are present
in particular part numbers and for some cases we have no IIO device
at all, but do have a temperature sensor.

So if people are going to actually use this to compensate outputs
(not sure which ones are actually temperature sensitive btw ;)
then if those are IIO supported devices, then probably makes sense
for this to be an IIO device.  It may make sense anyway if there
is any chance of adding temperature compensation to the drivers
in kernel.  I suspect the only use that would actually be made
is as a trip point if something has gone horribly wrong, but
I might be wrong!

Conclusion. I also don't feel strongly on this one as it kind of
sits between IIO and hwmon, but probably ever so slightly on the
IIO side as monitoring a sensor chip, not some other device.

Thanks,

Jonathan

> 
> Guenter

Jeff LaBundy Oct. 23, 2019, 2:04 a.m. UTC | #5

Hi Jonathan and Guenter,

On Tue, Oct 22, 2019 at 12:38:38PM +0100, Jonathan Cameron wrote:
> On Mon, 21 Oct 2019 20:22:44 -0700
> Guenter Roeck <[e-mail]> wrote:
> 
> > On 10/21/19 7:26 PM, Jeff LaBundy wrote:
> > > Hi Guenter,
> > > 
> > > Thank you for your prompt review.
> > > 
> > > On Mon, Oct 21, 2019 at 08:38:25AM -0700, Guenter Roeck wrote:  
> > >> On Sun, Oct 20, 2019 at 11:11:19PM -0500, Jeff LaBundy wrote:  
> > >>> This patch adds support for the Azoteq IQS620AT temperature sensor,
> > >>> capable of reporting its absolute die temperature.
> > >>>
> > >>> Signed-off-by: Jeff LaBundy <[e-mail]>  
> > >>
> > >> Seems to me this might be more feasible as iio driver.
> > >> Jonathan, what do you think ?
> > >>  
> > > 
> > > Interestingly enough, this actually started as an iio driver; however the
> > > "When to Use" slide of [0] made me suspect that conventional devices with
> > > the temperature sensing element integrated on the die belong in hwmon.
> > > 
> > > I then found the highly similar ad7314, which Jonathan himself appears to
> > > have converted from iio to hwmon. Therefore, I placed this where existing
> > > drivers seemed to match the most, especially since the temperature sensors
> > > in iio generally use IR or a thermocouple.
> > > 
> > > That being said, I would be happy to move this into iio so long as Jonathan
> > > does not mind, as it would limit the blast radius of this patch series.
> > >   
> > 
> > I don't recall why the ad7314 driver was moved. With a conversion time of 40uS
> > it is most definitely not a typical use case for a hwmon sensor.
> 
> I'll be honest, I can't remember either ;)
> > 
> > Anyway, not worth arguing about. Just don't complain later. There is an
> > iio->hwmon bridge, but no hwmon->iio bridge, so the decision does have some
> > impact. Specifically, userspace will have to implement both hwmon and iio
> > access to handle the chip.
> 
> So I had a very quick look at one of the data sheets.  The temperature sensor
> here is described as: 
> 
> "The IQS620(A) provides temperature monitoring capabilities which can be used for temperature
> change detection in order to ensure the integrity of other sensing technology".
> 
> Superficially this sounds like it's probably inappropriate for any sort
> of system temperature monitoring.  It's really just there to allow
> for clever compensation algorithms for the other bits on this chip
> (much like the temperature sensors we almost always get on a decent
> IMU).
> 

Correct on all counts. The "charge transfer" sensing mechanism employed by these
devices is sensitive to temperature, and they employ a compensation algorithm to
account for drift.

Of the five devices in the series, the IQS620A and IQS621 expose the output of
the temperature monitoring network to the outside world. However, the values are
purely relative. The IQS620AT, however, is calibrated at the factory such that
it can apply an internal scaling factor and offset in order to provide absolute
measurements.

The MFD driver checks these calibration values to determine if this driver can
be loaded, or if the device is a plain IQS620A (no 'T') in which case only the
input and PWM drivers are loaded.

> Normally we'd just tack an extra channel for the temperature sensor on
> to the the the sensor it is integrated with.  This is a bit more
> complex though as we have 3 different IIO sensors that are present
> in particular part numbers and for some cases we have no IIO device
> at all, but do have a temperature sensor.
> 
> So if people are going to actually use this to compensate outputs
> (not sure which ones are actually temperature sensitive btw ;)
> then if those are IIO supported devices, then probably makes sense
> for this to be an IIO device.  It may make sense anyway if there
> is any chance of adding temperature compensation to the drivers
> in kernel.  I suspect the only use that would actually be made
> is as a trip point if something has gone horribly wrong, but
> I might be wrong!
> 

Correct again; in my opinion this device is unlikely to be chosen for any sort
of system-level temperature monitoring. It's really meant for contactless key/
button/switch sensing and PWM control; a subset of the devices simply offer the
internal temperature measurement to the outside world as a bonus in case it is
useful. Hence, this patch.

> Conclusion. I also don't feel strongly on this one as it kind of
> sits between IIO and hwmon, but probably ever so slightly on the
> IIO side as monitoring a sensor chip, not some other device.
> 

Agreed on all counts; I'll move this to iio. Thank you both for the discussion.

> Thanks,
> 
> Jonathan
> 
> > 
> > Guenter
> 
> 

Kind regards,
Jeff LaBundy

Patch

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 13a6b4a..3e56be6 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -385,6 +385,16 @@  config SENSORS_ATXP1
 	  This driver can also be built as a module. If so, the module
 	  will be called atxp1.
 
+config SENSORS_IQS620AT
+	tristate "Azoteq IQS620AT temperature sensor"
+	depends on MFD_IQS62X
+	help
+	  Say Y here if you want to build support for the Azoteq IQS620AT
+	  temperature sensor.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called iqs620at-temp.
+
 config SENSORS_DS620
 	tristate "Dallas Semiconductor DS620"
 	depends on I2C
@@ -1593,7 +1603,7 @@  config SENSORS_ADS7871
 
 config SENSORS_AMC6821
 	tristate "Texas Instruments AMC6821"
-	depends on I2C 
+	depends on I2C
 	help
 	  If you say yes here you get support for the Texas Instruments
 	  AMC6821 hardware monitoring chips.
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 40c036e..2360add 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -83,6 +83,7 @@  obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
 obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
 obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
 obj-$(CONFIG_SENSORS_INA3221)	+= ina3221.o
+obj-$(CONFIG_SENSORS_IQS620AT)	+= iqs620at-temp.o
 obj-$(CONFIG_SENSORS_IT87)	+= it87.o
 obj-$(CONFIG_SENSORS_JC42)	+= jc42.o
 obj-$(CONFIG_SENSORS_K8TEMP)	+= k8temp.o
diff --git a/drivers/hwmon/iqs620at-temp.c b/drivers/hwmon/iqs620at-temp.c
new file mode 100644
index 0000000..0af49b6
--- /dev/null
+++ b/drivers/hwmon/iqs620at-temp.c
@@ -0,0 +1,96 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Azoteq IQS620AT Temperature Sensor
+ *
+ * Copyright (C) 2019
+ * Author: Jeff LaBundy <[e-mail]>
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/kernel.h>
+#include <linux/mfd/iqs62x.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define IQS620_TEMP_UI_OUT			0x1A
+
+static umode_t iqs620_temp_is_visible(const void *drvdata,
+				      enum hwmon_sensor_types type,
+				      u32 attr, int channel)
+{
+	if (type != hwmon_temp || attr != hwmon_temp_input)
+		return 0;
+
+	return 0444;
+}
+
+static int iqs620_temp_read(struct device *dev, enum hwmon_sensor_types type,
+			    u32 attr, int channel, long *val)
+{
+	struct iqs62x_core *iqs62x = dev_get_drvdata(dev);
+	int error;
+	__le16 val_buf;
+
+	if (type != hwmon_temp || attr != hwmon_temp_input)
+		return -EINVAL;
+
+	error = regmap_raw_read(iqs62x->map, IQS620_TEMP_UI_OUT, &val_buf,
+				sizeof(val_buf));
+	if (error)
+		return error;
+
+	*val = (le16_to_cpu(val_buf) - 100) * 1000;
+
+	return 0;
+}
+
+static const struct hwmon_ops iqs620_temp_ops = {
+	.is_visible = iqs620_temp_is_visible,
+	.read = iqs620_temp_read,
+};
+
+static const struct hwmon_channel_info *iqs620_temp_channel_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	NULL
+};
+
+static const struct hwmon_chip_info iqs620_temp_chip_info = {
+	.ops = &iqs620_temp_ops,
+	.info = iqs620_temp_channel_info,
+};
+
+static int iqs620_temp_probe(struct platform_device *pdev)
+{
+	struct iqs62x_core *iqs62x = dev_get_drvdata(pdev->dev.parent);
+	struct device *hdev;
+	int error = 0;
+
+	hdev = devm_hwmon_device_register_with_info(&pdev->dev,
+						    iqs62x->dev_desc->dev_name,
+						    iqs62x,
+						    &iqs620_temp_chip_info,
+						    NULL);
+	if (IS_ERR(hdev)) {
+		error = PTR_ERR(hdev);
+		dev_err(&pdev->dev, "Failed to register device: %d\n", error);
+	}
+
+	return error;
+}
+
+static struct platform_driver iqs620_temp_platform_driver = {
+	.driver = {
+		.name	= IQS620_DRV_NAME_TEMP,
+	},
+	.probe		= iqs620_temp_probe,
+};
+module_platform_driver(iqs620_temp_platform_driver);
+
+MODULE_AUTHOR("Jeff LaBundy <[e-mail]>");
+MODULE_DESCRIPTION("Azoteq IQS620AT Temperature Sensor");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" IQS620_DRV_NAME_TEMP);