* [PATCH 0/2] i2c: recovery: add bus_free callback
@ 2018-07-10 22:24 Wolfram Sang
2018-07-10 22:24 ` [PATCH 1/2] i2c: recovery: add get_bus_free callback Wolfram Sang
2018-07-10 22:24 ` [PATCH 2/2] i2c: rcar: use the new " Wolfram Sang
0 siblings, 2 replies; 8+ messages in thread
From: Wolfram Sang @ 2018-07-10 22:24 UTC (permalink / raw)
To: linux-i2c; +Cc: linux-renesas-soc, Yoshihiro Shimoda, Wolfram Sang
Add a new callback for IP cores which cannot report SDA directly but only a
'bus free' state. Implement this for the Renesas R-Car driver.
It has been tested with a Renesas Lager board (R-Car H2).
A branch (with further improvements) can be found here:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/i2c/recovery/write-byte-fix
Looking forward to comments.
Kind regards,
Wolfram
Wolfram Sang (2):
i2c: recovery: add get_bus_free callback
i2c: rcar: use the new get_bus_free callback
drivers/i2c/busses/i2c-rcar.c | 21 +++++++++++----------
drivers/i2c/i2c-core-base.c | 27 +++++++++++++++++++++++----
include/linux/i2c.h | 3 +++
3 files changed, 37 insertions(+), 14 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] i2c: recovery: add get_bus_free callback
2018-07-10 22:24 [PATCH 0/2] i2c: recovery: add bus_free callback Wolfram Sang
@ 2018-07-10 22:24 ` Wolfram Sang
2018-07-11 6:08 ` Peter Rosin
2018-07-17 8:48 ` Wolfram Sang
2018-07-10 22:24 ` [PATCH 2/2] i2c: rcar: use the new " Wolfram Sang
1 sibling, 2 replies; 8+ messages in thread
From: Wolfram Sang @ 2018-07-10 22:24 UTC (permalink / raw)
To: linux-i2c; +Cc: linux-renesas-soc, Yoshihiro Shimoda, Wolfram Sang
Some IP cores have an internal 'bus free' logic which may be more
advanced than just checking if SDA is high. Add a separate callback to
get this status. Filling it is optional.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/i2c-core-base.c | 27 +++++++++++++++++++++++----
include/linux/i2c.h | 3 +++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index c7995efd58ea..59f8dfc5be36 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -158,6 +158,22 @@ static void set_sda_gpio_value(struct i2c_adapter *adap, int val)
gpiod_set_value_cansleep(adap->bus_recovery_info->sda_gpiod, val);
}
+static int i2c_generic_bus_free(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+ int ret = -EOPNOTSUPP;
+
+ if (bri->get_bus_free)
+ ret = bri->get_bus_free(adap);
+ else if (bri->get_sda)
+ ret = bri->get_sda(adap);
+
+ if (ret < 0)
+ return ret;
+
+ return ret ? 0 : -EBUSY;
+}
+
/*
* We are generating clock pulses. ndelay() determines durating of clk pulses.
* We will generate clock with rate 100 KHz and so duration of both clock levels
@@ -169,7 +185,7 @@ static void set_sda_gpio_value(struct i2c_adapter *adap, int val)
int i2c_generic_scl_recovery(struct i2c_adapter *adap)
{
struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
- int i = 0, val = 1, ret = 0;
+ int i = 0, val = 1, ret;
if (bri->prepare_recovery)
bri->prepare_recovery(adap);
@@ -207,14 +223,17 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap)
bri->set_sda(adap, val);
ndelay(RECOVERY_NDELAY / 2);
- /* Break if SDA is high */
- if (val && bri->get_sda) {
- ret = bri->get_sda(adap) ? 0 : -EBUSY;
+ if (val) {
+ ret = i2c_generic_bus_free(adap);
if (ret == 0)
break;
}
}
+ /* If we can't check bus status, assume recovery worked */
+ if (ret == -EOPNOTSUPP)
+ ret = 0;
+
if (bri->unprepare_recovery)
bri->unprepare_recovery(adap);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 9d1818c56775..60e224428a85 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -587,6 +587,8 @@ struct i2c_timings {
* @set_sda: This sets/clears the SDA line. This or get_sda() is mandatory for
* generic SCL recovery. Populated internally, if sda_gpio is a valid GPIO,
* for generic GPIO recovery.
+ * @get_bus_free: Returns the bus free state as seen from the IP core in case it
+ * has a more complex internal logic than just reading SDA. Optional.
* @prepare_recovery: This will be called before starting recovery. Platform may
* configure padmux here for SDA/SCL line or something else they want.
* @unprepare_recovery: This will be called after completing recovery. Platform
@@ -601,6 +603,7 @@ struct i2c_bus_recovery_info {
void (*set_scl)(struct i2c_adapter *adap, int val);
int (*get_sda)(struct i2c_adapter *adap);
void (*set_sda)(struct i2c_adapter *adap, int val);
+ int (*get_bus_free)(struct i2c_adapter *adap);
void (*prepare_recovery)(struct i2c_adapter *adap);
void (*unprepare_recovery)(struct i2c_adapter *adap);
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] i2c: rcar: use the new get_bus_free callback
2018-07-10 22:24 [PATCH 0/2] i2c: recovery: add bus_free callback Wolfram Sang
2018-07-10 22:24 ` [PATCH 1/2] i2c: recovery: add get_bus_free callback Wolfram Sang
@ 2018-07-10 22:24 ` Wolfram Sang
2018-07-17 8:48 ` Wolfram Sang
1 sibling, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2018-07-10 22:24 UTC (permalink / raw)
To: linux-i2c; +Cc: linux-renesas-soc, Yoshihiro Shimoda, Wolfram Sang
To break out of recovery as early as possible, feed back the bus_free
logic state.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-rcar.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 5e310efd9446..76154747971e 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -179,8 +179,6 @@ static void rcar_i2c_set_scl(struct i2c_adapter *adap, int val)
rcar_i2c_write(priv, ICMCR, priv->recovery_icmcr);
};
-/* No get_sda, because the HW only reports its bus free logic, not SDA itself */
-
static void rcar_i2c_set_sda(struct i2c_adapter *adap, int val)
{
struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
@@ -193,10 +191,19 @@ static void rcar_i2c_set_sda(struct i2c_adapter *adap, int val)
rcar_i2c_write(priv, ICMCR, priv->recovery_icmcr);
};
+static int rcar_i2c_get_bus_free(struct i2c_adapter *adap)
+{
+ struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
+
+ return !(rcar_i2c_read(priv, ICMCR) & FSDA);
+
+};
+
static struct i2c_bus_recovery_info rcar_i2c_bri = {
.get_scl = rcar_i2c_get_scl,
.set_scl = rcar_i2c_set_scl,
.set_sda = rcar_i2c_set_sda,
+ .get_bus_free = rcar_i2c_get_bus_free,
.recover_bus = i2c_generic_scl_recovery,
};
static void rcar_i2c_init(struct rcar_i2c_priv *priv)
@@ -211,7 +218,7 @@ static void rcar_i2c_init(struct rcar_i2c_priv *priv)
static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
{
- int i, ret;
+ int i;
for (i = 0; i < LOOP_TIMEOUT; i++) {
/* make sure that bus is not busy */
@@ -222,13 +229,7 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
/* Waiting did not help, try to recover */
priv->recovery_icmcr = MDBS | OBPC | FSDA | FSCL;
- ret = i2c_recover_bus(&priv->adap);
-
- /* No failure when recovering, so check bus busy bit again */
- if (ret == 0)
- ret = (rcar_i2c_read(priv, ICMCR) & FSDA) ? -EBUSY : 0;
-
- return ret;
+ return i2c_recover_bus(&priv->adap);
}
static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, struct i2c_timings *t)
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] i2c: recovery: add get_bus_free callback
2018-07-10 22:24 ` [PATCH 1/2] i2c: recovery: add get_bus_free callback Wolfram Sang
@ 2018-07-11 6:08 ` Peter Rosin
2018-07-11 16:52 ` Wolfram Sang
2018-07-17 8:48 ` Wolfram Sang
1 sibling, 1 reply; 8+ messages in thread
From: Peter Rosin @ 2018-07-11 6:08 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c; +Cc: linux-renesas-soc, Yoshihiro Shimoda
On 2018-07-11 00:24, Wolfram Sang wrote:
> Some IP cores have an internal 'bus free' logic which may be more
> advanced than just checking if SDA is high. Add a separate callback to
> get this status. Filling it is optional.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/i2c/i2c-core-base.c | 27 +++++++++++++++++++++++----
> include/linux/i2c.h | 3 +++
> 2 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index c7995efd58ea..59f8dfc5be36 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -158,6 +158,22 @@ static void set_sda_gpio_value(struct i2c_adapter *adap, int val)
> gpiod_set_value_cansleep(adap->bus_recovery_info->sda_gpiod, val);
> }
>
> +static int i2c_generic_bus_free(struct i2c_adapter *adap)
> +{
> + struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> + int ret = -EOPNOTSUPP;
> +
> + if (bri->get_bus_free)
> + ret = bri->get_bus_free(adap);
> + else if (bri->get_sda)
> + ret = bri->get_sda(adap);
> +
> + if (ret < 0)
> + return ret;
> +
> + return ret ? 0 : -EBUSY;
> +}
Hmmm, the name i2c_generic_bus_free suggests that scl should also be checked,
because the *bus* isn't really free unless both lines are high? No? Or, maybe
just rename to i2c_generic_sda_free (or something)?
But that's of course just a nit...
More importantly, isn't a ->get_bus_free implementation a sufficient requirement
for recovery? I.e. even if both ->get_sda and ->set_sda are missing?
Cheers,
Peter
> +
> /*
> * We are generating clock pulses. ndelay() determines durating of clk pulses.
> * We will generate clock with rate 100 KHz and so duration of both clock levels
> @@ -169,7 +185,7 @@ static void set_sda_gpio_value(struct i2c_adapter *adap, int val)
> int i2c_generic_scl_recovery(struct i2c_adapter *adap)
> {
> struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> - int i = 0, val = 1, ret = 0;
> + int i = 0, val = 1, ret;
>
> if (bri->prepare_recovery)
> bri->prepare_recovery(adap);
> @@ -207,14 +223,17 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap)
> bri->set_sda(adap, val);
> ndelay(RECOVERY_NDELAY / 2);
>
> - /* Break if SDA is high */
> - if (val && bri->get_sda) {
> - ret = bri->get_sda(adap) ? 0 : -EBUSY;
> + if (val) {
> + ret = i2c_generic_bus_free(adap);
> if (ret == 0)
> break;
> }
> }
>
> + /* If we can't check bus status, assume recovery worked */
> + if (ret == -EOPNOTSUPP)
> + ret = 0;
> +
> if (bri->unprepare_recovery)
> bri->unprepare_recovery(adap);
>
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 9d1818c56775..60e224428a85 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -587,6 +587,8 @@ struct i2c_timings {
> * @set_sda: This sets/clears the SDA line. This or get_sda() is mandatory for
> * generic SCL recovery. Populated internally, if sda_gpio is a valid GPIO,
> * for generic GPIO recovery.
> + * @get_bus_free: Returns the bus free state as seen from the IP core in case it
> + * has a more complex internal logic than just reading SDA. Optional.
> * @prepare_recovery: This will be called before starting recovery. Platform may
> * configure padmux here for SDA/SCL line or something else they want.
> * @unprepare_recovery: This will be called after completing recovery. Platform
> @@ -601,6 +603,7 @@ struct i2c_bus_recovery_info {
> void (*set_scl)(struct i2c_adapter *adap, int val);
> int (*get_sda)(struct i2c_adapter *adap);
> void (*set_sda)(struct i2c_adapter *adap, int val);
> + int (*get_bus_free)(struct i2c_adapter *adap);
>
> void (*prepare_recovery)(struct i2c_adapter *adap);
> void (*unprepare_recovery)(struct i2c_adapter *adap);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] i2c: recovery: add get_bus_free callback
2018-07-11 6:08 ` Peter Rosin
@ 2018-07-11 16:52 ` Wolfram Sang
2018-07-12 17:39 ` Wolfram Sang
0 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2018-07-11 16:52 UTC (permalink / raw)
To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-renesas-soc, Yoshihiro Shimoda
[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]
> Hmmm, the name i2c_generic_bus_free suggests that scl should also be checked,
> because the *bus* isn't really free unless both lines are high? No? Or, maybe
> just rename to i2c_generic_sda_free (or something)?
Well, technically, bus recovery is just for resurrecting a stuck low
SDA. So, I'd think checking if SDA went high again should do it.
However, there is HW (at least Renesas R-Car) which cannot read SDA
directly. It will just return the result of its internal 'bus_free'
logic. Whatever that is. I think it wants to see a STOP but that may be
only part of it.
I wanted to have an option to make use of such a feature if we can't
read SDA. It is better than nothing. But since it doesn't reflect the
state of SDA directly, I decided for another callback.
> More importantly, isn't a ->get_bus_free implementation a sufficient requirement
> for recovery? I.e. even if both ->get_sda and ->set_sda are missing?
In my case, it isn't. It needs set_sda (== STOP) to achieve a useful
result. Hmm, I think this needs better documentation...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] i2c: recovery: add get_bus_free callback
2018-07-11 16:52 ` Wolfram Sang
@ 2018-07-12 17:39 ` Wolfram Sang
0 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2018-07-12 17:39 UTC (permalink / raw)
To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-renesas-soc, Yoshihiro Shimoda
[-- Attachment #1: Type: text/plain, Size: 503 bytes --]
> > More importantly, isn't a ->get_bus_free implementation a sufficient requirement
> > for recovery? I.e. even if both ->get_sda and ->set_sda are missing?
>
> In my case, it isn't. It needs set_sda (== STOP) to achieve a useful
> result. Hmm, I think this needs better documentation...
On a second thought, it may be possible to simply merge get_sda and
get_bus_free in the future. For now, I'd like to keep them seperate
until we have implemented bus recovery for some more hardware.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] i2c: recovery: add get_bus_free callback
2018-07-10 22:24 ` [PATCH 1/2] i2c: recovery: add get_bus_free callback Wolfram Sang
2018-07-11 6:08 ` Peter Rosin
@ 2018-07-17 8:48 ` Wolfram Sang
1 sibling, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2018-07-17 8:48 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-i2c, linux-renesas-soc, Yoshihiro Shimoda
[-- Attachment #1: Type: text/plain, Size: 354 bytes --]
On Wed, Jul 11, 2018 at 12:24:22AM +0200, Wolfram Sang wrote:
> Some IP cores have an internal 'bus free' logic which may be more
> advanced than just checking if SDA is high. Add a separate callback to
> get this status. Filling it is optional.
>
> 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] 8+ messages in thread
* Re: [PATCH 2/2] i2c: rcar: use the new get_bus_free callback
2018-07-10 22:24 ` [PATCH 2/2] i2c: rcar: use the new " Wolfram Sang
@ 2018-07-17 8:48 ` Wolfram Sang
0 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2018-07-17 8:48 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-i2c, linux-renesas-soc, Yoshihiro Shimoda
[-- Attachment #1: Type: text/plain, Size: 256 bytes --]
On Wed, Jul 11, 2018 at 12:24:23AM +0200, Wolfram Sang wrote:
> To break out of recovery as early as possible, feed back the bus_free
> logic state.
>
> 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] 8+ messages in thread
end of thread, other threads:[~2018-07-17 9:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-10 22:24 [PATCH 0/2] i2c: recovery: add bus_free callback Wolfram Sang
2018-07-10 22:24 ` [PATCH 1/2] i2c: recovery: add get_bus_free callback Wolfram Sang
2018-07-11 6:08 ` Peter Rosin
2018-07-11 16:52 ` Wolfram Sang
2018-07-12 17:39 ` Wolfram Sang
2018-07-17 8:48 ` Wolfram Sang
2018-07-10 22:24 ` [PATCH 2/2] i2c: rcar: use the new " Wolfram Sang
2018-07-17 8:48 ` Wolfram Sang
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.