Re: [PATCH v5 1/2] hwmon: new driver for ST stts751 thermal sensor
On Tue, Jan 31, 2017 at 12:47 PM, Guenter Roeck <li...@roeck-us.net> wrote: > On 01/30/2017 11:11 PM, Andrea Merello wrote: >> >> On Mon, Jan 30, 2017 at 3:24 PM, Guenter Roeck <li...@roeck-us.net> wrote: >>> >>> On 01/30/2017 01:40 AM, Andrea Merello wrote: >>>> >>>> >>>> This patch adds a HWMON driver for ST Microelectronics STTS751 >>>> temperature sensors. >>>> >>> >>> Overall looks good. One thing I noticed, though: In the log function >>> calls, >>> you use a mix of priv->dev and >dev as log device. Also, you >>> sometimes >>> use >client->dev even when a local client variable is available. >>> I can fix up the latter (no need to resend for that), but please have a >>> look at the overall use and either send a fixed up version or let me know >>> if the log devices are all intentional as-is. >> >> >> Hmm, you are right: there is a bit of mess here. It's not intentional >> indeed.. >> >> I gave a look to lm90.c and lm63.c; it seems no one uses the hwmon >> device for this purpose. We can probably stick to >dev or >> >client->dev when a local client variable is not already >> available. > > > It is really your call as the driver's author to make if you want to use > the hwmon device or >dev. I've decided to stick with >dev for all dev_dbg(), while I would use the hwmon device for dev_notice() and dev_warn(), that are supposed to be eventually read by the user. However, while keeping an eye on the log messages I've realized that if the temperature remains out-of-limit for a lot of time, especially if the conversion rate is set fast, we tend to spam the log with alert messages.. I learned that you are usually concerned by noisy drivers, so I guess you might want to mitigate this.. I could use dev_notice_once(), but it seems inappropriate here, or I can silence the message at least until one reads the chip. Lm90 does even disable the alarm until that happen, but it has only one alarm; here we have two, so it seems not correct here. Any suggestion? >> >> >> Also, it looks like we can change some >client->dev in >> >dev in probe function when calling i2c read/write functions. >> >> If you ACK, I'll fix and resend. >> > SGTM. > > Thanks, > Guenter > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/2] hwmon: new driver for ST stts751 thermal sensor
This patch adds a HWMON driver for ST Microelectronics STTS751
temperature sensors.
Changes since V3:
- fix sysfs_notify() filename
- simplify error exit path in stts751_update_temp(): kill useless goto
- fix set function were racy due to lock incorrectly placed
- use msecs_to_jiffies instead of manual calculation
- fix jiffies and msecs were mixed up
- don't mark cache as good unless reads really succeded to fill up it
- when updating therm, hyst keeps relative value rather than abs value
- don't should in dev_dbg when chip is detected
- don't ever generate two uevents
- clarify clear-on-read behaviour of status reg alarm flags
- simplify mDEG to HW conversion function
- fix chip stops due to incorrect resolution configuration
- fix incorrectly reading negative numbers from HW
Changes since V4:
- fix continuation lines (style, remove uncecessary, ..)
- shorten a unnecessary verbose dev_warn() message
- fix set_interval() exitpath didn't return error
- change uncecesary neg-logic
- change file symbolic permissions with octal
- move chip revision test in probe(), change dev_notice() in dev_dbg()
- use "return 0" on success (non functional change)
- add further checks in i2c detect function
- fix i2c detect function read wrong reg while checking for manufacturer ID
Thanks-to: LABBE Corentin [for suggestions]
Thanks-to: Guenter Roeck [for suggestion and discussions]
Signed-off-by: Andrea Merello <andrea.mere...@gmail.com>
Cc: LABBE Corentin <clabbe.montj...@gmail.com>
Cc: Guenter Roeck <li...@roeck-us.net>
Cc: Jean Delvare <jdelv...@suse.com>
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/stts751.c | 819
3 files changed, 830 insertions(+)
create mode 100644 drivers/hwmon/stts751.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index d1f82f2..8fdd241 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1448,6 +1448,16 @@ config SENSORS_SCH5636
This driver can also be built as a module. If so, the module
will be called sch5636.
+config SENSORS_STTS751
+ tristate "ST Microelectronics STTS751"
+ depends on I2C
+ help
+ If you say yes here you get support for STTS751
+ temperature sensor chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called stts751.
+
config SENSORS_SMM665
tristate "Summit Microelectronics SMM665"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 7476b22..1114130 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_SMM665)+= smm665.o
obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o
obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
+obj-$(CONFIG_SENSORS_STTS751) += stts751.o
obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o
obj-$(CONFIG_SENSORS_TC74) += tc74.o
obj-$(CONFIG_SENSORS_THMC50) += thmc50.o
diff --git a/drivers/hwmon/stts751.c b/drivers/hwmon/stts751.c
new file mode 100644
index 000..4154294
--- /dev/null
+++ b/drivers/hwmon/stts751.c
@@ -0,0 +1,819 @@
+/*
+ * STTS751 sensor driver
+ *
+ * Copyright (C) 2016-2017 Istituto Italiano di Tecnologia - RBCS - EDL
+ * Robotics, Brain and Cognitive Sciences department
+ * Electronic Design Laboratory
+ *
+ * Written by Andrea Merello <andrea.mere...@gmail.com>
+ *
+ * Based on LM95241 driver and LM90 driver
+ *
+ * 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEVNAME "stts751"
+
+static const unsigned short normal_i2c[] = {
+ 0x48, 0x49, 0x38, 0x39, /* STTS751-0 */
+ 0x4A, 0x4B, 0x3A, 0x3B, /* STTS751-1 */
+ I2C_CLIENT_END };
+
+#define STTS751_REG_TEMP_H 0x00
+#define STTS751_REG_STATUS 0x01
+#define STTS751_STATUS_TRIPT BIT(0)
+#define STTS751_STATUS_TRIPL BIT(5)
+#define STTS751_STATUS_TRIPH BIT(6)
+#define STTS751_REG_TEMP_L 0x02
+#define STTS751_REG_CONF 0x03
+#define STTS751_CONF_RES_MASK 0x0C
+#define STTS751_CONF_RES_SHIFT 2
+#define STTS751_CONF_EVENT_DIS BIT(7)
+#define STTS751_CONF_STOP BIT(6)
+#define STTS751_REG_RATE 0x04
+#define STTS751_REG_HLIM_H 0x05
+#define STTS751_REG_HLIM_L 0x06
+#d
Re: [PATCH v4 1/2] hwmon: new driver for ST stts751 thermal sensor
On Sun, Jan 29, 2017 at 7:18 PM, Guenter Roeck <li...@roeck-us.net> wrote: > On 01/26/2017 06:17 AM, Andrea Merello wrote: >> >> This patch adds a HWMON driver for ST Microelectronics STTS751 >> temperature sensors. >> >> Changes since V3: >> - fix sysfs_notify() filename >> - simplify error exit path in stts751_update_temp(): kill useless goto >> - fix set function were racy due to lock incorrectly placed >> - use msecs_to_jiffies instead of manual calculation >> - fix jiffies and msecs were mixed up >> - don't mark cache as good unless reads really succeded to fill up it >> - when updating therm, hyst keeps relative value rather than abs value >> - don't should in dev_dbg when chip is detected >> - don't ever generate two uevents >> - clarify clear-on-read behaviour of status reg alarm flags >> - simplify mDEG to HW conversion function >> - fix chip stops due to incorrect resolution configuration >> - fix incorrectly reading negative numbers from HW >> >> Thanks-to: LABBE Corentin [for suggestions] >> Signed-off-by: Andrea Merello <andrea.mere...@gmail.com> >> Cc: LABBE Corentin <clabbe.montj...@gmail.com> >> Cc: Guenter Roeck <li...@roeck-us.net> >> Cc: Jean Delvare <jdelv...@suse.com> > > > [ ... ] > >> + >> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_input, NULL, 0); >> +static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_min, >> set_min, 0); >> +static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_max, >> set_max, 0); >> +static SENSOR_DEVICE_ATTR(temp1_min_alarm, S_IRUGO, show_min_alarm, NULL, >> 0); >> +static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, show_max_alarm, NULL, >> 0); >> +static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_therm, >> + set_therm, 0); >> +static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_hyst, >> + set_hyst, 0); >> +static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_therm_trip, >> NULL, 0); >> +static SENSOR_DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, >> + show_interval, set_interval, 0); > > > I forgot to mention: Symbolic permissions ran out of favor upstream. > Please use octals above to make the annoying checkpatch warnings go away. OK > Thanks, > Guenter > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] hwmon: new driver for ST stts751 thermal sensor
On Sun, Jan 29, 2017 at 7:13 PM, Guenter Roeck <li...@roeck-us.net> wrote:
> On 01/26/2017 06:17 AM, Andrea Merello wrote:
>>
>> This patch adds a HWMON driver for ST Microelectronics STTS751
>> temperature sensors.
>>
>> Changes since V3:
>> - fix sysfs_notify() filename
>> - simplify error exit path in stts751_update_temp(): kill useless goto
>> - fix set function were racy due to lock incorrectly placed
>> - use msecs_to_jiffies instead of manual calculation
>> - fix jiffies and msecs were mixed up
>> - don't mark cache as good unless reads really succeded to fill up it
>> - when updating therm, hyst keeps relative value rather than abs value
>> - don't should in dev_dbg when chip is detected
>> - don't ever generate two uevents
>> - clarify clear-on-read behaviour of status reg alarm flags
>> - simplify mDEG to HW conversion function
>> - fix chip stops due to incorrect resolution configuration
>> - fix incorrectly reading negative numbers from HW
>>
>> Thanks-to: LABBE Corentin [for suggestions]
>> Signed-off-by: Andrea Merello <andrea.mere...@gmail.com>
>> Cc: LABBE Corentin <clabbe.montj...@gmail.com>
>> Cc: Guenter Roeck <li...@roeck-us.net>
>> Cc: Jean Delvare <jdelv...@suse.com>
>> ---
>> drivers/hwmon/Kconfig | 10 +
>> drivers/hwmon/Makefile | 1 +
>> drivers/hwmon/stts751.c | 803
>>
>> 3 files changed, 814 insertions(+)
>> create mode 100644 drivers/hwmon/stts751.c
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index d1f82f2..8fdd241 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -1448,6 +1448,16 @@ config SENSORS_SCH5636
>> This driver can also be built as a module. If so, the module
>> will be called sch5636.
>>
>> +config SENSORS_STTS751
>> + tristate "ST Microelectronics STTS751"
>> + depends on I2C
>> + help
>> + If you say yes here you get support for STTS751
>> + temperature sensor chips.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called stts751.
>> +
>> config SENSORS_SMM665
>> tristate "Summit Microelectronics SMM665"
>> depends on I2C
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 7476b22..1114130 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_SMM665)+= smm665.o
>> obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
>> obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o
>> obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
>> +obj-$(CONFIG_SENSORS_STTS751) += stts751.o
>> obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o
>> obj-$(CONFIG_SENSORS_TC74) += tc74.o
>> obj-$(CONFIG_SENSORS_THMC50) += thmc50.o
>> diff --git a/drivers/hwmon/stts751.c b/drivers/hwmon/stts751.c
>> new file mode 100644
>> index 000..de7298f
>> --- /dev/null
>> +++ b/drivers/hwmon/stts751.c
>> @@ -0,0 +1,803 @@
>> +/*
>> + * STTS751 sensor driver
>> + *
>> + * Copyright (C) 2016-2017 Istituto Italiano di Tecnologia - RBCS - EDL
>> + * Robotics, Brain and Cognitive Sciences department
>> + * Electronic Design Laboratory
>> + *
>> + * Written by Andrea Merello <andrea.mere...@gmail.com>
>> + *
>> + * Based on LM95241 driver and LM90 driver
>> + *
>> + * 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
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define DEVNAME "stts751"
>> +
>> +static const unsigned short normal_i2c[] = {
>> + 0x48, 0x49, 0x38, 0x39, /* STTS751-0 */
>> + 0x4A, 0x4B, 0x3A
[PATCH v4 1/2] hwmon: new driver for ST stts751 thermal sensor
This patch adds a HWMON driver for ST Microelectronics STTS751
temperature sensors.
Changes since V3:
- fix sysfs_notify() filename
- simplify error exit path in stts751_update_temp(): kill useless goto
- fix set function were racy due to lock incorrectly placed
- use msecs_to_jiffies instead of manual calculation
- fix jiffies and msecs were mixed up
- don't mark cache as good unless reads really succeded to fill up it
- when updating therm, hyst keeps relative value rather than abs value
- don't should in dev_dbg when chip is detected
- don't ever generate two uevents
- clarify clear-on-read behaviour of status reg alarm flags
- simplify mDEG to HW conversion function
- fix chip stops due to incorrect resolution configuration
- fix incorrectly reading negative numbers from HW
Thanks-to: LABBE Corentin [for suggestions]
Signed-off-by: Andrea Merello <andrea.mere...@gmail.com>
Cc: LABBE Corentin <clabbe.montj...@gmail.com>
Cc: Guenter Roeck <li...@roeck-us.net>
Cc: Jean Delvare <jdelv...@suse.com>
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/stts751.c | 803
3 files changed, 814 insertions(+)
create mode 100644 drivers/hwmon/stts751.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index d1f82f2..8fdd241 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1448,6 +1448,16 @@ config SENSORS_SCH5636
This driver can also be built as a module. If so, the module
will be called sch5636.
+config SENSORS_STTS751
+ tristate "ST Microelectronics STTS751"
+ depends on I2C
+ help
+ If you say yes here you get support for STTS751
+ temperature sensor chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called stts751.
+
config SENSORS_SMM665
tristate "Summit Microelectronics SMM665"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 7476b22..1114130 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_SMM665)+= smm665.o
obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o
obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
+obj-$(CONFIG_SENSORS_STTS751) += stts751.o
obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o
obj-$(CONFIG_SENSORS_TC74) += tc74.o
obj-$(CONFIG_SENSORS_THMC50) += thmc50.o
diff --git a/drivers/hwmon/stts751.c b/drivers/hwmon/stts751.c
new file mode 100644
index 000..de7298f
--- /dev/null
+++ b/drivers/hwmon/stts751.c
@@ -0,0 +1,803 @@
+/*
+ * STTS751 sensor driver
+ *
+ * Copyright (C) 2016-2017 Istituto Italiano di Tecnologia - RBCS - EDL
+ * Robotics, Brain and Cognitive Sciences department
+ * Electronic Design Laboratory
+ *
+ * Written by Andrea Merello <andrea.mere...@gmail.com>
+ *
+ * Based on LM95241 driver and LM90 driver
+ *
+ * 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEVNAME "stts751"
+
+static const unsigned short normal_i2c[] = {
+ 0x48, 0x49, 0x38, 0x39, /* STTS751-0 */
+ 0x4A, 0x4B, 0x3A, 0x3B, /* STTS751-1 */
+ I2C_CLIENT_END };
+
+#define STTS751_REG_TEMP_H 0x00
+#define STTS751_REG_STATUS 0x01
+#define STTS751_STATUS_TRIPT BIT(0)
+#define STTS751_STATUS_TRIPL BIT(5)
+#define STTS751_STATUS_TRIPH BIT(6)
+#define STTS751_REG_TEMP_L 0x02
+#define STTS751_REG_CONF 0x03
+#define STTS751_CONF_RES_MASK 0x0C
+#define STTS751_CONF_RES_SHIFT 2
+#define STTS751_CONF_EVENT_DIS BIT(7)
+#define STTS751_CONF_STOP BIT(6)
+#define STTS751_REG_RATE 0x04
+#define STTS751_REG_HLIM_H 0x05
+#define STTS751_REG_HLIM_L 0x06
+#define STTS751_REG_LLIM_H 0x07
+#define STTS751_REG_LLIM_L 0x08
+#define STTS751_REG_TLIM 0x20
+#define STTS751_REG_HYST 0x21
+#define STTS751_REG_SMBUS_TO 0x22
+
+#define STTS751_REG_PROD_ID0xFD
+#define STTS751_REG_MAN_ID 0xFE
+#define STTS751_REG_REV_ID 0xFF
+
+#define STTS751_0_PROD_ID 0x00
+#define STTS751_1_PROD_ID 0x01
+#define ST_MAN_ID 0x53
+
+/*
+ * Possible update intervals are (in mS):
+ * 16000, 8000, 4000, 2000, 1000, 500, 250, 125, 62.5, 31.25
+ * However we are not going to complicat
Re: [PATCH v3 1/2] hwmon: new driver for ST stts751 thermal sensor
On Tue, Jan 24, 2017 at 9:18 PM, Guenter Roeck <li...@roeck-us.net> wrote:
> On Tue, Jan 24, 2017 at 04:02:20PM +0100, Andrea Merello wrote:
>> This patch adds a HWMON driver for ST Microelectronics STTS751
>> temperature sensors.
>>
>> It does support manual-triggered conversions as well as automatic
>> conversions. The latter is used when the "event" or "therm" function
>> is present (declaring the physical wire is attached in the DT).
>>
>> Thanks-to: LABBE Corentin [for suggestions]
>> Signed-off-by: Andrea Merello <andrea.mere...@gmail.com>
>> Cc: LABBE Corentin <clabbe.montj...@gmail.com>
>> Cc: Guenter Roeck <li...@roeck-us.net>
>> Cc: Jean Delvare <jdelv...@suse.com>
>> ---
>
> No change log, meaning we have to review the entire driver again or look up
> older versions and comments ourselves. This will take a while. For now, just a
> few quick commnents. For the future, please consider providing a changelog.
Ok, of course. I'll do.
BTW most I've done is basically what we discussed; If you want I can
provide you the list of commits (before squashing/rebase), that should
be basically similar to a changelog.
>> drivers/hwmon/Kconfig | 10 +
>> drivers/hwmon/Makefile | 1 +
>> drivers/hwmon/stts751.c | 809
>>
>> 3 files changed, 820 insertions(+)
>> create mode 100644 drivers/hwmon/stts751.c
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index d1f82f2..8fdd241 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -1448,6 +1448,16 @@ config SENSORS_SCH5636
>> This driver can also be built as a module. If so, the module
>> will be called sch5636.
>>
>> +config SENSORS_STTS751
>> + tristate "ST Microelectronics STTS751"
>> + depends on I2C
>> + help
>> + If you say yes here you get support for STTS751
>> + temperature sensor chips.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called stts751.
>> +
>> config SENSORS_SMM665
>> tristate "Summit Microelectronics SMM665"
>> depends on I2C
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 7476b22..1114130 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_SMM665) += smm665.o
>> obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
>> obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o
>> obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
>> +obj-$(CONFIG_SENSORS_STTS751)+= stts751.o
>> obj-$(CONFIG_SENSORS_AMC6821)+= amc6821.o
>> obj-$(CONFIG_SENSORS_TC74) += tc74.o
>> obj-$(CONFIG_SENSORS_THMC50) += thmc50.o
>> diff --git a/drivers/hwmon/stts751.c b/drivers/hwmon/stts751.c
>> new file mode 100644
>> index 000..0d4716f
>> --- /dev/null
>> +++ b/drivers/hwmon/stts751.c
>> @@ -0,0 +1,809 @@
>> +/*
>> + * STTS751 sensor driver
>> + *
>> + * Copyright (C) 2016-2017 Istituto Italiano di Tecnologia - RBCS - EDL
>> + * Robotics, Brain and Cognitive Sciences department
>> + * Electronic Design Laboratory
>> + *
>> + * Written by Andrea Merello <andrea.mere...@gmail.com>
>> + *
>> + * Based on LM95241 driver and LM90 driver
>> + *
>> + * 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
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define DEVNAME "stts751"
>> +
>> +static const unsigned short normal_i2c[] = {
>> + 0x48, 0x49, 0x38, 0x39, /* STTS751-0 */
>> + 0x4A, 0x4B, 0x3A, 0x3B, /* STTS751-1 */
>> + I2C_CLIENT_END };
>> +
>> +#define STTS751_REG_TEMP_H 0x00
>> +#define STTS
[PATCH v3 1/2] hwmon: new driver for ST stts751 thermal sensor
This patch adds a HWMON driver for ST Microelectronics STTS751
temperature sensors.
It does support manual-triggered conversions as well as automatic
conversions. The latter is used when the "event" or "therm" function
is present (declaring the physical wire is attached in the DT).
Thanks-to: LABBE Corentin [for suggestions]
Signed-off-by: Andrea Merello <andrea.mere...@gmail.com>
Cc: LABBE Corentin <clabbe.montj...@gmail.com>
Cc: Guenter Roeck <li...@roeck-us.net>
Cc: Jean Delvare <jdelv...@suse.com>
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/stts751.c | 809
3 files changed, 820 insertions(+)
create mode 100644 drivers/hwmon/stts751.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index d1f82f2..8fdd241 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1448,6 +1448,16 @@ config SENSORS_SCH5636
This driver can also be built as a module. If so, the module
will be called sch5636.
+config SENSORS_STTS751
+ tristate "ST Microelectronics STTS751"
+ depends on I2C
+ help
+ If you say yes here you get support for STTS751
+ temperature sensor chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called stts751.
+
config SENSORS_SMM665
tristate "Summit Microelectronics SMM665"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 7476b22..1114130 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_SMM665)+= smm665.o
obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o
obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
+obj-$(CONFIG_SENSORS_STTS751) += stts751.o
obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o
obj-$(CONFIG_SENSORS_TC74) += tc74.o
obj-$(CONFIG_SENSORS_THMC50) += thmc50.o
diff --git a/drivers/hwmon/stts751.c b/drivers/hwmon/stts751.c
new file mode 100644
index 000..0d4716f
--- /dev/null
+++ b/drivers/hwmon/stts751.c
@@ -0,0 +1,809 @@
+/*
+ * STTS751 sensor driver
+ *
+ * Copyright (C) 2016-2017 Istituto Italiano di Tecnologia - RBCS - EDL
+ * Robotics, Brain and Cognitive Sciences department
+ * Electronic Design Laboratory
+ *
+ * Written by Andrea Merello <andrea.mere...@gmail.com>
+ *
+ * Based on LM95241 driver and LM90 driver
+ *
+ * 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEVNAME "stts751"
+
+static const unsigned short normal_i2c[] = {
+ 0x48, 0x49, 0x38, 0x39, /* STTS751-0 */
+ 0x4A, 0x4B, 0x3A, 0x3B, /* STTS751-1 */
+ I2C_CLIENT_END };
+
+#define STTS751_REG_TEMP_H 0x00
+#define STTS751_REG_STATUS 0x01
+#define STTS751_STATUS_TRIPT BIT(0)
+#define STTS751_STATUS_TRIPL BIT(5)
+#define STTS751_STATUS_TRIPH BIT(6)
+#define STTS751_REG_TEMP_L 0x02
+#define STTS751_REG_CONF 0x03
+#define STTS751_CONF_RES_MASK 0x0C
+#define STTS751_CONF_RES_SHIFT 2
+#define STTS751_CONF_EVENT_DIS BIT(7)
+#define STTS751_CONF_STOP BIT(6)
+#define STTS751_REG_RATE 0x04
+#define STTS751_REG_HLIM_H 0x05
+#define STTS751_REG_HLIM_L 0x06
+#define STTS751_REG_LLIM_H 0x07
+#define STTS751_REG_LLIM_L 0x08
+#define STTS751_REG_TLIM 0x20
+#define STTS751_REG_HYST 0x21
+#define STTS751_REG_SMBUS_TO 0x22
+
+#define STTS751_REG_PROD_ID0xFD
+#define STTS751_REG_MAN_ID 0xFE
+#define STTS751_REG_REV_ID 0xFF
+
+#define STTS751_0_PROD_ID 0x00
+#define STTS751_1_PROD_ID 0x01
+#define ST_MAN_ID 0x53
+
+/*
+ * Possible update intervals are (in mS):
+ * 16000, 8000, 4000, 2000, 1000, 500, 250, 125, 62.5, 31.25
+ * However we are not going to complicate things too much and we stick to the
+ * approx value in mS.
+ */
+static const int stts751_intervals[] = {
+ 16000, 8000, 4000, 2000, 1000, 500, 250, 125, 63, 31
+};
+
+static const struct i2c_device_id stts751_id[] = {
+ { "stts751", 0 },
+ { }
+};
+
+struct stts751_priv {
+ struct device *dev;
+ struct i2c_client *client;
+ struct mutex access_lock;
+ u8 interval;
+ int res;
+ int event_max, event_min;
+ int th
Re: [PATCH v2 2/2] hwmon: new driver for ST stts751 thermal sensor
On Fri, Nov 4, 2016 at 5:01 PM, Guenter Roeck <li...@roeck-us.net> wrote: > On Fri, Nov 04, 2016 at 04:43:46PM +0100, Andrea Merello wrote: >> On Fri, Nov 4, 2016 at 4:09 PM, Guenter Roeck <li...@roeck-us.net> wrote: >> > On Fri, Nov 04, 2016 at 09:24:25AM +0100, Andrea Merello wrote: >> >> On Thu, Nov 3, 2016 at 10:16 PM, Guenter Roeck <li...@roeck-us.net> wrote: >> >> > On Thu, Nov 03, 2016 at 08:35:27AM +0100, Andrea Merello wrote: >> >> >> On Wed, Nov 2, 2016 at 11:48 PM, Guenter Roeck <li...@roeck-us.net> >> >> >> wrote: >> >> >> > On Wed, Nov 02, 2016 at 09:11:35AM +0100, Andrea Merello wrote: >> >> >> >> On Mon, Oct 31, 2016 at 5:18 PM, Guenter Roeck <li...@roeck-us.net> >> >> >> >> wrote: >> >> >> >> > On 10/31/2016 08:27 AM, Andrea Merello wrote: >> >> >> >> > >> >> >> >> >>>> >> >> >> >> >>>>>> + if (ret < 0) >> >> >> >> >>>>>> + return ret; >> >> >> >> >>>>>> + >> >> >> >> >>>>>> + priv->max_alert = priv->max_alert || !!(ret & >> >> >> >> >>>>>> STTS751_STATUS_TRIPH); >> >> >> >> >>>>>> + priv->min_alert = priv->min_alert || !!(ret & >> >> >> >> >>>>>> STTS751_STATUS_TRIPL); >> >> >> >> >>>>>> + >> >> >> >> >>>>> >> >> >> >> >>>>> >> >> >> >> >>>>> I am not really happy about the caching of alert values. This >> >> >> >> >>>>> isn't >> >> >> >> >>>>> really >> >> >> >> >>>>> common. Usually we report current alert values when reading. >> >> >> >> >>>>> I don't >> >> >> >> >>>>> really >> >> >> >> >>>>> see why this driver should be different to all the others. >> >> >> >> >>>> >> >> >> >> >>>> >> >> >> >> >>>> >> >> >> >> >>>> OK >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> I've tried to avoid caching, but as soon as I read the status >> >> >> >> >> register >> >> >> >> >> in the alert handler, the device clears that reg, and it will >> >> >> >> >> remain >> >> >> >> >> cleared up to the next conversion. >> >> >> >> >> >> >> >> >> >> So for example if the reader is blocked on poll over the alert >> >> >> >> >> attribute, it gets unblocked on alert, and it will very likely >> >> >> >> >> read >> >> >> >> >> zero.. >> >> >> >> >> >> >> >> >> >> The only way I can see for avoiding caching, and for changing >> >> >> >> >> the API >> >> >> >> >> about clearing events as you asked below, is performing a >> >> >> >> >> temperature >> >> >> >> >> read from the temperature registers and doing software compare >> >> >> >> >> with >> >> >> >> >> the limits in show_[max/min]_alert. >> >> >> >> >> >> >> >> >> >> Does this sounds OK for you ? >> >> >> >> >> >> >> >> >> > >> >> >> >> > Not really. That could be done from user space by just reading the >> >> >> >> > temperature >> >> >> >> > and comparing it against limits. >> >> >> >> >> >> >> >> Sorry, I don't see the point here.. Why the fact that something >> >> >> >> could >> >> >> >> be done in userspace should suggest we don't want to do it in the >> >> >> >> driver? We do, for example, a simple calculation to giv
Re: [PATCH v2 2/2] hwmon: new driver for ST stts751 thermal sensor
On Thu, Nov 3, 2016 at 10:16 PM, Guenter Roeck <li...@roeck-us.net> wrote: > On Thu, Nov 03, 2016 at 08:35:27AM +0100, Andrea Merello wrote: >> On Wed, Nov 2, 2016 at 11:48 PM, Guenter Roeck <li...@roeck-us.net> wrote: >> > On Wed, Nov 02, 2016 at 09:11:35AM +0100, Andrea Merello wrote: >> >> On Mon, Oct 31, 2016 at 5:18 PM, Guenter Roeck <li...@roeck-us.net> wrote: >> >> > On 10/31/2016 08:27 AM, Andrea Merello wrote: >> >> > >> >> >>>> >> >> >>>>>> + if (ret < 0) >> >> >>>>>> + return ret; >> >> >>>>>> + >> >> >>>>>> + priv->max_alert = priv->max_alert || !!(ret & >> >> >>>>>> STTS751_STATUS_TRIPH); >> >> >>>>>> + priv->min_alert = priv->min_alert || !!(ret & >> >> >>>>>> STTS751_STATUS_TRIPL); >> >> >>>>>> + >> >> >>>>> >> >> >>>>> >> >> >>>>> I am not really happy about the caching of alert values. This isn't >> >> >>>>> really >> >> >>>>> common. Usually we report current alert values when reading. I don't >> >> >>>>> really >> >> >>>>> see why this driver should be different to all the others. >> >> >>>> >> >> >>>> >> >> >>>> >> >> >>>> OK >> >> >> >> >> >> >> >> >> I've tried to avoid caching, but as soon as I read the status register >> >> >> in the alert handler, the device clears that reg, and it will remain >> >> >> cleared up to the next conversion. >> >> >> >> >> >> So for example if the reader is blocked on poll over the alert >> >> >> attribute, it gets unblocked on alert, and it will very likely read >> >> >> zero.. >> >> >> >> >> >> The only way I can see for avoiding caching, and for changing the API >> >> >> about clearing events as you asked below, is performing a temperature >> >> >> read from the temperature registers and doing software compare with >> >> >> the limits in show_[max/min]_alert. >> >> >> >> >> >> Does this sounds OK for you ? >> >> >> >> >> > >> >> > Not really. That could be done from user space by just reading the >> >> > temperature >> >> > and comparing it against limits. >> >> >> >> Sorry, I don't see the point here.. Why the fact that something could >> >> be done in userspace should suggest we don't want to do it in the >> >> driver? We do, for example, a simple calculation to give an absolute >> >> value for the hysteresis; we could do this also in userspace, but we >> >> do this in the driver in order to support the proper API.. >> >> >> > Datasheet says: >> > >> > T HIGH : [6] Bit = 1 indicates temperature high limit has been exceeded (T >> > A > >> > high limit). T HIGH is cleared when the status register is read, provided >> > the condition no longer exists. >> > T LOW : [5] Bit = 1 indicates the is at or below the low limit (T A ≤ low >> > limit). T LOW is cleared when the status register is read, provided the >> > condition no longer exists. >> > >> > This isn't exactly what you are claiming, since it says "cleared ... >> > provided >> > that the condition no longer exists", which would be more in line with >> > other >> > chips. >> >> Yes, I know.. I've also read that on the datasheet.. And I tried >> reading the status register in both the alert handler and the show >> function, but it didn't worked :/ >> >> So I started to suspect the status register got actually clear after >> reading up to the next conversion, and to test this, I simply read it >> twice in the alert handler. The first read always read 1, as expected; >> the second time I always got zero, even if the temperature was still >> over-limit.. And I soon got another event, that behaved in the same >> manner. >> > Question is if the event bit is set again after the next conversion; this > would > be relatively easy to handle (cache the status register fo
Re: [PATCH v2 2/2] hwmon: new driver for ST stts751 thermal sensor
On Mon, Oct 31, 2016 at 5:18 PM, Guenter Roeck <li...@roeck-us.net> wrote:
> On 10/31/2016 08:27 AM, Andrea Merello wrote:
>
>>>>
>>>>>> + if (ret < 0)
>>>>>> + return ret;
>>>>>> +
>>>>>> + priv->max_alert = priv->max_alert || !!(ret &
>>>>>> STTS751_STATUS_TRIPH);
>>>>>> + priv->min_alert = priv->min_alert || !!(ret &
>>>>>> STTS751_STATUS_TRIPL);
>>>>>> +
>>>>>
>>>>>
>>>>> I am not really happy about the caching of alert values. This isn't
>>>>> really
>>>>> common. Usually we report current alert values when reading. I don't
>>>>> really
>>>>> see why this driver should be different to all the others.
>>>>
>>>>
>>>>
>>>> OK
>>
>>
>> I've tried to avoid caching, but as soon as I read the status register
>> in the alert handler, the device clears that reg, and it will remain
>> cleared up to the next conversion.
>>
>> So for example if the reader is blocked on poll over the alert
>> attribute, it gets unblocked on alert, and it will very likely read
>> zero..
>>
>> The only way I can see for avoiding caching, and for changing the API
>> about clearing events as you asked below, is performing a temperature
>> read from the temperature registers and doing software compare with
>> the limits in show_[max/min]_alert.
>>
>> Does this sounds OK for you ?
>>
>
> Not really. That could be done from user space by just reading the
> temperature
> and comparing it against limits.
Sorry, I don't see the point here.. Why the fact that something could
be done in userspace should suggest we don't want to do it in the
driver? We do, for example, a simple calculation to give an absolute
value for the hysteresis; we could do this also in userspace, but we
do this in the driver in order to support the proper API..
> If the chip clears the alert bit on read, why would you need a set attribute
> to clear it ?
That was used to clear the cached value, not the chip reg.
> From what you are saying, the next alert should update all
> alert bits, meaning they would be auto-updated at that time. User space
> would then always read the current state.
>
> Sure, you would cache the value
> in the alert function to be able to read it in the read function, but I
> don't see why all the complexity around it in your code would be needed.
The next alert is, by definition, an alert, so it will always update
the cached value with a '1'. We don't have anything that tell us when
the alert is gone. No one would clear the cached value with '0' when
the alert is gone. (ok, we have two alert bits, so the _other_ one of
them will be set to zero, but that's not the point)..
So, in order to report the current alarm value, without relying to the
user to clear it, as you asked for, caching seems not an option..
> You might want to update the alert status in case there is no interrupt
> support, though (maybe you do already; I didn't re-check the code).
OK, good point, I'll make sure it works event in this case.
> The 'sensors' code doesn't know that it is supposed to write into the
> alert attribute to clear the status, thus unless you have a special
> application, an alert would never be cleared. That doesn't sound very
> appealing to me.
So, OK to change the API as you said, however I don't see how this can
be done except for doing the manual temperature/limit comparison in
the driver:
When you get into show_[max/min]_alert function you can't rely on
reading the status register (because as I said in my prev mail it
could have been just cleared by reading it in the event handler), but
as I explained above it seems to me that you cannot also rely on a
value cached in the event handler with this API, because no one will
ever clear it..
In other words you will see a status register that could be zero even
when we have the alarm, or a cached value that is still 1 even if the
alarm has gone time ago.. I could clear the cached value in the
show_[max/min]_alert function itself, but this doesn't seems enough to
me for reporting to the userspace the _current_ alarm value, as you
asked for.
Or am I missing something ?
>
>>>>
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static void stts751_alert(struct i2c_client *client,
>>>>>> + enum i2c_alert_protocol type, unsigned int data)
>>>>>> +{
>>>>>> + int ret;
>>>>>> + struct stts751_priv *pri
Re: [PATCH v2 2/2] hwmon: new driver for ST stts751 thermal sensor
On Wed, Oct 19, 2016 at 4:50 PM, Guenter Roeck <li...@roeck-us.net> wrote: > On 10/19/2016 05:16 AM, Andrea Merello wrote: >> >> On Wed, Oct 5, 2016 at 1:51 AM, Guenter Roeck <li...@roeck-us.net> wrote: >>> >>> On Tue, Sep 27, 2016 at 02:07:27PM +0200, Andrea Merello wrote: >>>> >>>> This patch adds a HWMON driver for ST Microelectronics STTS751 >>>> temperature sensors. >>>> >>>> It does support temperature reading (of course), SMBUS alert >>>> functionality and "therm" output. >>>> >>>> Thanks-to: LABBE Corentin [for suggestions] >>>> Thanks-to: Guenter Roeck [for suggestions/discussions] >>>> Signed-off-by: Andrea Merello <andrea.mere...@gmail.com> >>>> Cc: LABBE Corentin <clabbe.montj...@gmail.com> >>>> Cc: Guenter Roeck <li...@roeck-us.net> >>>> Cc: Jean Delvare <jdelv...@suse.com> >>>> --- >>>> drivers/hwmon/Kconfig | 12 +- >>>> drivers/hwmon/Makefile | 2 +- >>>> drivers/hwmon/stts751.c | 805 >>>> >>>> 3 files changed, 817 insertions(+), 2 deletions(-) >>>> create mode 100644 drivers/hwmon/stts751.c >>>> >>>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig >>>> index eaf2f91..4e70b42 100644 >>>> --- a/drivers/hwmon/Kconfig >>>> +++ b/drivers/hwmon/Kconfig >>>> @@ -1448,6 +1448,16 @@ config SENSORS_SCH5636 >>>> This driver can also be built as a module. If so, the module >>>> will be called sch5636. >>>> >>>> +config SENSORS_STTS751 >>>> + tristate "ST Microelectronics STTS751" >>>> + depends on I2C >>>> + help >>>> + If you say yes here you get support for STTS751 >>>> + temperature sensor chips. >>>> + >>>> + This driver can also be built as a module. If so, the module >>>> + will be called stts751. >>>> + >>>> config SENSORS_SMM665 >>>> tristate "Summit Microelectronics SMM665" >>>> depends on I2C >>>> @@ -1506,7 +1516,7 @@ config SENSORS_ADS7871 >>>> >>>> config SENSORS_AMC6821 >>>> tristate "Texas Instruments AMC6821" >>>> - depends on I2C >>>> + depends on I2C >>> >>> >>> ??? >> >> >> My editor is kind enough to automatically kill trailing spaces.. I'll >> take care of moving this one in another patch in next series version.. >> >>> >>>> help >>>> If you say yes here you get support for the Texas Instruments >>>> AMC6821 hardware monitoring chips. >>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile >>>> index fe87d28..1114130 100644 >>>> --- a/drivers/hwmon/Makefile >>>> +++ b/drivers/hwmon/Makefile >>>> @@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_SMM665) += smm665.o >>>> obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o >>>> obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o >>>> obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o >>>> +obj-$(CONFIG_SENSORS_STTS751) += stts751.o >>>> obj-$(CONFIG_SENSORS_AMC6821)+= amc6821.o >>>> obj-$(CONFIG_SENSORS_TC74) += tc74.o >>>> obj-$(CONFIG_SENSORS_THMC50) += thmc50.o >>>> @@ -169,4 +170,3 @@ obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o >>>> obj-$(CONFIG_PMBUS) += pmbus/ >>>> >>>> ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG >>>> - >>> >>> >>> Looks like an unnecessary whitespace change. >> >> >> Like above.. >> >>> >>>> diff --git a/drivers/hwmon/stts751.c b/drivers/hwmon/stts751.c >>>> new file mode 100644 >>>> index 000..8358e04 >>>> --- /dev/null >>>> +++ b/drivers/hwmon/stts751.c >>>> @@ -0,0 +1,805 @@ >>>> +/* >>>> + * STTS751 sensor driver >>>> + * >>>> + * Copyright (C) 2016 Istituto Italiano di Tecnologia - RBCS - EDL >>>> + * Robotics, Brain and Cognitive Sciences department >>>> + * Electronic Design Laboratory >>>> + * >>>> + * Written by Andrea Merello <andrea.mere...@gmail.com> >>>> + * >
Re: [PATCH v2 2/2] hwmon: new driver for ST stts751 thermal sensor
On Wed, Oct 5, 2016 at 1:51 AM, Guenter Roeck <li...@roeck-us.net> wrote: > On Tue, Sep 27, 2016 at 02:07:27PM +0200, Andrea Merello wrote: >> This patch adds a HWMON driver for ST Microelectronics STTS751 >> temperature sensors. >> >> It does support temperature reading (of course), SMBUS alert >> functionality and "therm" output. >> >> Thanks-to: LABBE Corentin [for suggestions] >> Thanks-to: Guenter Roeck [for suggestions/discussions] >> Signed-off-by: Andrea Merello <andrea.mere...@gmail.com> >> Cc: LABBE Corentin <clabbe.montj...@gmail.com> >> Cc: Guenter Roeck <li...@roeck-us.net> >> Cc: Jean Delvare <jdelv...@suse.com> >> --- >> drivers/hwmon/Kconfig | 12 +- >> drivers/hwmon/Makefile | 2 +- >> drivers/hwmon/stts751.c | 805 >> >> 3 files changed, 817 insertions(+), 2 deletions(-) >> create mode 100644 drivers/hwmon/stts751.c >> >> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig >> index eaf2f91..4e70b42 100644 >> --- a/drivers/hwmon/Kconfig >> +++ b/drivers/hwmon/Kconfig >> @@ -1448,6 +1448,16 @@ config SENSORS_SCH5636 >> This driver can also be built as a module. If so, the module >> will be called sch5636. >> >> +config SENSORS_STTS751 >> + tristate "ST Microelectronics STTS751" >> + depends on I2C >> + help >> + If you say yes here you get support for STTS751 >> + temperature sensor chips. >> + >> + This driver can also be built as a module. If so, the module >> + will be called stts751. >> + >> config SENSORS_SMM665 >> tristate "Summit Microelectronics SMM665" >> depends on I2C >> @@ -1506,7 +1516,7 @@ config SENSORS_ADS7871 >> >> config SENSORS_AMC6821 >> tristate "Texas Instruments AMC6821" >> - depends on I2C >> + depends on I2C > > ??? My editor is kind enough to automatically kill trailing spaces.. I'll take care of moving this one in another patch in next series version.. > >> help >> If you say yes here you get support for the Texas Instruments >> AMC6821 hardware monitoring chips. >> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile >> index fe87d28..1114130 100644 >> --- a/drivers/hwmon/Makefile >> +++ b/drivers/hwmon/Makefile >> @@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_SMM665) += smm665.o >> obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o >> obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o >> obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o >> +obj-$(CONFIG_SENSORS_STTS751)+= stts751.o >> obj-$(CONFIG_SENSORS_AMC6821)+= amc6821.o >> obj-$(CONFIG_SENSORS_TC74) += tc74.o >> obj-$(CONFIG_SENSORS_THMC50) += thmc50.o >> @@ -169,4 +170,3 @@ obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o >> obj-$(CONFIG_PMBUS) += pmbus/ >> >> ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG >> - > > Looks like an unnecessary whitespace change. Like above.. > >> diff --git a/drivers/hwmon/stts751.c b/drivers/hwmon/stts751.c >> new file mode 100644 >> index 000..8358e04 >> --- /dev/null >> +++ b/drivers/hwmon/stts751.c >> @@ -0,0 +1,805 @@ >> +/* >> + * STTS751 sensor driver >> + * >> + * Copyright (C) 2016 Istituto Italiano di Tecnologia - RBCS - EDL >> + * Robotics, Brain and Cognitive Sciences department >> + * Electronic Design Laboratory >> + * >> + * Written by Andrea Merello <andrea.mere...@gmail.com> >> + * >> + * Based on LM95241 driver and LM90 driver >> + * >> + * 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. >> + */ >> + >> + > No douple empty lines please. OK. BTW do you use some enhanced "checkpatch" script in order to catch issues like these? > >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >&
[PATCH v2 1/2] DT: add binding documentation for STTS751
Signed-off-by: Andrea Merello <andrea.mere...@gmail.com>
Cc: Rob Herring <robh...@kernel.org>
Cc: Mark Rutland <mark.rutl...@arm.com>
---
Documentation/devicetree/bindings/hwmon/stts751.txt | 19 +++
1 file changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/stts751.txt
diff --git a/Documentation/devicetree/bindings/hwmon/stts751.txt
b/Documentation/devicetree/bindings/hwmon/stts751.txt
new file mode 100644
index 000..277ad46
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/stts751.txt
@@ -0,0 +1,19 @@
+* STTS751 thermometer.
+
+Required node properties:
+- compatible: "stts751"
+- reg: I2C bus address of the device
+
+Optional properties:
+- has-therm: specifies that the "therm" pin is wired
+- has-event: specifies that the "event" pin is wired
+- smbus-timeout-disable: when set, the smbus timeout function will be disabled
+
+Example stts751 node:
+
+temp-sensor {
+ compatible = "stts751";
+ reg = <0x48>;
+ has-therm;
+ has-event;
+}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] hwmon: new driver for ST stts751 thermal sensor
This patch adds a HWMON driver for ST Microelectronics STTS751
temperature sensors.
It does support temperature reading (of course), SMBUS alert
functionality and "therm" output.
Thanks-to: LABBE Corentin [for suggestions]
Thanks-to: Guenter Roeck [for suggestions/discussions]
Signed-off-by: Andrea Merello <andrea.mere...@gmail.com>
Cc: LABBE Corentin <clabbe.montj...@gmail.com>
Cc: Guenter Roeck <li...@roeck-us.net>
Cc: Jean Delvare <jdelv...@suse.com>
---
drivers/hwmon/Kconfig | 12 +-
drivers/hwmon/Makefile | 2 +-
drivers/hwmon/stts751.c | 805
3 files changed, 817 insertions(+), 2 deletions(-)
create mode 100644 drivers/hwmon/stts751.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index eaf2f91..4e70b42 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1448,6 +1448,16 @@ config SENSORS_SCH5636
This driver can also be built as a module. If so, the module
will be called sch5636.
+config SENSORS_STTS751
+ tristate "ST Microelectronics STTS751"
+ depends on I2C
+ help
+ If you say yes here you get support for STTS751
+ temperature sensor chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called stts751.
+
config SENSORS_SMM665
tristate "Summit Microelectronics SMM665"
depends on I2C
@@ -1506,7 +1516,7 @@ config SENSORS_ADS7871
config SENSORS_AMC6821
tristate "Texas Instruments AMC6821"
- depends on I2C
+ depends on I2C
help
If you say yes here you get support for the Texas Instruments
AMC6821 hardware monitoring chips.
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index fe87d28..1114130 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_SMM665)+= smm665.o
obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o
obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
+obj-$(CONFIG_SENSORS_STTS751) += stts751.o
obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o
obj-$(CONFIG_SENSORS_TC74) += tc74.o
obj-$(CONFIG_SENSORS_THMC50) += thmc50.o
@@ -169,4 +170,3 @@ obj-$(CONFIG_SENSORS_WM8350)+= wm8350-hwmon.o
obj-$(CONFIG_PMBUS)+= pmbus/
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
-
diff --git a/drivers/hwmon/stts751.c b/drivers/hwmon/stts751.c
new file mode 100644
index 000..8358e04
--- /dev/null
+++ b/drivers/hwmon/stts751.c
@@ -0,0 +1,805 @@
+/*
+ * STTS751 sensor driver
+ *
+ * Copyright (C) 2016 Istituto Italiano di Tecnologia - RBCS - EDL
+ * Robotics, Brain and Cognitive Sciences department
+ * Electronic Design Laboratory
+ *
+ * Written by Andrea Merello <andrea.mere...@gmail.com>
+ *
+ * Based on LM95241 driver and LM90 driver
+ *
+ * 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEVNAME "stts751"
+
+static const unsigned short normal_i2c[] = {
+ 0x48, 0x49, 0x38, 0x39, /* STTS751-0 */
+ 0x4A, 0x4B, 0x3A, 0x3B, /* STTS751-1 */
+ I2C_CLIENT_END };
+
+#define STTS751_REG_TEMP_H 0x00
+#define STTS751_REG_STATUS 0x01
+#define STTS751_STATUS_TRIPL BIT(5)
+#define STTS751_STATUS_TRIPH BIT(6)
+#define STTS751_STATUS_BUSYBIT(8)
+#define STTS751_REG_TEMP_L 0x02
+#define STTS751_REG_CONF 0x03
+#define STTS751_CONF_RES_MASK 0x0C
+#define STTS751_CONF_RES_SHIFT 2
+#define STTS751_CONF_EVENT_DIS BIT(7)
+#define STTS751_CONF_STOP BIT(6)
+#define STTS751_REG_RATE 0x04
+#define STTS751_REG_HLIM_H 0x05
+#define STTS751_REG_HLIM_L 0x06
+#define STTS751_REG_LLIM_H 0x07
+#define STTS751_REG_LLIM_L 0x08
+#define STTS751_REG_ONESHOT0x0F
+#define STTS751_REG_TLIM 0x20
+#define STTS751_REG_HYST 0x21
+#define STTS751_REG_SMBUS_TO 0x22
+
+#define STTS751_REG_PROD_ID0xFD
+#define STTS751_REG_MAN_ID 0xFE
+#define STTS751_REG_REV_ID 0xFF
+
+#define STTS751_0_PROD_ID 0x00
+#define STTS751_1_PROD_ID 0x01
+#define ST_MAN_ID 0x53
+
+/* stick with HW defaults */
+#define STTS751_THERM_DEFAULT 85000
+#define STTS751_HYST_DEFAULT 1
+#define STTS751_EVENT_MAX_DEFAULT 85000
+#define STTS751_EVENT_MIN_DEFAULT
[PATCH v2 0/2] Add driver for ST stts751 thermal sensor
This series adds new driver for ST stts751 thermal sensor Changes since v1: - removed the "manual mode" logic; always use automatic mode - changed the way 16-bit data is read from two 8-bit regs (now similar to lm90) - code simplifications - comment style fixes Andrea Merello (2): DT: add binding documentation for STTS751 hwmon: new driver for ST stts751 thermal sensor .../devicetree/bindings/hwmon/stts751.txt | 19 + drivers/hwmon/Kconfig | 12 +- drivers/hwmon/Makefile | 2 +- drivers/hwmon/stts751.c| 805 + 4 files changed, 836 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwmon/stts751.txt create mode 100644 drivers/hwmon/stts751.c Cc: Rob Herring <robh...@kernel.org> Cc: Mark Rutland <mark.rutl...@arm.com> Cc: LABBE Corentin <clabbe.montj...@gmail.com> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jean Delvare <jdelv...@suse.com> -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] hwmon: new driver for ST stts751 thermal sensor
On Thu, Sep 8, 2016 at 3:35 PM, Guenter Roeck <li...@roeck-us.net> wrote: > > On 09/08/2016 12:42 AM, Andrea Merello wrote: >> >> > [ ... ] > >> >> Will look if I can find another driver with similar issue to snoop >> at.. >> >> Try lm90.c:lm90_read16(). >> >> >> >> I see.. This is reasonable, and I will implement this trick in stts751 >> driver.. This seems to have good chances to read correctly, but it can still >> go racy for example if the converter ends a conversion between the first two >> reads, and we get preempted in between the last two reads (so that the HW >> ends another conversion). IMHO looking _also_ at time, and eventually >> retry, makes things even more robust.. >> > As a general comment, please use a mailer which splits lines at 80 columns or > less. Thanks for pointing this out. (Probably I just need to remember to switch gmail in plain text mode..) > > > While I understand your concern, the time check is complete overkill. > What is the probability for a double bad reading, and on top of that one that > includes a change in the high temperature register ? One in a Million ? > With a downside of a wrong fraction in the "error" case ? And what does that > really mean, on a chip with a specified accuracy of +/- 1 degrees C ? Yes, I see your point, and this is indeed true. Probably I have been too paranoid on this... > Even your solution to drop the fraction in some cases doesn't really solve > anything. > It now reports, say, 30.0 degrees C instead of 30.875 degrees C. How is that > better > than a "wrong" 30.X degrees C, where X is any other value it might report ? You're right, I could let the "wrong" fraction data. (but the point of my solution was not this indeed). >> I'd say we could add time check and retry (with proper cpu_relax() to avoid >> those hot loops) also to lm90 rather than removing it from stts651 :) >> > Only if you can show that the problem is seen in the real world. > > >> >> >> >> >> + break; >> + >> + /* if we are going on with a racy read, >> don't pretend to be >> +* super-precise, just use the MSBs .. >> +*/ >> + frac = 0; >> + } >> + >> +exit: >> + mutex_unlock(>access_lock); >> + if (ret) >> + return ret; >> + >> + /* use integer2, because when we fallback to the >> "MSB-only" compromise >> +* this is the more recent one >> +*/ >> + priv->temp = stts751_to_deg(integer2, frac); >> >> >> All other drivers I am aware of manage to handle conversions >> from 16 bit chip >> values to temperatures and from temperatures to chip values with >> a single parameter. >> >> >> I'll check this.. BTW This chip wants two separated reads from two >> registers, one for the integer and one for the fractional part. The purpose >> of this function is interpreting these two raw HW values to convert in a >> number. Is this so bad ? >> >> Again, have a look at lm90.c:lm90_read16() and the conversion functions >> in drivers which have to read two registers to read the temperature. >> >> Is it bad ? It makes the code more complicated than it has to be and >> increases code size, so, yes, I consider it bad. >> >> >> It does not look like that the simple register combination that lm90 does >> could also work for the integer/fractional format of the stts751. >> > Why ? The register formats are exactly the same. Aah, I didn't saw temp_from_s16() in lm90.c that does the conversion afterwards, thus I assumed the data read from lm90 didn't need any conversion and that the reg format was different.. sorry.. Now I got what you meant :) >> I'll check again more carefully to see if for some math trick the two things >> eventually end up to produce the same result, but right now it seems to me >> that we need to do some math over the two read values in stts751. And the >> same for the reverse direction conversion. >> >> If the point above turns out to be true, then I honestly can't see how >&
