On 06/22/2017 05:07 AM, Kirill Esipov wrote:
2017-06-22 1:24 GMT+03:00 Guenter Roeck <linux@xxxxxxxxxxxx <mailto:linux@xxxxxxxxxxxx>>: 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@xxxxxxxxx <mailto:yesipov@xxxxxxxxx>> > --- > 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