[V1,05/10] thermal: da9062/61: Thermal junction temperature monitoring driver

Commit Message

Steve Twiss Oct. 6, 2016, 8:43 a.m. UTC

From: Steve Twiss <[e-mail]>

Add junction temperature monitoring supervisor device driver, compatible
with the DA9062 and DA9061 PMICs.

If the PMIC's internal junction temperature rises above TEMP_WARN (125
degC) an interrupt is issued. This TEMP_WARN level is defined as the
THERMAL_TRIP_HOT trip-wire inside the device driver. A kernel work queue
is configured to repeatedly poll this temperature trip-wire, between 1 and
10 second intervals (defaulting at 3 seconds).

This first level of temperature supervision is intended for non-invasive
temperature control, where the necessary measures for cooling the system
down are left to the host software. In this case, inside the thermal
notification function da9062_thermal_notify().

Signed-off-by: Steve Twiss <[e-mail]>

---
This patch applies against linux-next and v4.8

Regards,
Steve Twiss, Dialog Semiconductor Ltd.


 drivers/thermal/Kconfig          |  10 ++
 drivers/thermal/Makefile         |   1 +
 drivers/thermal/da9062-thermal.c | 313 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 324 insertions(+)
 create mode 100644 drivers/thermal/da9062-thermal.c

Comments

Keerthy Oct. 7, 2016, 5:28 a.m. UTC | #1

Steve,

On Thursday 06 October 2016 02:13 PM, Steve Twiss wrote:
> From: Steve Twiss <[e-mail]>
>
> Add junction temperature monitoring supervisor device driver, compatible
> with the DA9062 and DA9061 PMICs.
>
> If the PMIC's internal junction temperature rises above TEMP_WARN (125
> degC) an interrupt is issued. This TEMP_WARN level is defined as the
> THERMAL_TRIP_HOT trip-wire inside the device driver. A kernel work queue
> is configured to repeatedly poll this temperature trip-wire, between 1 and
> 10 second intervals (defaulting at 3 seconds).
>
> This first level of temperature supervision is intended for non-invasive
> temperature control, where the necessary measures for cooling the system
> down are left to the host software. In this case, inside the thermal
> notification function da9062_thermal_notify().
>
> Signed-off-by: Steve Twiss <[e-mail]>
>
> ---
> This patch applies against linux-next and v4.8
>
> Regards,
> Steve Twiss, Dialog Semiconductor Ltd.
>
>
>  drivers/thermal/Kconfig          |  10 ++
>  drivers/thermal/Makefile         |   1 +
>  drivers/thermal/da9062-thermal.c | 313 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 324 insertions(+)
>  create mode 100644 drivers/thermal/da9062-thermal.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 2d702ca..da58e54 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -272,6 +272,16 @@ config DB8500_CPUFREQ_COOLING
>  	  bound cpufreq cooling device turns active to set CPU frequency low to
>  	  cool down the CPU.
>
> +config DA9062_THERMAL
> +	tristate "DA9062/DA9061 Dialog Semiconductor thermal driver"
> +	depends on MFD_DA9062
> +	depends on OF
> +	help
> +	  Enable this for the Dialog Semiconductor thermal sensor driver.
> +	  This will report PMIC junction over-temperature for one thermal trip
> +	  zone.
> +	  Compatible with the DA9062 and DA9061 PMICs.
> +
>  config INTEL_POWERCLAMP
>  	tristate "Intel PowerClamp idle injection driver"
>  	depends on THERMAL
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 10b07c1..0a2b3f2 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_ARMADA_THERMAL)	+= armada_thermal.o
>  obj-$(CONFIG_TANGO_THERMAL)	+= tango_thermal.o
>  obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
>  obj-$(CONFIG_DB8500_CPUFREQ_COOLING)	+= db8500_cpufreq_cooling.o
> +obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o
>  obj-$(CONFIG_INTEL_POWERCLAMP)	+= intel_powerclamp.o
>  obj-$(CONFIG_X86_PKG_TEMP_THERMAL)	+= x86_pkg_temp_thermal.o
>  obj-$(CONFIG_INTEL_SOC_DTS_IOSF_CORE)	+= intel_soc_dts_iosf.o
> diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
> new file mode 100644
> index 0000000..feeabf6
> --- /dev/null
> +++ b/drivers/thermal/da9062-thermal.c
> @@ -0,0 +1,313 @@
> +/*
> + * Thermal device driver for DA9062 and DA9061
> + * Copyright (C) 2016  Dialog Semiconductor Ltd.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/workqueue.h>
> +#include <linux/regmap.h>
> +#include <linux/thermal.h>
> +#include <linux/of.h>
> +
> +#include <linux/mfd/da9062/core.h>
> +#include <linux/mfd/da9062/registers.h>
> +
> +#define DA9062_DEFAULT_POLLING_MS_PERIOD	3000
> +#define DA9062_MAX_POLLING_MS_PERIOD		10000
> +#define DA9062_MIN_POLLING_MS_PERIOD		1000
> +
> +#define DA9062_MILLI_CELSIUS(t)			((t)*1000)
> +
> +struct da9062_thermal_config {
> +	const char *name;
> +};
> +
> +struct da9062_thermal {
> +	struct da9062 *hw;
> +	struct delayed_work work;
> +	struct thermal_zone_device *zone;
> +	enum thermal_device_mode mode;
> +	unsigned int polling_period;
> +	struct mutex lock;
> +	int temperature;
> +	int irq;
> +	const struct da9062_thermal_config *config;
> +	struct device *dev;
> +};
> +
> +static void da9062_thermal_poll_on(struct work_struct *work)
> +{
> +	struct da9062_thermal *thermal = container_of(work,
> +						struct da9062_thermal,
> +						work.work);
> +	unsigned int val;
> +	int ret;
> +
> +	/* clear E_TEMP */
> +	ret = regmap_write(thermal->hw->regmap,
> +				DA9062AA_EVENT_B,
> +				DA9062AA_E_TEMP_MASK);
> +	if (ret < 0) {
> +		dev_err(thermal->dev,
> +			"Cannot clear the TJUNC temperature status\n");
> +		goto err_enable_irq;
> +	}
> +
> +	/* Now read E_TEMP again: it is acting like a status bit.
> +	 * If over-temperature, then this status will be true.
> +	 * If not over-temperature, this staus will be false.
> +	 */
> +	ret = regmap_read(thermal->hw->regmap,
> +			  DA9062AA_EVENT_B,
> +			  &val);
> +	if (ret < 0) {
> +		dev_err(thermal->dev,
> +			"Cannot check the TJUNC temperature status\n");
> +		goto err_enable_irq;
> +	} else {
> +		if (val & DA9062AA_E_TEMP_MASK) {
> +			mutex_lock(&thermal->lock);
> +			thermal->temperature = DA9062_MILLI_CELSIUS(125);
> +			mutex_unlock(&thermal->lock);
> +			thermal_zone_device_update(thermal->zone);
> +
> +			schedule_delayed_work(&thermal->work,
> +				msecs_to_jiffies(thermal->polling_period));
> +			return;
> +		} else {
> +			mutex_lock(&thermal->lock);
> +			thermal->temperature = DA9062_MILLI_CELSIUS(0);
> +			mutex_unlock(&thermal->lock);
> +			thermal_zone_device_update(thermal->zone);
> +		}
> +	}
> +
> +err_enable_irq:
> +	enable_irq(thermal->irq);
> +}
> +
> +static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
> +{
> +	struct da9062_thermal *thermal = data;
> +
> +	disable_irq_nosync(thermal->irq);
> +	schedule_delayed_work(&thermal->work, 0);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int da9062_thermal_get_mode(struct thermal_zone_device *z,
> +				   enum thermal_device_mode *mode)
> +{
> +	struct da9062_thermal *thermal = z->devdata;
> +	*mode = thermal->mode;
> +	return 0;
> +}
> +
> +static int da9062_thermal_get_trip_type(struct thermal_zone_device *z,
> +				int trip,
> +				enum thermal_trip_type *type)
> +{
> +	struct da9062_thermal *thermal = z->devdata;
> +
> +	switch (trip) {
> +	case 0:
> +		*type = THERMAL_TRIP_HOT;

125C is pretty hot. Just a suggestion, do look at THERMAL_TRIP_CRITICAL.

> +		break;
> +	default:
> +		dev_err(thermal->dev,
> +			"Driver does not support more than 1 trip-wire\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int da9062_thermal_get_trip_temp(struct thermal_zone_device *z,
> +					int trip,
> +					int *temp)
> +{
> +	struct da9062_thermal *thermal = z->devdata;
> +
> +	switch (trip) {
> +	case 0:
> +		*temp = DA9062_MILLI_CELSIUS(125);
> +		break;
> +	default:
> +		dev_err(thermal->dev,
> +			"Driver does not support more than 1 trip-wire\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int da9062_thermal_notify(struct thermal_zone_device *z,
> +				 int trip,
> +				 enum thermal_trip_type type)
> +{
> +	struct da9062_thermal *thermal = z->devdata;
> +
> +	switch (type) {
> +	case THERMAL_TRIP_HOT:
> +		dev_warn(thermal->dev, "Reached HOT (125degC) temperature\n");

Any cooling action? What is done once you reach this high temperature?

> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int da9062_thermal_get_temp(struct thermal_zone_device *z,
> +				   int *temp)
> +{
> +	struct da9062_thermal *thermal = z->devdata;
> +
> +	mutex_lock(&thermal->lock);
> +	*temp = thermal->temperature;
> +	mutex_unlock(&thermal->lock);
> +
> +	return 0;
> +}
> +
> +static struct thermal_zone_device_ops da9062_thermal_ops = {
> +	.get_temp	= da9062_thermal_get_temp,
> +	.get_mode	= da9062_thermal_get_mode,
> +	.get_trip_type	= da9062_thermal_get_trip_type,
> +	.get_trip_temp	= da9062_thermal_get_trip_temp,
> +	.notify		= da9062_thermal_notify,
> +};
> +
> +static const struct da9062_thermal_config da9062_config = {
> +	.name = "da9062-thermal",
> +};
> +
> +static const struct da9062_thermal_config da9061_config = {
> +	.name = "da9061-thermal",
> +};
> +
> +static const struct of_device_id da9062_compatible_reg_id_table[] = {
> +	{ .compatible = "dlg,da9062-thermal", .data = &da9062_config },
> +	{ .compatible = "dlg,da9061-thermal", .data = &da9061_config },

Two separate compatible values. Do you have anything different apart 
from the name? Why use 2 compatibles when there is absolutely no difference?

> +	{ },
> +};
> +
> +static int da9062_thermal_probe(struct platform_device *pdev)
> +{
> +	struct da9062 *chip = dev_get_drvdata(pdev->dev.parent);
> +	struct da9062_thermal *thermal;
> +	unsigned int pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
> +	const struct of_device_id *match;
> +	int ret = 0;
> +
> +	match = of_match_node(da9062_compatible_reg_id_table,
> +			      pdev->dev.of_node);
> +	if (!match)
> +		return -ENXIO;
> +
> +	if (pdev->dev.of_node) {
> +		if (!of_property_read_u32(pdev->dev.of_node,
> +					"dlg,tjunc-temp-polling-period-ms",
> +					&pp_tmp)) {
> +			if (pp_tmp < DA9062_MIN_POLLING_MS_PERIOD ||
> +				pp_tmp > DA9062_MAX_POLLING_MS_PERIOD)
> +				pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
> +		}
> +
> +		dev_dbg(&pdev->dev,
> +			 "TJUNC temp polling period set at %d ms\n",
> +			 pp_tmp);
> +	}
> +
> +	thermal = devm_kzalloc(&pdev->dev, sizeof(struct da9062_thermal),
> +			     GFP_KERNEL);
> +	if (!thermal) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	thermal->config = match->data;
> +
> +	INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
> +	mutex_init(&thermal->lock);

thermal_zone_device_register itself does
INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);

refer: drivers/thermal/thermal_core.c

It does a periodic monitoring of the temperature as well. Do you really 
want to have an additional work for monitoring temperature in your 
driver also?

> +
> +	thermal->zone = thermal_zone_device_register(thermal->config->name,
> +					1, 0, thermal,
> +					&da9062_thermal_ops, NULL, 0,
> +					0);
> +	if (IS_ERR(thermal->zone)) {
> +		dev_err(&pdev->dev, "Cannot register thermal zone device\n");
> +		ret = PTR_ERR(thermal->zone);
> +		goto err;
> +	}
> +
> +	ret = platform_get_irq_byname(pdev, "THERMAL");
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to get platform IRQ.\n");
> +		goto err_zone;
> +	}
> +	thermal->irq = ret;
> +
> +	ret = request_threaded_irq(thermal->irq, NULL,
> +				   da9062_thermal_irq_handler,
> +				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +				   "THERMAL", thermal);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Failed to request thermal device IRQ.\n");
> +		goto err_zone;
> +	}
> +
> +	thermal->hw = chip;
> +	thermal->polling_period = pp_tmp;
> +	thermal->mode = THERMAL_DEVICE_ENABLED;
> +	thermal->dev = &pdev->dev;
> +
> +	platform_set_drvdata(pdev, thermal);
> +	return 0;
> +
> +err_zone:
> +	thermal_zone_device_unregister(thermal->zone);
> +err:
> +	return ret;
> +}
> +
> +static int da9062_thermal_remove(struct platform_device *pdev)
> +{
> +	struct	da9062_thermal *thermal = platform_get_drvdata(pdev);
> +
> +	free_irq(thermal->irq, thermal);
> +	thermal_zone_device_unregister(thermal->zone);
> +	cancel_delayed_work_sync(&thermal->work);
> +	return 0;
> +}
> +
> +static struct platform_driver da9062_thermal_driver = {
> +	.probe	= da9062_thermal_probe,
> +	.remove	= da9062_thermal_remove,
> +	.driver	= {
> +		.name	= "da9062-thermal",
> +		.of_match_table = da9062_compatible_reg_id_table,
> +	},
> +};
> +
> +module_platform_driver(da9062_thermal_driver);
> +
> +MODULE_AUTHOR("Steve Twiss, Dialog Semiconductor");
> +MODULE_DESCRIPTION("Thermal TJUNC device driver for Dialog DA9062 and DA9061");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:da9062-thermal");
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Steve Twiss Oct. 7, 2016, 5:48 p.m. UTC | #2

Hi,

On 07 October 2016 06:29, Keerthy [mailto:[e-mail]] wrote:
> On Thursday 06 October 2016 02:13 PM, Steve Twiss wrote:
> > From: Steve Twiss <[e-mail]>

[...]

> > +
> > +static int da9062_thermal_get_trip_type(struct thermal_zone_device *z,
> > +				int trip,
> > +				enum thermal_trip_type *type)
> > +{
> > +	struct da9062_thermal *thermal = z->devdata;
> > +
> > +	switch (trip) {
> > +	case 0:
> > +		*type = THERMAL_TRIP_HOT;
> 
> 125C is pretty hot. Just a suggestion, do look at THERMAL_TRIP_CRITICAL.
> 

Quite warm.
I got this from the Documentation/devicetree/bindings/thermal/thermal.txt
	"hot":		A trip point to notify emergency
	"critical":	Hardware not reliable.

The junction temperature supervision characteristics for DA9061 supports three
levels and 125degC is the lowest "warning" level.  This warning level is intended
for non-invasive temperature control where the necessary mitigation is under
software control of the system.

The other two levels are hotter than 125degC and it is not possible to intervene
using software. The hardware will automatically power off.

[...]

> > +static int da9062_thermal_notify(struct thermal_zone_device *z,
> > +				 int trip,
> > +				 enum thermal_trip_type type)
> > +{
> > +	struct da9062_thermal *thermal = z->devdata;
> > +
> > +	switch (type) {
> > +	case THERMAL_TRIP_HOT:
> > +		dev_warn(thermal->dev, "Reached HOT (125degC)
> temperature\n");
> 
> Any cooling action? What is done once you reach this high temperature?

There is nothing to do in the device driver. This is just a stub for board specific additions
to mitigate the temperature. There is a similar function in the  RCAR thermal driver which
contains a FIXME comment. But, apart from helping with my testing, it doesn't add
anything in the driver. It can be removed.

[...]

> > +static const struct da9062_thermal_config da9062_config = {
> > +	.name = "da9062-thermal",
> > +};
> > +
> > +static const struct da9062_thermal_config da9061_config = {
> > +	.name = "da9061-thermal",
> > +};
> > +
> > +static const struct of_device_id da9062_compatible_reg_id_table[] = {
> > +	{ .compatible = "dlg,da9062-thermal", .data = &da9062_config },
> > +	{ .compatible = "dlg,da9061-thermal", .data = &da9061_config },
> 
> Two separate compatible values. Do you have anything different apart
> from the name? Why use 2 compatibles when there is absolutely no
> difference?

Yes.
This was a comment for the watchdog device driver as well. My concern was having 
multiple devices (61 and 62) in the same system -- and allowing the driver to report 
the hardware difference.

There is discussion going on about this in other threads. Not certain of the final outcome
yet, apart from my existing proposal should be changed.

-- From Guenter Roeck:
-- http://www.spinics.net/lists/linux-watchdog/msg10040.html

[...]

> > +	INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
> > +	mutex_init(&thermal->lock);
> 
> thermal_zone_device_register itself does
> INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);
> 
> refer: drivers/thermal/thermal_core.c
> 
> It does a periodic monitoring of the temperature as well. Do you really
> want to have an additional work for monitoring temperature in your
> driver also?

I will take a look at this for V2.

Regards,
Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Steve Twiss Oct. 14, 2016, 1:07 p.m. UTC | #3

An update.

On: 07 October 2016 18:49, Steve Twiss wrote:
> On 07 October 2016 06:29, Keerthy [mailto:[e-mail]] wrote:
> > On Thursday 06 October 2016 02:13 PM, Steve Twiss wrote:
> > > From: Steve Twiss <[e-mail]>
[...]
> > > +static const struct da9062_thermal_config da9062_config = {
> > > +	.name = "da9062-thermal",
> > > +};
> > > +
> > > +static const struct da9062_thermal_config da9061_config = {
> > > +	.name = "da9061-thermal",
> > > +};
> > > +
> > > +static const struct of_device_id da9062_compatible_reg_id_table[] = {
> > > +	{ .compatible = "dlg,da9062-thermal", .data = &da9062_config },
> > > +	{ .compatible = "dlg,da9061-thermal", .data = &da9061_config },
> >
> > Two separate compatible values. Do you have anything different apart
> > from the name? Why use 2 compatibles when there is absolutely no
> > difference?
> 
> Yes.
> This was a comment for the watchdog device driver as well. My concern was having
> multiple devices (61 and 62) in the same system -- and allowing the driver to report
> the hardware difference.
> 
> There is discussion going on about this in other threads. Not certain of the
> final outcome yet, apart from my existing proposal should be changed.

An answer to this came from comments by Dmitry Torokhov and Guenter Roeck, who
suggested this: https://lkml.org/lkml/2016/10/7/641

I will take a look at this for V2.

Regards,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Steve Twiss Oct. 20, 2016, 1:02 p.m. UTC | #4

On  07 October 2016 06:29 Keerthy wrote:

> On Thursday 06 October 2016 02:13 PM, Steve Twiss wrote:
> > From: Steve Twiss <[e-mail]>
> > 
> > +static int da9062_thermal_probe(struct platform_device *pdev)
> > +{
> > +	struct da9062 *chip = dev_get_drvdata(pdev->dev.parent);
> > +	struct da9062_thermal *thermal;
> > +	unsigned int pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
> > +	const struct of_device_id *match;
> > +	int ret = 0;
> > +
> > +	match = of_match_node(da9062_compatible_reg_id_table,
> > +			      pdev->dev.of_node);
> > +	if (!match)
> > +		return -ENXIO;
> > +
> > +	if (pdev->dev.of_node) {
> > +		if (!of_property_read_u32(pdev->dev.of_node,
> > +					"dlg,tjunc-temp-polling-period-ms",
> > +					&pp_tmp)) {
> > +			if (pp_tmp < DA9062_MIN_POLLING_MS_PERIOD ||
> > +				pp_tmp > DA9062_MAX_POLLING_MS_PERIOD)
> > +				pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
> > +		}
> > +
> > +		dev_dbg(&pdev->dev,
> > +			 "TJUNC temp polling period set at %d ms\n",
> > +			 pp_tmp);
> > +	}
> > +
> > +	thermal = devm_kzalloc(&pdev->dev, sizeof(struct da9062_thermal),
> > +			     GFP_KERNEL);
> > +	if (!thermal) {
> > +		ret = -ENOMEM;
> > +		goto err;
> > +	}
> > +
> > +	thermal->config = match->data;
> > +
> > +	INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
> > +	mutex_init(&thermal->lock);
> 
> thermal_zone_device_register itself does
> INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);
> 
> refer: drivers/thermal/thermal_core.c
> 
> It does a periodic monitoring of the temperature as well. Do you really
> want to have an additional work for monitoring temperature in your
> driver also?

The thermal triggering mechanism is interrupt based and happens when the
temperature rises above a given threshold level. The component cannot
return an exact temperature, it only has knowledge if the temperature is
above or below a given threshold value. A status bit must be polled to
detect when the temperature falls below that threshold level again.

If I was to use the core's polling_delay it would request get_temp() every
x milliseconds. This would work: I could test the status bit to decide if
the temperature was above or below the threshold, and enable or disable the
interrupt accordingly. But during normal operation, when the temperature
is in the normal range, I would be polling through I2C every x milliseconds
instead of just waiting for an IRQ trigger event before starting my polling
operations to detect when the temperature level fell below the threshold.

Since an over-temperature is supposed to be a very rare event, I decided to
go with the IRQ based trigger and then kick-off a separate work-queue to
poll the status bit and detect when the temperature dropped below the
threshold level. This seemed a more efficient way of handling this.

I have looked through thermal core, and there doesn't seem to be an obvious
way of toggling on/off thermal core polling when I need to poll the
temperature through get_temp(). So, I was going to stick with this method for
PATCH V2.

Regards,
Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Steve Twiss Oct. 20, 2016, 2:21 p.m. UTC | #5

On 20 October 2016 14:03 Steve Twiss wrote:

> On  07 October 2016 06:29 Keerthy wrote:
> > On Thursday 06 October 2016 02:13 PM, Steve Twiss wrote:
> > > From: Steve Twiss <[e-mail]>
> > >
> > > +	INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
> > > +	mutex_init(&thermal->lock);
> >
> > thermal_zone_device_register itself does
> > INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check);
> >
> > refer: drivers/thermal/thermal_core.c
> >
> > It does a periodic monitoring of the temperature as well. Do you really
> > want to have an additional work for monitoring temperature in your
> > driver also?
> 
> The thermal triggering mechanism is interrupt based and happens when the
> temperature rises above a given threshold level. The component cannot
> return an exact temperature, it only has knowledge if the temperature is
> above or below a given threshold value. A status bit must be polled to
> detect when the temperature falls below that threshold level again.
> 
> If I was to use the core's polling_delay it would request get_temp() every
> x milliseconds. This would work: I could test the status bit to decide if
> the temperature was above or below the threshold, and enable or disable
> the interrupt accordingly. But during normal operation, when the temperature
> is in the normal range, I would be polling through I2C every x milliseconds
> instead of just waiting for an IRQ trigger event before starting my polling
> operations to detect when the temperature level fell below the threshold.
> 
> Since an over-temperature is supposed to be a very rare event, I decided to
> go with the IRQ based trigger and then kick-off a separate work-queue to
> poll the status bit and detect when the temperature dropped below the
> threshold level. This seemed a more efficient way of handling this.
> 
> I have looked through thermal core, and there doesn't seem to be an
> obvious way of toggling on/off thermal core polling when I need to poll the
> temperature through get_temp(). So, I was going to stick with this method
> for PATCH V2.

Ok. There doesn't seem to be any formal way to do this in the thermal core, 
but after a second look, it seems like I can hijack the polling_delay value and
turn the polling on and off from my device driver. There is no API to do this,
but it seems to be possible.

I start with polling off, wait for an over-temperature IRQ trigger and tweak
the polling_delay value, say, like this:

static irqreturn_t da9062_thermal_irq_handler(int irq, void *data) {
	struct da9062_thermal *thermal = data;
	disable_irq_nosync(thermal->irq);
	thermal->zone->polling_delay = 3000;
	thermal_zone_device_update(thermal->zone);
	return IRQ_HANDLED;
}

Then when polling starts and get_temp() is called, I can re-enable the IRQ again
if the temperature has dropped.
At that point I can also turn off the thermal core polling by re-writing the 
polling_delay. Like this:

static int da9062_thermal_get_temp(struct thermal_zone_device *z,
				   int *temp)
{
	struct da9062_thermal *thermal = z->devdata;
	...
	/* if temp has dropped */
	thermal->zone->polling_delay = 0;
	enable_irq(thermal->irq);
}

I'm not certain if this is acceptable, accessing the thermal core like this.
I will send this patch separately as a RFC I think.

Regards,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 2d702ca..da58e54 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -272,6 +272,16 @@  config DB8500_CPUFREQ_COOLING
 	  bound cpufreq cooling device turns active to set CPU frequency low to
 	  cool down the CPU.
 
+config DA9062_THERMAL
+	tristate "DA9062/DA9061 Dialog Semiconductor thermal driver"
+	depends on MFD_DA9062
+	depends on OF
+	help
+	  Enable this for the Dialog Semiconductor thermal sensor driver.
+	  This will report PMIC junction over-temperature for one thermal trip
+	  zone.
+	  Compatible with the DA9062 and DA9061 PMICs.
+
 config INTEL_POWERCLAMP
 	tristate "Intel PowerClamp idle injection driver"
 	depends on THERMAL
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 10b07c1..0a2b3f2 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -38,6 +38,7 @@  obj-$(CONFIG_ARMADA_THERMAL)	+= armada_thermal.o
 obj-$(CONFIG_TANGO_THERMAL)	+= tango_thermal.o
 obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
 obj-$(CONFIG_DB8500_CPUFREQ_COOLING)	+= db8500_cpufreq_cooling.o
+obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o
 obj-$(CONFIG_INTEL_POWERCLAMP)	+= intel_powerclamp.o
 obj-$(CONFIG_X86_PKG_TEMP_THERMAL)	+= x86_pkg_temp_thermal.o
 obj-$(CONFIG_INTEL_SOC_DTS_IOSF_CORE)	+= intel_soc_dts_iosf.o
diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
new file mode 100644
index 0000000..feeabf6
--- /dev/null
+++ b/drivers/thermal/da9062-thermal.c
@@ -0,0 +1,313 @@ 
+/*
+ * Thermal device driver for DA9062 and DA9061
+ * Copyright (C) 2016  Dialog Semiconductor Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/workqueue.h>
+#include <linux/regmap.h>
+#include <linux/thermal.h>
+#include <linux/of.h>
+
+#include <linux/mfd/da9062/core.h>
+#include <linux/mfd/da9062/registers.h>
+
+#define DA9062_DEFAULT_POLLING_MS_PERIOD	3000
+#define DA9062_MAX_POLLING_MS_PERIOD		10000
+#define DA9062_MIN_POLLING_MS_PERIOD		1000
+
+#define DA9062_MILLI_CELSIUS(t)			((t)*1000)
+
+struct da9062_thermal_config {
+	const char *name;
+};
+
+struct da9062_thermal {
+	struct da9062 *hw;
+	struct delayed_work work;
+	struct thermal_zone_device *zone;
+	enum thermal_device_mode mode;
+	unsigned int polling_period;
+	struct mutex lock;
+	int temperature;
+	int irq;
+	const struct da9062_thermal_config *config;
+	struct device *dev;
+};
+
+static void da9062_thermal_poll_on(struct work_struct *work)
+{
+	struct da9062_thermal *thermal = container_of(work,
+						struct da9062_thermal,
+						work.work);
+	unsigned int val;
+	int ret;
+
+	/* clear E_TEMP */
+	ret = regmap_write(thermal->hw->regmap,
+				DA9062AA_EVENT_B,
+				DA9062AA_E_TEMP_MASK);
+	if (ret < 0) {
+		dev_err(thermal->dev,
+			"Cannot clear the TJUNC temperature status\n");
+		goto err_enable_irq;
+	}
+
+	/* Now read E_TEMP again: it is acting like a status bit.
+	 * If over-temperature, then this status will be true.
+	 * If not over-temperature, this staus will be false.
+	 */
+	ret = regmap_read(thermal->hw->regmap,
+			  DA9062AA_EVENT_B,
+			  &val);
+	if (ret < 0) {
+		dev_err(thermal->dev,
+			"Cannot check the TJUNC temperature status\n");
+		goto err_enable_irq;
+	} else {
+		if (val & DA9062AA_E_TEMP_MASK) {
+			mutex_lock(&thermal->lock);
+			thermal->temperature = DA9062_MILLI_CELSIUS(125);
+			mutex_unlock(&thermal->lock);
+			thermal_zone_device_update(thermal->zone);
+
+			schedule_delayed_work(&thermal->work,
+				msecs_to_jiffies(thermal->polling_period));
+			return;
+		} else {
+			mutex_lock(&thermal->lock);
+			thermal->temperature = DA9062_MILLI_CELSIUS(0);
+			mutex_unlock(&thermal->lock);
+			thermal_zone_device_update(thermal->zone);
+		}
+	}
+
+err_enable_irq:
+	enable_irq(thermal->irq);
+}
+
+static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
+{
+	struct da9062_thermal *thermal = data;
+
+	disable_irq_nosync(thermal->irq);
+	schedule_delayed_work(&thermal->work, 0);
+
+	return IRQ_HANDLED;
+}
+
+static int da9062_thermal_get_mode(struct thermal_zone_device *z,
+				   enum thermal_device_mode *mode)
+{
+	struct da9062_thermal *thermal = z->devdata;
+	*mode = thermal->mode;
+	return 0;
+}
+
+static int da9062_thermal_get_trip_type(struct thermal_zone_device *z,
+				int trip,
+				enum thermal_trip_type *type)
+{
+	struct da9062_thermal *thermal = z->devdata;
+
+	switch (trip) {
+	case 0:
+		*type = THERMAL_TRIP_HOT;
+		break;
+	default:
+		dev_err(thermal->dev,
+			"Driver does not support more than 1 trip-wire\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int da9062_thermal_get_trip_temp(struct thermal_zone_device *z,
+					int trip,
+					int *temp)
+{
+	struct da9062_thermal *thermal = z->devdata;
+
+	switch (trip) {
+	case 0:
+		*temp = DA9062_MILLI_CELSIUS(125);
+		break;
+	default:
+		dev_err(thermal->dev,
+			"Driver does not support more than 1 trip-wire\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int da9062_thermal_notify(struct thermal_zone_device *z,
+				 int trip,
+				 enum thermal_trip_type type)
+{
+	struct da9062_thermal *thermal = z->devdata;
+
+	switch (type) {
+	case THERMAL_TRIP_HOT:
+		dev_warn(thermal->dev, "Reached HOT (125degC) temperature\n");
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int da9062_thermal_get_temp(struct thermal_zone_device *z,
+				   int *temp)
+{
+	struct da9062_thermal *thermal = z->devdata;
+
+	mutex_lock(&thermal->lock);
+	*temp = thermal->temperature;
+	mutex_unlock(&thermal->lock);
+
+	return 0;
+}
+
+static struct thermal_zone_device_ops da9062_thermal_ops = {
+	.get_temp	= da9062_thermal_get_temp,
+	.get_mode	= da9062_thermal_get_mode,
+	.get_trip_type	= da9062_thermal_get_trip_type,
+	.get_trip_temp	= da9062_thermal_get_trip_temp,
+	.notify		= da9062_thermal_notify,
+};
+
+static const struct da9062_thermal_config da9062_config = {
+	.name = "da9062-thermal",
+};
+
+static const struct da9062_thermal_config da9061_config = {
+	.name = "da9061-thermal",
+};
+
+static const struct of_device_id da9062_compatible_reg_id_table[] = {
+	{ .compatible = "dlg,da9062-thermal", .data = &da9062_config },
+	{ .compatible = "dlg,da9061-thermal", .data = &da9061_config },
+	{ },
+};
+
+static int da9062_thermal_probe(struct platform_device *pdev)
+{
+	struct da9062 *chip = dev_get_drvdata(pdev->dev.parent);
+	struct da9062_thermal *thermal;
+	unsigned int pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
+	const struct of_device_id *match;
+	int ret = 0;
+
+	match = of_match_node(da9062_compatible_reg_id_table,
+			      pdev->dev.of_node);
+	if (!match)
+		return -ENXIO;
+
+	if (pdev->dev.of_node) {
+		if (!of_property_read_u32(pdev->dev.of_node,
+					"dlg,tjunc-temp-polling-period-ms",
+					&pp_tmp)) {
+			if (pp_tmp < DA9062_MIN_POLLING_MS_PERIOD ||
+				pp_tmp > DA9062_MAX_POLLING_MS_PERIOD)
+				pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
+		}
+
+		dev_dbg(&pdev->dev,
+			 "TJUNC temp polling period set at %d ms\n",
+			 pp_tmp);
+	}
+
+	thermal = devm_kzalloc(&pdev->dev, sizeof(struct da9062_thermal),
+			     GFP_KERNEL);
+	if (!thermal) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	thermal->config = match->data;
+
+	INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
+	mutex_init(&thermal->lock);
+
+	thermal->zone = thermal_zone_device_register(thermal->config->name,
+					1, 0, thermal,
+					&da9062_thermal_ops, NULL, 0,
+					0);
+	if (IS_ERR(thermal->zone)) {
+		dev_err(&pdev->dev, "Cannot register thermal zone device\n");
+		ret = PTR_ERR(thermal->zone);
+		goto err;
+	}
+
+	ret = platform_get_irq_byname(pdev, "THERMAL");
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to get platform IRQ.\n");
+		goto err_zone;
+	}
+	thermal->irq = ret;
+
+	ret = request_threaded_irq(thermal->irq, NULL,
+				   da9062_thermal_irq_handler,
+				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+				   "THERMAL", thermal);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Failed to request thermal device IRQ.\n");
+		goto err_zone;
+	}
+
+	thermal->hw = chip;
+	thermal->polling_period = pp_tmp;
+	thermal->mode = THERMAL_DEVICE_ENABLED;
+	thermal->dev = &pdev->dev;
+
+	platform_set_drvdata(pdev, thermal);
+	return 0;
+
+err_zone:
+	thermal_zone_device_unregister(thermal->zone);
+err:
+	return ret;
+}
+
+static int da9062_thermal_remove(struct platform_device *pdev)
+{
+	struct	da9062_thermal *thermal = platform_get_drvdata(pdev);
+
+	free_irq(thermal->irq, thermal);
+	thermal_zone_device_unregister(thermal->zone);
+	cancel_delayed_work_sync(&thermal->work);
+	return 0;
+}
+
+static struct platform_driver da9062_thermal_driver = {
+	.probe	= da9062_thermal_probe,
+	.remove	= da9062_thermal_remove,
+	.driver	= {
+		.name	= "da9062-thermal",
+		.of_match_table = da9062_compatible_reg_id_table,
+	},
+};
+
+module_platform_driver(da9062_thermal_driver);
+
+MODULE_AUTHOR("Steve Twiss, Dialog Semiconductor");
+MODULE_DESCRIPTION("Thermal TJUNC device driver for Dialog DA9062 and DA9061");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:da9062-thermal");