Skip to contentSkip to conversation

iio: hx711: fix bug in reset functionality

PATCH Superseded Andreas Klinger 2 participants 3 messages Nov 30, 2017 – Dec 5, 2017

Subsystems (2)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Andreas Klinger

author maintainer
#0 lore raw

PATCH v2 2/2 iio: hx711: fix bug in reset functionality

return value in hx711_reset() should indicate status of dout otherwise the calling function is reporting an error als false positive

wait up to 1 second until the device is in normal state
by changing the channel on every trigger event it turned out that the former 100 ms are not enough for waiting until the device is in normal mode

---
 drivers/iio/adc/hx711.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/iio/adc/hx711.c +6 -7
index d773fd69a76d..eabd2e66aca3 100644
@@ -153,15 +153,16 @@ static int hx711_wait_for_ready(struct hx711_data *hx711_data)
 	int i, val;
 
 	/*
-	 * a maximum reset cycle time of 56 ms was measured.
-	 * we round it up to 100 ms
+	 * in some rare cases the reset takes quite a long time
+	 * especially when the channel is changed.
+	 * Allow up to one second for it
 	 */
 	for (i = 0; i < 100; i++) {
 		val = gpiod_get_value(hx711_data->gpiod_dout);
 		if (!val)
 			break;
-		/* sleep at least 1 ms */
-		msleep(1);
+		/* sleep at least 10 ms */
+		msleep(10);
 	}
 	if (val)
 		return -EIO;
@@ -203,9 +204,7 @@ static int hx711_reset(struct hx711_data *hx711_data)
 		 * after a dummy read we need to wait vor readiness
 		 * for not mixing gain pulses with the clock
 		 */
-		ret = hx711_wait_for_ready(hx711_data);
-		if (ret)
-			return ret;
+		val = hx711_wait_for_ready(hx711_data);
 	}
 
 	return val;
Signature · 4 lines
2.1.4


-- 
maintainer
#1 lore raw
Quoted 6 lines · On Thu, 30 Nov 2017 22:44:45 +0100 Andreas Klinger <[email protected]> wrote:

return value in hx711_reset() should indicate status of dout otherwise the calling function is reporting an error als false positive

wait up to 1 second until the device is in normal state
by changing the channel on every trigger event it turned out that the former 100 ms are not enough for waiting until the device is in normal mode

What is the ultimate result of this? I'm trying to assess if we want to marked it for stable trees or just apply it ready for the next merge window. It think it means that two reads too close to each other will always result in an error for the second one rather than the intended delay and then succeed?

Thanks,

Jonathan

Quoted 42 lines
---
 drivers/iio/adc/hx711.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/iio/adc/hx711.c +6 -7
index d773fd69a76d..eabd2e66aca3 100644
@@ -153,15 +153,16 @@ static int hx711_wait_for_ready(struct hx711_data *hx711_data)
 	int i, val;
 
 	/*
-	 * a maximum reset cycle time of 56 ms was measured.
-	 * we round it up to 100 ms
+	 * in some rare cases the reset takes quite a long time
+	 * especially when the channel is changed.
+	 * Allow up to one second for it
 	 */
 	for (i = 0; i < 100; i++) {
 		val = gpiod_get_value(hx711_data->gpiod_dout);
 		if (!val)
 			break;
-		/* sleep at least 1 ms */
-		msleep(1);
+		/* sleep at least 10 ms */
+		msleep(10);
 	}
 	if (val)
 		return -EIO;
@@ -203,9 +204,7 @@ static int hx711_reset(struct hx711_data *hx711_data)
 		 * after a dummy read we need to wait vor readiness
 		 * for not mixing gain pulses with the clock
 		 */
-		ret = hx711_wait_for_ready(hx711_data);
-		if (ret)
-			return ret;
+		val = hx711_wait_for_ready(hx711_data);
 	}
 
 	return val;
author maintainer
#2 lore raw

Hi Jonathan,

see answer to your question below.

Andreas

Jonathan Cameron <[email protected]> schrieb am Sat, 02. Dec 13:22:

Quoted 10 lines
Quoted 6 lines · On Thu, 30 Nov 2017 22:44:45 +0100 Andreas Klinger <[email protected]> wrote:

return value in hx711_reset() should indicate status of dout otherwise the calling function is reporting an error als false positive

wait up to 1 second until the device is in normal state
by changing the channel on every trigger event it turned out that the former 100 ms are not enough for waiting until the device is in normal mode

Quoted 5 lines

What is the ultimate result of this? I'm trying to assess if we want to marked it for stable trees or just apply it ready for the next merge window. It think it means that two reads too close to each other will always result in an error for the second one rather than the intended delay and then succeed?

Exactly. If one is accessing to fast the second access will fail. Especially when using a trigger the second channel is never read without error.

Quoted 47 lines

Thanks,

Jonathan

Quoted 42 lines
---
 drivers/iio/adc/hx711.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/iio/adc/hx711.c +6 -7
index d773fd69a76d..eabd2e66aca3 100644
@@ -153,15 +153,16 @@ static int hx711_wait_for_ready(struct hx711_data *hx711_data)
 	int i, val;
 
 	/*
-	 * a maximum reset cycle time of 56 ms was measured.
-	 * we round it up to 100 ms
+	 * in some rare cases the reset takes quite a long time
+	 * especially when the channel is changed.
+	 * Allow up to one second for it
 	 */
 	for (i = 0; i < 100; i++) {
 		val = gpiod_get_value(hx711_data->gpiod_dout);
 		if (!val)
 			break;
-		/* sleep at least 1 ms */
-		msleep(1);
+		/* sleep at least 10 ms */
+		msleep(10);
 	}
 	if (val)
 		return -EIO;
@@ -203,9 +204,7 @@ static int hx711_reset(struct hx711_data *hx711_data)
 		 * after a dummy read we need to wait vor readiness
 		 * for not mixing gain pulses with the clock
 		 */
-		ret = hx711_wait_for_ready(hx711_data);
-		if (ret)
-			return ret;
+		val = hx711_wait_for_ready(hx711_data);
 	}
 
 	return val;