linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] rtc: isl12022: cleanups and hwmon support
@ 2022-08-30 10:01 Rasmus Villemoes
  2022-08-30 10:01 ` [PATCH 6/6] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Rasmus Villemoes @ 2022-08-30 10:01 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Jean Delvare, Guenter Roeck,
	linux-rtc, linux-kernel, linux-hwmon
  Cc: Rasmus Villemoes

This series does a few cleanups of the isl12022 driver,

- removes use of deprecated function
- removes some  redundant code
- switches to regmap API instead of private helpers

and finally hooks up the temperatur sensor to hwmon.

Rasmus Villemoes (6):
  rtc: isl12022: stop using deprecated devm_rtc_device_register()
  rtc: isl12022: simplify some expressions
  rtc: isl12022: use dev_set_drvdata() instead of i2c_set_clientdata()
  rtc: isl12022: drop redundant write to HR register
  rtc: isl12022: switch to using regmap API
  rtc: isl12022: add support for temperature sensor

 drivers/rtc/Kconfig        |   1 +
 drivers/rtc/rtc-isl12022.c | 195 ++++++++++++++++++++-----------------
 2 files changed, 107 insertions(+), 89 deletions(-)

-- 
2.37.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 6/6] rtc: isl12022: add support for temperature sensor
  2022-08-30 10:01 [PATCH 0/6] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
@ 2022-08-30 10:01 ` Rasmus Villemoes
  2022-08-30 13:13   ` Guenter Roeck
  2022-09-14 15:12   ` Alexandre Belloni
  2022-09-09  8:15 ` [PATCH 0/6] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
  2022-09-21 11:46 ` [PATCH v2 0/9] " Rasmus Villemoes
  2 siblings, 2 replies; 19+ messages in thread
From: Rasmus Villemoes @ 2022-08-30 10:01 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Jean Delvare, Guenter Roeck
  Cc: Rasmus Villemoes, linux-rtc, linux-kernel, linux-hwmon

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/rtc/rtc-isl12022.c | 81 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
index b295ec92ee17..1bd72f436318 100644
--- a/drivers/rtc/rtc-isl12022.c
+++ b/drivers/rtc/rtc-isl12022.c
@@ -17,6 +17,8 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 
 /* ISL register offsets */
 #define ISL12022_REG_SC		0x00
@@ -30,6 +32,9 @@
 #define ISL12022_REG_SR		0x07
 #define ISL12022_REG_INT	0x08
 
+#define ISL12022_REG_BETA	0x0d
+#define ISL12022_REG_TEMP_L	0x28
+
 /* ISL register bits */
 #define ISL12022_HR_MIL		(1 << 7)	/* military or 24 hour time */
 
@@ -38,6 +43,7 @@
 
 #define ISL12022_INT_WRTC	(1 << 6)
 
+#define ISL12022_BETA_TSE	(1 << 7)
 
 static struct i2c_driver isl12022_driver;
 
@@ -48,6 +54,79 @@ struct isl12022 {
 	bool write_enabled;	/* true if write enable is set */
 };
 
+/*
+ * A user-initiated temperature conversion is not started by this function,
+ * so the temperature is updated once every ~60 seconds.
+ */
+static int isl12022_hwmon_read_temp(struct device *dev, s32 *mC)
+{
+	struct isl12022 *isl12022 = dev_get_drvdata(dev);
+	struct regmap *regmap = isl12022->regmap;
+	u8 temp_buf[2];
+	s32 temp;
+	int ret;
+
+	ret = regmap_bulk_read(regmap, ISL12022_REG_TEMP_L,
+			       temp_buf, sizeof(temp_buf));
+	if (ret)
+		return ret;
+	/*
+	 * Temperature is represented as a 10-bit number, unit half-Kelvins.
+	 */
+	temp = (temp_buf[1] << 8) | temp_buf[0];
+	temp *= 500;
+	temp -= 273000;
+
+	*mC = temp;
+
+	return 0;
+}
+
+static ssize_t
+isl12022_hwmon_show_temp(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	int ret;
+	s32 temp;
+
+	ret = isl12022_hwmon_read_temp(dev, &temp);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", temp);
+}
+static SENSOR_DEVICE_ATTR(temp1_input, 0444, isl12022_hwmon_show_temp,
+			  NULL, 0);
+
+static struct attribute *isl12022_hwmon_attrs[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(isl12022_hwmon);
+
+static void isl12022_hwmon_register(struct device *dev)
+{
+	struct isl12022 *isl12022;
+	struct device *hwmon;
+	int ret;
+
+	if (!IS_REACHABLE(CONFIG_HWMON))
+		return;
+
+	isl12022 = dev_get_drvdata(dev);
+
+	ret = regmap_update_bits(isl12022->regmap, ISL12022_REG_BETA,
+				 ISL12022_BETA_TSE, ISL12022_BETA_TSE);
+	if (ret) {
+		dev_warn(dev, "unable to enable temperature sensor\n");
+		return;
+	}
+
+	hwmon = devm_hwmon_device_register_with_groups(dev, "isl12022", isl12022,
+						       isl12022_hwmon_groups);
+	if (IS_ERR(hwmon))
+		dev_warn(dev, "unable to register hwmon device: %pe\n", hwmon);
+}
+
 /*
  * In the routines that deal directly with the isl12022 hardware, we use
  * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch.
@@ -179,6 +258,8 @@ static int isl12022_probe(struct i2c_client *client)
 		return PTR_ERR(isl12022->regmap);
 	}
 
+	isl12022_hwmon_register(&client->dev);
+
 	isl12022->rtc = devm_rtc_allocate_device(&client->dev);
 	if (IS_ERR(isl12022->rtc))
 		return PTR_ERR(isl12022->rtc);
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/6] rtc: isl12022: add support for temperature sensor
  2022-08-30 10:01 ` [PATCH 6/6] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
@ 2022-08-30 13:13   ` Guenter Roeck
  2022-09-14 15:12   ` Alexandre Belloni
  1 sibling, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2022-08-30 13:13 UTC (permalink / raw)
  To: Rasmus Villemoes, Alessandro Zummo, Alexandre Belloni, Jean Delvare
  Cc: linux-rtc, linux-kernel, linux-hwmon

On 8/30/22 03:01, Rasmus Villemoes wrote:
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>   drivers/rtc/rtc-isl12022.c | 81 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 81 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
> index b295ec92ee17..1bd72f436318 100644
> --- a/drivers/rtc/rtc-isl12022.c
> +++ b/drivers/rtc/rtc-isl12022.c
> @@ -17,6 +17,8 @@
>   #include <linux/of.h>
>   #include <linux/of_device.h>
>   #include <linux/regmap.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
>   
>   /* ISL register offsets */
>   #define ISL12022_REG_SC		0x00
> @@ -30,6 +32,9 @@
>   #define ISL12022_REG_SR		0x07
>   #define ISL12022_REG_INT	0x08
>   
> +#define ISL12022_REG_BETA	0x0d
> +#define ISL12022_REG_TEMP_L	0x28
> +
>   /* ISL register bits */
>   #define ISL12022_HR_MIL		(1 << 7)	/* military or 24 hour time */
>   
> @@ -38,6 +43,7 @@
>   
>   #define ISL12022_INT_WRTC	(1 << 6)
>   
> +#define ISL12022_BETA_TSE	(1 << 7)
>   
>   static struct i2c_driver isl12022_driver;
>   
> @@ -48,6 +54,79 @@ struct isl12022 {
>   	bool write_enabled;	/* true if write enable is set */
>   };
>   
> +/*
> + * A user-initiated temperature conversion is not started by this function,
> + * so the temperature is updated once every ~60 seconds.
> + */
> +static int isl12022_hwmon_read_temp(struct device *dev, s32 *mC)
> +{
> +	struct isl12022 *isl12022 = dev_get_drvdata(dev);
> +	struct regmap *regmap = isl12022->regmap;
> +	u8 temp_buf[2];
> +	s32 temp;
> +	int ret;
> +
> +	ret = regmap_bulk_read(regmap, ISL12022_REG_TEMP_L,
> +			       temp_buf, sizeof(temp_buf));
> +	if (ret)
> +		return ret;
> +	/*
> +	 * Temperature is represented as a 10-bit number, unit half-Kelvins.
> +	 */
> +	temp = (temp_buf[1] << 8) | temp_buf[0];
> +	temp *= 500;
> +	temp -= 273000;
> +
> +	*mC = temp;
> +
> +	return 0;
> +}
> +
> +static ssize_t
> +isl12022_hwmon_show_temp(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	int ret;
> +	s32 temp;
> +
> +	ret = isl12022_hwmon_read_temp(dev, &temp);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", temp);
> +}
> +static SENSOR_DEVICE_ATTR(temp1_input, 0444, isl12022_hwmon_show_temp,
> +			  NULL, 0);
> +
> +static struct attribute *isl12022_hwmon_attrs[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(isl12022_hwmon);
> +
> +static void isl12022_hwmon_register(struct device *dev)
> +{
> +	struct isl12022 *isl12022;
> +	struct device *hwmon;
> +	int ret;
> +
> +	if (!IS_REACHABLE(CONFIG_HWMON))
> +		return;
> +
> +	isl12022 = dev_get_drvdata(dev);
> +
> +	ret = regmap_update_bits(isl12022->regmap, ISL12022_REG_BETA,
> +				 ISL12022_BETA_TSE, ISL12022_BETA_TSE);
> +	if (ret) {
> +		dev_warn(dev, "unable to enable temperature sensor\n");
> +		return;
> +	}
> +
> +	hwmon = devm_hwmon_device_register_with_groups(dev, "isl12022", isl12022,
> +						       isl12022_hwmon_groups);

Please use devm_hwmon_device_register_with_info().

Thanks,
Guenter

> +	if (IS_ERR(hwmon))
> +		dev_warn(dev, "unable to register hwmon device: %pe\n", hwmon);
> +}
> +
>   /*
>    * In the routines that deal directly with the isl12022 hardware, we use
>    * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch.
> @@ -179,6 +258,8 @@ static int isl12022_probe(struct i2c_client *client)
>   		return PTR_ERR(isl12022->regmap);
>   	}
>   
> +	isl12022_hwmon_register(&client->dev);
> +
>   	isl12022->rtc = devm_rtc_allocate_device(&client->dev);
>   	if (IS_ERR(isl12022->rtc))
>   		return PTR_ERR(isl12022->rtc);


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/6] rtc: isl12022: cleanups and hwmon support
  2022-08-30 10:01 [PATCH 0/6] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
  2022-08-30 10:01 ` [PATCH 6/6] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
@ 2022-09-09  8:15 ` Rasmus Villemoes
  2022-09-14 15:16   ` Alexandre Belloni
  2022-09-21 11:46 ` [PATCH v2 0/9] " Rasmus Villemoes
  2 siblings, 1 reply; 19+ messages in thread
From: Rasmus Villemoes @ 2022-09-09  8:15 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Jean Delvare, Guenter Roeck,
	linux-rtc, linux-kernel, linux-hwmon

On 30/08/2022 12.01, Rasmus Villemoes wrote:
> This series does a few cleanups of the isl12022 driver,
> 
> - removes use of deprecated function
> - removes some  redundant code
> - switches to regmap API instead of private helpers
> 
> and finally hooks up the temperatur sensor to hwmon.

ping. Any comments before I respin 6/6 to address Guenter's request?

Rasmus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/6] rtc: isl12022: add support for temperature sensor
  2022-08-30 10:01 ` [PATCH 6/6] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
  2022-08-30 13:13   ` Guenter Roeck
@ 2022-09-14 15:12   ` Alexandre Belloni
  2022-09-21  7:58     ` Rasmus Villemoes
  1 sibling, 1 reply; 19+ messages in thread
From: Alexandre Belloni @ 2022-09-14 15:12 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Alessandro Zummo, Jean Delvare, Guenter Roeck, linux-rtc,
	linux-kernel, linux-hwmon

On 30/08/2022 12:01:52+0200, Rasmus Villemoes wrote:
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  drivers/rtc/rtc-isl12022.c | 81 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
> index b295ec92ee17..1bd72f436318 100644
> --- a/drivers/rtc/rtc-isl12022.c
> +++ b/drivers/rtc/rtc-isl12022.c
> @@ -17,6 +17,8 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/regmap.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>

You should keep that list ordered

>  
>  /* ISL register offsets */
>  #define ISL12022_REG_SC		0x00
> @@ -30,6 +32,9 @@
>  #define ISL12022_REG_SR		0x07
>  #define ISL12022_REG_INT	0x08
>  
> +#define ISL12022_REG_BETA	0x0d
> +#define ISL12022_REG_TEMP_L	0x28
> +
>  /* ISL register bits */
>  #define ISL12022_HR_MIL		(1 << 7)	/* military or 24 hour time */
>  
> @@ -38,6 +43,7 @@
>  
>  #define ISL12022_INT_WRTC	(1 << 6)
>  
> +#define ISL12022_BETA_TSE	(1 << 7)
>  
>  static struct i2c_driver isl12022_driver;
>  
> @@ -48,6 +54,79 @@ struct isl12022 {
>  	bool write_enabled;	/* true if write enable is set */
>  };
>  
> +/*
> + * A user-initiated temperature conversion is not started by this function,
> + * so the temperature is updated once every ~60 seconds.
> + */
> +static int isl12022_hwmon_read_temp(struct device *dev, s32 *mC)
> +{
> +	struct isl12022 *isl12022 = dev_get_drvdata(dev);
> +	struct regmap *regmap = isl12022->regmap;
> +	u8 temp_buf[2];
> +	s32 temp;
> +	int ret;
> +
> +	ret = regmap_bulk_read(regmap, ISL12022_REG_TEMP_L,
> +			       temp_buf, sizeof(temp_buf));
> +	if (ret)
> +		return ret;
> +	/*
> +	 * Temperature is represented as a 10-bit number, unit half-Kelvins.
> +	 */
> +	temp = (temp_buf[1] << 8) | temp_buf[0];
> +	temp *= 500;
> +	temp -= 273000;
> +
> +	*mC = temp;
> +
> +	return 0;
> +}
> +
> +static ssize_t
> +isl12022_hwmon_show_temp(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	int ret;
> +	s32 temp;
> +
> +	ret = isl12022_hwmon_read_temp(dev, &temp);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", temp);
> +}
> +static SENSOR_DEVICE_ATTR(temp1_input, 0444, isl12022_hwmon_show_temp,
> +			  NULL, 0);
> +
> +static struct attribute *isl12022_hwmon_attrs[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(isl12022_hwmon);
> +
> +static void isl12022_hwmon_register(struct device *dev)
> +{
> +	struct isl12022 *isl12022;
> +	struct device *hwmon;
> +	int ret;
> +
> +	if (!IS_REACHABLE(CONFIG_HWMON))
> +		return;
> +
> +	isl12022 = dev_get_drvdata(dev);
> +
> +	ret = regmap_update_bits(isl12022->regmap, ISL12022_REG_BETA,
> +				 ISL12022_BETA_TSE, ISL12022_BETA_TSE);
> +	if (ret) {
> +		dev_warn(dev, "unable to enable temperature sensor\n");
> +		return;
> +	}
> +
> +	hwmon = devm_hwmon_device_register_with_groups(dev, "isl12022", isl12022,
> +						       isl12022_hwmon_groups);
> +	if (IS_ERR(hwmon))
> +		dev_warn(dev, "unable to register hwmon device: %pe\n", hwmon);
> +}
> +
>  /*
>   * In the routines that deal directly with the isl12022 hardware, we use
>   * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch.
> @@ -179,6 +258,8 @@ static int isl12022_probe(struct i2c_client *client)
>  		return PTR_ERR(isl12022->regmap);
>  	}
>  
> +	isl12022_hwmon_register(&client->dev);
> +
>  	isl12022->rtc = devm_rtc_allocate_device(&client->dev);
>  	if (IS_ERR(isl12022->rtc))
>  		return PTR_ERR(isl12022->rtc);
> -- 
> 2.37.2
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/6] rtc: isl12022: cleanups and hwmon support
  2022-09-09  8:15 ` [PATCH 0/6] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
@ 2022-09-14 15:16   ` Alexandre Belloni
  0 siblings, 0 replies; 19+ messages in thread
From: Alexandre Belloni @ 2022-09-14 15:16 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Alessandro Zummo, Jean Delvare, Guenter Roeck, linux-rtc,
	linux-kernel, linux-hwmon

On 09/09/2022 10:15:22+0200, Rasmus Villemoes wrote:
> On 30/08/2022 12.01, Rasmus Villemoes wrote:
> > This series does a few cleanups of the isl12022 driver,
> > 
> > - removes use of deprecated function
> > - removes some  redundant code
> > - switches to regmap API instead of private helpers
> > 
> > and finally hooks up the temperatur sensor to hwmon.
> 
> ping. Any comments before I respin 6/6 to address Guenter's request?
> 

This series is missing one patch setting the datetime range supported by
the RTC which I guess is 2000 to 2099.

> Rasmus

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/6] rtc: isl12022: add support for temperature sensor
  2022-09-14 15:12   ` Alexandre Belloni
@ 2022-09-21  7:58     ` Rasmus Villemoes
  2022-09-21  9:10       ` Alexandre Belloni
  0 siblings, 1 reply; 19+ messages in thread
From: Rasmus Villemoes @ 2022-09-21  7:58 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Alessandro Zummo, Jean Delvare, Guenter Roeck, linux-rtc,
	linux-kernel, linux-hwmon

On 14/09/2022 17.12, Alexandre Belloni wrote:
> On 30/08/2022 12:01:52+0200, Rasmus Villemoes wrote:
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> ---
>>  drivers/rtc/rtc-isl12022.c | 81 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 81 insertions(+)
>>
>> diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
>> index b295ec92ee17..1bd72f436318 100644
>> --- a/drivers/rtc/rtc-isl12022.c
>> +++ b/drivers/rtc/rtc-isl12022.c
>> @@ -17,6 +17,8 @@
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>>  #include <linux/regmap.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
> 
> You should keep that list ordered

While the three lines of context happen to be sorted, the list as a
whole is not. I can of course include a patch sorting it.

Rasmus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/6] rtc: isl12022: add support for temperature sensor
  2022-09-21  7:58     ` Rasmus Villemoes
@ 2022-09-21  9:10       ` Alexandre Belloni
  0 siblings, 0 replies; 19+ messages in thread
From: Alexandre Belloni @ 2022-09-21  9:10 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Alessandro Zummo, Jean Delvare, Guenter Roeck, linux-rtc,
	linux-kernel, linux-hwmon

On 21/09/2022 09:58:16+0200, Rasmus Villemoes wrote:
> On 14/09/2022 17.12, Alexandre Belloni wrote:
> > On 30/08/2022 12:01:52+0200, Rasmus Villemoes wrote:
> >> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> >> ---
> >>  drivers/rtc/rtc-isl12022.c | 81 ++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 81 insertions(+)
> >>
> >> diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
> >> index b295ec92ee17..1bd72f436318 100644
> >> --- a/drivers/rtc/rtc-isl12022.c
> >> +++ b/drivers/rtc/rtc-isl12022.c
> >> @@ -17,6 +17,8 @@
> >>  #include <linux/of.h>
> >>  #include <linux/of_device.h>
> >>  #include <linux/regmap.h>
> >> +#include <linux/hwmon.h>
> >> +#include <linux/hwmon-sysfs.h>
> > 
> > You should keep that list ordered
> 
> While the three lines of context happen to be sorted, the list as a
> whole is not. I can of course include a patch sorting it.
> 

Nevermind then


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 0/9] rtc: isl12022: cleanups and hwmon support
  2022-08-30 10:01 [PATCH 0/6] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
  2022-08-30 10:01 ` [PATCH 6/6] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
  2022-09-09  8:15 ` [PATCH 0/6] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
@ 2022-09-21 11:46 ` Rasmus Villemoes
  2022-09-21 11:46   ` [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
                     ` (2 more replies)
  2 siblings, 3 replies; 19+ messages in thread
From: Rasmus Villemoes @ 2022-09-21 11:46 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Jean Delvare, Guenter Roeck,
	linux-rtc, linux-kernel, linux-hwmon
  Cc: Rasmus Villemoes

This series does a few cleanups of the isl12022 driver,

- removes use of deprecated function
- removes some redundant code
- switches to regmap API instead of private helpers

It also provides range_min, range_max values and finally hooks up the
temperatur sensor to hwmon.

Changes in v2:

- Use devm_hwmon_device_register_with_info() (Guenter), and use the
  opportunity to also expose an update_interval with the constant
  value 60000.

- Convert some dev_dbgs to use %ptR, remove a dev_info() (Alexandre)

- Add patch setting range_min, range_max (Alexandre)

Not changed in v2: For now, I've kept the low-battery
dev_warn(). Implementing support for the ioctls RTC_VL_READ/RTC_VL_CLR
is on my radar, but I will need some more time for reading up on and
deciding on the details of what the chip provides and how to map that
information to the various defined RTC_VL_* bits. I also need to
figure out a good way to reliably trigger a low-battery condition so I
can test the ioctl support. So I hope these patches can go in for now
and not be blocked on the series expanding even further.

Rasmus Villemoes (9):
  rtc: isl12022: stop using deprecated devm_rtc_device_register()
  rtc: isl12022: specify range_min and range_max
  rtc: isl12022: drop a dev_info()
  rtc: isl12022: simplify some expressions
  rtc: isl12022: use %ptR
  rtc: isl12022: use dev_set_drvdata() instead of i2c_set_clientdata()
  rtc: isl12022: drop redundant write to HR register
  rtc: isl12022: switch to using regmap API
  rtc: isl12022: add support for temperature sensor

 drivers/rtc/Kconfig        |   1 +
 drivers/rtc/rtc-isl12022.c | 237 ++++++++++++++++++++-----------------
 2 files changed, 131 insertions(+), 107 deletions(-)

-- 
2.37.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor
  2022-09-21 11:46 ` [PATCH v2 0/9] " Rasmus Villemoes
@ 2022-09-21 11:46   ` Rasmus Villemoes
  2022-09-21 14:13     ` Guenter Roeck
  2022-10-26 13:38     ` [PATCH v3 " Rasmus Villemoes
  2022-10-07 13:51   ` [PATCH v2 0/9] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
  2022-10-13 21:31   ` Alexandre Belloni
  2 siblings, 2 replies; 19+ messages in thread
From: Rasmus Villemoes @ 2022-09-21 11:46 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Jean Delvare, Guenter Roeck
  Cc: Rasmus Villemoes, linux-rtc, linux-kernel, linux-hwmon

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/rtc/rtc-isl12022.c | 104 +++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
index ca677c4265e6..f3efe61c81e5 100644
--- a/drivers/rtc/rtc-isl12022.c
+++ b/drivers/rtc/rtc-isl12022.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>
+#include <linux/hwmon.h>
 
 /* ISL register offsets */
 #define ISL12022_REG_SC		0x00
@@ -30,6 +31,9 @@
 #define ISL12022_REG_SR		0x07
 #define ISL12022_REG_INT	0x08
 
+#define ISL12022_REG_BETA	0x0d
+#define ISL12022_REG_TEMP_L	0x28
+
 /* ISL register bits */
 #define ISL12022_HR_MIL		(1 << 7)	/* military or 24 hour time */
 
@@ -38,6 +42,7 @@
 
 #define ISL12022_INT_WRTC	(1 << 6)
 
+#define ISL12022_BETA_TSE	(1 << 7)
 
 static struct i2c_driver isl12022_driver;
 
@@ -46,6 +51,103 @@ struct isl12022 {
 	struct regmap *regmap;
 };
 
+static umode_t isl12022_hwmon_is_visible(const void *data,
+					 enum hwmon_sensor_types type,
+					 u32 attr, int channel)
+{
+	if (type == hwmon_chip && attr == hwmon_chip_update_interval)
+		return 0444;
+
+	if (type == hwmon_temp && attr == hwmon_temp_input)
+		return 0444;
+
+	return 0;
+}
+
+/*
+ * A user-initiated temperature conversion is not started by this function,
+ * so the temperature is updated once every ~60 seconds.
+ */
+static int isl12022_hwmon_read_temp(struct device *dev, long *mC)
+{
+	struct isl12022 *isl12022 = dev_get_drvdata(dev);
+	struct regmap *regmap = isl12022->regmap;
+	u8 temp_buf[2];
+	int temp, ret;
+
+	ret = regmap_bulk_read(regmap, ISL12022_REG_TEMP_L,
+			       temp_buf, sizeof(temp_buf));
+	if (ret)
+		return ret;
+	/*
+	 * Temperature is represented as a 10-bit number, unit half-Kelvins.
+	 */
+	temp = (temp_buf[1] << 8) | temp_buf[0];
+	temp *= 500;
+	temp -= 273000;
+
+	*mC = temp;
+
+	return 0;
+}
+
+static int isl12022_hwmon_read(struct device *dev,
+			       enum hwmon_sensor_types type,
+			       u32 attr, int channel, long *val)
+{
+	if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
+		*val = 60000;
+		return 0;
+	}
+
+	if (type == hwmon_temp && attr == hwmon_temp_input) {
+		return isl12022_hwmon_read_temp(dev, val);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static const struct hwmon_channel_info *isl12022_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL),
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops isl12022_hwmon_ops = {
+	.is_visible = isl12022_hwmon_is_visible,
+	.read = isl12022_hwmon_read,
+};
+
+static const struct hwmon_chip_info isl12022_hwmon_chip_info = {
+	.ops = &isl12022_hwmon_ops,
+	.info = isl12022_hwmon_info,
+};
+
+static void isl12022_hwmon_register(struct device *dev)
+{
+	struct isl12022 *isl12022;
+	struct device *hwmon;
+	int ret;
+
+	if (!IS_REACHABLE(CONFIG_HWMON))
+		return;
+
+	isl12022 = dev_get_drvdata(dev);
+
+	ret = regmap_update_bits(isl12022->regmap, ISL12022_REG_BETA,
+				 ISL12022_BETA_TSE, ISL12022_BETA_TSE);
+	if (ret) {
+		dev_warn(dev, "unable to enable temperature sensor\n");
+		return;
+	}
+
+	hwmon = devm_hwmon_device_register_with_info(dev, "isl12022", isl12022,
+						     &isl12022_hwmon_chip_info,
+						     NULL);
+	if (IS_ERR(hwmon))
+		dev_warn(dev, "unable to register hwmon device: %pe\n", hwmon);
+}
+
 /*
  * In the routines that deal directly with the isl12022 hardware, we use
  * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch.
@@ -160,6 +262,8 @@ static int isl12022_probe(struct i2c_client *client)
 		return PTR_ERR(isl12022->regmap);
 	}
 
+	isl12022_hwmon_register(&client->dev);
+
 	isl12022->rtc = devm_rtc_allocate_device(&client->dev);
 	if (IS_ERR(isl12022->rtc))
 		return PTR_ERR(isl12022->rtc);
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor
  2022-09-21 11:46   ` [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
@ 2022-09-21 14:13     ` Guenter Roeck
  2022-09-23  8:40       ` Rasmus Villemoes
  2022-10-26 13:38     ` [PATCH v3 " Rasmus Villemoes
  1 sibling, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2022-09-21 14:13 UTC (permalink / raw)
  To: Rasmus Villemoes, Alessandro Zummo, Alexandre Belloni, Jean Delvare
  Cc: linux-rtc, linux-kernel, linux-hwmon

On 9/21/22 04:46, Rasmus Villemoes wrote:
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>   drivers/rtc/rtc-isl12022.c | 104 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 104 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
> index ca677c4265e6..f3efe61c81e5 100644
> --- a/drivers/rtc/rtc-isl12022.c
> +++ b/drivers/rtc/rtc-isl12022.c
> @@ -17,6 +17,7 @@
>   #include <linux/of.h>
>   #include <linux/of_device.h>
>   #include <linux/regmap.h>
> +#include <linux/hwmon.h>
>   
>   /* ISL register offsets */
>   #define ISL12022_REG_SC		0x00
> @@ -30,6 +31,9 @@
>   #define ISL12022_REG_SR		0x07
>   #define ISL12022_REG_INT	0x08
>   
> +#define ISL12022_REG_BETA	0x0d
> +#define ISL12022_REG_TEMP_L	0x28
> +
>   /* ISL register bits */
>   #define ISL12022_HR_MIL		(1 << 7)	/* military or 24 hour time */
>   
> @@ -38,6 +42,7 @@
>   
>   #define ISL12022_INT_WRTC	(1 << 6)
>   
> +#define ISL12022_BETA_TSE	(1 << 7)
>   
>   static struct i2c_driver isl12022_driver;
>   
> @@ -46,6 +51,103 @@ struct isl12022 {
>   	struct regmap *regmap;
>   };
>   
> +static umode_t isl12022_hwmon_is_visible(const void *data,
> +					 enum hwmon_sensor_types type,
> +					 u32 attr, int channel)
> +{
> +	if (type == hwmon_chip && attr == hwmon_chip_update_interval)
> +		return 0444;
> +
> +	if (type == hwmon_temp && attr == hwmon_temp_input)
> +		return 0444;
> +
> +	return 0;
> +}
> +
> +/*
> + * A user-initiated temperature conversion is not started by this function,
> + * so the temperature is updated once every ~60 seconds.
> + */
> +static int isl12022_hwmon_read_temp(struct device *dev, long *mC)
> +{
> +	struct isl12022 *isl12022 = dev_get_drvdata(dev);
> +	struct regmap *regmap = isl12022->regmap;
> +	u8 temp_buf[2];
> +	int temp, ret;
> +
> +	ret = regmap_bulk_read(regmap, ISL12022_REG_TEMP_L,
> +			       temp_buf, sizeof(temp_buf));
> +	if (ret)
> +		return ret;
> +	/*
> +	 * Temperature is represented as a 10-bit number, unit half-Kelvins.
> +	 */
> +	temp = (temp_buf[1] << 8) | temp_buf[0];
> +	temp *= 500;
> +	temp -= 273000;
> +
> +	*mC = temp;
> +
> +	return 0;
> +}
> +
> +static int isl12022_hwmon_read(struct device *dev,
> +			       enum hwmon_sensor_types type,
> +			       u32 attr, int channel, long *val)
> +{
> +	if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
> +		*val = 60000;
> +		return 0;
> +	}

It is not the purpose of the update_interval attribute to inform the
user what the update interval of this chip happens to be. The purpose
of the attribute is to inform the chip what update interval it should use.

 From the documentation:
	Some devices have a variable update rate or interval.
	This attribute can be used to change it to the desired value.

The patch is not in the hwmon subsystem, to it is up to the maintainers
to decide if they want to accept the patch anyway. Please don't expect
a reviewed tag, though.

> +
> +	if (type == hwmon_temp && attr == hwmon_temp_input) {
> +		return isl12022_hwmon_read_temp(dev, val);
> +	}

Nit: { } is unnecessary.

Guenter

> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static const struct hwmon_channel_info *isl12022_hwmon_info[] = {
> +	HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL),
> +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
> +	NULL
> +};
> +
> +static const struct hwmon_ops isl12022_hwmon_ops = {
> +	.is_visible = isl12022_hwmon_is_visible,
> +	.read = isl12022_hwmon_read,
> +};
> +
> +static const struct hwmon_chip_info isl12022_hwmon_chip_info = {
> +	.ops = &isl12022_hwmon_ops,
> +	.info = isl12022_hwmon_info,
> +};
> +
> +static void isl12022_hwmon_register(struct device *dev)
> +{
> +	struct isl12022 *isl12022;
> +	struct device *hwmon;
> +	int ret;
> +
> +	if (!IS_REACHABLE(CONFIG_HWMON))
> +		return;
> +
> +	isl12022 = dev_get_drvdata(dev);
> +
> +	ret = regmap_update_bits(isl12022->regmap, ISL12022_REG_BETA,
> +				 ISL12022_BETA_TSE, ISL12022_BETA_TSE);
> +	if (ret) {
> +		dev_warn(dev, "unable to enable temperature sensor\n");
> +		return;
> +	}
> +
> +	hwmon = devm_hwmon_device_register_with_info(dev, "isl12022", isl12022,
> +						     &isl12022_hwmon_chip_info,
> +						     NULL);
> +	if (IS_ERR(hwmon))
> +		dev_warn(dev, "unable to register hwmon device: %pe\n", hwmon);
> +}
> +
>   /*
>    * In the routines that deal directly with the isl12022 hardware, we use
>    * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch.
> @@ -160,6 +262,8 @@ static int isl12022_probe(struct i2c_client *client)
>   		return PTR_ERR(isl12022->regmap);
>   	}
>   
> +	isl12022_hwmon_register(&client->dev);
> +
>   	isl12022->rtc = devm_rtc_allocate_device(&client->dev);
>   	if (IS_ERR(isl12022->rtc))
>   		return PTR_ERR(isl12022->rtc);


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor
  2022-09-21 14:13     ` Guenter Roeck
@ 2022-09-23  8:40       ` Rasmus Villemoes
  2022-09-23 13:51         ` Guenter Roeck
  0 siblings, 1 reply; 19+ messages in thread
From: Rasmus Villemoes @ 2022-09-23  8:40 UTC (permalink / raw)
  To: Guenter Roeck, Alessandro Zummo, Alexandre Belloni, Jean Delvare
  Cc: linux-rtc, linux-kernel, linux-hwmon

On 21/09/2022 16.13, Guenter Roeck wrote:
> On 9/21/22 04:46, Rasmus Villemoes wrote:

>> +static int isl12022_hwmon_read(struct device *dev,
>> +                   enum hwmon_sensor_types type,
>> +                   u32 attr, int channel, long *val)
>> +{
>> +    if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
>> +        *val = 60000;
>> +        return 0;
>> +    }
> 
> It is not the purpose of the update_interval attribute to inform the
> user what the update interval of this chip happens to be. The purpose
> of the attribute is to inform the chip what update interval it should use.

Well, I think it's a completely natural thing to expose a fixed and
known update_interval as a 0444 property, and it might even be useful to
userspace to know that there's no point reading the sensor any more
often than that. And I didn't come up with this by myself, there's
already at least a couple of instances of a 0444 update_interval.

I'll leave it to the RTC maintainers to decide, it's easy enough to remove.

Rasmus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor
  2022-09-23  8:40       ` Rasmus Villemoes
@ 2022-09-23 13:51         ` Guenter Roeck
  0 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2022-09-23 13:51 UTC (permalink / raw)
  To: Rasmus Villemoes, Alessandro Zummo, Alexandre Belloni, Jean Delvare
  Cc: linux-rtc, linux-kernel, linux-hwmon

On 9/23/22 01:40, Rasmus Villemoes wrote:
> On 21/09/2022 16.13, Guenter Roeck wrote:
>> On 9/21/22 04:46, Rasmus Villemoes wrote:
> 
>>> +static int isl12022_hwmon_read(struct device *dev,
>>> +                   enum hwmon_sensor_types type,
>>> +                   u32 attr, int channel, long *val)
>>> +{
>>> +    if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
>>> +        *val = 60000;
>>> +        return 0;
>>> +    }
>>
>> It is not the purpose of the update_interval attribute to inform the
>> user what the update interval of this chip happens to be. The purpose
>> of the attribute is to inform the chip what update interval it should use.
> 
> Well, I think it's a completely natural thing to expose a fixed and
> known update_interval as a 0444 property, and it might even be useful to
> userspace to know that there's no point reading the sensor any more
> often than that. And I didn't come up with this by myself, there's
> already at least a couple of instances of a 0444 update_interval.
> 

That doesn't make it better. It is still an abuse of the ABI.

Guenter

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/9] rtc: isl12022: cleanups and hwmon support
  2022-09-21 11:46 ` [PATCH v2 0/9] " Rasmus Villemoes
  2022-09-21 11:46   ` [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
@ 2022-10-07 13:51   ` Rasmus Villemoes
  2022-10-13 21:31   ` Alexandre Belloni
  2 siblings, 0 replies; 19+ messages in thread
From: Rasmus Villemoes @ 2022-10-07 13:51 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Jean Delvare, Guenter Roeck,
	linux-rtc, linux-kernel, linux-hwmon

On 21/09/2022 13.46, Rasmus Villemoes wrote:
> This series does a few cleanups of the isl12022 driver,
> 
> - removes use of deprecated function
> - removes some redundant code
> - switches to regmap API instead of private helpers
> 
> It also provides range_min, range_max values and finally hooks up the
> temperatur sensor to hwmon.
> 

Ping. Alexandre, anything I need to do to move this forward?

Thanks,
Rasmus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/9] rtc: isl12022: cleanups and hwmon support
  2022-09-21 11:46 ` [PATCH v2 0/9] " Rasmus Villemoes
  2022-09-21 11:46   ` [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
  2022-10-07 13:51   ` [PATCH v2 0/9] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
@ 2022-10-13 21:31   ` Alexandre Belloni
  2 siblings, 0 replies; 19+ messages in thread
From: Alexandre Belloni @ 2022-10-13 21:31 UTC (permalink / raw)
  To: Alessandro Zummo, Rasmus Villemoes, linux-rtc, Jean Delvare,
	Guenter Roeck, linux-hwmon, linux-kernel

On Wed, 21 Sep 2022 13:46:15 +0200, Rasmus Villemoes wrote:
> This series does a few cleanups of the isl12022 driver,
> 
> - removes use of deprecated function
> - removes some redundant code
> - switches to regmap API instead of private helpers
> 
> It also provides range_min, range_max values and finally hooks up the
> temperatur sensor to hwmon.
> 
> [...]

Applied, thanks!

[1/9] rtc: isl12022: stop using deprecated devm_rtc_device_register()
      commit: a35a2ad2b88a66732ac442ad5f86dc49af51673f
[2/9] rtc: isl12022: specify range_min and range_max
      commit: ca03b7a2c0b098321365f69538823d1bcc860552
[3/9] rtc: isl12022: drop a dev_info()
      commit: 43a96b9cf67770d4bb46267e1554d3d8b4cf78ac
[4/9] rtc: isl12022: simplify some expressions
      commit: ca35887186b7c53f26c42aee1285ba213adb4365
[5/9] rtc: isl12022: use %ptR
      commit: 7093b8a471f48d49891da2108f44fd64742408cb
[6/9] rtc: isl12022: use dev_set_drvdata() instead of i2c_set_clientdata()
      commit: 31b108acc50cddf3d16472ead45c4cd0d1337289
[7/9] rtc: isl12022: drop redundant write to HR register
      commit: 0a2abbfd8586d396a8581ebf9b96fd5746f08b14
[8/9] rtc: isl12022: switch to using regmap API
      commit: b1a1baa657c738e8bb0107ce304f5e78b9847f37

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 9/9] rtc: isl12022: add support for temperature sensor
  2022-09-21 11:46   ` [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
  2022-09-21 14:13     ` Guenter Roeck
@ 2022-10-26 13:38     ` Rasmus Villemoes
  2022-10-26 14:46       ` Guenter Roeck
  2022-11-04 11:02       ` [PATCH v4] " Rasmus Villemoes
  1 sibling, 2 replies; 19+ messages in thread
From: Rasmus Villemoes @ 2022-10-26 13:38 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Jean Delvare, Guenter Roeck
  Cc: Rasmus Villemoes, linux-rtc, linux-kernel, linux-hwmon

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v3: drop 0444 update_interval property.

v2 of patches 1-8 are already upstream (b1a1baa657c7 and parents).

 drivers/rtc/rtc-isl12022.c | 94 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
index ca677c4265e6..a3b0de3393f5 100644
--- a/drivers/rtc/rtc-isl12022.c
+++ b/drivers/rtc/rtc-isl12022.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>
+#include <linux/hwmon.h>
 
 /* ISL register offsets */
 #define ISL12022_REG_SC		0x00
@@ -30,6 +31,9 @@
 #define ISL12022_REG_SR		0x07
 #define ISL12022_REG_INT	0x08
 
+#define ISL12022_REG_BETA	0x0d
+#define ISL12022_REG_TEMP_L	0x28
+
 /* ISL register bits */
 #define ISL12022_HR_MIL		(1 << 7)	/* military or 24 hour time */
 
@@ -38,6 +42,7 @@
 
 #define ISL12022_INT_WRTC	(1 << 6)
 
+#define ISL12022_BETA_TSE	(1 << 7)
 
 static struct i2c_driver isl12022_driver;
 
@@ -46,6 +51,93 @@ struct isl12022 {
 	struct regmap *regmap;
 };
 
+static umode_t isl12022_hwmon_is_visible(const void *data,
+					 enum hwmon_sensor_types type,
+					 u32 attr, int channel)
+{
+	if (type == hwmon_temp && attr == hwmon_temp_input)
+		return 0444;
+
+	return 0;
+}
+
+/*
+ * A user-initiated temperature conversion is not started by this function,
+ * so the temperature is updated once every ~60 seconds.
+ */
+static int isl12022_hwmon_read_temp(struct device *dev, long *mC)
+{
+	struct isl12022 *isl12022 = dev_get_drvdata(dev);
+	struct regmap *regmap = isl12022->regmap;
+	u8 temp_buf[2];
+	int temp, ret;
+
+	ret = regmap_bulk_read(regmap, ISL12022_REG_TEMP_L,
+			       temp_buf, sizeof(temp_buf));
+	if (ret)
+		return ret;
+	/*
+	 * Temperature is represented as a 10-bit number, unit half-Kelvins.
+	 */
+	temp = (temp_buf[1] << 8) | temp_buf[0];
+	temp *= 500;
+	temp -= 273000;
+
+	*mC = temp;
+
+	return 0;
+}
+
+static int isl12022_hwmon_read(struct device *dev,
+			       enum hwmon_sensor_types type,
+			       u32 attr, int channel, long *val)
+{
+	if (type == hwmon_temp && attr == hwmon_temp_input)
+		return isl12022_hwmon_read_temp(dev, val);
+
+	return -EOPNOTSUPP;
+}
+
+static const struct hwmon_channel_info *isl12022_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops isl12022_hwmon_ops = {
+	.is_visible = isl12022_hwmon_is_visible,
+	.read = isl12022_hwmon_read,
+};
+
+static const struct hwmon_chip_info isl12022_hwmon_chip_info = {
+	.ops = &isl12022_hwmon_ops,
+	.info = isl12022_hwmon_info,
+};
+
+static void isl12022_hwmon_register(struct device *dev)
+{
+	struct isl12022 *isl12022;
+	struct device *hwmon;
+	int ret;
+
+	if (!IS_REACHABLE(CONFIG_HWMON))
+		return;
+
+	isl12022 = dev_get_drvdata(dev);
+
+	ret = regmap_update_bits(isl12022->regmap, ISL12022_REG_BETA,
+				 ISL12022_BETA_TSE, ISL12022_BETA_TSE);
+	if (ret) {
+		dev_warn(dev, "unable to enable temperature sensor\n");
+		return;
+	}
+
+	hwmon = devm_hwmon_device_register_with_info(dev, "isl12022", isl12022,
+						     &isl12022_hwmon_chip_info,
+						     NULL);
+	if (IS_ERR(hwmon))
+		dev_warn(dev, "unable to register hwmon device: %pe\n", hwmon);
+}
+
 /*
  * In the routines that deal directly with the isl12022 hardware, we use
  * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch.
@@ -160,6 +252,8 @@ static int isl12022_probe(struct i2c_client *client)
 		return PTR_ERR(isl12022->regmap);
 	}
 
+	isl12022_hwmon_register(&client->dev);
+
 	isl12022->rtc = devm_rtc_allocate_device(&client->dev);
 	if (IS_ERR(isl12022->rtc))
 		return PTR_ERR(isl12022->rtc);
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 9/9] rtc: isl12022: add support for temperature sensor
  2022-10-26 13:38     ` [PATCH v3 " Rasmus Villemoes
@ 2022-10-26 14:46       ` Guenter Roeck
  2022-11-04 11:02       ` [PATCH v4] " Rasmus Villemoes
  1 sibling, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2022-10-26 14:46 UTC (permalink / raw)
  To: Rasmus Villemoes, Alessandro Zummo, Alexandre Belloni, Jean Delvare
  Cc: linux-rtc, linux-kernel, linux-hwmon

On 10/26/22 06:38, Rasmus Villemoes wrote:
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

There should be some description above. Other than that,

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

 > ---
> v3: drop 0444 update_interval property.
> 
> v2 of patches 1-8 are already upstream (b1a1baa657c7 and parents).
> 
>   drivers/rtc/rtc-isl12022.c | 94 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 94 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
> index ca677c4265e6..a3b0de3393f5 100644
> --- a/drivers/rtc/rtc-isl12022.c
> +++ b/drivers/rtc/rtc-isl12022.c
> @@ -17,6 +17,7 @@
>   #include <linux/of.h>
>   #include <linux/of_device.h>
>   #include <linux/regmap.h>
> +#include <linux/hwmon.h>
>   
>   /* ISL register offsets */
>   #define ISL12022_REG_SC		0x00
> @@ -30,6 +31,9 @@
>   #define ISL12022_REG_SR		0x07
>   #define ISL12022_REG_INT	0x08
>   
> +#define ISL12022_REG_BETA	0x0d
> +#define ISL12022_REG_TEMP_L	0x28
> +
>   /* ISL register bits */
>   #define ISL12022_HR_MIL		(1 << 7)	/* military or 24 hour time */
>   
> @@ -38,6 +42,7 @@
>   
>   #define ISL12022_INT_WRTC	(1 << 6)
>   
> +#define ISL12022_BETA_TSE	(1 << 7)
>   
>   static struct i2c_driver isl12022_driver;
>   
> @@ -46,6 +51,93 @@ struct isl12022 {
>   	struct regmap *regmap;
>   };
>   
> +static umode_t isl12022_hwmon_is_visible(const void *data,
> +					 enum hwmon_sensor_types type,
> +					 u32 attr, int channel)
> +{
> +	if (type == hwmon_temp && attr == hwmon_temp_input)
> +		return 0444;
> +
> +	return 0;
> +}
> +
> +/*
> + * A user-initiated temperature conversion is not started by this function,
> + * so the temperature is updated once every ~60 seconds.
> + */
> +static int isl12022_hwmon_read_temp(struct device *dev, long *mC)
> +{
> +	struct isl12022 *isl12022 = dev_get_drvdata(dev);
> +	struct regmap *regmap = isl12022->regmap;
> +	u8 temp_buf[2];
> +	int temp, ret;
> +
> +	ret = regmap_bulk_read(regmap, ISL12022_REG_TEMP_L,
> +			       temp_buf, sizeof(temp_buf));
> +	if (ret)
> +		return ret;
> +	/*
> +	 * Temperature is represented as a 10-bit number, unit half-Kelvins.
> +	 */
> +	temp = (temp_buf[1] << 8) | temp_buf[0];
> +	temp *= 500;
> +	temp -= 273000;
> +
> +	*mC = temp;
> +
> +	return 0;
> +}
> +
> +static int isl12022_hwmon_read(struct device *dev,
> +			       enum hwmon_sensor_types type,
> +			       u32 attr, int channel, long *val)
> +{
> +	if (type == hwmon_temp && attr == hwmon_temp_input)
> +		return isl12022_hwmon_read_temp(dev, val);
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static const struct hwmon_channel_info *isl12022_hwmon_info[] = {
> +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
> +	NULL
> +};
> +
> +static const struct hwmon_ops isl12022_hwmon_ops = {
> +	.is_visible = isl12022_hwmon_is_visible,
> +	.read = isl12022_hwmon_read,
> +};
> +
> +static const struct hwmon_chip_info isl12022_hwmon_chip_info = {
> +	.ops = &isl12022_hwmon_ops,
> +	.info = isl12022_hwmon_info,
> +};
> +
> +static void isl12022_hwmon_register(struct device *dev)
> +{
> +	struct isl12022 *isl12022;
> +	struct device *hwmon;
> +	int ret;
> +
> +	if (!IS_REACHABLE(CONFIG_HWMON))
> +		return;
> +
> +	isl12022 = dev_get_drvdata(dev);
> +
> +	ret = regmap_update_bits(isl12022->regmap, ISL12022_REG_BETA,
> +				 ISL12022_BETA_TSE, ISL12022_BETA_TSE);
> +	if (ret) {
> +		dev_warn(dev, "unable to enable temperature sensor\n");
> +		return;
> +	}
> +
> +	hwmon = devm_hwmon_device_register_with_info(dev, "isl12022", isl12022,
> +						     &isl12022_hwmon_chip_info,
> +						     NULL);
> +	if (IS_ERR(hwmon))
> +		dev_warn(dev, "unable to register hwmon device: %pe\n", hwmon);
> +}
> +
>   /*
>    * In the routines that deal directly with the isl12022 hardware, we use
>    * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch.
> @@ -160,6 +252,8 @@ static int isl12022_probe(struct i2c_client *client)
>   		return PTR_ERR(isl12022->regmap);
>   	}
>   
> +	isl12022_hwmon_register(&client->dev);
> +
>   	isl12022->rtc = devm_rtc_allocate_device(&client->dev);
>   	if (IS_ERR(isl12022->rtc))
>   		return PTR_ERR(isl12022->rtc);


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v4] rtc: isl12022: add support for temperature sensor
  2022-10-26 13:38     ` [PATCH v3 " Rasmus Villemoes
  2022-10-26 14:46       ` Guenter Roeck
@ 2022-11-04 11:02       ` Rasmus Villemoes
  2022-11-14 21:41         ` Alexandre Belloni
  1 sibling, 1 reply; 19+ messages in thread
From: Rasmus Villemoes @ 2022-11-04 11:02 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Jean Delvare, Guenter Roeck
  Cc: Rasmus Villemoes, linux-rtc, linux-kernel, linux-hwmon

The isl12022 has built-in temperature compensation effective over the
range -40C to +85C. It exposes the average of the last two temperature
measurements as a 10-bit value in half-Kelvins. Make this available
via the hwmon framework.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v4: add a real commit log, include Guenter's R-b.

v3 at https://lore.kernel.org/lkml/20221026133847.1193422-1-linux@rasmusvillemoes.dk/

 drivers/rtc/rtc-isl12022.c | 94 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
index ca677c4265e6..a3b0de3393f5 100644
--- a/drivers/rtc/rtc-isl12022.c
+++ b/drivers/rtc/rtc-isl12022.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>
+#include <linux/hwmon.h>
 
 /* ISL register offsets */
 #define ISL12022_REG_SC		0x00
@@ -30,6 +31,9 @@
 #define ISL12022_REG_SR		0x07
 #define ISL12022_REG_INT	0x08
 
+#define ISL12022_REG_BETA	0x0d
+#define ISL12022_REG_TEMP_L	0x28
+
 /* ISL register bits */
 #define ISL12022_HR_MIL		(1 << 7)	/* military or 24 hour time */
 
@@ -38,6 +42,7 @@
 
 #define ISL12022_INT_WRTC	(1 << 6)
 
+#define ISL12022_BETA_TSE	(1 << 7)
 
 static struct i2c_driver isl12022_driver;
 
@@ -46,6 +51,93 @@ struct isl12022 {
 	struct regmap *regmap;
 };
 
+static umode_t isl12022_hwmon_is_visible(const void *data,
+					 enum hwmon_sensor_types type,
+					 u32 attr, int channel)
+{
+	if (type == hwmon_temp && attr == hwmon_temp_input)
+		return 0444;
+
+	return 0;
+}
+
+/*
+ * A user-initiated temperature conversion is not started by this function,
+ * so the temperature is updated once every ~60 seconds.
+ */
+static int isl12022_hwmon_read_temp(struct device *dev, long *mC)
+{
+	struct isl12022 *isl12022 = dev_get_drvdata(dev);
+	struct regmap *regmap = isl12022->regmap;
+	u8 temp_buf[2];
+	int temp, ret;
+
+	ret = regmap_bulk_read(regmap, ISL12022_REG_TEMP_L,
+			       temp_buf, sizeof(temp_buf));
+	if (ret)
+		return ret;
+	/*
+	 * Temperature is represented as a 10-bit number, unit half-Kelvins.
+	 */
+	temp = (temp_buf[1] << 8) | temp_buf[0];
+	temp *= 500;
+	temp -= 273000;
+
+	*mC = temp;
+
+	return 0;
+}
+
+static int isl12022_hwmon_read(struct device *dev,
+			       enum hwmon_sensor_types type,
+			       u32 attr, int channel, long *val)
+{
+	if (type == hwmon_temp && attr == hwmon_temp_input)
+		return isl12022_hwmon_read_temp(dev, val);
+
+	return -EOPNOTSUPP;
+}
+
+static const struct hwmon_channel_info *isl12022_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops isl12022_hwmon_ops = {
+	.is_visible = isl12022_hwmon_is_visible,
+	.read = isl12022_hwmon_read,
+};
+
+static const struct hwmon_chip_info isl12022_hwmon_chip_info = {
+	.ops = &isl12022_hwmon_ops,
+	.info = isl12022_hwmon_info,
+};
+
+static void isl12022_hwmon_register(struct device *dev)
+{
+	struct isl12022 *isl12022;
+	struct device *hwmon;
+	int ret;
+
+	if (!IS_REACHABLE(CONFIG_HWMON))
+		return;
+
+	isl12022 = dev_get_drvdata(dev);
+
+	ret = regmap_update_bits(isl12022->regmap, ISL12022_REG_BETA,
+				 ISL12022_BETA_TSE, ISL12022_BETA_TSE);
+	if (ret) {
+		dev_warn(dev, "unable to enable temperature sensor\n");
+		return;
+	}
+
+	hwmon = devm_hwmon_device_register_with_info(dev, "isl12022", isl12022,
+						     &isl12022_hwmon_chip_info,
+						     NULL);
+	if (IS_ERR(hwmon))
+		dev_warn(dev, "unable to register hwmon device: %pe\n", hwmon);
+}
+
 /*
  * In the routines that deal directly with the isl12022 hardware, we use
  * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch.
@@ -160,6 +252,8 @@ static int isl12022_probe(struct i2c_client *client)
 		return PTR_ERR(isl12022->regmap);
 	}
 
+	isl12022_hwmon_register(&client->dev);
+
 	isl12022->rtc = devm_rtc_allocate_device(&client->dev);
 	if (IS_ERR(isl12022->rtc))
 		return PTR_ERR(isl12022->rtc);
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v4] rtc: isl12022: add support for temperature sensor
  2022-11-04 11:02       ` [PATCH v4] " Rasmus Villemoes
@ 2022-11-14 21:41         ` Alexandre Belloni
  0 siblings, 0 replies; 19+ messages in thread
From: Alexandre Belloni @ 2022-11-14 21:41 UTC (permalink / raw)
  To: Alessandro Zummo, Guenter Roeck, Jean Delvare, Rasmus Villemoes
  Cc: linux-rtc, linux-hwmon, linux-kernel

On Fri, 4 Nov 2022 12:02:25 +0100, Rasmus Villemoes wrote:
> The isl12022 has built-in temperature compensation effective over the
> range -40C to +85C. It exposes the average of the last two temperature
> measurements as a 10-bit value in half-Kelvins. Make this available
> via the hwmon framework.
> 
> 

Applied, thanks!

[1/1] rtc: isl12022: add support for temperature sensor
      commit: 1087656c079ddd3bb240a3c1f108c37b9b8f1457

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-11-14 21:41 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-30 10:01 [PATCH 0/6] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
2022-08-30 10:01 ` [PATCH 6/6] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
2022-08-30 13:13   ` Guenter Roeck
2022-09-14 15:12   ` Alexandre Belloni
2022-09-21  7:58     ` Rasmus Villemoes
2022-09-21  9:10       ` Alexandre Belloni
2022-09-09  8:15 ` [PATCH 0/6] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
2022-09-14 15:16   ` Alexandre Belloni
2022-09-21 11:46 ` [PATCH v2 0/9] " Rasmus Villemoes
2022-09-21 11:46   ` [PATCH v2 9/9] rtc: isl12022: add support for temperature sensor Rasmus Villemoes
2022-09-21 14:13     ` Guenter Roeck
2022-09-23  8:40       ` Rasmus Villemoes
2022-09-23 13:51         ` Guenter Roeck
2022-10-26 13:38     ` [PATCH v3 " Rasmus Villemoes
2022-10-26 14:46       ` Guenter Roeck
2022-11-04 11:02       ` [PATCH v4] " Rasmus Villemoes
2022-11-14 21:41         ` Alexandre Belloni
2022-10-07 13:51   ` [PATCH v2 0/9] rtc: isl12022: cleanups and hwmon support Rasmus Villemoes
2022-10-13 21:31   ` Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).