[PATCH 0/3] thermal: add devm_ version of thermal_zone register and driver for max77620

[PATCH 0/3] thermal: add devm_ version of thermal_zone register and driver for max77620

[Laxman Dewangan]

The series add the devm_ version of thermal_zone_of_sensor_register/unregister
and use this in new thermal driver for max77620.

The header file for max77620 is part of MFD patch
	https://lkml.org/lkml/2016/2/11/186

Laxman Dewangan (3):
  thermal: of-thermal: Add devm version of
    thermal_zone_of_sensor_register
  thermal: Add devm_thermal_zone_of_sensor_register() in managed devices
    list
  thermal: max77620: Add thermal driver for reporting junction temp

 Documentation/driver-model/devres.txt |   4 +
 drivers/thermal/Kconfig               |   9 ++
 drivers/thermal/Makefile              |   1 +
 drivers/thermal/of-thermal.c          |  81 ++++++++++++++++++
 drivers/thermal/thermal-max77620.c    | 151 ++++++++++++++++++++++++++++++++++
 include/linux/thermal.h               |  18 ++++
 6 files changed, 264 insertions(+)
 create mode 100644 drivers/thermal/thermal-max77620.c

-- 
2.1.4

[PATCH 1/3] thermal: of-thermal: Add devm version of thermal_zone_of_sensor_register

[Laxman Dewangan]

Add resource managed version of thermal_zone_of_sensor_register() and
thermal_zone_of_sensor_unregister().

This helps in reducing the code size in error path, remove of
driver remove callbacks and making proper sequence for deallocations.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/thermal/of-thermal.c | 81 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/thermal.h      | 18 ++++++++++
 2 files changed, 99 insertions(+)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 9043f8f..1d9f8dc 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -555,6 +555,87 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(thermal_zone_of_sensor_unregister);
 
+static void devm_thermal_zone_of_sensor_release(struct device *dev, void *res)
+{
+	thermal_zone_of_sensor_unregister(dev,
+					  *(struct thermal_zone_device **)res);
+}
+
+static int devm_thermal_zone_of_sensor_match(struct device *dev, void *res,
+					     void *data)
+{
+	struct thermal_zone_device **r = res;
+
+	if (WARN_ON(!r || !*r))
+		return 0;
+
+	return *r == data;
+}
+
+/**
+ * devm_thermal_zone_of_sensor_register - Resource managed version of
+ *				thermal_zone_of_sensor_register()
+ * @dev: a valid struct device pointer of a sensor device. Must contain
+ *       a valid .of_node, for the sensor node.
+ * @sensor_id: a sensor identifier, in case the sensor IP has more
+ *	       than one sensors
+ * @data: a private pointer (owned by the caller) that will be passed
+ *	  back, when a temperature reading is needed.
+ * @ops: struct thermal_zone_of_device_ops *. Must contain at least .get_temp.
+ *
+ * Refer thermal_zone_of_sensor_register() for more details.
+ *
+ * Return: On success returns a valid struct thermal_zone_device,
+ * otherwise, it returns a corresponding ERR_PTR(). Caller must
+ * check the return value with help of IS_ERR() helper.
+ * Registered hermal_zone_device device will automatically be
+ * released when device is unbounded.
+ */
+struct thermal_zone_device *
+devm_thermal_zone_of_sensor_register(struct device *dev, int sensor_id,
+		void *data, const struct thermal_zone_of_device_ops *ops)
+{
+	struct thermal_zone_device **ptr, *tzd;
+
+	ptr = devres_alloc(devm_thermal_zone_of_sensor_release, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	tzd = thermal_zone_of_sensor_register(dev, sensor_id, data, ops);
+	if (IS_ERR(tzd)) {
+		devres_free(ptr);
+		return tzd;
+	}
+
+	*ptr = tzd;
+	devres_add(dev, ptr);
+
+	return tzd;
+}
+EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_register);
+
+/**
+ * devm_thermal_zone_of_sensor_unregister - Resource managed version of
+ *				thermal_zone_of_sensor_unregister().
+ * @dev: Device for which which resource was allocated.
+ * @tzd: a pointer to struct thermal_zone_device where the sensor is registered.
+ *
+ * This function removes the sensor callbacks and private data from the
+ * thermal zone device registered with devm_thermal_zone_of_sensor_register()
+ * API. It will also silent the zone by remove the .get_temp() and .get_trend()
+ * thermal zone device callbacks.
+ * Normally this function will not need to be called and the resource
+ * management code will ensure that the resource is freed.
+ */
+void devm_thermal_zone_of_sensor_unregister(struct device *dev,
+				       struct thermal_zone_device *tzd)
+{
+	WARN_ON(devres_release(dev, devm_thermal_zone_of_sensor_release,
+			       devm_thermal_zone_of_sensor_match, tzd));
+}
+EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_unregister);
+
 /***   functions parsing device tree nodes   ***/
 
 /**
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index e13a1ac..4c43b21 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -362,6 +362,11 @@ thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
 				const struct thermal_zone_of_device_ops *ops);
 void thermal_zone_of_sensor_unregister(struct device *dev,
 				       struct thermal_zone_device *tz);
+struct thermal_zone_device *
+devm_thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
+				const struct thermal_zone_of_device_ops *ops);
+void devm_thermal_zone_of_sensor_unregister(struct device *dev,
+				       struct thermal_zone_device *tz);
 #else
 static inline struct thermal_zone_device *
 thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
@@ -376,6 +381,19 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
 {
 }
 
+static inline struct thermal_zone_device *
+devm_thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
+				const struct thermal_zone_of_device_ops *ops)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline
+void devm_thermal_zone_of_sensor_unregister(struct device *dev,
+				       struct thermal_zone_device *tz)
+{
+}
+
 #endif
 
 #if IS_ENABLED(CONFIG_THERMAL)
-- 
2.1.4

Re: [PATCH 1/3] thermal: of-thermal: Add devm version of thermal_zone_of_sensor_register

[Eduardo Valentin]

Hello Laxman,


Minor as follows.

Can you please run ./scripts/checkpatch.pl --strict on this and remove
the warnings, errors, checks?

On Fri, Mar 04, 2016 at 07:10:08PM +0530, Laxman Dewangan wrote:
> Add resource managed version of thermal_zone_of_sensor_register() and
> thermal_zone_of_sensor_unregister().
> 
> This helps in reducing the code size in error path, remove of
> driver remove callbacks and making proper sequence for deallocations.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  drivers/thermal/of-thermal.c | 81 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/thermal.h      | 18 ++++++++++

Could you also please document these under
Documentation/thermal/sysfs-api.txt?

>  2 files changed, 99 insertions(+)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index 9043f8f..1d9f8dc 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -555,6 +555,87 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_of_sensor_unregister);
>  
> +static void devm_thermal_zone_of_sensor_release(struct device *dev, void *res)
> +{
> +	thermal_zone_of_sensor_unregister(dev,
> +					  *(struct thermal_zone_device **)res);
> +}
> +
> +static int devm_thermal_zone_of_sensor_match(struct device *dev, void *res,
> +					     void *data)
> +{
> +	struct thermal_zone_device **r = res;
> +
> +	if (WARN_ON(!r || !*r))
> +		return 0;
> +
> +	return *r == data;
> +}
> +
> +/**
> + * devm_thermal_zone_of_sensor_register - Resource managed version of
> + *				thermal_zone_of_sensor_register()
> + * @dev: a valid struct device pointer of a sensor device. Must contain
> + *       a valid .of_node, for the sensor node.
> + * @sensor_id: a sensor identifier, in case the sensor IP has more
> + *	       than one sensors
> + * @data: a private pointer (owned by the caller) that will be passed
> + *	  back, when a temperature reading is needed.
> + * @ops: struct thermal_zone_of_device_ops *. Must contain at least .get_temp.
> + *
> + * Refer thermal_zone_of_sensor_register() for more details.
> + *
> + * Return: On success returns a valid struct thermal_zone_device,
> + * otherwise, it returns a corresponding ERR_PTR(). Caller must
> + * check the return value with help of IS_ERR() helper.
> + * Registered hermal_zone_device device will automatically be
> + * released when device is unbounded.
> + */
> +struct thermal_zone_device *
> +devm_thermal_zone_of_sensor_register(struct device *dev, int sensor_id,
> +		void *data, const struct thermal_zone_of_device_ops *ops)
> +{
> +	struct thermal_zone_device **ptr, *tzd;
> +
> +	ptr = devres_alloc(devm_thermal_zone_of_sensor_release, sizeof(*ptr),
> +			   GFP_KERNEL);
> +	if (!ptr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	tzd = thermal_zone_of_sensor_register(dev, sensor_id, data, ops);
> +	if (IS_ERR(tzd)) {
> +		devres_free(ptr);
> +		return tzd;
> +	}
> +
> +	*ptr = tzd;
> +	devres_add(dev, ptr);
> +
> +	return tzd;
> +}
> +EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_register);
> +
> +/**
> + * devm_thermal_zone_of_sensor_unregister - Resource managed version of
> + *				thermal_zone_of_sensor_unregister().
> + * @dev: Device for which which resource was allocated.
> + * @tzd: a pointer to struct thermal_zone_device where the sensor is registered.
> + *
> + * This function removes the sensor callbacks and private data from the
> + * thermal zone device registered with devm_thermal_zone_of_sensor_register()
> + * API. It will also silent the zone by remove the .get_temp() and .get_trend()
> + * thermal zone device callbacks.
> + * Normally this function will not need to be called and the resource
> + * management code will ensure that the resource is freed.
> + */
> +void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> +				       struct thermal_zone_device *tzd)
> +{
> +	WARN_ON(devres_release(dev, devm_thermal_zone_of_sensor_release,
> +			       devm_thermal_zone_of_sensor_match, tzd));
> +}
> +EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_unregister);
> +
>  /***   functions parsing device tree nodes   ***/
>  
>  /**
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index e13a1ac..4c43b21 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -362,6 +362,11 @@ thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
>  				const struct thermal_zone_of_device_ops *ops);
>  void thermal_zone_of_sensor_unregister(struct device *dev,
>  				       struct thermal_zone_device *tz);
> +struct thermal_zone_device *
> +devm_thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> +				const struct thermal_zone_of_device_ops *ops);
> +void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> +				       struct thermal_zone_device *tz);
>  #else
>  static inline struct thermal_zone_device *
>  thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> @@ -376,6 +381,19 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
>  {
>  }
>  
> +static inline struct thermal_zone_device *
> +devm_thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> +				const struct thermal_zone_of_device_ops *ops)
> +{
> +	return ERR_PTR(-ENODEV);
> +}
> +
> +static inline
> +void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> +				       struct thermal_zone_device *tz)
> +{
> +}
> +
>  #endif
>  
>  #if IS_ENABLED(CONFIG_THERMAL)
> -- 
> 2.1.4
> 

Re: [PATCH 1/3] thermal: of-thermal: Add devm version of thermal_zone_of_sensor_register

[Laxman Dewangan]

Hi Edurado,
Thanks for review.


On Wednesday 09 March 2016 02:59 AM, Eduardo Valentin wrote:
> Hello Laxman,
>
>
> Minor as follows.
>
> Can you please run ./scripts/checkpatch.pl --strict on this and remove
> the warnings, errors, checks?

Taken from existing function and so borrowed the error. Will fix in next 
revision.


>
> On Fri, Mar 04, 2016 at 07:10:08PM +0530, Laxman Dewangan wrote:
>> Add resource managed version of thermal_zone_of_sensor_register() and
>> thermal_zone_of_sensor_unregister().
>>
>> This helps in reducing the code size in error path, remove of
>> driver remove callbacks and making proper sequence for deallocations.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>> ---
>>   drivers/thermal/of-thermal.c | 81 ++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/thermal.h      | 18 ++++++++++
> Could you also please document these under
> Documentation/thermal/sysfs-api.txt?
>
>
Sure, I saw that thermal_zone_of_sensor_register/unregister is also not 
documented.
So I will add the details for this first and then add for newly added 
interfaces.

Will take care in next series.

[PATCH 2/3] thermal: Add devm_thermal_zone_of_sensor_register() in managed devices list

[Laxman Dewangan]

thermal_zone_of_sensor_register() and thermal_zone_of_sensor_unregister()
gained their devm_ wrappers. Add these APIs in the list of managed devices.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 Documentation/driver-model/devres.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index 5930d9a..dd7ce58 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -342,6 +342,10 @@ SLAVE DMA ENGINE
 SPI
   devm_spi_register_master()
 
+THERMAL
+ devm_thermal_zone_of_sensor_register()
+ devm_thermal_zone_of_sensor_unregister()
+
 WATCHDOG
   devm_watchdog_register_device()
   devm_watchdog_unregister_device()
-- 
2.1.4

[PATCH 3/3] thermal: max77620: Add thermal driver for reporting junction temp

[Laxman Dewangan]

Maxim Semiconductor Max77620 supports alarm interrupts when
its die temperature crosses 120C and 140C. These threshold
temperatures are not configurable.

Add thermal driver to register PMIC die temperature as thermal
zone sensor and capture the die temperature warning interrupts
to notifying the client.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/thermal/Kconfig            |   9 +++
 drivers/thermal/Makefile           |   1 +
 drivers/thermal/thermal-max77620.c | 151 +++++++++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 drivers/thermal/thermal-max77620.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 5e7c97a..faba1a3 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -194,6 +194,15 @@ config IMX_THERMAL
 	  cpufreq is used as the cooling device to throttle CPUs when the
 	  passive trip is crossed.
 
+config MAX77620_THERMAL
+	tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
+	depends on MFD_MAX77620
+	depends on OF
+	help
+	  Support for die junction temperature warning alarm for Maxim
+	  Semiconductor PMIC MAX77620 device. Device generates alert
+	  signal/interrupt when die temperature cross its threshold.
+
 config SPEAR_THERMAL
 	tristate "SPEAr thermal sensor driver"
 	depends on PLAT_SPEAR || COMPILE_TEST
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 8e9cbc3..c6bc2bd 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_DOVE_THERMAL)  	+= dove_thermal.o
 obj-$(CONFIG_DB8500_THERMAL)	+= db8500_thermal.o
 obj-$(CONFIG_ARMADA_THERMAL)	+= armada_thermal.o
 obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
+obj-$(CONFIG_MAX77620_THERMAL)	+= thermal-max77620.o
 obj-$(CONFIG_DB8500_CPUFREQ_COOLING)	+= db8500_cpufreq_cooling.o
 obj-$(CONFIG_INTEL_POWERCLAMP)	+= intel_powerclamp.o
 obj-$(CONFIG_X86_PKG_TEMP_THERMAL)	+= x86_pkg_temp_thermal.o
diff --git a/drivers/thermal/thermal-max77620.c b/drivers/thermal/thermal-max77620.c
new file mode 100644
index 0000000..846da62
--- /dev/null
+++ b/drivers/thermal/thermal-max77620.c
@@ -0,0 +1,151 @@
+/*
+ * Junction temperature thermal driver for Maxim Max77620.
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Author: Laxman Dewangan <ldewangan@nvidia.com>
+ *	   Mallikarjun Kasoju <mkasoju@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max77620.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/thermal.h>
+
+#define MAX77620_NORMAL_OPERATING_TEMP	100000
+#define MAX77620_TJALARM1_TEMP		120000
+#define MAX77620_TJALARM1_TEMP		140000
+
+struct max77620_therm_info {
+	struct device			*dev;
+	struct regmap			*rmap;
+	struct thermal_zone_device	*tz_device;
+	int				irq_tjalarm1;
+	int				irq_tjalarm2;
+};
+
+static int max77620_thermal_read_temp(void *data, int *temp)
+{
+	struct max77620_therm_info *mtherm = data;
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(mtherm->rmap, MAX77620_REG_STATLBT, &val);
+	if (ret < 0) {
+		dev_err(mtherm->dev, "Failed to read STATLBT, %d\n", ret);
+		return -EINVAL;
+	}
+
+	if (val & MAX77620_IRQ_TJALRM2_MASK)
+		*temp = MAX77620_TJALARM2_TEMP;
+	else if (val & MAX77620_IRQ_TJALRM1_MASK)
+		*temp = MAX77620_TJALARM1_TEMP;
+	else
+		*temp = MAX77620_NORMAL_OPERATING_TEMP;
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops max77620_thermal_ops = {
+	.get_temp = max77620_thermal_read_temp,
+};
+
+static irqreturn_t max77620_thermal_irq(int irq, void *data)
+{
+	struct max77620_therm_info *mtherm = data;
+
+	if (irq == mtherm->irq_tjalarm1)
+		dev_warn(mtherm->dev, "Junction Temp Alarm1(120C) occurred\n");
+	else if (irq == mtherm->irq_tjalarm2)
+		dev_warn(mtherm->dev, "Junction Temp Alarm2(140C) occurred\n");
+
+	thermal_zone_device_update(mtherm->tz_device);
+
+	return IRQ_HANDLED;
+}
+
+static int max77620_thermal_probe(struct platform_device *pdev)
+{
+	struct max77620_therm_info *mtherm;
+	int ret;
+
+	if (!pdev->dev.of_node)
+		pdev->dev.of_node = pdev->dev.parent->of_node;
+
+	mtherm = devm_kzalloc(&pdev->dev, sizeof(*mtherm), GFP_KERNEL);
+	if (!mtherm)
+		return -ENOMEM;
+
+	mtherm->irq_tjalarm1 = platform_get_irq(pdev, 0);
+	mtherm->irq_tjalarm2 = platform_get_irq(pdev, 1);
+	if ((mtherm->irq_tjalarm1 < 0) || (mtherm->irq_tjalarm2 < 0)) {
+		dev_err(&pdev->dev, "Alarm irq number not available\n");
+		return -EINVAL;
+	}
+
+	mtherm->dev = &pdev->dev;
+	mtherm->rmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!mtherm->rmap) {
+		dev_err(&pdev->dev, "Failed to get parent regmap\n");
+		return -ENODEV;
+	}
+
+	mtherm->tz_device = devm_thermal_zone_of_sensor_register(&pdev->dev, 0,
+				mtherm, &max77620_thermal_ops);
+	if (IS_ERR(mtherm->tz_device)) {
+		ret = PTR_ERR(mtherm->tz_device);
+		dev_err(&pdev->dev, "Failed to register thermal zone, %d\n",
+			ret);
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, mtherm->irq_tjalarm1, NULL,
+					max77620_thermal_irq,
+					IRQF_ONESHOT | IRQF_SHARED,
+					dev_name(&pdev->dev), mtherm);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to request irq1, %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, mtherm->irq_tjalarm2, NULL,
+					max77620_thermal_irq,
+					IRQF_ONESHOT | IRQF_SHARED,
+					dev_name(&pdev->dev), mtherm);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to request irq2, %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, mtherm);
+	return 0;
+}
+
+static struct platform_device_id max77620_thermal_devtype[] = {
+	{ .name = "max77620-thermal", },
+	{},
+};
+
+static struct platform_driver max77620_thermal_driver = {
+	.driver = {
+		.name = "max77620-thermal",
+	},
+	.probe = max77620_thermal_probe,
+	.id_table = max77620_thermal_devtype,
+};
+
+module_platform_driver(max77620_thermal_driver);
+
+MODULE_DESCRIPTION("Max77620 Junction temperature Thermal driver");
+MODULE_ALIAS("platform:max77620-thermal");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_AUTHOR("Mallikarjun Kasoju <mkasoju@nvidia.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

Re: [PATCH 3/3] thermal: max77620: Add thermal driver for reporting junction temp

[Laxman Dewangan]

On Friday 04 March 2016 07:10 PM, Laxman Dewangan wrote:
> >
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/thermal.h>
> +
> +#define MAX77620_NORMAL_OPERATING_TEMP	100000
> +#define MAX77620_TJALARM1_TEMP		120000
> +#define MAX77620_TJALARM1_TEMP		140000
>
Oops, Second one is MAX77620_TJALARM2_TEMP.
Will send other version once I will have some more feedback.

Re: [PATCH 3/3] thermal: max77620: Add thermal driver for reporting junction temp

[Eduardo Valentin]

Hello Laxman,

Thanks for working on this. Impressed how simplified these drivers are
becoming. I really liked you added the so waited devm_ helpers. Very
minor comments as follows (now that you will send a new version anyway).

On Fri, Mar 04, 2016 at 07:10:10PM +0530, Laxman Dewangan wrote:
> Maxim Semiconductor Max77620 supports alarm interrupts when
> its die temperature crosses 120C and 140C. These threshold
> temperatures are not configurable.
> 
> Add thermal driver to register PMIC die temperature as thermal
> zone sensor and capture the die temperature warning interrupts
> to notifying the client.

Are any of these critical?

> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  drivers/thermal/Kconfig            |   9 +++
>  drivers/thermal/Makefile           |   1 +
>  drivers/thermal/thermal-max77620.c | 151 +++++++++++++++++++++++++++++++++++++

Given that it is a DT based driver, please add also a binding entry
under Documentation/devicetree/bindings/thermal/


>  3 files changed, 161 insertions(+)
>  create mode 100644 drivers/thermal/thermal-max77620.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 5e7c97a..faba1a3 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -194,6 +194,15 @@ config IMX_THERMAL
>  	  cpufreq is used as the cooling device to throttle CPUs when the
>  	  passive trip is crossed.
>  
> +config MAX77620_THERMAL
> +	tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
> +	depends on MFD_MAX77620

Can this be COMPILE_TEST'ed?

> +	depends on OF

> +	help
> +	  Support for die junction temperature warning alarm for Maxim
> +	  Semiconductor PMIC MAX77620 device. Device generates alert
> +	  signal/interrupt when die temperature cross its threshold.
> +

Somehow checkpatch.pl --strict is complaining about this entry. Can you
please check?


>  config SPEAR_THERMAL
>  	tristate "SPEAr thermal sensor driver"
>  	depends on PLAT_SPEAR || COMPILE_TEST
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 8e9cbc3..c6bc2bd 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_DOVE_THERMAL)  	+= dove_thermal.o
>  obj-$(CONFIG_DB8500_THERMAL)	+= db8500_thermal.o
>  obj-$(CONFIG_ARMADA_THERMAL)	+= armada_thermal.o
>  obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
> +obj-$(CONFIG_MAX77620_THERMAL)	+= thermal-max77620.o
>  obj-$(CONFIG_DB8500_CPUFREQ_COOLING)	+= db8500_cpufreq_cooling.o
>  obj-$(CONFIG_INTEL_POWERCLAMP)	+= intel_powerclamp.o
>  obj-$(CONFIG_X86_PKG_TEMP_THERMAL)	+= x86_pkg_temp_thermal.o
> diff --git a/drivers/thermal/thermal-max77620.c b/drivers/thermal/thermal-max77620.c
> new file mode 100644
> index 0000000..846da62
> --- /dev/null
> +++ b/drivers/thermal/thermal-max77620.c
> @@ -0,0 +1,151 @@
> +/*
> + * Junction temperature thermal driver for Maxim Max77620.
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * Author: Laxman Dewangan <ldewangan@nvidia.com>
> + *	   Mallikarjun Kasoju <mkasoju@nvidia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/max77620.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/thermal.h>
> +
> +#define MAX77620_NORMAL_OPERATING_TEMP	100000
> +#define MAX77620_TJALARM1_TEMP		120000
> +#define MAX77620_TJALARM1_TEMP		140000
> +
> +struct max77620_therm_info {
> +	struct device			*dev;
> +	struct regmap			*rmap;
> +	struct thermal_zone_device	*tz_device;
> +	int				irq_tjalarm1;
> +	int				irq_tjalarm2;
> +};
> +
> +static int max77620_thermal_read_temp(void *data, int *temp)
> +{
> +	struct max77620_therm_info *mtherm = data;
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(mtherm->rmap, MAX77620_REG_STATLBT, &val);
> +	if (ret < 0) {
> +		dev_err(mtherm->dev, "Failed to read STATLBT, %d\n", ret);
> +		return -EINVAL;
> +	}
> +
> +	if (val & MAX77620_IRQ_TJALRM2_MASK)
> +		*temp = MAX77620_TJALARM2_TEMP;
> +	else if (val & MAX77620_IRQ_TJALRM1_MASK)
> +		*temp = MAX77620_TJALARM1_TEMP;
> +	else
> +		*temp = MAX77620_NORMAL_OPERATING_TEMP;

So, no way at all to get a temp?

> +	return 0;
> +}
> +
> +static const struct thermal_zone_of_device_ops max77620_thermal_ops = {
> +	.get_temp = max77620_thermal_read_temp,
> +};
> +
> +static irqreturn_t max77620_thermal_irq(int irq, void *data)
> +{
> +	struct max77620_therm_info *mtherm = data;
> +
> +	if (irq == mtherm->irq_tjalarm1)
> +		dev_warn(mtherm->dev, "Junction Temp Alarm1(120C) occurred\n");
> +	else if (irq == mtherm->irq_tjalarm2)
> +		dev_warn(mtherm->dev, "Junction Temp Alarm2(140C) occurred\n");
> +
> +	thermal_zone_device_update(mtherm->tz_device);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int max77620_thermal_probe(struct platform_device *pdev)
> +{
> +	struct max77620_therm_info *mtherm;
> +	int ret;
> +
> +	if (!pdev->dev.of_node)
> +		pdev->dev.of_node = pdev->dev.parent->of_node;

Why is this needed?

> +
> +	mtherm = devm_kzalloc(&pdev->dev, sizeof(*mtherm), GFP_KERNEL);
> +	if (!mtherm)
> +		return -ENOMEM;
> +
> +	mtherm->irq_tjalarm1 = platform_get_irq(pdev, 0);
> +	mtherm->irq_tjalarm2 = platform_get_irq(pdev, 1);
> +	if ((mtherm->irq_tjalarm1 < 0) || (mtherm->irq_tjalarm2 < 0)) {
> +		dev_err(&pdev->dev, "Alarm irq number not available\n");
> +		return -EINVAL;
> +	}
> +
> +	mtherm->dev = &pdev->dev;
> +	mtherm->rmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!mtherm->rmap) {
> +		dev_err(&pdev->dev, "Failed to get parent regmap\n");
> +		return -ENODEV;
> +	}
> +
> +	mtherm->tz_device = devm_thermal_zone_of_sensor_register(&pdev->dev, 0,
> +				mtherm, &max77620_thermal_ops);
> +	if (IS_ERR(mtherm->tz_device)) {
> +		ret = PTR_ERR(mtherm->tz_device);
> +		dev_err(&pdev->dev, "Failed to register thermal zone, %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, mtherm->irq_tjalarm1, NULL,
> +					max77620_thermal_irq,
> +					IRQF_ONESHOT | IRQF_SHARED,
> +					dev_name(&pdev->dev), mtherm);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to request irq1, %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, mtherm->irq_tjalarm2, NULL,
> +					max77620_thermal_irq,
> +					IRQF_ONESHOT | IRQF_SHARED,
> +					dev_name(&pdev->dev), mtherm);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to request irq2, %d\n", ret);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, mtherm);
nit: empty line here.
> +	return 0;
> +}
> +
> +static struct platform_device_id max77620_thermal_devtype[] = {
> +	{ .name = "max77620-thermal", },
> +	{},
> +};
> +
> +static struct platform_driver max77620_thermal_driver = {
> +	.driver = {
> +		.name = "max77620-thermal",
> +	},
> +	.probe = max77620_thermal_probe,
> +	.id_table = max77620_thermal_devtype,
> +};
> +
> +module_platform_driver(max77620_thermal_driver);
> +
> +MODULE_DESCRIPTION("Max77620 Junction temperature Thermal driver");
> +MODULE_ALIAS("platform:max77620-thermal");
> +MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
> +MODULE_AUTHOR("Mallikarjun Kasoju <mkasoju@nvidia.com>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.1.4
> 

Re: [PATCH 3/3] thermal: max77620: Add thermal driver for reporting junction temp

[Laxman Dewangan]

On Wednesday 09 March 2016 02:54 AM, Eduardo Valentin wrote:
>

> On Fri, Mar 04, 2016 at 07:10:10PM +0530, Laxman Dewangan wrote:
>> Maxim Semiconductor Max77620 supports alarm interrupts when
>> its die temperature crosses 120C and 140C. These threshold
>> temperatures are not configurable.
>>
>> Add thermal driver to register PMIC die temperature as thermal
>> zone sensor and capture the die temperature warning interrupts
>> to notifying the client.
> Are any of these critical?

Datasheet says that two alarm interrupt at 120 and 140degC. 165 degC is 
shutdown temp on which PMIC get shutdown.
So just says as the warning interrupt.


>
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>> ---
>>   drivers/thermal/Kconfig            |   9 +++
>>   drivers/thermal/Makefile           |   1 +
>>   drivers/thermal/thermal-max77620.c | 151 +++++++++++++++++++++++++++++++++++++
> Given that it is a DT based driver, please add also a binding entry
> under Documentation/devicetree/bindings/thermal/

There is no new DT property and so did not added. But will add the doc 
saying the mandatory properties from thermal framework i.e. 
#thermal-sensor-cells

>
> +	depends on MFD_MAX77620
> Can this be COMPILE_TEST'ed?

Yes, I compile and tested .

>> +	depends on OF
>> +	help
>> +	  Support for die junction temperature warning alarm for Maxim
>> +	  Semiconductor PMIC MAX77620 device. Device generates alert
>> +	  signal/interrupt when die temperature cross its threshold.
>> +
> Somehow checkpatch.pl --strict is complaining about this entry. Can you
> please check?

The help should be minimum 4 lines otherwise warning is generated. I 
made it for next patch.

Second warning is:
/**
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#57:
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 174 lines checked

0001-thermal-max77620-Add-thermal-driver-for-reporting-ju.patch has 
style problems, please review.
**/

This is because new file get added and I think we can ignore it.

>> +	if (val & MAX77620_IRQ_TJALRM2_MASK)
>> +		*temp = MAX77620_TJALARM2_TEMP;
>> +	else if (val & MAX77620_IRQ_TJALRM1_MASK)
>> +		*temp = MAX77620_TJALARM1_TEMP;
>> +	else
>> +		*temp = MAX77620_NORMAL_OPERATING_TEMP;
> So, no way at all to get a temp?

yaah, there is no way other than getting the bit for whether temp 
crossed threshold or not.

>
>> +
>> +	if (!pdev->dev.of_node)
>> +		pdev->dev.of_node = pdev->dev.parent->of_node;
> Why is this needed?

This driver is sub mfd devices and it is registered without device node 
pointer.
The DT binding doc for the mfdmax77620 is flat, does not have thermal 
sub node.

hence to get the of_node for sensor, I am making the device node as node 
pointer for thermal sensor.
I can overwrite as
  pdev->dev.of_node = pdev->dev.parent->of_node also without check for 
simplification.