Message ID | e2c150ce1312a524ea0a524f3d476f910333ae82.1486327509.git.chunkeey@googlemail.com |
---|---|
State | Not Applicable, archived |
Headers | show |
On 02/05/2017 01:03 PM, Christian Lamparter wrote: > This patch adds thermal_zone temperature sensor support > to the lm90 module. The feature has to be enabled > separately via the Kconfig option: > CONFIG_SENSORS_LM90_THERMAL_DEVICE > > The LM90 supports two (three for MAX6695 and MAX6696) > temperature sensors. The local sensor is integrated > into the LM90 chip. The remote sensors are connected > to external temperature sensing diodes. > > Cc: Wei Ni <wni@nvidia.com> > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com> Since the hwnmon core now supports thermal registration, and since the lm90 driver has already been converted to using the new hwmon API, I would rather like to understand why using the hwmon core for this purpose is insufficient, and I would prefer to address the deficiencies in the hwmon core and not in the driver. Thanks, Guenter > --- > --- > drivers/hwmon/Kconfig | 11 +++++++ > drivers/hwmon/lm90.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 102 insertions(+) > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 190d270b20a2..9df70ad21a21 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1085,6 +1085,17 @@ config SENSORS_LM90 > This driver can also be built as a module. If so, the module > will be called lm90. > > +config SENSORS_LM90_THERMAL_DEVICE > + bool "Support for thermal-zone temperature sensors" > + depends on SENSORS_LM90 && THERMAL_OF > + help > + If you say yes here you get support for thermal-zone temperature > + sensors for the lm90 module and all supported chips. > + > + Refer to Documentation/devicetree/bindings/thermal/thermal.txt > + and Documentation/devicetree/bindings/hwmon/lm90.txt on how to > + use it for your platform. > + > config SENSORS_LM92 > tristate "National Semiconductor LM92 and compatibles" > depends on I2C > diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c > index 841f2428e84a..584550c7336d 100644 > --- a/drivers/hwmon/lm90.c > +++ b/drivers/hwmon/lm90.c > @@ -95,6 +95,9 @@ > #include <linux/sysfs.h> > #include <linux/interrupt.h> > #include <linux/regulator/consumer.h> > +#include <linux/of.h> > +#include <linux/thermal.h> > +#include <dt-bindings/thermal/lm90.h> > > /* > * Addresses to scan > @@ -1643,6 +1646,90 @@ static const struct hwmon_ops lm90_ops = { > .write = lm90_write, > }; > > +#ifdef CONFIG_SENSORS_LM90_THERMAL_DEVICE > + > +static int lm90_read_local_temp(void *data, int *temp) > +{ > + *temp = lm90_get_temp11(data, LOCAL_TEMP); > + return 0; > +} > + > +static int lm90_read_remote_temp(void *data, int *temp) > +{ > + *temp = lm90_get_temp11(data, REMOTE_TEMP); > + return 0; > +} > + > +static int lm90_read_remote2_temp(void *data, int *temp) > +{ > + *temp = lm90_get_temp11(data, REMOTE2_TEMP); > + return 0; > +} > + > +static const struct thermal_zone_of_device_ops local_temp_sensor = { > + .get_temp = lm90_read_local_temp, > +}; > + > +static const struct thermal_zone_of_device_ops remote_temp_sensor = { > + .get_temp = lm90_read_remote_temp, > +}; > + > +static const struct thermal_zone_of_device_ops remote2_temp_sensor = { > + .get_temp = lm90_read_remote2_temp, > +}; > + > +static const struct thermal_zone_sensors_struct { > + int sensor_id; > + const char *name; > + const struct thermal_zone_of_device_ops *ops; > + u32 flag_mask; > + u32 flag_required; > +} thermal_zone_sensors[] = { > + { LM90_REMOTE_TEMPERATURE, "remote", &remote_temp_sensor }, > + { LM90_LOCAL_TEMPERATURE, "local", &local_temp_sensor }, > + { LM90_REMOTE2_TEMPERATURE, "second remote", &remote2_temp_sensor, > + LM90_HAVE_TEMP3, LM90_HAVE_TEMP3 }, > +}; > + > +static int lm90_setup_thermal_device(struct device *dev, > + struct lm90_data *data) > +{ > + const struct thermal_zone_sensors_struct *entry; > + struct thermal_zone_device *therm_dev; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(thermal_zone_sensors); i++) { > + entry = &thermal_zone_sensors[i]; > + > + if ((data->flags & entry->flag_mask) != entry->flag_required) > + continue; > + > + therm_dev = devm_thermal_zone_of_sensor_register(dev, > + entry->sensor_id, data, entry->ops); > + > + /* > + * if a sensor device isn't requested in a thermal-zone the > + * call to devm_thermal_zone_of_sensor_register will fail > + * with -ENODEV. In this case we can ignore/skip it. > + */ > + > + if (IS_ERR(therm_dev) && (PTR_ERR(therm_dev) != -ENODEV)) { > + dev_err(dev, "Failed to register %s thermal_zone sensor device\n", > + entry->name); > + return PTR_ERR(therm_dev); > + } > + } > + return 0; > +} > +#else > +static int lm90_setup_thermal_device(struct device __maybe_unused *dev, > + struct lm90_data __maybe_unused *data) > +{ > + return 0; > +} > +#endif > + > + > static int lm90_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -1765,6 +1852,10 @@ static int lm90_probe(struct i2c_client *client, > if (IS_ERR(hwmon_dev)) > return PTR_ERR(hwmon_dev); > > + err = lm90_setup_thermal_device(dev, data); > + if (err) > + return err; > + > if (client->irq) { > dev_dbg(dev, "IRQ: %d\n", client->irq); > err = devm_request_threaded_irq(dev, client->irq, > -- 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
On Sunday, February 5, 2017 7:10:53 PM CET Guenter Roeck wrote: > On 02/05/2017 01:03 PM, Christian Lamparter wrote: > > This patch adds thermal_zone temperature sensor support > > to the lm90 module. The feature has to be enabled > > separately via the Kconfig option: > > CONFIG_SENSORS_LM90_THERMAL_DEVICE > > > > The LM90 supports two (three for MAX6695 and MAX6696) > > temperature sensors. The local sensor is integrated > > into the LM90 chip. The remote sensors are connected > > to external temperature sensing diodes. > > > > Cc: Wei Ni <wni@nvidia.com> > > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com> > > Since the hwnmon core now supports thermal registration, and since the lm90 > driver has already been converted to using the new hwmon API, I would > rather like to understand why using the hwmon core for this purpose is > insufficient, and I would prefer to address the deficiencies in the hwmon > core and not in the driver. Hey, that's great, I completely missed that! Yes, this makes the changes to lm90.c obsolete. However, what about the device-tree updates in 1/2? I'm asking because the hwmon device node still needs the #thermal-sensor-cells property defined for thermal_zone_of_sensor_register(). Without it, the sensor will be skipped and the thermal-zones will do nothing (no regulation). I can respin the device-tree patch (local and remote need to trade places). What do you think? Regards, Christian [0] <http://lxr.free-electrons.com/source/drivers/thermal/of-thermal.c#L495> -- 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
On Mon, Feb 06, 2017 at 05:01:31PM +0100, Christian Lamparter wrote: > On Sunday, February 5, 2017 7:10:53 PM CET Guenter Roeck wrote: > > On 02/05/2017 01:03 PM, Christian Lamparter wrote: > > > This patch adds thermal_zone temperature sensor support > > > to the lm90 module. The feature has to be enabled > > > separately via the Kconfig option: > > > CONFIG_SENSORS_LM90_THERMAL_DEVICE > > > > > > The LM90 supports two (three for MAX6695 and MAX6696) > > > temperature sensors. The local sensor is integrated > > > into the LM90 chip. The remote sensors are connected > > > to external temperature sensing diodes. > > > > > > Cc: Wei Ni <wni@nvidia.com> > > > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com> > > > > Since the hwnmon core now supports thermal registration, and since the lm90 > > driver has already been converted to using the new hwmon API, I would > > rather like to understand why using the hwmon core for this purpose is > > insufficient, and I would prefer to address the deficiencies in the hwmon > > core and not in the driver. > > Hey, that's great, I completely missed that! Yes, this makes the changes > to lm90.c obsolete. > > However, what about the device-tree updates in 1/2? I'm asking because > the hwmon device node still needs the #thermal-sensor-cells property > defined for thermal_zone_of_sensor_register(). Without it, the sensor > will be skipped and the thermal-zones will do nothing (no regulation). > I can respin the device-tree patch (local and remote need to trade > places). What do you think? > Sure, no problem with that. Does supporting this require changes in the hwmon core ? Thanks, Guenter -- 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
On Sun, Feb 05, 2017 at 10:03:15PM +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> > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com> > --- > I'm aware that there was atleast one previous attempt to add > thermal_zone temperature sensors for the LM90 module. This was > discussed on: > <http://www.gossamer-threads.com/lists/linux/kernel/1992853> > <https://lkml.org/lkml/2014/3/4/194> > > This RFC is meant to get it going again. As I would really > like to have this functionality for the Netgear WNDR4700. > This router uses a G781 to measure the SoCs temperature > in order to regulate a TC654 fan-controller. > <https://git.lede-project.org/?p=source.git;a=commit;h=9e0fd1b52ad1f805a308bf6a5a13236f352fd962> > --- > 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 Acked-by: Rob Herring <robh@kernel.org> -- 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
On Wednesday, February 8, 2017 4:30:34 PM CET Rob Herring wrote: > On Sun, Feb 05, 2017 at 10:03:15PM +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> > > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com> > > --- > > I'm aware that there was atleast one previous attempt to add > > thermal_zone temperature sensors for the LM90 module. This was > > discussed on: > > <http://www.gossamer-threads.com/lists/linux/kernel/1992853> > > <https://lkml.org/lkml/2014/3/4/194> > > > > This RFC is meant to get it going again. As I would really > > like to have this functionality for the Netgear WNDR4700. > > This router uses a G781 to measure the SoCs temperature > > in order to regulate a TC654 fan-controller. > > <https://git.lede-project.org/?p=source.git;a=commit;h=9e0fd1b52ad1f805a308bf6a5a13236f352fd962> > > --- > > 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 > > Acked-by: Rob Herring <robh@kernel.org> > Ok thanks. Just a quick note: Don't apply this yet. I'm working on a patch, but this has to wait for the weekend. OnT: From Guenter Roeck: > Sure, no problem with that. Does supporting this require changes in the hwmon > core ? No changes to hwmon's core are required. It's basically this 1/1 patch with +#define LM90_LOCAL_TEMPERATURE 1 (updated to 0) +#define LM90_REMOTE_TEMPERATURE 0 (updated to 1) swapped and a small update to lm90.c. This way the defines are doing something in the driver [0]: static const u8 lm90_temp_index[3] = { LOCAL_TEMP, REMOTE_TEMP, REMOTE2_TEMP }; will become something like: static const u8 lm90_temp_index[3] = { [LM90_LOCAL_TEMPERATURE] = LOCAL_TEMP, [LM90_REMOTE_TEMPERATURE] = REMOTE_TEMP, [LM90_REMOTE2_TEMPERATURE] = REMOTE2_TEMP }; Regards, Christian [0] <http://lxr.free-electrons.com/source/drivers/hwmon/lm90.c#L1018> -- 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 --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 5e637e2b3ff9..8c2ea8a18064 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..39d90f3e63ee --- /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_REMOTE_TEMPERATURE 0 +#define LM90_LOCAL_TEMPERATURE 1 +#define LM90_REMOTE2_TEMPERATURE 2 + +#endif
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> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com> --- I'm aware that there was atleast one previous attempt to add thermal_zone temperature sensors for the LM90 module. This was discussed on: <http://www.gossamer-threads.com/lists/linux/kernel/1992853> <https://lkml.org/lkml/2014/3/4/194> This RFC is meant to get it going again. As I would really like to have this functionality for the Netgear WNDR4700. This router uses a G781 to measure the SoCs temperature in order to regulate a TC654 fan-controller. <https://git.lede-project.org/?p=source.git;a=commit;h=9e0fd1b52ad1f805a308bf6a5a13236f352fd962> --- 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