Message ID | 1450310296-12153-1-git-send-email-akinobu.mita@gmail.com |
---|---|
State | Superseded |
Headers | show |
Hi, Sorry for the very late review. This seems very valuable. However, instead of creating a new sysfs interface, I think this should be using the hwmon framework. I've just check and it doesn't seem to be an issue to register an hwmon device from an other framework, some ethernet and graphic drivers are doing it. Can you try to do that? On 17/12/2015 at 08:58:16 +0900, Akinobu Mita wrote : > DS3231 has the temperature registers with a resolution of 0.25 > degree celsius. This enables to get the value through sysfs file. > > # cat /sys/class/i2c-adapter/i2c-9/9-0068/temperature > 22.50 > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > Cc: Alessandro Zummo <a.zummo@towertech.it> > Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com> > Cc: rtc-linux@googlegroups.com > --- > drivers/rtc/rtc-ds1307.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 117 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c > index aa705bb..c0cd447 100644 > --- a/drivers/rtc/rtc-ds1307.c > +++ b/drivers/rtc/rtc-ds1307.c > @@ -19,6 +19,7 @@ > #include <linux/rtc.h> > #include <linux/slab.h> > #include <linux/string.h> > +#include <linux/delay.h> > > /* > * We can't determine type by probing, but if we expect pre-Linux code > @@ -75,6 +76,7 @@ enum ds_type { > # define DS1337_BIT_nEOSC 0x80 > # define DS1339_BIT_BBSQI 0x20 > # define DS3231_BIT_BBSQW 0x40 /* same as BBSQI */ > +# define DS3231_BIT_CONV 0x20 > # define DS1337_BIT_RS2 0x10 > # define DS1337_BIT_RS1 0x08 > # define DS1337_BIT_INTCN 0x04 > @@ -89,6 +91,7 @@ enum ds_type { > # define DS1340_BIT_OSF 0x80 > #define DS1337_REG_STATUS 0x0f > # define DS1337_BIT_OSF 0x80 > +# define DS3231_BIT_BSY 0x04 > # define DS1337_BIT_A2I 0x02 > # define DS1337_BIT_A1I 0x01 > #define DS1339_REG_ALARM1_SECS 0x07 > @@ -110,8 +113,10 @@ struct ds1307 { > struct bin_attribute *nvram; > enum ds_type type; > unsigned long flags; > -#define HAS_NVRAM 0 /* bit 0 == sysfs file active */ > -#define HAS_ALARM 1 /* bit 1 == irq claimed */ > +#define HAS_NVRAM 0 /* bit 0 == nvram sysfs file active */ > +#define HAS_ALARM 1 /* bit 1 == irq claimed */ > +#define HAS_TEMP 2 /* bit 2 == temperature sysfs file active */ > + > struct i2c_client *client; > struct rtc_device *rtc; > s32 (*read_block_data)(const struct i2c_client *client, u8 command, > @@ -851,6 +856,112 @@ out: > return; > } > > +/*----------------------------------------------------------------------*/ > + > +/* > + * Temperature sensor support for ds3231 devices. > + */ > + > +#define DS3231_REG_TEMPERATURE 0x11 > + > +static ssize_t ds3231_sysfs_show_temperature(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct ds1307 *ds1307 = dev_get_drvdata(dev); > + struct i2c_client *client = ds1307->client; > + u8 temp_buf[2]; > + s16 temp; > + bool negative; > + int control; > + int status; > + s32 ret; > + unsigned long timeout; > + > + status = i2c_smbus_read_byte_data(client, DS1337_REG_STATUS); > + if (status < 0) > + return status; > + > + /* > + * Start user-initiated temperature conversion > + */ > + if (!(status & DS3231_BIT_BSY)) { > + struct mutex *lock = &ds1307->rtc->ops_lock; > + > + mutex_lock(lock); > + > + control = i2c_smbus_read_byte_data(client, DS1337_REG_CONTROL); > + if (control < 0) { > + mutex_unlock(lock); > + return control; > + } > + ret = i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL, > + control | DS3231_BIT_CONV); > + if (ret) > + return ret; > + > + mutex_unlock(lock); > + > + /* > + * A user-initiated temperature conversion does not affect > + * the BSY bit for approximately 2ms. > + */ > + usleep_range(2000, 3000); > + } > + > + /* > + * Wait until the conversion is finished > + */ > + timeout = jiffies + HZ; > + > + do { > + control = i2c_smbus_read_byte_data(client, DS1337_REG_CONTROL); > + if (control < 0) > + return control; > + if (!(control & DS3231_BIT_CONV)) > + break; > + if (time_after(jiffies, timeout)) > + return -EIO; > + usleep_range(2000, 3000); > + } while (1); > + > + ret = ds1307->read_block_data(ds1307->client, DS3231_REG_TEMPERATURE, > + sizeof(temp_buf), temp_buf); > + if (ret < 0) > + return ret; > + if (ret != sizeof(temp_buf)) > + return -EIO; > + > + /* > + * 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; > + negative = (temp < 0) ? true : false; > + temp = abs(temp); > + > + return sprintf(buf, "%s%d.%d\n", negative ? "-" : "", > + temp >> 2, 25 * (temp & 0x3)); > +} > +static DEVICE_ATTR(temperature, S_IRUGO, ds3231_sysfs_show_temperature, NULL); > + > +static int ds1307_temperature_register(struct ds1307 *ds1307) > +{ > + if (ds1307->type != ds_3231) > + return 0; > + > + if (!device_create_file(&ds1307->client->dev, &dev_attr_temperature)) > + set_bit(HAS_TEMP, &ds1307->flags); > + > + return 0; > +} > + > +static void ds1307_temperature_unregister(struct ds1307 *ds1307) > +{ > + if (test_and_clear_bit(HAS_TEMP, &ds1307->flags)) > + device_remove_file(&ds1307->client->dev, &dev_attr_temperature); > +} > + > static int ds1307_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -1191,6 +1302,8 @@ read_rtc: > } > } > > + ds1307_temperature_register(ds1307); > + > return 0; > > exit: > @@ -1201,6 +1314,8 @@ static int ds1307_remove(struct i2c_client *client) > { > struct ds1307 *ds1307 = i2c_get_clientdata(client); > > + ds1307_temperature_unregister(ds1307); > + > if (test_and_clear_bit(HAS_NVRAM, &ds1307->flags)) > sysfs_remove_bin_file(&client->dev.kobj, ds1307->nvram); > > -- > 1.9.1 >
2016-01-21 7:51 GMT+09:00 Alexandre Belloni <alexandre.belloni@free-electrons.com>: > This seems very valuable. However, instead of creating a new sysfs > interface, I think this should be using the hwmon framework. I've just > check and it doesn't seem to be an issue to register an hwmon device > from an other framework, some ethernet and graphic drivers are doing it. > > Can you try to do that? Sounds good. It can be done with small changes from this patch.
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c index aa705bb..c0cd447 100644 --- a/drivers/rtc/rtc-ds1307.c +++ b/drivers/rtc/rtc-ds1307.c @@ -19,6 +19,7 @@ #include <linux/rtc.h> #include <linux/slab.h> #include <linux/string.h> +#include <linux/delay.h> /* * We can't determine type by probing, but if we expect pre-Linux code @@ -75,6 +76,7 @@ enum ds_type { # define DS1337_BIT_nEOSC 0x80 # define DS1339_BIT_BBSQI 0x20 # define DS3231_BIT_BBSQW 0x40 /* same as BBSQI */ +# define DS3231_BIT_CONV 0x20 # define DS1337_BIT_RS2 0x10 # define DS1337_BIT_RS1 0x08 # define DS1337_BIT_INTCN 0x04 @@ -89,6 +91,7 @@ enum ds_type { # define DS1340_BIT_OSF 0x80 #define DS1337_REG_STATUS 0x0f # define DS1337_BIT_OSF 0x80 +# define DS3231_BIT_BSY 0x04 # define DS1337_BIT_A2I 0x02 # define DS1337_BIT_A1I 0x01 #define DS1339_REG_ALARM1_SECS 0x07 @@ -110,8 +113,10 @@ struct ds1307 { struct bin_attribute *nvram; enum ds_type type; unsigned long flags; -#define HAS_NVRAM 0 /* bit 0 == sysfs file active */ -#define HAS_ALARM 1 /* bit 1 == irq claimed */ +#define HAS_NVRAM 0 /* bit 0 == nvram sysfs file active */ +#define HAS_ALARM 1 /* bit 1 == irq claimed */ +#define HAS_TEMP 2 /* bit 2 == temperature sysfs file active */ + struct i2c_client *client; struct rtc_device *rtc; s32 (*read_block_data)(const struct i2c_client *client, u8 command, @@ -851,6 +856,112 @@ out: return; } +/*----------------------------------------------------------------------*/ + +/* + * Temperature sensor support for ds3231 devices. + */ + +#define DS3231_REG_TEMPERATURE 0x11 + +static ssize_t ds3231_sysfs_show_temperature(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct ds1307 *ds1307 = dev_get_drvdata(dev); + struct i2c_client *client = ds1307->client; + u8 temp_buf[2]; + s16 temp; + bool negative; + int control; + int status; + s32 ret; + unsigned long timeout; + + status = i2c_smbus_read_byte_data(client, DS1337_REG_STATUS); + if (status < 0) + return status; + + /* + * Start user-initiated temperature conversion + */ + if (!(status & DS3231_BIT_BSY)) { + struct mutex *lock = &ds1307->rtc->ops_lock; + + mutex_lock(lock); + + control = i2c_smbus_read_byte_data(client, DS1337_REG_CONTROL); + if (control < 0) { + mutex_unlock(lock); + return control; + } + ret = i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL, + control | DS3231_BIT_CONV); + if (ret) + return ret; + + mutex_unlock(lock); + + /* + * A user-initiated temperature conversion does not affect + * the BSY bit for approximately 2ms. + */ + usleep_range(2000, 3000); + } + + /* + * Wait until the conversion is finished + */ + timeout = jiffies + HZ; + + do { + control = i2c_smbus_read_byte_data(client, DS1337_REG_CONTROL); + if (control < 0) + return control; + if (!(control & DS3231_BIT_CONV)) + break; + if (time_after(jiffies, timeout)) + return -EIO; + usleep_range(2000, 3000); + } while (1); + + ret = ds1307->read_block_data(ds1307->client, DS3231_REG_TEMPERATURE, + sizeof(temp_buf), temp_buf); + if (ret < 0) + return ret; + if (ret != sizeof(temp_buf)) + return -EIO; + + /* + * 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; + negative = (temp < 0) ? true : false; + temp = abs(temp); + + return sprintf(buf, "%s%d.%d\n", negative ? "-" : "", + temp >> 2, 25 * (temp & 0x3)); +} +static DEVICE_ATTR(temperature, S_IRUGO, ds3231_sysfs_show_temperature, NULL); + +static int ds1307_temperature_register(struct ds1307 *ds1307) +{ + if (ds1307->type != ds_3231) + return 0; + + if (!device_create_file(&ds1307->client->dev, &dev_attr_temperature)) + set_bit(HAS_TEMP, &ds1307->flags); + + return 0; +} + +static void ds1307_temperature_unregister(struct ds1307 *ds1307) +{ + if (test_and_clear_bit(HAS_TEMP, &ds1307->flags)) + device_remove_file(&ds1307->client->dev, &dev_attr_temperature); +} + static int ds1307_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -1191,6 +1302,8 @@ read_rtc: } } + ds1307_temperature_register(ds1307); + return 0; exit: @@ -1201,6 +1314,8 @@ static int ds1307_remove(struct i2c_client *client) { struct ds1307 *ds1307 = i2c_get_clientdata(client); + ds1307_temperature_unregister(ds1307); + if (test_and_clear_bit(HAS_NVRAM, &ds1307->flags)) sysfs_remove_bin_file(&client->dev.kobj, ds1307->nvram);
DS3231 has the temperature registers with a resolution of 0.25 degree celsius. This enables to get the value through sysfs file. # cat /sys/class/i2c-adapter/i2c-9/9-0068/temperature 22.50 Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> Cc: Alessandro Zummo <a.zummo@towertech.it> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com> Cc: rtc-linux@googlegroups.com --- drivers/rtc/rtc-ds1307.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 117 insertions(+), 2 deletions(-)