[PATCH 1/4] w1: ds2413: output_write() cosmetic fixes / simplify

[PATCH 1/4] w1: ds2413: output_write() cosmetic fixes / simplify

[Mariusz Bialonczyk]

Make the output_write simpler.
Based on Jean-Francois Dagenais code from:
49695ac46861 ("w1: ds2408: reset on output_write retry with readback")

Signed-off-by: Mariusz Bialonczyk <manio@skyboo.net>
---
 drivers/w1/slaves/w1_ds2413.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2413.c b/drivers/w1/slaves/w1_ds2413.c
index 492e3d010321..cd3763df69ac 100644
--- a/drivers/w1/slaves/w1_ds2413.c
+++ b/drivers/w1/slaves/w1_ds2413.c
@@ -69,6 +69,7 @@ static ssize_t output_write(struct file *filp, struct kobject *kobj,
 	struct w1_slave *sl = kobj_to_w1_slave(kobj);
 	u8 w1_buf[3];
 	unsigned int retries = W1_F3A_RETRIES;
+	ssize_t bytes_written = -EIO;
 
 	if (count != 1 || off != 0)
 		return -EFAULT;
@@ -78,7 +79,7 @@ static ssize_t output_write(struct file *filp, struct kobject *kobj,
 	dev_dbg(&sl->dev, "mutex locked");
 
 	if (w1_reset_select_slave(sl))
-		goto error;
+		goto out;
 
 	/* according to the DS2413 datasheet the most significant 6 bits
 	   should be set to "1"s, so do it now */
@@ -91,18 +92,20 @@ static ssize_t output_write(struct file *filp, struct kobject *kobj,
 		w1_write_block(sl->master, w1_buf, 3);
 
 		if (w1_read_8(sl->master) == W1_F3A_SUCCESS_CONFIRM_BYTE) {
-			mutex_unlock(&sl->master->bus_mutex);
-			dev_dbg(&sl->dev, "mutex unlocked, retries:%d", retries);
-			return 1;
+			bytes_written = 1;
+			goto out;
 		}
 		if (w1_reset_resume_command(sl->master))
-			goto error;
+			goto out; /* unrecoverable error */
+
+		dev_warn(&sl->dev, "PIO_ACCESS_WRITE error, retries left: %d\n", retries);
 	}
 
-error:
+out:
 	mutex_unlock(&sl->master->bus_mutex);
-	dev_dbg(&sl->dev, "mutex unlocked in error, retries:%d", retries);
-	return -EIO;
+	dev_dbg(&sl->dev, "%s, mutex unlocked, retries: %d\n",
+		(bytes_written > 0) ? "succeeded" : "error", retries);
+	return bytes_written;
 }
 
 static BIN_ATTR(output, S_IRUGO | S_IWUSR | S_IWGRP, NULL, output_write, 1);
-- 
2.19.0.rc1

[PATCH 2/4] w1: ds2413: add retry support to state_read()

[Mariusz Bialonczyk]

The state_read() was calling PIO_ACCESS_READ once and bail out if it
failed for this first time.
This commit is improving this to trying more times before it give up,
similarly as the write call is currently doing.

Signed-off-by: Mariusz Bialonczyk <manio@skyboo.net>
---
 drivers/w1/slaves/w1_ds2413.c | 37 +++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2413.c b/drivers/w1/slaves/w1_ds2413.c
index cd3763df69ac..d63778c70568 100644
--- a/drivers/w1/slaves/w1_ds2413.c
+++ b/drivers/w1/slaves/w1_ds2413.c
@@ -30,6 +30,9 @@ static ssize_t state_read(struct file *filp, struct kobject *kobj,
 			  size_t count)
 {
 	struct w1_slave *sl = kobj_to_w1_slave(kobj);
+	unsigned int retries = W1_F3A_RETRIES;
+	ssize_t bytes_read = -EIO;
+
 	dev_dbg(&sl->dev,
 		"Reading %s kobj: %p, off: %0#10x, count: %zu, buff addr: %p",
 		bin_attr->attr.name, kobj, (unsigned int)off, count, buf);
@@ -42,22 +45,30 @@ static ssize_t state_read(struct file *filp, struct kobject *kobj,
 	mutex_lock(&sl->master->bus_mutex);
 	dev_dbg(&sl->dev, "mutex locked");
 
-	if (w1_reset_select_slave(sl)) {
-		mutex_unlock(&sl->master->bus_mutex);
-		return -EIO;
-	}
+	if (w1_reset_select_slave(sl))
+		goto out;
 
-	w1_write_8(sl->master, W1_F3A_FUNC_PIO_ACCESS_READ);
-	*buf = w1_read_8(sl->master);
+	while (retries--) {
+		w1_write_8(sl->master, W1_F3A_FUNC_PIO_ACCESS_READ);
 
-	mutex_unlock(&sl->master->bus_mutex);
-	dev_dbg(&sl->dev, "mutex unlocked");
+		*buf = w1_read_8(sl->master);
+		/* check for correct complement */
+		if ((*buf & 0x0F) == ((~*buf >> 4) & 0x0F)) {
+			bytes_read = 1;
+			goto out;
+		}
 
-	/* check for correct complement */
-	if ((*buf & 0x0F) != ((~*buf >> 4) & 0x0F))
-		return -EIO;
-	else
-		return 1;
+		if (w1_reset_resume_command(sl->master))
+			goto out; /* unrecoverable error */
+
+		dev_warn(&sl->dev, "PIO_ACCESS_READ error, retries left: %d\n", retries);
+	}
+
+out:
+	mutex_unlock(&sl->master->bus_mutex);
+	dev_dbg(&sl->dev, "%s, mutex unlocked, retries: %d\n",
+		(bytes_read > 0) ? "succeeded" : "error", retries);
+	return bytes_read;
 }
 
 static BIN_ATTR_RO(state, 1);
-- 
2.19.0.rc1

[PATCH 3/4] w1: ds2413: when the slave is not responding during read, select it again

[Mariusz Bialonczyk]

The protocol is not allowing to obtain a byte of 0xff for PIO_ACCESS_READ
call. It is very likely that the slave was not addressed properly and
it is just not respoding (leaving the bus in logic high state) during
the read of sampled PIO value.
We cannot just call w1_reset_resume_command() because the problem will
persist, instead try selecting (addressing) the slave again.

Signed-off-by: Mariusz Bialonczyk <manio@skyboo.net>
---
 drivers/w1/slaves/w1_ds2413.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2413.c b/drivers/w1/slaves/w1_ds2413.c
index d63778c70568..1854461d4d56 100644
--- a/drivers/w1/slaves/w1_ds2413.c
+++ b/drivers/w1/slaves/w1_ds2413.c
@@ -24,6 +24,7 @@
 #define W1_F3A_FUNC_PIO_ACCESS_READ        0xF5
 #define W1_F3A_FUNC_PIO_ACCESS_WRITE       0x5A
 #define W1_F3A_SUCCESS_CONFIRM_BYTE        0xAA
+#define W1_F3A_INVALID_PIO_STATE           0xFF
 
 static ssize_t state_read(struct file *filp, struct kobject *kobj,
 			  struct bin_attribute *bin_attr, char *buf, loff_t off,
@@ -45,6 +46,7 @@ static ssize_t state_read(struct file *filp, struct kobject *kobj,
 	mutex_lock(&sl->master->bus_mutex);
 	dev_dbg(&sl->dev, "mutex locked");
 
+next:
 	if (w1_reset_select_slave(sl))
 		goto out;
 
@@ -52,8 +54,13 @@ static ssize_t state_read(struct file *filp, struct kobject *kobj,
 		w1_write_8(sl->master, W1_F3A_FUNC_PIO_ACCESS_READ);
 
 		*buf = w1_read_8(sl->master);
-		/* check for correct complement */
-		if ((*buf & 0x0F) == ((~*buf >> 4) & 0x0F)) {
+		if (*buf == W1_F3A_INVALID_PIO_STATE) {
+			/* slave didn't respond, try to select it again */
+			dev_warn(&sl->dev, "slave device did not respond to PIO_ACCESS_READ, " \
+					    "reselecting, retries left: %d\n", retries);
+			goto next;
+		} else if ((*buf & 0x0F) == ((~*buf >> 4) & 0x0F)) {
+			/* complement is correct */
 			bytes_read = 1;
 			goto out;
 		}
-- 
2.19.0.rc1

[PATCH 3/4 v2] w1: ds2413: when the slave is not responding during read, select it again

[Mariusz Bialonczyk]

The protocol is not allowing to obtain a byte of 0xff for PIO_ACCESS_READ
call. It is very likely that the slave was not addressed properly and
it is just not respoding (leaving the bus in logic high state) during
the read of sampled PIO value.
We cannot just call w1_reset_resume_command() because the problem will
persist, instead try selecting (addressing) the slave again.

Signed-off-by: Mariusz Bialonczyk <manio@skyboo.net>
---
Changes in v2:
    Optimize the if statement so the most common is checked first.

 drivers/w1/slaves/w1_ds2413.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/w1/slaves/w1_ds2413.c b/drivers/w1/slaves/w1_ds2413.c
index d63778c70568..21f08ac8a4e0 100644
--- a/drivers/w1/slaves/w1_ds2413.c
+++ b/drivers/w1/slaves/w1_ds2413.c
@@ -24,6 +24,7 @@
 #define W1_F3A_FUNC_PIO_ACCESS_READ        0xF5
 #define W1_F3A_FUNC_PIO_ACCESS_WRITE       0x5A
 #define W1_F3A_SUCCESS_CONFIRM_BYTE        0xAA
+#define W1_F3A_INVALID_PIO_STATE           0xFF
 
 static ssize_t state_read(struct file *filp, struct kobject *kobj,
 			  struct bin_attribute *bin_attr, char *buf, loff_t off,
@@ -45,6 +46,7 @@ static ssize_t state_read(struct file *filp, struct kobject *kobj,
 	mutex_lock(&sl->master->bus_mutex);
 	dev_dbg(&sl->dev, "mutex locked");
 
+next:
 	if (w1_reset_select_slave(sl))
 		goto out;
 
@@ -52,10 +54,15 @@ static ssize_t state_read(struct file *filp, struct kobject *kobj,
 		w1_write_8(sl->master, W1_F3A_FUNC_PIO_ACCESS_READ);
 
 		*buf = w1_read_8(sl->master);
-		/* check for correct complement */
 		if ((*buf & 0x0F) == ((~*buf >> 4) & 0x0F)) {
+			/* complement is correct */
 			bytes_read = 1;
 			goto out;
+		} else if (*buf == W1_F3A_INVALID_PIO_STATE) {
+			/* slave didn't respond, try to select it again */
+			dev_warn(&sl->dev, "slave device did not respond to PIO_ACCESS_READ, " \
+					    "reselecting, retries left: %d\n", retries);
+			goto next;
 		}
 
 		if (w1_reset_resume_command(sl->master))
-- 
2.19.0.rc1

[PATCH 4/4] w1: ds2805: rename w1_family struct, fixing c-p typo

[Mariusz Bialonczyk]

Signed-off-by: Mariusz Bialonczyk <manio@skyboo.net>
---
 drivers/w1/slaves/w1_ds2805.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2805.c b/drivers/w1/slaves/w1_ds2805.c
index 29348d283a65..ab349604531a 100644
--- a/drivers/w1/slaves/w1_ds2805.c
+++ b/drivers/w1/slaves/w1_ds2805.c
@@ -288,7 +288,7 @@ static struct w1_family_ops w1_f0d_fops = {
 	.remove_slave   = w1_f0d_remove_slave,
 };
 
-static struct w1_family w1_family_2d = {
+static struct w1_family w1_family_0d = {
 	.fid = W1_EEPROM_DS2805,
 	.fops = &w1_f0d_fops,
 };
@@ -296,13 +296,13 @@ static struct w1_family w1_family_2d = {
 static int __init w1_f0d_init(void)
 {
 	pr_info("%s()\n", __func__);
-	return w1_register_family(&w1_family_2d);
+	return w1_register_family(&w1_family_0d);
 }
 
 static void __exit w1_f0d_fini(void)
 {
 	pr_info("%s()\n", __func__);
-	w1_unregister_family(&w1_family_2d);
+	w1_unregister_family(&w1_family_0d);
 }
 
 module_init(w1_f0d_init);
-- 
2.19.0.rc1

Re: [PATCH 4/4] w1: ds2805: rename w1_family struct, fixing c-p typo

[Greg KH]

On Mon, May 20, 2019 at 09:05:58AM +0200, Mariusz Bialonczyk wrote:
> Signed-off-by: Mariusz Bialonczyk <manio@skyboo.net>
> ---
>  drivers/w1/slaves/w1_ds2805.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

I can not take patches without any changelog text, sorry.

greg k-h

[PATCH 4/4 v2] w1: ds2805: rename w1_family struct, fixing c-p typo

[Mariusz Bialonczyk]

The ds2805 has a structure named: w1_family_2d, which surely
comes from a w1_ds2431 module. This commit fixes this name to
prevent confusion and mark a correct family name.

Signed-off-by: Mariusz Bialonczyk <manio@skyboo.net>
---
Changes in v2:
    Added a missing commit msg.

 drivers/w1/slaves/w1_ds2805.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2805.c b/drivers/w1/slaves/w1_ds2805.c
index 29348d283a65..ab349604531a 100644
--- a/drivers/w1/slaves/w1_ds2805.c
+++ b/drivers/w1/slaves/w1_ds2805.c
@@ -288,7 +288,7 @@ static struct w1_family_ops w1_f0d_fops = {
 	.remove_slave   = w1_f0d_remove_slave,
 };
 
-static struct w1_family w1_family_2d = {
+static struct w1_family w1_family_0d = {
 	.fid = W1_EEPROM_DS2805,
 	.fops = &w1_f0d_fops,
 };
@@ -296,13 +296,13 @@ static struct w1_family w1_family_2d = {
 static int __init w1_f0d_init(void)
 {
 	pr_info("%s()\n", __func__);
-	return w1_register_family(&w1_family_2d);
+	return w1_register_family(&w1_family_0d);
 }
 
 static void __exit w1_f0d_fini(void)
 {
 	pr_info("%s()\n", __func__);
-	w1_unregister_family(&w1_family_2d);
+	w1_unregister_family(&w1_family_0d);
 }
 
 module_init(w1_f0d_init);
-- 
2.19.0.rc1

Re: [PATCH 4/4] w1: ds2805: rename w1_family struct, fixing c-p typo

[Mariusz Bialonczyk]

On Fri, 24 May 2019 20:21:09 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, May 20, 2019 at 09:05:58AM +0200, Mariusz Bialonczyk wrote:
> > Signed-off-by: Mariusz Bialonczyk <manio@skyboo.net>
> > ---
> >  drivers/w1/slaves/w1_ds2805.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> I can not take patches without any changelog text, sorry.
Greg,
I'm sorry, I thought that a subject commit line in such
simple commits is enough.

I just resend this single one with commit message filled:
https://lore.kernel.org/lkml/20190525084538.29389-1-manio@skyboo.net/

Thanks!

regards,
-- 
Mariusz Białończyk | xmpp/e-mail: manio@skyboo.net
https://skyboo.net | https://github.com/manio