[PATCH] rtc: ds3232: add temperature support

[PATCH] rtc: ds3232: add temperature support

[Kirill Esipov]

DS3232/DS3234 has the temperature registers with a resolution of
0.25 degree celsius. This enables to get the value through hwmon.

	# cat /sys/class/hwmon/hwmon0/temp1_input
	37250

Signed-off-by: Kirill Esipov <yesipov@gmail.com>
---
 drivers/rtc/Kconfig      |  9 ++++++
 drivers/rtc/rtc-ds3232.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8d3b95728326..b4a6a916d4df 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -791,6 +791,15 @@ config RTC_DRV_DS3232
 	  This driver can also be built as a module.  If so, the module
 	  will be called rtc-ds3232.
 
+config RTC_DRV_DS3232_HWMON
+	bool "HWMON support for Dallas/Maxim DS3232/DS3234"
+	depends on RTC_DRV_DS3232 && HWMON
+	depends on !(RTC_DRV_DS3232=y && HWMON=m)
+	default y
+	help
+	  Say Y here if you want to expose temperature sensor data on
+	  rtc-ds3232
+
 config RTC_DRV_PCF2127
 	tristate "NXP PCF2127"
 	depends on RTC_I2C_AND_SPI
diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
index deff431a37c4..f94ff0685942 100644
--- a/drivers/rtc/rtc-ds3232.c
+++ b/drivers/rtc/rtc-ds3232.c
@@ -22,6 +22,8 @@
 #include <linux/bcd.h>
 #include <linux/slab.h>
 #include <linux/regmap.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 
 #define DS3232_REG_SECONDS      0x00
 #define DS3232_REG_MINUTES      0x01
@@ -275,6 +277,86 @@ static int ds3232_update_alarm(struct device *dev, unsigned int enabled)
 	return ret;
 }
 
+/*----------------------------------------------------------------------*/
+
+#ifdef CONFIG_RTC_DRV_DS3232_HWMON
+
+/*
+ * Temperature sensor support for ds3232/ds3234 devices.
+ */
+
+#define DS3232_REG_TEMPERATURE	0x11
+
+/*
+ * A user-initiated temperature conversion is not started by this function,
+ * so the temperature is updated once every 64 seconds.
+ */
+static int ds3232_hwmon_read_temp(struct device *dev, s32 *mC)
+{
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
+	u8 temp_buf[2];
+	s16 temp;
+	int ret;
+
+	ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_TEMPERATURE, temp_buf,
+				sizeof(temp_buf));
+
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Temperature is represented as a 10-bit code with a resolution of
+	 * 0.25 degree celsius and encoded in two's complement format.
+	 */
+	temp = (temp_buf[0] << 8) | temp_buf[1];
+	temp >>= 6;
+	*mC = temp * 250;
+
+	return 0;
+}
+
+static ssize_t ds3232_hwmon_show_temp(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int ret;
+	s32 temp;
+
+	ret = ds3232_hwmon_read_temp(dev, &temp);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", temp);
+}
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, ds3232_hwmon_show_temp,
+			NULL, 0);
+
+static struct attribute *ds3232_hwmon_attrs[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(ds3232_hwmon);
+
+static void ds3232_hwmon_register(struct device *dev, const char *name)
+{
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
+	struct device *hwmon_dev;
+
+	hwmon_dev = devm_hwmon_device_register_with_groups(dev, name, ds3232,
+							   ds3232_hwmon_groups);
+	if (IS_ERR(hwmon_dev)) {
+		dev_warn(dev, "unable to register hwmon device %ld\n",
+			 PTR_ERR(hwmon_dev));
+	}
+}
+
+#else
+
+static void ds3232_hwmon_register(struct ds3232 *ds3232)
+{
+}
+
+#endif
+
 static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct ds3232 *ds3232 = dev_get_drvdata(dev);
@@ -366,6 +448,8 @@ static int ds3232_probe(struct device *dev, struct regmap *regmap, int irq,
 	if (ds3232->irq > 0)
 		device_init_wakeup(dev, 1);
 
+	ds3232_hwmon_register(dev, name);
+
 	ds3232->rtc = devm_rtc_device_register(dev, name, &ds3232_rtc_ops,
 						THIS_MODULE);
 	if (IS_ERR(ds3232->rtc))
-- 
2.7.4

Re: rtc: ds3232: add temperature support

[Guenter Roeck]

On Wed, Jun 21, 2017 at 05:49:43PM +0300, Kirill Esipov wrote:
> DS3232/DS3234 has the temperature registers with a resolution of
> 0.25 degree celsius. This enables to get the value through hwmon.
> 
> 	# cat /sys/class/hwmon/hwmon0/temp1_input
> 	37250
> 
> Signed-off-by: Kirill Esipov <yesipov@gmail.com>
> ---
>  drivers/rtc/Kconfig      |  9 ++++++
>  drivers/rtc/rtc-ds3232.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 8d3b95728326..b4a6a916d4df 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -791,6 +791,15 @@ config RTC_DRV_DS3232
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called rtc-ds3232.
>  
> +config RTC_DRV_DS3232_HWMON
> +	bool "HWMON support for Dallas/Maxim DS3232/DS3234"
> +	depends on RTC_DRV_DS3232 && HWMON
> +	depends on !(RTC_DRV_DS3232=y && HWMON=m)
> +	default y
> +	help
> +	  Say Y here if you want to expose temperature sensor data on
> +	  rtc-ds3232
> +
>  config RTC_DRV_PCF2127
>  	tristate "NXP PCF2127"
>  	depends on RTC_I2C_AND_SPI
> diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
> index deff431a37c4..f94ff0685942 100644
> --- a/drivers/rtc/rtc-ds3232.c
> +++ b/drivers/rtc/rtc-ds3232.c
> @@ -22,6 +22,8 @@
>  #include <linux/bcd.h>
>  #include <linux/slab.h>
>  #include <linux/regmap.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
>  
>  #define DS3232_REG_SECONDS      0x00
>  #define DS3232_REG_MINUTES      0x01
> @@ -275,6 +277,86 @@ static int ds3232_update_alarm(struct device *dev, unsigned int enabled)
>  	return ret;
>  }
>  
> +/*----------------------------------------------------------------------*/
> +
> +#ifdef CONFIG_RTC_DRV_DS3232_HWMON
> +
> +/*
> + * Temperature sensor support for ds3232/ds3234 devices.
> + */
> +
> +#define DS3232_REG_TEMPERATURE	0x11
> +
> +/*
> + * A user-initiated temperature conversion is not started by this function,
> + * so the temperature is updated once every 64 seconds.
> + */
> +static int ds3232_hwmon_read_temp(struct device *dev, s32 *mC)
> +{
> +	struct ds3232 *ds3232 = dev_get_drvdata(dev);
> +	u8 temp_buf[2];
> +	s16 temp;
> +	int ret;
> +
> +	ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_TEMPERATURE, temp_buf,
> +				sizeof(temp_buf));
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Temperature is represented as a 10-bit code with a resolution of
> +	 * 0.25 degree celsius and encoded in two's complement format.
> +	 */
> +	temp = (temp_buf[0] << 8) | temp_buf[1];
> +	temp >>= 6;
> +	*mC = temp * 250;
> +
> +	return 0;
> +}
> +
> +static ssize_t ds3232_hwmon_show_temp(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	int ret;
> +	s32 temp;
> +
> +	ret = ds3232_hwmon_read_temp(dev, &temp);
> +	if (ret < 0)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", temp);
> +}
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, ds3232_hwmon_show_temp,
> +			NULL, 0);
> +
> +static struct attribute *ds3232_hwmon_attrs[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(ds3232_hwmon);
> +
> +static void ds3232_hwmon_register(struct device *dev, const char *name)
> +{
> +	struct ds3232 *ds3232 = dev_get_drvdata(dev);
> +	struct device *hwmon_dev;
> +
> +	hwmon_dev = devm_hwmon_device_register_with_groups(dev, name, ds3232,
> +							   ds3232_hwmon_groups);

Any reason for not using devm_hwmon_device_register_with_info() ?

Guenter

> +	if (IS_ERR(hwmon_dev)) {
> +		dev_warn(dev, "unable to register hwmon device %ld\n",
> +			 PTR_ERR(hwmon_dev));
> +	}
> +}
> +
> +#else
> +
> +static void ds3232_hwmon_register(struct ds3232 *ds3232)
> +{
> +}
> +
> +#endif
> +
>  static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
>  {
>  	struct ds3232 *ds3232 = dev_get_drvdata(dev);
> @@ -366,6 +448,8 @@ static int ds3232_probe(struct device *dev, struct regmap *regmap, int irq,
>  	if (ds3232->irq > 0)
>  		device_init_wakeup(dev, 1);
>  
> +	ds3232_hwmon_register(dev, name);
> +
>  	ds3232->rtc = devm_rtc_device_register(dev, name, &ds3232_rtc_ops,
>  						THIS_MODULE);
>  	if (IS_ERR(ds3232->rtc))

Re: rtc: ds3232: add temperature support

[Kirill Esipov]

2017-06-22 1:24 GMT+03:00 Guenter Roeck <linux@roeck-us.net>:
> On Wed, Jun 21, 2017 at 05:49:43PM +0300, Kirill Esipov wrote:
>> DS3232/DS3234 has the temperature registers with a resolution of
>> 0.25 degree celsius. This enables to get the value through hwmon.
>>
>>       # cat /sys/class/hwmon/hwmon0/temp1_input
>>       37250
>>
>> Signed-off-by: Kirill Esipov <yesipov@gmail.com>
>> ---
>>  drivers/rtc/Kconfig      |  9 ++++++
>>  drivers/rtc/rtc-ds3232.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 93 insertions(+)
>>
>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>> index 8d3b95728326..b4a6a916d4df 100644
>> --- a/drivers/rtc/Kconfig
>> +++ b/drivers/rtc/Kconfig
>> @@ -791,6 +791,15 @@ config RTC_DRV_DS3232
>>         This driver can also be built as a module.  If so, the module
>>         will be called rtc-ds3232.
>>
>> +config RTC_DRV_DS3232_HWMON
>> +     bool "HWMON support for Dallas/Maxim DS3232/DS3234"
>> +     depends on RTC_DRV_DS3232 && HWMON
>> +     depends on !(RTC_DRV_DS3232=y && HWMON=m)
>> +     default y
>> +     help
>> +       Say Y here if you want to expose temperature sensor data on
>> +       rtc-ds3232
>> +
>>  config RTC_DRV_PCF2127
>>       tristate "NXP PCF2127"
>>       depends on RTC_I2C_AND_SPI
>> diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
>> index deff431a37c4..f94ff0685942 100644
>> --- a/drivers/rtc/rtc-ds3232.c
>> +++ b/drivers/rtc/rtc-ds3232.c
>> @@ -22,6 +22,8 @@
>>  #include <linux/bcd.h>
>>  #include <linux/slab.h>
>>  #include <linux/regmap.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>>
>>  #define DS3232_REG_SECONDS      0x00
>>  #define DS3232_REG_MINUTES      0x01
>> @@ -275,6 +277,86 @@ static int ds3232_update_alarm(struct device *dev, unsigned int enabled)
>>       return ret;
>>  }
>>
>> +/*----------------------------------------------------------------------*/
>> +
>> +#ifdef CONFIG_RTC_DRV_DS3232_HWMON
>> +
>> +/*
>> + * Temperature sensor support for ds3232/ds3234 devices.
>> + */
>> +
>> +#define DS3232_REG_TEMPERATURE       0x11
>> +
>> +/*
>> + * A user-initiated temperature conversion is not started by this function,
>> + * so the temperature is updated once every 64 seconds.
>> + */
>> +static int ds3232_hwmon_read_temp(struct device *dev, s32 *mC)
>> +{
>> +     struct ds3232 *ds3232 = dev_get_drvdata(dev);
>> +     u8 temp_buf[2];
>> +     s16 temp;
>> +     int ret;
>> +
>> +     ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_TEMPERATURE, temp_buf,
>> +                             sizeof(temp_buf));
>> +
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     /*
>> +      * Temperature is represented as a 10-bit code with a resolution of
>> +      * 0.25 degree celsius and encoded in two's complement format.
>> +      */
>> +     temp = (temp_buf[0] << 8) | temp_buf[1];
>> +     temp >>= 6;
>> +     *mC = temp * 250;
>> +
>> +     return 0;
>> +}
>> +
>> +static ssize_t ds3232_hwmon_show_temp(struct device *dev,
>> +                             struct device_attribute *attr, char *buf)
>> +{
>> +     int ret;
>> +     s32 temp;
>> +
>> +     ret = ds3232_hwmon_read_temp(dev, &temp);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     return sprintf(buf, "%d\n", temp);
>> +}
>> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, ds3232_hwmon_show_temp,
>> +                     NULL, 0);
>> +
>> +static struct attribute *ds3232_hwmon_attrs[] = {
>> +     &sensor_dev_attr_temp1_input.dev_attr.attr,
>> +     NULL,
>> +};
>> +ATTRIBUTE_GROUPS(ds3232_hwmon);
>> +
>> +static void ds3232_hwmon_register(struct device *dev, const char *name)
>> +{
>> +     struct ds3232 *ds3232 = dev_get_drvdata(dev);
>> +     struct device *hwmon_dev;
>> +
>> +     hwmon_dev = devm_hwmon_device_register_with_groups(dev, name, ds3232,
>> +                                                        ds3232_hwmon_groups);
>
> Any reason for not using devm_hwmon_device_register_with_info() ?
>


Just to keep uniformity,  because other rtc drivers with hwmon
(rtc-ds1307, rtc-rv3029c2) use
devm_hwmon_device_register_with_groups().


> Guenter
>
>> +     if (IS_ERR(hwmon_dev)) {
>> +             dev_warn(dev, "unable to register hwmon device %ld\n",
>> +                      PTR_ERR(hwmon_dev));
>> +     }
>> +}
>> +
>> +#else
>> +
>> +static void ds3232_hwmon_register(struct ds3232 *ds3232)
>> +{
>> +}
>> +
>> +#endif
>> +
>>  static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
>>  {
>>       struct ds3232 *ds3232 = dev_get_drvdata(dev);
>> @@ -366,6 +448,8 @@ static int ds3232_probe(struct device *dev, struct regmap *regmap, int irq,
>>       if (ds3232->irq > 0)
>>               device_init_wakeup(dev, 1);
>>
>> +     ds3232_hwmon_register(dev, name);
>> +
>>       ds3232->rtc = devm_rtc_device_register(dev, name, &ds3232_rtc_ops,
>>                                               THIS_MODULE);
>>       if (IS_ERR(ds3232->rtc))



-- 
Kirill Esipov

Re: rtc: ds3232: add temperature support

[Guenter Roeck]

On 06/22/2017 05:07 AM, Kirill Esipov wrote:
> 
> 
> 2017-06-22 1:24 GMT+03:00 Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>>:
> 
>     On Wed, Jun 21, 2017 at 05:49:43PM +0300, Kirill Esipov wrote:
>      > DS3232/DS3234 has the temperature registers with a resolution of
>      > 0.25 degree celsius. This enables to get the value through hwmon.
>      >
>      >       # cat /sys/class/hwmon/hwmon0/temp1_input
>      >       37250
>      >
>      > Signed-off-by: Kirill Esipov <yesipov@gmail.com <mailto:yesipov@gmail.com>>
>      > ---
>      >  drivers/rtc/Kconfig      |  9 ++++++
>      >  drivers/rtc/rtc-ds3232.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
>      >  2 files changed, 93 insertions(+)
>      >
>      > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>      > index 8d3b95728326..b4a6a916d4df 100644
>      > --- a/drivers/rtc/Kconfig
>      > +++ b/drivers/rtc/Kconfig
>      > @@ -791,6 +791,15 @@ config RTC_DRV_DS3232
>      >         This driver can also be built as a module.  If so, the module
>      >         will be called rtc-ds3232.
>      >
>      > +config RTC_DRV_DS3232_HWMON
>      > +     bool "HWMON support for Dallas/Maxim DS3232/DS3234"
>      > +     depends on RTC_DRV_DS3232 && HWMON
>      > +     depends on !(RTC_DRV_DS3232=y && HWMON=m)
>      > +     default y
>      > +     help
>      > +       Say Y here if you want to expose temperature sensor data on
>      > +       rtc-ds3232
>      > +
>      >  config RTC_DRV_PCF2127
>      >       tristate "NXP PCF2127"
>      >       depends on RTC_I2C_AND_SPI
>      > diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
>      > index deff431a37c4..f94ff0685942 100644
>      > --- a/drivers/rtc/rtc-ds3232.c
>      > +++ b/drivers/rtc/rtc-ds3232.c
>      > @@ -22,6 +22,8 @@
>      >  #include <linux/bcd.h>
>      >  #include <linux/slab.h>
>      >  #include <linux/regmap.h>
>      > +#include <linux/hwmon.h>
>      > +#include <linux/hwmon-sysfs.h>
>      >
>      >  #define DS3232_REG_SECONDS      0x00
>      >  #define DS3232_REG_MINUTES      0x01
>      > @@ -275,6 +277,86 @@ static int ds3232_update_alarm(struct device *dev, unsigned int enabled)
>      >       return ret;
>      >  }
>      >
>      > +/*----------------------------------------------------------------------*/
>      > +
>      > +#ifdef CONFIG_RTC_DRV_DS3232_HWMON
>      > +
>      > +/*
>      > + * Temperature sensor support for ds3232/ds3234 devices.
>      > + */
>      > +
>      > +#define DS3232_REG_TEMPERATURE       0x11
>      > +
>      > +/*
>      > + * A user-initiated temperature conversion is not started by this function,
>      > + * so the temperature is updated once every 64 seconds.
>      > + */
>      > +static int ds3232_hwmon_read_temp(struct device *dev, s32 *mC)
>      > +{
>      > +     struct ds3232 *ds3232 = dev_get_drvdata(dev);
>      > +     u8 temp_buf[2];
>      > +     s16 temp;
>      > +     int ret;
>      > +
>      > +     ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_TEMPERATURE, temp_buf,
>      > +                             sizeof(temp_buf));
>      > +
>      > +     if (ret < 0)
>      > +             return ret;
>      > +
>      > +     /*
>      > +      * Temperature is represented as a 10-bit code with a resolution of
>      > +      * 0.25 degree celsius and encoded in two's complement format.
>      > +      */
>      > +     temp = (temp_buf[0] << 8) | temp_buf[1];
>      > +     temp >>= 6;
>      > +     *mC = temp * 250;
>      > +
>      > +     return 0;
>      > +}
>      > +
>      > +static ssize_t ds3232_hwmon_show_temp(struct device *dev,
>      > +                             struct device_attribute *attr, char *buf)
>      > +{
>      > +     int ret;
>      > +     s32 temp;
>      > +
>      > +     ret = ds3232_hwmon_read_temp(dev, &temp);
>      > +     if (ret < 0)
>      > +             return ret;
>      > +
>      > +     return sprintf(buf, "%d\n", temp);
>      > +}
>      > +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, ds3232_hwmon_show_temp,
>      > +                     NULL, 0);
>      > +
>      > +static struct attribute *ds3232_hwmon_attrs[] = {
>      > +     &sensor_dev_attr_temp1_input.dev_attr.attr,
>      > +     NULL,
>      > +};
>      > +ATTRIBUTE_GROUPS(ds3232_hwmon);
>      > +
>      > +static void ds3232_hwmon_register(struct device *dev, const char *name)
>      > +{
>      > +     struct ds3232 *ds3232 = dev_get_drvdata(dev);
>      > +     struct device *hwmon_dev;
>      > +
>      > +     hwmon_dev = devm_hwmon_device_register_with_groups(dev, name, ds3232,
>      > +                                                        ds3232_hwmon_groups);
> 
>     Any reason for not using devm_hwmon_device_register_with_info() ?
> 
> 
> 
> Just to keep uniformity,  because other rtc drivers with hwmon (rtc-ds1307, rtc-rv3029c2) use
> devm_hwmon_device_register_with_groups().
> 

Hmm. Odd reason. With this line of argument, a new API should never be used because
"everyone else uses the old API". How about converting the other drivers to use
new API instead ?

The idea behind the new API was to simplify drivers and make them independent
of the sysfs ABI. That doesn't mean that drivers _have_ to use that API, though,
so feel free to stick with the above.

Guenter

> 
>     Guenter
> 
>      > +     if (IS_ERR(hwmon_dev)) {
>      > +             dev_warn(dev, "unable to register hwmon device %ld\n",
>      > +                      PTR_ERR(hwmon_dev));
>      > +     }
>      > +}
>      > +
>      > +#else
>      > +
>      > +static void ds3232_hwmon_register(struct ds3232 *ds3232)
>      > +{
>      > +}
>      > +
>      > +#endif
>      > +
>      >  static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
>      >  {
>      >       struct ds3232 *ds3232 = dev_get_drvdata(dev);
>      > @@ -366,6 +448,8 @@ static int ds3232_probe(struct device *dev, struct regmap *regmap, int irq,
>      >       if (ds3232->irq > 0)
>      >               device_init_wakeup(dev, 1);
>      >
>      > +     ds3232_hwmon_register(dev, name);
>      > +
>      >       ds3232->rtc = devm_rtc_device_register(dev, name, &ds3232_rtc_ops,
>      >                                               THIS_MODULE);
>      >       if (IS_ERR(ds3232->rtc))
> 
> 
> 
> 
> -- 
> Kirill Esipov

Re: rtc: ds3232: add temperature support

[Kirill Esipov]

2017-06-22 16:44 GMT+03:00 Guenter Roeck <linux@roeck-us.net>:
> On 06/22/2017 05:07 AM, Kirill Esipov wrote:
>>
>>
>>
>> 2017-06-22 1:24 GMT+03:00 Guenter Roeck <linux@roeck-us.net
>> <mailto:linux@roeck-us.net>>:
>>
>>     On Wed, Jun 21, 2017 at 05:49:43PM +0300, Kirill Esipov wrote:
>>      > DS3232/DS3234 has the temperature registers with a resolution of
>>      > 0.25 degree celsius. This enables to get the value through hwmon.
>>      >
>>      >       # cat /sys/class/hwmon/hwmon0/temp1_input
>>      >       37250
>>      >
>>      > Signed-off-by: Kirill Esipov <yesipov@gmail.com
>> <mailto:yesipov@gmail.com>>
>>
>>      > ---
>>      >  drivers/rtc/Kconfig      |  9 ++++++
>>      >  drivers/rtc/rtc-ds3232.c | 84
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>      >  2 files changed, 93 insertions(+)
>>      >
>>      > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>>      > index 8d3b95728326..b4a6a916d4df 100644
>>      > --- a/drivers/rtc/Kconfig
>>      > +++ b/drivers/rtc/Kconfig
>>      > @@ -791,6 +791,15 @@ config RTC_DRV_DS3232
>>      >         This driver can also be built as a module.  If so, the
>> module
>>      >         will be called rtc-ds3232.
>>      >
>>      > +config RTC_DRV_DS3232_HWMON
>>      > +     bool "HWMON support for Dallas/Maxim DS3232/DS3234"
>>      > +     depends on RTC_DRV_DS3232 && HWMON
>>      > +     depends on !(RTC_DRV_DS3232=y && HWMON=m)
>>      > +     default y
>>      > +     help
>>      > +       Say Y here if you want to expose temperature sensor data on
>>      > +       rtc-ds3232
>>      > +
>>      >  config RTC_DRV_PCF2127
>>      >       tristate "NXP PCF2127"
>>      >       depends on RTC_I2C_AND_SPI
>>      > diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
>>      > index deff431a37c4..f94ff0685942 100644
>>      > --- a/drivers/rtc/rtc-ds3232.c
>>      > +++ b/drivers/rtc/rtc-ds3232.c
>>      > @@ -22,6 +22,8 @@
>>      >  #include <linux/bcd.h>
>>      >  #include <linux/slab.h>
>>      >  #include <linux/regmap.h>
>>      > +#include <linux/hwmon.h>
>>      > +#include <linux/hwmon-sysfs.h>
>>      >
>>      >  #define DS3232_REG_SECONDS      0x00
>>      >  #define DS3232_REG_MINUTES      0x01
>>      > @@ -275,6 +277,86 @@ static int ds3232_update_alarm(struct device
>> *dev, unsigned int enabled)
>>      >       return ret;
>>      >  }
>>      >
>>      >
>> +/*----------------------------------------------------------------------*/
>>      > +
>>      > +#ifdef CONFIG_RTC_DRV_DS3232_HWMON
>>      > +
>>      > +/*
>>      > + * Temperature sensor support for ds3232/ds3234 devices.
>>      > + */
>>      > +
>>      > +#define DS3232_REG_TEMPERATURE       0x11
>>      > +
>>      > +/*
>>      > + * A user-initiated temperature conversion is not started by this
>> function,
>>      > + * so the temperature is updated once every 64 seconds.
>>      > + */
>>      > +static int ds3232_hwmon_read_temp(struct device *dev, s32 *mC)
>>      > +{
>>      > +     struct ds3232 *ds3232 = dev_get_drvdata(dev);
>>      > +     u8 temp_buf[2];
>>      > +     s16 temp;
>>      > +     int ret;
>>      > +
>>      > +     ret = regmap_bulk_read(ds3232->regmap,
>> DS3232_REG_TEMPERATURE, temp_buf,
>>      > +                             sizeof(temp_buf));
>>      > +
>>      > +     if (ret < 0)
>>      > +             return ret;
>>      > +
>>      > +     /*
>>      > +      * Temperature is represented as a 10-bit code with a
>> resolution of
>>      > +      * 0.25 degree celsius and encoded in two's complement
>> format.
>>      > +      */
>>      > +     temp = (temp_buf[0] << 8) | temp_buf[1];
>>      > +     temp >>= 6;
>>      > +     *mC = temp * 250;
>>      > +
>>      > +     return 0;
>>      > +}
>>      > +
>>      > +static ssize_t ds3232_hwmon_show_temp(struct device *dev,
>>      > +                             struct device_attribute *attr, char
>> *buf)
>>      > +{
>>      > +     int ret;
>>      > +     s32 temp;
>>      > +
>>      > +     ret = ds3232_hwmon_read_temp(dev, &temp);
>>      > +     if (ret < 0)
>>      > +             return ret;
>>      > +
>>      > +     return sprintf(buf, "%d\n", temp);
>>      > +}
>>      > +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
>> ds3232_hwmon_show_temp,
>>      > +                     NULL, 0);
>>      > +
>>      > +static struct attribute *ds3232_hwmon_attrs[] = {
>>      > +     &sensor_dev_attr_temp1_input.dev_attr.attr,
>>      > +     NULL,
>>      > +};
>>      > +ATTRIBUTE_GROUPS(ds3232_hwmon);
>>      > +
>>      > +static void ds3232_hwmon_register(struct device *dev, const char
>> *name)
>>      > +{
>>      > +     struct ds3232 *ds3232 = dev_get_drvdata(dev);
>>      > +     struct device *hwmon_dev;
>>      > +
>>      > +     hwmon_dev = devm_hwmon_device_register_with_groups(dev, name,
>> ds3232,
>>      > +
>> ds3232_hwmon_groups);
>>
>>     Any reason for not using devm_hwmon_device_register_with_info() ?
>>
>>
>>
>> Just to keep uniformity,  because other rtc drivers with hwmon
>> (rtc-ds1307, rtc-rv3029c2) use
>> devm_hwmon_device_register_with_groups().
>>
>
> Hmm. Odd reason. With this line of argument, a new API should never be used
> because "everyone else uses the old API".

Well, I had task to add temperature feature to my device, and the
cheapest (easiest)
way was to make it as in same device drivers. I did it and decided to share.

> How about converting the other drivers to  use new API instead ?

Ok, at first I'll try to use new hwmon API for current driver (rtc-ds3232) .


> The idea behind the new API was to simplify drivers and make them
> independent
> of the sysfs ABI. That doesn't mean that drivers _have_ to use that API,
> though,
> so feel free to stick with the above.
>
> Guenter
>
>
>>
>>     Guenter
>>
>>      > +     if (IS_ERR(hwmon_dev)) {
>>      > +             dev_warn(dev, "unable to register hwmon device
>> %ld\n",
>>      > +                      PTR_ERR(hwmon_dev));
>>      > +     }
>>      > +}
>>      > +
>>      > +#else
>>      > +
>>      > +static void ds3232_hwmon_register(struct ds3232 *ds3232)
>>      > +{
>>      > +}
>>      > +
>>      > +#endif
>>      > +
>>      >  static int ds3232_alarm_irq_enable(struct device *dev, unsigned
>> int enabled)
>>      >  {
>>      >       struct ds3232 *ds3232 = dev_get_drvdata(dev);
>>      > @@ -366,6 +448,8 @@ static int ds3232_probe(struct device *dev,
>> struct regmap *regmap, int irq,
>>      >       if (ds3232->irq > 0)
>>      >               device_init_wakeup(dev, 1);
>>      >
>>      > +     ds3232_hwmon_register(dev, name);
>>      > +
>>      >       ds3232->rtc = devm_rtc_device_register(dev, name,
>> &ds3232_rtc_ops,
>>      >                                               THIS_MODULE);
>>      >       if (IS_ERR(ds3232->rtc))
>>
>>
>>
>>
>> --
>> Kirill Esipov
>
>



-- 
Kirill Esipov

Re: [PATCH] rtc: ds3232: add temperature support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]

Hi Kirill,

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.12-rc6 next-20170621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kirill-Esipov/rtc-ds3232-add-temperature-support/20170622-065247
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: x86_64-randconfig-x008-201725 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/rtc/rtc-ds3232.c: In function 'ds3232_probe':
>> drivers/rtc/rtc-ds3232.c:451:24: error: passing argument 1 of 'ds3232_hwmon_register' from incompatible pointer type [-Werror=incompatible-pointer-types]
     ds3232_hwmon_register(dev, name);
                           ^~~
   drivers/rtc/rtc-ds3232.c:354:13: note: expected 'struct ds3232 *' but argument is of type 'struct device *'
    static void ds3232_hwmon_register(struct ds3232 *ds3232)
                ^~~~~~~~~~~~~~~~~~~~~
>> drivers/rtc/rtc-ds3232.c:451:2: error: too many arguments to function 'ds3232_hwmon_register'
     ds3232_hwmon_register(dev, name);
     ^~~~~~~~~~~~~~~~~~~~~
   drivers/rtc/rtc-ds3232.c:354:13: note: declared here
    static void ds3232_hwmon_register(struct ds3232 *ds3232)
                ^~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/ds3232_hwmon_register +451 drivers/rtc/rtc-ds3232.c

   445		if (ret)
   446			return ret;
   447	
   448		if (ds3232->irq > 0)
   449			device_init_wakeup(dev, 1);
   450	
 > 451		ds3232_hwmon_register(dev, name);
   452	
   453		ds3232->rtc = devm_rtc_device_register(dev, name, &ds3232_rtc_ops,
   454							THIS_MODULE);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26277 bytes --]

Re: [PATCH] rtc: ds3232: add temperature support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]

Hi Kirill,

[auto build test WARNING on abelloni/rtc-next]
[also build test WARNING on v4.12-rc6 next-20170621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kirill-Esipov/rtc-ds3232-add-temperature-support/20170622-065247
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: x86_64-randconfig-h0-06220808 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/rtc/rtc-ds3232.c: In function 'ds3232_probe':
>> drivers/rtc/rtc-ds3232.c:451:24: warning: passing argument 1 of 'ds3232_hwmon_register' from incompatible pointer type
     ds3232_hwmon_register(dev, name);
                           ^
   drivers/rtc/rtc-ds3232.c:354:13: note: expected 'struct ds3232 *' but argument is of type 'struct device *'
    static void ds3232_hwmon_register(struct ds3232 *ds3232)
                ^
   drivers/rtc/rtc-ds3232.c:451:2: error: too many arguments to function 'ds3232_hwmon_register'
     ds3232_hwmon_register(dev, name);
     ^
   drivers/rtc/rtc-ds3232.c:354:13: note: declared here
    static void ds3232_hwmon_register(struct ds3232 *ds3232)
                ^

vim +/ds3232_hwmon_register +451 drivers/rtc/rtc-ds3232.c

   435		ds3232 = devm_kzalloc(dev, sizeof(*ds3232), GFP_KERNEL);
   436		if (!ds3232)
   437			return -ENOMEM;
   438	
   439		ds3232->regmap = regmap;
   440		ds3232->irq = irq;
   441		ds3232->dev = dev;
   442		dev_set_drvdata(dev, ds3232);
   443	
   444		ret = ds3232_check_rtc_status(dev);
   445		if (ret)
   446			return ret;
   447	
   448		if (ds3232->irq > 0)
   449			device_init_wakeup(dev, 1);
   450	
 > 451		ds3232_hwmon_register(dev, name);
   452	
   453		ds3232->rtc = devm_rtc_device_register(dev, name, &ds3232_rtc_ops,
   454							THIS_MODULE);
   455		if (IS_ERR(ds3232->rtc))
   456			return PTR_ERR(ds3232->rtc);
   457	
   458		if (ds3232->irq > 0) {
   459			ret = devm_request_threaded_irq(dev, ds3232->irq, NULL,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 20328 bytes --]