[PATCH linux v1 0/3] Export 1-wire thermal sensors as hwmon device

[PATCH linux v1 0/3] Export 1-wire thermal sensors as hwmon device

[Jaghathiswari Rankappagounder Natarajan]

Our board has 4 DS18B20 1-wire temperature sensors. Each 1-wire bus and the
sensor under it is already configured against the Linux 1-wire driver
(called w1). They have a sysfs file(e.g.
/sys/bus/w1/devices/w1_bus_master1/28-000007704f4c/w1_slave) which, when read,
runs code to read the temperature. We'd like the temperatures to show up in
hwmon, so that the BMC IPMI sensor plumbing can forward those to host.

This patchset is based on linux mainline version v4.10.

Tested:
Yes; On a board with 4 DS18B20 1-wire temperature sensors:
root@zaius:/sys/class/hwmon# ls
hwmon0  hwmon1  hwmon2  hwmon3  hwmon4  hwmon5
root@zaius:/sys/class/hwmon# cd hwmon0
root@zaius:/sys/class/hwmon/hwmon0# ls
device       name         subsystem    temp1_input  uevent
root@zaius:/sys/class/hwmon/hwmon0# cat temp1_input
24500
root@zaius:/sys/class/hwmon/hwmon0# cd ..
root@zaius:/sys/class/hwmon# cd hwmon1
root@zaius:/sys/class/hwmon/hwmon1# cat temp1_input
26562
root@zaius:/sys/class/hwmon/hwmon1# cd ..
root@zaius:/sys/class/hwmon# cd hwmon2
root@zaius:/sys/class/hwmon/hwmon2# cat temp1_input
27250
root@zaius:/sys/class/hwmon/hwmon2# cd ..
root@zaius:/sys/class/hwmon# cd hwmon3
root@zaius:/sys/class/hwmon/hwmon3# cat temp1_input
22250
root@zaius:/sys/class/hwmon/hwmon3#

Jaghathiswari Rankappagounder Natarajan (3):
  drivers: w1: add hwmon support structures
  drivers: w1: refactor w1_slave_show to make the temp reading
    functionality separate
  drivers: w1: add hwmon temp support for w1_therm

 drivers/w1/slaves/w1_therm.c | 124 ++++++++++++++++++++++++++++++++-----------
 drivers/w1/w1.c              |  20 ++++++-
 drivers/w1/w1.h              |   2 +
 drivers/w1/w1_family.h       |   2 +
 4 files changed, 117 insertions(+), 31 deletions(-)

--
2.13.1.508.gb3defc5cc-goog

[PATCH linux v1 1/3] drivers: w1: add hwmon support structures

[Jaghathiswari Rankappagounder Natarajan]

This patch has changes to w1.h/w1.c/w1_family.h generic files to
add (optional) hwmon support structures

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
Change-Id: I17f7c670e5cd7a3826add720ffa928c3e1aa0abc
---
 drivers/w1/w1.c        | 20 +++++++++++++++++++-
 drivers/w1/w1.h        |  2 ++
 drivers/w1/w1_family.h |  2 ++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index e213c678bbfe..e3efabccd284 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -32,6 +32,7 @@
 #include <linux/sched.h>
 #include <linux/kthread.h>
 #include <linux/freezer.h>
+#include <linux/hwmon.h>

 #include <linux/atomic.h>

@@ -659,13 +660,30 @@ static int w1_family_notify(unsigned long action, struct w1_slave *sl)
 				return err;
 			}
 		}
-
+#ifdef CONFIG_HWMON
+		if (fops->hwmon_groups && sl->hwmon) {
+			struct device *hwmon
+				= hwmon_device_register_with_groups(&sl->dev,
+						"w1_slave_temp", sl,
+						fops->hwmon_groups);
+			if (IS_ERR(hwmon)) {
+				dev_warn(&sl->dev,
+					 "could not create hwmon device\n");
+			} else {
+				sl->hwmon = hwmon;
+			}
+		}
+#endif
 		break;
 	case BUS_NOTIFY_DEL_DEVICE:
 		if (fops->remove_slave)
 			sl->family->fops->remove_slave(sl);
 		if (fops->groups)
 			sysfs_remove_groups(&sl->dev.kobj, fops->groups);
+#ifdef CONFIG_HWMON
+		if (fops->hwmon_groups && sl->hwmon)
+			hwmon_device_unregister(sl->hwmon);
+#endif
 		break;
 	}
 	return 0;
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index 129895f562b0..74ef1a543856 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -83,6 +83,7 @@ struct w1_reg_num
  * @family: module for device family type
  * @family_data: pointer for use by the family module
  * @dev: kernel device identifier
+ * @hwmon: pointer to hwmon device
  *
  */
 struct w1_slave
@@ -99,6 +100,7 @@ struct w1_slave
 	struct w1_family	*family;
 	void			*family_data;
 	struct device		dev;
+	struct device		*hwmon;
 };

 typedef void (*w1_slave_found_callback)(struct w1_master *, u64);
diff --git a/drivers/w1/w1_family.h b/drivers/w1/w1_family.h
index 10a7a0767187..fc582df5c28e 100644
--- a/drivers/w1/w1_family.h
+++ b/drivers/w1/w1_family.h
@@ -55,12 +55,14 @@ struct w1_slave;
  * @add_slave: add_slave
  * @remove_slave: remove_slave
  * @groups: sysfs group
+ * @hwmon_groups: hwmon sysfs group
  */
 struct w1_family_ops
 {
 	int  (* add_slave)(struct w1_slave *);
 	void (* remove_slave)(struct w1_slave *);
 	const struct attribute_group **groups;
+	const struct attribute_group **hwmon_groups;
 };

 /**
--
2.13.1.508.gb3defc5cc-goog

[PATCH linux v1 2/3] drivers: w1: refactor w1_slave_show to make the temp reading functionality separate

[Jaghathiswari Rankappagounder Natarajan]

Inside the w1_slave_show function refactor the code to read the temp into a
separate function.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
Change-Id: I35741f1345cd2bcb0754ca25c0258843af7ba579
---
 drivers/w1/slaves/w1_therm.c | 82 +++++++++++++++++++++++++++-----------------
 1 file changed, 51 insertions(+), 31 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 82611f197b0a..88f4cc8c3908 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -64,6 +64,12 @@ struct w1_therm_family_data {
 	atomic_t refcnt;
 };

+struct therm_info {
+	u8 rom[9];
+	u8 crc;
+	u8 verdict;
+};
+
 /* return the address of the refcnt in the family data */
 #define THERM_REFCNT(family_data) \
 	(&((struct w1_therm_family_data *)family_data)->refcnt)
@@ -427,33 +433,23 @@ static ssize_t w1_slave_store(struct device *device,
 	return ret ? : size;
 }

-static ssize_t w1_slave_show(struct device *device,
-	struct device_attribute *attr, char *buf)
+static ssize_t read_therm(struct device *device,
+			  struct w1_slave *sl, struct therm_info *info)
 {
-	struct w1_slave *sl = dev_to_w1_slave(device);
 	struct w1_master *dev = sl->master;
-	u8 rom[9], crc, verdict, external_power;
-	int i, ret, max_trying = 10;
-	ssize_t c = PAGE_SIZE;
-	u8 *family_data = sl->family_data;
+	u8 external_power;
+	int ret, max_trying = 10;

 	ret = mutex_lock_interruptible(&dev->bus_mutex);
 	if (ret != 0)
 		goto post_unlock;

-	if (!sl->family_data) {
-		ret = -ENODEV;
-		goto pre_unlock;
-	}
-
-	/* prevent the slave from going away in sleep */
-	atomic_inc(THERM_REFCNT(family_data));
-	memset(rom, 0, sizeof(rom));
+	memset(info->rom, 0, sizeof(info->rom));

 	while (max_trying--) {

-		verdict = 0;
-		crc = 0;
+		info->verdict = 0;
+		info->crc = 0;

 		if (!w1_reset_select_slave(sl)) {
 			int count = 0;
@@ -496,30 +492,57 @@ static ssize_t w1_slave_show(struct device *device,
 			if (!w1_reset_select_slave(sl)) {

 				w1_write_8(dev, W1_READ_SCRATCHPAD);
-				count = w1_read_block(dev, rom, 9);
+				count = w1_read_block(dev, info->rom, 9);
 				if (count != 9) {
 					dev_warn(device, "w1_read_block() "
 						"returned %u instead of 9.\n",
 						count);
 				}

-				crc = w1_calc_crc8(rom, 8);
+				info->crc = w1_calc_crc8(info->rom, 8);

-				if (rom[8] == crc)
-					verdict = 1;
+				if (info->rom[8] == info->crc)
+					info->verdict = 1;
 			}
 		}

-		if (verdict)
+		if (info->verdict)
 			break;
 	}

+pre_unlock:
+	mutex_unlock(&dev->bus_mutex);
+
+post_unlock:
+	return ret;
+}
+
+static ssize_t w1_slave_show(struct device *device,
+			     struct device_attribute *attr, char *buf)
+{
+	struct w1_slave *sl = dev_to_w1_slave(device);
+	u8 *family_data = sl->family_data;
+	struct therm_info info;
+	int ret;
+	ssize_t c = PAGE_SIZE;
+	int i;
+
+	if (!sl->family_data)
+		return -ENODEV;
+
+	/* prevent the slave from going away in sleep */
+	atomic_inc(THERM_REFCNT(family_data));
+
+	ret = read_therm(device, sl, &info);
+	if (ret)
+		goto dec_refcnt;
+
 	for (i = 0; i < 9; ++i)
-		c -= snprintf(buf + PAGE_SIZE - c, c, "%02x ", rom[i]);
+		c -= snprintf(buf + PAGE_SIZE - c, c, "%02x ", info.rom[i]);
 	c -= snprintf(buf + PAGE_SIZE - c, c, ": crc=%02x %s\n",
-		      crc, (verdict) ? "YES" : "NO");
-	if (verdict)
-		memcpy(family_data, rom, sizeof(rom));
+		      info.crc, (info.verdict) ? "YES" : "NO");
+	if (info.verdict)
+		memcpy(family_data, info.rom, sizeof(info.rom));
 	else
 		dev_warn(device, "Read failed CRC check\n");

@@ -528,13 +551,10 @@ static ssize_t w1_slave_show(struct device *device,
 			      ((u8 *)family_data)[i]);

 	c -= snprintf(buf + PAGE_SIZE - c, c, "t=%d\n",
-		w1_convert_temp(rom, sl->family->fid));
+			w1_convert_temp(info.rom, sl->family->fid));
 	ret = PAGE_SIZE - c;

-pre_unlock:
-	mutex_unlock(&dev->bus_mutex);
-
-post_unlock:
+dec_refcnt:
 	atomic_dec(THERM_REFCNT(family_data));
 	return ret;
 }
--
2.13.1.508.gb3defc5cc-goog

[PATCH linux v1 3/3] drivers: w1: add hwmon temp support for w1_therm

[Jaghathiswari Rankappagounder Natarajan]

This change adds hwmon temp support for w1_therm.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
Change-Id: I66c0837d63b5a5839227319608eb8061b917f5fa
---
 drivers/w1/slaves/w1_therm.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 88f4cc8c3908..dba2d21fa731 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -105,8 +105,12 @@ static ssize_t w1_slave_store(struct device *device,
 static ssize_t w1_seq_show(struct device *device,
 	struct device_attribute *attr, char *buf);

+static ssize_t temp1_input_show(struct device *device,
+	struct device_attribute *attr, char *buf);
+
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
+static DEVICE_ATTR_RO(temp1_input);

 static struct attribute *w1_therm_attrs[] = {
 	&dev_attr_w1_slave.attr,
@@ -118,19 +122,27 @@ static struct attribute *w1_ds28ea00_attrs[] = {
 	&dev_attr_w1_seq.attr,
 	NULL,
 };
+static struct attribute *w1_therm_hwmon_attrs[] = {
+	&dev_attr_temp1_input.attr,
+	NULL,
+};
+
 ATTRIBUTE_GROUPS(w1_therm);
 ATTRIBUTE_GROUPS(w1_ds28ea00);
+ATTRIBUTE_GROUPS(w1_therm_hwmon);

 static struct w1_family_ops w1_therm_fops = {
 	.add_slave	= w1_therm_add_slave,
 	.remove_slave	= w1_therm_remove_slave,
 	.groups		= w1_therm_groups,
+	.hwmon_groups   = w1_therm_hwmon_groups,
 };

 static struct w1_family_ops w1_ds28ea00_fops = {
 	.add_slave	= w1_therm_add_slave,
 	.remove_slave	= w1_therm_remove_slave,
 	.groups		= w1_ds28ea00_groups,
+	.hwmon_groups   = w1_therm_hwmon_groups,
 };

 static struct w1_family w1_therm_family_DS18S20 = {
@@ -559,6 +571,38 @@ static ssize_t w1_slave_show(struct device *device,
 	return ret;
 }

+static ssize_t temp1_input_show(struct device *device,
+				struct device_attribute *attr, char *buf)
+{
+	struct w1_slave *sl = dev_get_drvdata(device);
+	u8 *family_data = sl->family_data;
+	struct therm_info info;
+	int ret;
+
+	if (!sl->family_data)
+		return -ENODEV;
+
+	/* prevent the slave from going away in sleep */
+	atomic_inc(THERM_REFCNT(family_data));
+
+	ret = read_therm(device, sl, &info);
+	if (ret)
+		goto dec_refcnt;
+
+	if (!info.verdict) {
+		dev_warn(device, "Read failed CRC check\n");
+		ret = -EIO;
+		goto dec_refcnt;
+	}
+
+	ret = sprintf(buf, "%d\n",
+			w1_convert_temp(info.rom, sl->family->fid));
+
+dec_refcnt:
+	atomic_dec(THERM_REFCNT(family_data));
+	return ret;
+}
+
 #define W1_42_CHAIN	0x99
 #define W1_42_CHAIN_OFF	0x3C
 #define W1_42_CHAIN_OFF_INV	0xC3
--
2.13.1.508.gb3defc5cc-goog

Re: [PATCH linux v1 1/3] drivers: w1: add hwmon support structures

[Guenter Roeck]

On Wed, Jun 14, 2017 at 02:59:42PM -0700, Jaghathiswari Rankappagounder Natarajan wrote:
> This patch has changes to w1.h/w1.c/w1_family.h generic files to
> add (optional) hwmon support structures
> 
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
> Change-Id: I17f7c670e5cd7a3826add720ffa928c3e1aa0abc
> ---
>  drivers/w1/w1.c        | 20 +++++++++++++++++++-
>  drivers/w1/w1.h        |  2 ++
>  drivers/w1/w1_family.h |  2 ++
>  3 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index e213c678bbfe..e3efabccd284 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -32,6 +32,7 @@
>  #include <linux/sched.h>
>  #include <linux/kthread.h>
>  #include <linux/freezer.h>
> +#include <linux/hwmon.h>
> 
>  #include <linux/atomic.h>
> 
> @@ -659,13 +660,30 @@ static int w1_family_notify(unsigned long action, struct w1_slave *sl)
>  				return err;
>  			}
>  		}
> -
> +#ifdef CONFIG_HWMON
> +		if (fops->hwmon_groups && sl->hwmon) {
> +			struct device *hwmon
> +				= hwmon_device_register_with_groups(&sl->dev,

You should really use hwmon_device_register_with_info().

Also, would devm_hwmon_device_register_with_info() work ?

> +						"w1_slave_temp", sl,
> +						fops->hwmon_groups);
> +			if (IS_ERR(hwmon)) {
> +				dev_warn(&sl->dev,
> +					 "could not create hwmon device\n");
> +			} else {
> +				sl->hwmon = hwmon;
> +			}
> +		}
> +#endif
>  		break;
>  	case BUS_NOTIFY_DEL_DEVICE:
>  		if (fops->remove_slave)
>  			sl->family->fops->remove_slave(sl);
>  		if (fops->groups)
>  			sysfs_remove_groups(&sl->dev.kobj, fops->groups);
> +#ifdef CONFIG_HWMON
> +		if (fops->hwmon_groups && sl->hwmon)
> +			hwmon_device_unregister(sl->hwmon);
> +#endif
>  		break;
>  	}
>  	return 0;
> diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
> index 129895f562b0..74ef1a543856 100644
> --- a/drivers/w1/w1.h
> +++ b/drivers/w1/w1.h
> @@ -83,6 +83,7 @@ struct w1_reg_num
>   * @family: module for device family type
>   * @family_data: pointer for use by the family module
>   * @dev: kernel device identifier
> + * @hwmon: pointer to hwmon device
>   *
>   */
>  struct w1_slave
> @@ -99,6 +100,7 @@ struct w1_slave
>  	struct w1_family	*family;
>  	void			*family_data;
>  	struct device		dev;
> +	struct device		*hwmon;
>  };
> 
>  typedef void (*w1_slave_found_callback)(struct w1_master *, u64);
> diff --git a/drivers/w1/w1_family.h b/drivers/w1/w1_family.h
> index 10a7a0767187..fc582df5c28e 100644
> --- a/drivers/w1/w1_family.h
> +++ b/drivers/w1/w1_family.h
> @@ -55,12 +55,14 @@ struct w1_slave;
>   * @add_slave: add_slave
>   * @remove_slave: remove_slave
>   * @groups: sysfs group
> + * @hwmon_groups: hwmon sysfs group
>   */
>  struct w1_family_ops
>  {
>  	int  (* add_slave)(struct w1_slave *);
>  	void (* remove_slave)(struct w1_slave *);
>  	const struct attribute_group **groups;
> +	const struct attribute_group **hwmon_groups;
>  };
> 
>  /**
> --
> 2.13.1.508.gb3defc5cc-goog
> 

Re: [PATCH linux v1 0/3] Export 1-wire thermal sensors as hwmon device

[Evgeniy Polyakov]

Hi

15.06.2017, 00:59, "Jaghathiswari Rankappagounder Natarajan" <jaghu@google.com>:
> Our board has 4 DS18B20 1-wire temperature sensors. Each 1-wire bus and the
> sensor under it is already configured against the Linux 1-wire driver
> (called w1). They have a sysfs file(e.g.
> /sys/bus/w1/devices/w1_bus_master1/28-000007704f4c/w1_slave) which, when read,
> runs code to read the temperature. We'd like the temperatures to show up in
> hwmon, so that the BMC IPMI sensor plumbing can forward those to host.
>
> This patchset is based on linux mainline version v4.10.

Looks good to me, thank you.

Acked-by: Evgeniy Polyakov <zbr@ioremap.net>