diff mbox

[1/2] devicetree: add lm90 thermal_zone sensor support

Message ID 0d274e32ad09daa2f6f7f27f1c36d39da526b66d.1486741517.git.chunkeey@googlemail.com
State Not Applicable, archived
Headers show

Commit Message

Christian Lamparter Feb. 10, 2017, 4:12 p.m. UTC 
This patch updates the LM90's devicetree definition to
include the #thermal-sensor-cells property as well as
the sensor constants in include/dt-bindings/thermal/lm90.h.

Cc: Wei Ni <wni@nvidia.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
I've reordered LM90_LOCAL_TEMPERATURE and LM90_REMOTE_TEMPERATURE.
Everything else is the same as in the RFC. This is the only
required patch and it only updates the documentation and binding.
[PATCH 2/2] is optional (as I said I would look at it).
---
 Documentation/devicetree/bindings/hwmon/lm90.txt |  6 ++++++
 MAINTAINERS                                      |  1 +
 include/dt-bindings/thermal/lm90.h               | 12 ++++++++++++
 3 files changed, 19 insertions(+)
 create mode 100644 include/dt-bindings/thermal/lm90.h

Comments

Guenter Roeck Feb. 10, 2017, 5:21 p.m. UTC | #1 
On Fri, Feb 10, 2017 at 05:12:30PM +0100, Christian Lamparter wrote:
> This patch integrates the LOCAL, REMOTE and REMOTE2
> channel definitions into the lm90.c driver.
> 
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
> This is an optional patch to showcase how the channel definition
> in the dt-bindings are mapped into the driver.
> In theory, this makes it possible to easily remap the channel
> indices. However, it does make the driver harder to read.

It also makes the driver dependent on external defines which are not controlled
by the driver. If anyone changes those defines to be non-sequential or to not
start with 0, we would be in trouble. Sure, that might and likely would result
in compile errors, but still ...

Besides, it is not complete. Anyone changing channel index values would
(at least) mess up alarm bit association.

If we want to do that kind of thing, it might make more sense to add some code
to provide the desired mapping to the hwmon core, and to let the hwmon core
handle it. No idea if that is even possible, though.

Is that really worth it ?

Thanks,
Guenter

> ---
>  drivers/hwmon/lm90.c | 61 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 40 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 841f2428e84a..aa67810000f9 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -95,6 +95,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/interrupt.h>
>  #include <linux/regulator/consumer.h>
> +#include <dt-bindings/thermal/lm90.h>
>  
>  /*
>   * Addresses to scan
> @@ -1016,23 +1017,33 @@ static int lm90_set_temphyst(struct lm90_data *data, long val)
>  }
>  
>  static const u8 lm90_temp_index[3] = {
> -	LOCAL_TEMP, REMOTE_TEMP, REMOTE2_TEMP
> +	[LM90_LOCAL_TEMPERATURE] = LOCAL_TEMP,
> +	[LM90_REMOTE_TEMPERATURE] = REMOTE_TEMP,
> +	[LM90_REMOTE2_TEMPERATURE] =  REMOTE2_TEMP
>  };
>  
>  static const u8 lm90_temp_min_index[3] = {
> -	LOCAL_LOW, REMOTE_LOW, REMOTE2_LOW
> +	[LM90_LOCAL_TEMPERATURE] = LOCAL_LOW,
> +	[LM90_REMOTE_TEMPERATURE] = REMOTE_LOW,
> +	[LM90_REMOTE2_TEMPERATURE] = REMOTE2_LOW
>  };
>  
>  static const u8 lm90_temp_max_index[3] = {
> -	LOCAL_HIGH, REMOTE_HIGH, REMOTE2_HIGH
> +	[LM90_LOCAL_TEMPERATURE] = LOCAL_HIGH,
> +	[LM90_REMOTE_TEMPERATURE] = REMOTE_HIGH,
> +	[LM90_REMOTE2_TEMPERATURE] = REMOTE2_HIGH
>  };
>  
>  static const u8 lm90_temp_crit_index[3] = {
> -	LOCAL_CRIT, REMOTE_CRIT, REMOTE2_CRIT
> +	[LM90_LOCAL_TEMPERATURE] = LOCAL_CRIT,
> +	[LM90_REMOTE_TEMPERATURE] = REMOTE_CRIT,
> +	[LM90_REMOTE2_TEMPERATURE] = REMOTE2_CRIT
>  };
>  
>  static const u8 lm90_temp_emerg_index[3] = {
> -	LOCAL_EMERG, REMOTE_EMERG, REMOTE2_EMERG
> +	[LM90_LOCAL_TEMPERATURE] = LOCAL_EMERG,
> +	[LM90_REMOTE_TEMPERATURE] = REMOTE_EMERG,
> +	[LM90_REMOTE2_TEMPERATURE] = REMOTE2_EMERG
>  };
>  
>  static const u8 lm90_min_alarm_bits[3] = { 5, 3, 11 };
> @@ -1654,6 +1665,10 @@ static int lm90_probe(struct i2c_client *client,
>  	struct lm90_data *data;
>  	int err;
>  
> +	BUILD_BUG_ON(LM90_LOCAL_TEMPERATURE == LM90_REMOTE_TEMPERATURE ||
> +		     LM90_REMOTE_TEMPERATURE == LM90_REMOTE2_TEMPERATURE ||
> +		     LM90_REMOTE2_TEMPERATURE == LM90_LOCAL_TEMPERATURE);
> +
>  	regulator = devm_regulator_get(dev, "vcc");
>  	if (IS_ERR(regulator))
>  		return PTR_ERR(regulator);
> @@ -1695,37 +1710,41 @@ static int lm90_probe(struct i2c_client *client,
>  	data->chip.ops = &lm90_ops;
>  	data->chip.info = data->info;
>  
> -	data->info[0] = &lm90_chip_info;
> -	data->info[1] = &data->temp_info;
> +	data->info[LM90_LOCAL_TEMPERATURE] = &lm90_chip_info;
> +	data->info[LM90_REMOTE_TEMPERATURE] = &data->temp_info;
>  
>  	info = &data->temp_info;
>  	info->type = hwmon_temp;
>  	info->config = data->channel_config;
>  
> -	data->channel_config[0] = HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX |
> -		HWMON_T_CRIT | HWMON_T_CRIT_HYST | HWMON_T_MIN_ALARM |
> -		HWMON_T_MAX_ALARM | HWMON_T_CRIT_ALARM;
> -	data->channel_config[1] = HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX |
> -		HWMON_T_CRIT | HWMON_T_CRIT_HYST | HWMON_T_MIN_ALARM |
> -		HWMON_T_MAX_ALARM | HWMON_T_CRIT_ALARM | HWMON_T_FAULT;
> +	data->channel_config[LM90_LOCAL_TEMPERATURE] = HWMON_T_INPUT |
> +		HWMON_T_MIN | HWMON_T_MAX | HWMON_T_CRIT | HWMON_T_CRIT_HYST |
> +		HWMON_T_MIN_ALARM | HWMON_T_MAX_ALARM | HWMON_T_CRIT_ALARM;
> +	data->channel_config[LM90_REMOTE_TEMPERATURE] = HWMON_T_INPUT |
> +		HWMON_T_MIN | HWMON_T_MAX | HWMON_T_CRIT | HWMON_T_CRIT_HYST |
> +		HWMON_T_MIN_ALARM | HWMON_T_MAX_ALARM | HWMON_T_CRIT_ALARM |
> +		HWMON_T_FAULT;
>  
>  	if (data->flags & LM90_HAVE_OFFSET)
> -		data->channel_config[1] |= HWMON_T_OFFSET;
> +		data->channel_config[LM90_REMOTE_TEMPERATURE] |= HWMON_T_OFFSET;
>  
>  	if (data->flags & LM90_HAVE_EMERGENCY) {
> -		data->channel_config[0] |= HWMON_T_EMERGENCY |
> -			HWMON_T_EMERGENCY_HYST;
> -		data->channel_config[1] |= HWMON_T_EMERGENCY |
> -			HWMON_T_EMERGENCY_HYST;
> +		data->channel_config[LM90_LOCAL_TEMPERATURE] |=
> +			HWMON_T_EMERGENCY | HWMON_T_EMERGENCY_HYST;
> +		data->channel_config[LM90_REMOTE_TEMPERATURE] |=
> +			HWMON_T_EMERGENCY | HWMON_T_EMERGENCY_HYST;
>  	}
>  
>  	if (data->flags & LM90_HAVE_EMERGENCY_ALARM) {
> -		data->channel_config[0] |= HWMON_T_EMERGENCY_ALARM;
> -		data->channel_config[1] |= HWMON_T_EMERGENCY_ALARM;
> +		data->channel_config[LM90_LOCAL_TEMPERATURE] |=
> +			HWMON_T_EMERGENCY_ALARM;
> +		data->channel_config[LM90_REMOTE_TEMPERATURE] |=
> +			HWMON_T_EMERGENCY_ALARM;
>  	}
>  
>  	if (data->flags & LM90_HAVE_TEMP3) {
> -		data->channel_config[2] = HWMON_T_INPUT |
> +		data->channel_config[LM90_REMOTE2_TEMPERATURE] =
> +			HWMON_T_INPUT |
>  			HWMON_T_MIN | HWMON_T_MAX |
>  			HWMON_T_CRIT | HWMON_T_CRIT_HYST |
>  			HWMON_T_EMERGENCY | HWMON_T_EMERGENCY_HYST |
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guenter Roeck Feb. 10, 2017, 11:51 p.m. UTC | #2 
On Fri, Feb 10, 2017 at 05:12:29PM +0100, Christian Lamparter wrote:
> This patch updates the LM90's devicetree definition to
> include the #thermal-sensor-cells property as well as
> the sensor constants in include/dt-bindings/thermal/lm90.h.
> 
> Cc: Wei Ni <wni@nvidia.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>

I don't really know who would normally apply a patch like this,
so I added it to hwmon-next.

Thanks,
Guenter

> ---
> I've reordered LM90_LOCAL_TEMPERATURE and LM90_REMOTE_TEMPERATURE.
> Everything else is the same as in the RFC. This is the only
> required patch and it only updates the documentation and binding.
> [PATCH 2/2] is optional (as I said I would look at it).
> ---
>  Documentation/devicetree/bindings/hwmon/lm90.txt |  6 ++++++
>  MAINTAINERS                                      |  1 +
>  include/dt-bindings/thermal/lm90.h               | 12 ++++++++++++
>  3 files changed, 19 insertions(+)
>  create mode 100644 include/dt-bindings/thermal/lm90.h
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/lm90.txt b/Documentation/devicetree/bindings/hwmon/lm90.txt
> index e8632486b9ef..97581266e329 100644
> --- a/Documentation/devicetree/bindings/hwmon/lm90.txt
> +++ b/Documentation/devicetree/bindings/hwmon/lm90.txt
> @@ -33,6 +33,11 @@ Optional properties:
>                LM90 "-ALERT" pin output.
>                See interrupt-controller/interrupts.txt for the format.
>  
> +- #thermal-sensor-cells: should be set to 1. See thermal/thermal.txt for
> +	      details. See <include/dt-bindings/thermal/lm90.h> for the
> +	      definition of the local, remote and 2nd remote sensor index
> +	      constants.
> +
>  Example LM90 node:
>  
>  temp-sensor {
> @@ -41,4 +46,5 @@ temp-sensor {
>  	vcc-supply = <&palmas_ldo6_reg>;
>  	interrupt-parent = <&gpio>;
>  	interrupts = <TEGRA_GPIO(O, 4) IRQ_TYPE_LEVEL_LOW>;
> +	#thermal-sensor-cells = <1>;
>  }
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2be620cea1ed..73972ccebc56 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7541,6 +7541,7 @@ S:	Maintained
>  F:	Documentation/hwmon/lm90
>  F:	Documentation/devicetree/bindings/hwmon/lm90.txt
>  F:	drivers/hwmon/lm90.c
> +F:	include/dt-bindings/thermal/lm90.h
>  
>  LM95234 HARDWARE MONITOR DRIVER
>  M:	Guenter Roeck <linux@roeck-us.net>
> diff --git a/include/dt-bindings/thermal/lm90.h b/include/dt-bindings/thermal/lm90.h
> new file mode 100644
> index 000000000000..8c2e3095f704
> --- /dev/null
> +++ b/include/dt-bindings/thermal/lm90.h
> @@ -0,0 +1,12 @@
> +/*
> + * This header provides constants for the LM90 thermal bindings.
> + */
> +
> +#ifndef _DT_BINDINGS_THERMAL_LM90_H_
> +#define _DT_BINDINGS_THERMAL_LM90_H_
> +
> +#define LM90_LOCAL_TEMPERATURE 0
> +#define LM90_REMOTE_TEMPERATURE 1
> +#define LM90_REMOTE2_TEMPERATURE 2
> +
> +#endif
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/lm90.txt b/Documentation/devicetree/bindings/hwmon/lm90.txt
index e8632486b9ef..97581266e329 100644
--- a/Documentation/devicetree/bindings/hwmon/lm90.txt
+++ b/Documentation/devicetree/bindings/hwmon/lm90.txt
@@ -33,6 +33,11 @@  Optional properties:
               LM90 "-ALERT" pin output.
               See interrupt-controller/interrupts.txt for the format.
 
+- #thermal-sensor-cells: should be set to 1. See thermal/thermal.txt for
+	      details. See <include/dt-bindings/thermal/lm90.h> for the
+	      definition of the local, remote and 2nd remote sensor index
+	      constants.
+
 Example LM90 node:
 
 temp-sensor {
@@ -41,4 +46,5 @@  temp-sensor {
 	vcc-supply = <&palmas_ldo6_reg>;
 	interrupt-parent = <&gpio>;
 	interrupts = <TEGRA_GPIO(O, 4) IRQ_TYPE_LEVEL_LOW>;
+	#thermal-sensor-cells = <1>;
 }
diff --git a/MAINTAINERS b/MAINTAINERS
index 2be620cea1ed..73972ccebc56 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7541,6 +7541,7 @@  S:	Maintained
 F:	Documentation/hwmon/lm90
 F:	Documentation/devicetree/bindings/hwmon/lm90.txt
 F:	drivers/hwmon/lm90.c
+F:	include/dt-bindings/thermal/lm90.h
 
 LM95234 HARDWARE MONITOR DRIVER
 M:	Guenter Roeck <linux@roeck-us.net>
diff --git a/include/dt-bindings/thermal/lm90.h b/include/dt-bindings/thermal/lm90.h
new file mode 100644
index 000000000000..8c2e3095f704
--- /dev/null
+++ b/include/dt-bindings/thermal/lm90.h
@@ -0,0 +1,12 @@ 
+/*
+ * This header provides constants for the LM90 thermal bindings.
+ */
+
+#ifndef _DT_BINDINGS_THERMAL_LM90_H_
+#define _DT_BINDINGS_THERMAL_LM90_H_
+
+#define LM90_LOCAL_TEMPERATURE 0
+#define LM90_REMOTE_TEMPERATURE 1
+#define LM90_REMOTE2_TEMPERATURE 2
+
+#endif