* [PATCH 0/2] i2c: rcar: use iopoll helpers for better timeout handling
@ 2020-08-29 20:38 Wolfram Sang
2020-08-29 20:38 ` [PATCH 1/2] i2c: rcar: improve bus busy detection Wolfram Sang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Wolfram Sang @ 2020-08-29 20:38 UTC (permalink / raw)
To: linux-i2c; +Cc: linux-renesas-soc, Wolfram Sang
Originally, I noticed that the timeout value for initiating bus recovery
was not optimal. While fixing it, I took the chance to convert its
handling to the iopoll helpers. And then, I converted the timeout
handling for resetting the device, too, while I was at it.
Tested on a Renesas Lager board (H2) and Salvator-XS (M3-N).
Wolfram Sang (2):
i2c: rcar: improve bus busy detection
i2c: rcar: refactor and shorten timeout when resetting
drivers/i2c/busses/i2c-rcar.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] i2c: rcar: improve bus busy detection
2020-08-29 20:38 [PATCH 0/2] i2c: rcar: use iopoll helpers for better timeout handling Wolfram Sang
@ 2020-08-29 20:38 ` Wolfram Sang
2020-09-09 8:44 ` Wolfram Sang
2020-08-29 20:38 ` [PATCH 2/2] i2c: rcar: refactor and shorten timeout when resetting Wolfram Sang
2020-08-30 8:30 ` [PATCH 0/2] i2c: rcar: use iopoll helpers for better timeout handling Niklas Söderlund
2 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2020-08-29 20:38 UTC (permalink / raw)
To: linux-i2c; +Cc: linux-renesas-soc, Wolfram Sang
I2C doesn't define a timeout for bus busy, so an arbitrary value like
LOOP_TIMEOUT is not a good idea. Let's use the timeout value in struct
adapter which is meant for such cases and is user-configurable (via
IOCTL). To reduce the load, wait 10us instead of 1us which is good
enough for the slow frequencies used by I2C. Finally, use the
poll_timeout helper instead of open coding it.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-rcar.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 0f73f0681a6e..ef10514f6215 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -19,6 +19,7 @@
#include <linux/err.h>
#include <linux/interrupt.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/i2c.h>
#include <linux/i2c-smbus.h>
#include <linux/kernel.h>
@@ -225,18 +226,18 @@ static void rcar_i2c_init(struct rcar_i2c_priv *priv)
static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
{
- int i;
+ int ret;
+ u32 val;
- for (i = 0; i < LOOP_TIMEOUT; i++) {
- /* make sure that bus is not busy */
- if (!(rcar_i2c_read(priv, ICMCR) & FSDA))
- return 0;
- udelay(1);
+ ret = readl_poll_timeout(priv->io + ICMCR, val, !(val & FSDA), 10,
+ priv->adap.timeout);
+ if (ret) {
+ /* Waiting did not help, try to recover */
+ priv->recovery_icmcr = MDBS | OBPC | FSDA | FSCL;
+ ret = i2c_recover_bus(&priv->adap);
}
- /* Waiting did not help, try to recover */
- priv->recovery_icmcr = MDBS | OBPC | FSDA | FSCL;
- return i2c_recover_bus(&priv->adap);
+ return ret;
}
static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv)
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] i2c: rcar: refactor and shorten timeout when resetting
2020-08-29 20:38 [PATCH 0/2] i2c: rcar: use iopoll helpers for better timeout handling Wolfram Sang
2020-08-29 20:38 ` [PATCH 1/2] i2c: rcar: improve bus busy detection Wolfram Sang
@ 2020-08-29 20:38 ` Wolfram Sang
2020-09-09 8:44 ` Wolfram Sang
2020-08-30 8:30 ` [PATCH 0/2] i2c: rcar: use iopoll helpers for better timeout handling Niklas Söderlund
2 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2020-08-29 20:38 UTC (permalink / raw)
To: linux-i2c; +Cc: linux-renesas-soc, Wolfram Sang
LOOP_TIMEOUT was only used back then because we didn't want to introduce
another constant. The timeout value can easily be a magnitude shorter
because the typical range is 3us - 8us. Refactor the code to use the
poll_timeout helper, use a specific timeout value and get rid of the
ugly LOOP_TIMEOUT constant.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-rcar.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index ef10514f6215..217def2d7cb4 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -150,9 +150,6 @@ struct rcar_i2c_priv {
#define rcar_i2c_priv_to_dev(p) ((p)->adap.dev.parent)
#define rcar_i2c_is_recv(p) ((p)->msg->flags & I2C_M_RD)
-#define LOOP_TIMEOUT 1024
-
-
static void rcar_i2c_write(struct rcar_i2c_priv *priv, int reg, u32 val)
{
writel(val, priv->io + reg);
@@ -765,20 +762,14 @@ static void rcar_i2c_release_dma(struct rcar_i2c_priv *priv)
/* I2C is a special case, we need to poll the status of a reset */
static int rcar_i2c_do_reset(struct rcar_i2c_priv *priv)
{
- int i, ret;
+ int ret;
ret = reset_control_reset(priv->rstc);
if (ret)
return ret;
- for (i = 0; i < LOOP_TIMEOUT; i++) {
- ret = reset_control_status(priv->rstc);
- if (ret == 0)
- return 0;
- udelay(1);
- }
-
- return -ETIMEDOUT;
+ return read_poll_timeout_atomic(reset_control_status, ret, ret == 0, 1,
+ 100, false, priv->rstc);
}
static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] i2c: rcar: use iopoll helpers for better timeout handling
2020-08-29 20:38 [PATCH 0/2] i2c: rcar: use iopoll helpers for better timeout handling Wolfram Sang
2020-08-29 20:38 ` [PATCH 1/2] i2c: rcar: improve bus busy detection Wolfram Sang
2020-08-29 20:38 ` [PATCH 2/2] i2c: rcar: refactor and shorten timeout when resetting Wolfram Sang
@ 2020-08-30 8:30 ` Niklas Söderlund
2 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2020-08-30 8:30 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-i2c, linux-renesas-soc
Hi Wolfram,
Neat work, I learnt something new.
On 2020-08-29 22:38:08 +0200, Wolfram Sang wrote:
> Originally, I noticed that the timeout value for initiating bus recovery
> was not optimal. While fixing it, I took the chance to convert its
> handling to the iopoll helpers. And then, I converted the timeout
> handling for resetting the device, too, while I was at it.
>
> Tested on a Renesas Lager board (H2) and Salvator-XS (M3-N).
>
> Wolfram Sang (2):
> i2c: rcar: improve bus busy detection
> i2c: rcar: refactor and shorten timeout when resetting
For the whole series,
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
> drivers/i2c/busses/i2c-rcar.c | 34 +++++++++++++---------------------
> 1 file changed, 13 insertions(+), 21 deletions(-)
>
> --
> 2.20.1
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] i2c: rcar: improve bus busy detection
2020-08-29 20:38 ` [PATCH 1/2] i2c: rcar: improve bus busy detection Wolfram Sang
@ 2020-09-09 8:44 ` Wolfram Sang
0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2020-09-09 8:44 UTC (permalink / raw)
To: linux-i2c; +Cc: linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 570 bytes --]
On Sat, Aug 29, 2020 at 10:38:09PM +0200, Wolfram Sang wrote:
> I2C doesn't define a timeout for bus busy, so an arbitrary value like
> LOOP_TIMEOUT is not a good idea. Let's use the timeout value in struct
> adapter which is meant for such cases and is user-configurable (via
> IOCTL). To reduce the load, wait 10us instead of 1us which is good
> enough for the slow frequencies used by I2C. Finally, use the
> poll_timeout helper instead of open coding it.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] i2c: rcar: refactor and shorten timeout when resetting
2020-08-29 20:38 ` [PATCH 2/2] i2c: rcar: refactor and shorten timeout when resetting Wolfram Sang
@ 2020-09-09 8:44 ` Wolfram Sang
0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2020-09-09 8:44 UTC (permalink / raw)
To: linux-i2c; +Cc: linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 491 bytes --]
On Sat, Aug 29, 2020 at 10:38:10PM +0200, Wolfram Sang wrote:
> LOOP_TIMEOUT was only used back then because we didn't want to introduce
> another constant. The timeout value can easily be a magnitude shorter
> because the typical range is 3us - 8us. Refactor the code to use the
> poll_timeout helper, use a specific timeout value and get rid of the
> ugly LOOP_TIMEOUT constant.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-09 8:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-29 20:38 [PATCH 0/2] i2c: rcar: use iopoll helpers for better timeout handling Wolfram Sang
2020-08-29 20:38 ` [PATCH 1/2] i2c: rcar: improve bus busy detection Wolfram Sang
2020-09-09 8:44 ` Wolfram Sang
2020-08-29 20:38 ` [PATCH 2/2] i2c: rcar: refactor and shorten timeout when resetting Wolfram Sang
2020-09-09 8:44 ` Wolfram Sang
2020-08-30 8:30 ` [PATCH 0/2] i2c: rcar: use iopoll helpers for better timeout handling Niklas Söderlund
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.