Re: [RESEND PATCH V1 8/9] hwmon: lm75: expose to thermal fw via DT nodes
On 07/18/2013 09:12 PM, Eduardo Valentin wrote:
> * PGP Signed: 07/18/2013 at 06:12:01 AM
>
> Hi Wei,
>
>
> On 18-07-2013 01:33, Wei Ni wrote:
>> On 07/17/2013 11:17 PM, Eduardo Valentin wrote:
>>> This patch adds to lm75 temperature sensor the possibility
>>> to expose itself as thermal zone device, registered on the
>>> thermal framework.
>>>
>>> The thermal zone is built only if a device tree node
>>> describing a thermal zone for this sensor is present
>>> inside the lm75 DT node. Otherwise, the driver behavior
>>> will be the same.
>>>
>>> Cc: Jean Delvare
>>> Cc: Guenter Roeck
>>> Cc: lm-sens...@lm-sensors.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Eduardo Valentin
>>> ---
>>> drivers/hwmon/lm75.c | 29 +
>>> 1 file changed, 29 insertions(+)
>>>
>>> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>>> index c03b490..0aa5e28 100644
>>> --- a/drivers/hwmon/lm75.c
>>> +++ b/drivers/hwmon/lm75.c
>>> @@ -27,6 +27,8 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> +#include
>>> #include "lm75.h"
>>>
>>>
>>> @@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
>>> /* Each client has this additional data */
>>> struct lm75_data {
>>> struct device *hwmon_dev;
>>> + struct thermal_zone_device *tz;
>>> struct mutexupdate_lock;
>>> u8 orig_conf;
>>> u8 resolution; /* In bits, between 9 and 12 */
>>> @@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct
>>> device *dev);
>>>
>>> /* sysfs attributes for hwmon */
>>>
>>> +static int lm75_read_temp(void *dev, unsigned long *temp)
>>> +{
>>> + struct lm75_data *data = lm75_update_device(dev);
>>> +
>>> + if (IS_ERR(data))
>>> + return PTR_ERR(data);
>>> +
>>> + *temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
>>> + (data->resolution - 8);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static ssize_t show_temp(struct device *dev, struct device_attribute *da,
>>> char *buf)
>>> {
>>> @@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct
>>> i2c_device_id *id)
>>> goto exit_remove;
>>> }
>>>
>>> + if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
>>> + data->tz = thermal_zone_of_device_register(&client->dev,
>>> + &client->dev,
>>> + lm75_read_temp);
>>
>> Hi, Eduardo
>> I have two questions:
>> 1. As we know, after register to the thermal framework, it will have
>> duplicate hwmon devices. I think lm-sensor maintainer would not like this.
>
> Yes I noticed. You have always the option to disable the
> CONFIG_THERMAL_HWMON in your build.
Yes, we can disable this config, but it will disable all hwmon interface
which registered by the thermal fw, it may affect other driver.
>
>> How about to add a flag to indicate it, which I talk about with
>> Durgadoss in your [RFC 1/4]patch.
>
> This would be very much appreciated, but I don't think this is a blocker
> for this series. We can of course include this patch on it. Having a
> flag to control this thermal fw to hwmon interface is actually a good idea.
Ok, I think I can post it as a separate patches.
>
>>
>> 2. I'm also trying to use your codes on lm90. The LM90 serial has more
>> then one sensors in one chip, local sensor, remote sensor and may have
>> remote2 sensor, so it mean there may have more than one thermal_zone
>> under the lm90 device node, will you consider it?
>>
>
> I haven't looked lm90 source code. How do you map it? Do you probe each
> sensor or do you probe 1 device and expose 3 sensors?
lm90.c is the driver for all LM90 register compatible chips, it probe 1
device, and normally it can expose local and remote sensor, and for
max669x chip, it also expose remote2 sensor.
>
> In first case, you can still reuse what is in this series.
>
> Later case, we need to change it slightly.
>
> That is the case which is pretty similar on OMAP5 for instance, where
> the device has three sensors. This patch series does not cover for this
> case. But it can be simply modified to get around it.
>
> We would need to allow more than one 'thermal_zone' nodes inside a
> single device. But then we would need to have a way to determine which
> sensor goes to which zone too.
Yes, agreed, we need a way to determine it.
I have tentative suggestions, how about following:
lm90 {
...
thermal_zone@0 {
type = "local_sensor";
...
}
thermal_zone@1 {
type = "remote_sensor";
}
}
And in the driver's probe routine, we can parse the thermal_zone node,
and pass it to thermal_zone_of_device_register() with the right .get_temp().
Wei.
>
>
>> Thanks.
>> Wei.
>>
>>> + if (IS_ERR(data->tz)) {
>>> + status = PTR_ERR(data->tz);
>>> + goto
Re: [RESEND PATCH V1 8/9] hwmon: lm75: expose to thermal fw via DT nodes
Hi Wei,
On 18-07-2013 01:33, Wei Ni wrote:
> On 07/17/2013 11:17 PM, Eduardo Valentin wrote:
>> This patch adds to lm75 temperature sensor the possibility
>> to expose itself as thermal zone device, registered on the
>> thermal framework.
>>
>> The thermal zone is built only if a device tree node
>> describing a thermal zone for this sensor is present
>> inside the lm75 DT node. Otherwise, the driver behavior
>> will be the same.
>>
>> Cc: Jean Delvare
>> Cc: Guenter Roeck
>> Cc: lm-sens...@lm-sensors.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Eduardo Valentin
>> ---
>> drivers/hwmon/lm75.c | 29 +
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>> index c03b490..0aa5e28 100644
>> --- a/drivers/hwmon/lm75.c
>> +++ b/drivers/hwmon/lm75.c
>> @@ -27,6 +27,8 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>> #include "lm75.h"
>>
>>
>> @@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
>> /* Each client has this additional data */
>> struct lm75_data {
>> struct device *hwmon_dev;
>> +struct thermal_zone_device *tz;
>> struct mutexupdate_lock;
>> u8 orig_conf;
>> u8 resolution; /* In bits, between 9 and 12 */
>> @@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device
>> *dev);
>>
>> /* sysfs attributes for hwmon */
>>
>> +static int lm75_read_temp(void *dev, unsigned long *temp)
>> +{
>> +struct lm75_data *data = lm75_update_device(dev);
>> +
>> +if (IS_ERR(data))
>> +return PTR_ERR(data);
>> +
>> +*temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
>> +(data->resolution - 8);
>> +
>> +return 0;
>> +}
>> +
>> static ssize_t show_temp(struct device *dev, struct device_attribute *da,
>> char *buf)
>> {
>> @@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct
>> i2c_device_id *id)
>> goto exit_remove;
>> }
>>
>> +if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
>> +data->tz = thermal_zone_of_device_register(&client->dev,
>> + &client->dev,
>> + lm75_read_temp);
>
> Hi, Eduardo
> I have two questions:
> 1. As we know, after register to the thermal framework, it will have
> duplicate hwmon devices. I think lm-sensor maintainer would not like this.
Yes I noticed. You have always the option to disable the
CONFIG_THERMAL_HWMON in your build.
> How about to add a flag to indicate it, which I talk about with
> Durgadoss in your [RFC 1/4]patch.
This would be very much appreciated, but I don't think this is a blocker
for this series. We can of course include this patch on it. Having a
flag to control this thermal fw to hwmon interface is actually a good idea.
>
> 2. I'm also trying to use your codes on lm90. The LM90 serial has more
> then one sensors in one chip, local sensor, remote sensor and may have
> remote2 sensor, so it mean there may have more than one thermal_zone
> under the lm90 device node, will you consider it?
>
I haven't looked lm90 source code. How do you map it? Do you probe each
sensor or do you probe 1 device and expose 3 sensors?
In first case, you can still reuse what is in this series.
Later case, we need to change it slightly.
That is the case which is pretty similar on OMAP5 for instance, where
the device has three sensors. This patch series does not cover for this
case. But it can be simply modified to get around it.
We would need to allow more than one 'thermal_zone' nodes inside a
single device. But then we would need to have a way to determine which
sensor goes to which zone too.
> Thanks.
> Wei.
>
>> +if (IS_ERR(data->tz)) {
>> +status = PTR_ERR(data->tz);
>> +goto exit_hwmon;
>> +}
>> +}
>> +
>> dev_info(&client->dev, "%s: sensor '%s'\n",
>> dev_name(data->hwmon_dev), client->name);
>>
>> return 0;
>>
>> +exit_hwmon:
>> +hwmon_device_unregister(data->hwmon_dev);
>> exit_remove:
>> sysfs_remove_group(&client->dev.kobj, &lm75_group);
>> return status;
>> @@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
>> {
>> struct lm75_data *data = i2c_get_clientdata(client);
>>
>> +thermal_zone_device_unregister(data->tz);
>> hwmon_device_unregister(data->hwmon_dev);
>> sysfs_remove_group(&client->dev.kobj, &lm75_group);
>> lm75_write_value(client, LM75_REG_CONF, data->orig_conf);
>>
>
>
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
signature.asc
Description: OpenPGP digital signature
Re: [RESEND PATCH V1 8/9] hwmon: lm75: expose to thermal fw via DT nodes
On Wed, Jul 17, 2013 at 11:17:27AM -0400, Eduardo Valentin wrote:
> This patch adds to lm75 temperature sensor the possibility
> to expose itself as thermal zone device, registered on the
> thermal framework.
>
> The thermal zone is built only if a device tree node
> describing a thermal zone for this sensor is present
> inside the lm75 DT node. Otherwise, the driver behavior
> will be the same.
>
As mentioned before, your usage of devicetree information is clearly
outside the scope for devicetree data.
One clear guideline for devicetree data is that it is supposed to be usable
across multiple operating systems. Your proposal is clearly operating system
and even subsystem specific. It does _not_ describe hardware.
NACK.
Guenter
> Cc: Jean Delvare
> Cc: Guenter Roeck
> Cc: lm-sens...@lm-sensors.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Eduardo Valentin
> ---
> drivers/hwmon/lm75.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index c03b490..0aa5e28 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -27,6 +27,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include "lm75.h"
>
>
> @@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
> /* Each client has this additional data */
> struct lm75_data {
> struct device *hwmon_dev;
> + struct thermal_zone_device *tz;
> struct mutexupdate_lock;
> u8 orig_conf;
> u8 resolution; /* In bits, between 9 and 12 */
> @@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device
> *dev);
>
> /* sysfs attributes for hwmon */
>
> +static int lm75_read_temp(void *dev, unsigned long *temp)
> +{
> + struct lm75_data *data = lm75_update_device(dev);
> +
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + *temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
> + (data->resolution - 8);
> +
> + return 0;
> +}
> +
> static ssize_t show_temp(struct device *dev, struct device_attribute *da,
>char *buf)
> {
> @@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct
> i2c_device_id *id)
> goto exit_remove;
> }
>
> + if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
> + data->tz = thermal_zone_of_device_register(&client->dev,
> +&client->dev,
> +lm75_read_temp);
> + if (IS_ERR(data->tz)) {
> + status = PTR_ERR(data->tz);
> + goto exit_hwmon;
> + }
> + }
> +
> dev_info(&client->dev, "%s: sensor '%s'\n",
>dev_name(data->hwmon_dev), client->name);
>
> return 0;
>
> +exit_hwmon:
> + hwmon_device_unregister(data->hwmon_dev);
> exit_remove:
> sysfs_remove_group(&client->dev.kobj, &lm75_group);
> return status;
> @@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
> {
> struct lm75_data *data = i2c_get_clientdata(client);
>
> + thermal_zone_device_unregister(data->tz);
> hwmon_device_unregister(data->hwmon_dev);
> sysfs_remove_group(&client->dev.kobj, &lm75_group);
> lm75_write_value(client, LM75_REG_CONF, data->orig_conf);
> --
> 1.8.2.1.342.gfa7285d
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RESEND PATCH V1 8/9] hwmon: lm75: expose to thermal fw via DT nodes
On 07/17/2013 11:17 PM, Eduardo Valentin wrote:
> This patch adds to lm75 temperature sensor the possibility
> to expose itself as thermal zone device, registered on the
> thermal framework.
>
> The thermal zone is built only if a device tree node
> describing a thermal zone for this sensor is present
> inside the lm75 DT node. Otherwise, the driver behavior
> will be the same.
>
> Cc: Jean Delvare
> Cc: Guenter Roeck
> Cc: lm-sens...@lm-sensors.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Eduardo Valentin
> ---
> drivers/hwmon/lm75.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index c03b490..0aa5e28 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -27,6 +27,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include "lm75.h"
>
>
> @@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
> /* Each client has this additional data */
> struct lm75_data {
> struct device *hwmon_dev;
> + struct thermal_zone_device *tz;
> struct mutexupdate_lock;
> u8 orig_conf;
> u8 resolution; /* In bits, between 9 and 12 */
> @@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device
> *dev);
>
> /* sysfs attributes for hwmon */
>
> +static int lm75_read_temp(void *dev, unsigned long *temp)
> +{
> + struct lm75_data *data = lm75_update_device(dev);
> +
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + *temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
> + (data->resolution - 8);
> +
> + return 0;
> +}
> +
> static ssize_t show_temp(struct device *dev, struct device_attribute *da,
>char *buf)
> {
> @@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct
> i2c_device_id *id)
> goto exit_remove;
> }
>
> + if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
> + data->tz = thermal_zone_of_device_register(&client->dev,
> +&client->dev,
> +lm75_read_temp);
Hi, Eduardo
I have two questions:
1. As we know, after register to the thermal framework, it will have
duplicate hwmon devices. I think lm-sensor maintainer would not like this.
How about to add a flag to indicate it, which I talk about with
Durgadoss in your [RFC 1/4]patch.
2. I'm also trying to use your codes on lm90. The LM90 serial has more
then one sensors in one chip, local sensor, remote sensor and may have
remote2 sensor, so it mean there may have more than one thermal_zone
under the lm90 device node, will you consider it?
Thanks.
Wei.
> + if (IS_ERR(data->tz)) {
> + status = PTR_ERR(data->tz);
> + goto exit_hwmon;
> + }
> + }
> +
> dev_info(&client->dev, "%s: sensor '%s'\n",
>dev_name(data->hwmon_dev), client->name);
>
> return 0;
>
> +exit_hwmon:
> + hwmon_device_unregister(data->hwmon_dev);
> exit_remove:
> sysfs_remove_group(&client->dev.kobj, &lm75_group);
> return status;
> @@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
> {
> struct lm75_data *data = i2c_get_clientdata(client);
>
> + thermal_zone_device_unregister(data->tz);
> hwmon_device_unregister(data->hwmon_dev);
> sysfs_remove_group(&client->dev.kobj, &lm75_group);
> lm75_write_value(client, LM75_REG_CONF, data->orig_conf);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RESEND PATCH V1 8/9] hwmon: lm75: expose to thermal fw via DT nodes
This patch adds to lm75 temperature sensor the possibility
to expose itself as thermal zone device, registered on the
thermal framework.
The thermal zone is built only if a device tree node
describing a thermal zone for this sensor is present
inside the lm75 DT node. Otherwise, the driver behavior
will be the same.
Cc: Jean Delvare
Cc: Guenter Roeck
Cc: lm-sens...@lm-sensors.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin
---
drivers/hwmon/lm75.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index c03b490..0aa5e28 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -27,6 +27,8 @@
#include
#include
#include
+#include
+#include
#include "lm75.h"
@@ -70,6 +72,7 @@ static const u8 LM75_REG_TEMP[3] = {
/* Each client has this additional data */
struct lm75_data {
struct device *hwmon_dev;
+ struct thermal_zone_device *tz;
struct mutexupdate_lock;
u8 orig_conf;
u8 resolution; /* In bits, between 9 and 12 */
@@ -92,6 +95,19 @@ static struct lm75_data *lm75_update_device(struct device
*dev);
/* sysfs attributes for hwmon */
+static int lm75_read_temp(void *dev, unsigned long *temp)
+{
+ struct lm75_data *data = lm75_update_device(dev);
+
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ *temp = ((data->temp[0] >> (16 - data->resolution)) * 1000) >>
+ (data->resolution - 8);
+
+ return 0;
+}
+
static ssize_t show_temp(struct device *dev, struct device_attribute *da,
char *buf)
{
@@ -271,11 +287,23 @@ lm75_probe(struct i2c_client *client, const struct
i2c_device_id *id)
goto exit_remove;
}
+ if (of_find_node_by_name(client->dev.of_node, "thermal_zone")) {
+ data->tz = thermal_zone_of_device_register(&client->dev,
+ &client->dev,
+ lm75_read_temp);
+ if (IS_ERR(data->tz)) {
+ status = PTR_ERR(data->tz);
+ goto exit_hwmon;
+ }
+ }
+
dev_info(&client->dev, "%s: sensor '%s'\n",
dev_name(data->hwmon_dev), client->name);
return 0;
+exit_hwmon:
+ hwmon_device_unregister(data->hwmon_dev);
exit_remove:
sysfs_remove_group(&client->dev.kobj, &lm75_group);
return status;
@@ -285,6 +313,7 @@ static int lm75_remove(struct i2c_client *client)
{
struct lm75_data *data = i2c_get_clientdata(client);
+ thermal_zone_device_unregister(data->tz);
hwmon_device_unregister(data->hwmon_dev);
sysfs_remove_group(&client->dev.kobj, &lm75_group);
lm75_write_value(client, LM75_REG_CONF, data->orig_conf);
--
1.8.2.1.342.gfa7285d
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
