* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).