Re: [U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover
On 07/Mar/2019 12:59, Heiko Schocher wrote: <...> > This change brings travis build to fail, see: > > https://travis-ci.org/hsdenx/u-boot-i2c/jobs/502984991#L1265 > > On which hardware have you tested this patchset? I didn't had -Werror enabled, so it slipped. > So, please, send a v4, Thanks! Ok. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover
Hello Ismael,
Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
Cosmetic change. Any call to the recover function would need to do the
same check afterwards, so it's sensible to make it part of the function.
Signed-off-by: Ismael Luceno
---
drivers/i2c/rcar_i2c.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
index 78ef8acacc..8fe0aadffa 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -81,12 +81,13 @@ static int rcar_i2c_finish(struct udevice *dev)
return ret;
}
-static void rcar_i2c_recover(struct udevice *dev)
+static int rcar_i2c_recover(struct udevice *dev)
{
struct rcar_i2c_priv *priv = dev_get_priv(dev);
u32 mcr = RCAR_I2C_ICMCR_MDBS | RCAR_I2C_ICMCR_OBPC;
u32 mcra = mcr | RCAR_I2C_ICMCR_FSDA;
int i;
+ u32 mstat;
/* Send 9 SCL pulses */
for (i = 0; i < 9; i++) {
@@ -106,6 +107,9 @@ static void rcar_i2c_recover(struct udevice *dev)
udelay(5);
writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR);
udelay(5);
+
+ mstat = readl(priv->base + RCAR_I2C_ICMSR);
+ return mstat & RCAR_I2C_ICMCR_FSDA ? -EBUSY : 0;
}
static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
@@ -125,9 +129,7 @@ static int rcar_i2c_set_addr(struct udevice *dev, u8 chip,
u8 read)
ret = wait_for_bit_le32(priv->base + RCAR_I2C_ICMCR,
RCAR_I2C_ICMCR_FSDA, false, 2, true);
if (ret) {
- rcar_i2c_recover(dev);
- val = readl(priv->base + RCAR_I2C_ICMSR);
- if (val & RCAR_I2C_ICMCR_FSDA) {
+ if (rcar_i2c_recover(dev)) {
dev_err(dev, "Bus busy, aborting\n");
return ret;
}
This change brings travis build to fail, see:
https://travis-ci.org/hsdenx/u-boot-i2c/jobs/502984991#L1265
On which hardware have you tested this patchset?
The fix is easy:
$ git diff
diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
index 8fe0aadffa..2ebae349ed 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -117,7 +117,6 @@ static int rcar_i2c_set_addr(struct udevice *dev, u8 chip,
u8 read)
struct rcar_i2c_priv *priv = dev_get_priv(dev);
u32 mask = RCAR_I2C_ICMSR_MAT |
(read ? RCAR_I2C_ICMSR_MDR : RCAR_I2C_ICMSR_MDE);
- u32 val;
int ret;
writel(0, priv->base + RCAR_I2C_ICMIER);
$
but I am unsure if you ever tested the changes...
So, please, send a v4, Thanks!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover
Hello Ismael, Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes: Cosmetic change. Any call to the recover function would need to do the same check afterwards, so it's sensible to make it part of the function. Signed-off-by: Ismael Luceno --- drivers/i2c/rcar_i2c.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Heiko Schocher bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover
On 3/6/19 11:46 AM, Ismael Luceno Cortes wrote:
> Cosmetic change. Any call to the recover function would need to do the
> same check afterwards, so it's sensible to make it part of the function.
>
> Signed-off-by: Ismael Luceno
Reviewed-by: Marek Vasut
> ---
> drivers/i2c/rcar_i2c.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
> index 78ef8acacc..8fe0aadffa 100644
> --- a/drivers/i2c/rcar_i2c.c
> +++ b/drivers/i2c/rcar_i2c.c
> @@ -81,12 +81,13 @@ static int rcar_i2c_finish(struct udevice *dev)
> return ret;
> }
>
> -static void rcar_i2c_recover(struct udevice *dev)
> +static int rcar_i2c_recover(struct udevice *dev)
> {
> struct rcar_i2c_priv *priv = dev_get_priv(dev);
> u32 mcr = RCAR_I2C_ICMCR_MDBS | RCAR_I2C_ICMCR_OBPC;
> u32 mcra = mcr | RCAR_I2C_ICMCR_FSDA;
> int i;
> + u32 mstat;
Nit: would be nice to keep these variables sorted in reverse xmas tree.
--
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover
Cosmetic change. Any call to the recover function would need to do the
same check afterwards, so it's sensible to make it part of the function.
Signed-off-by: Ismael Luceno
---
drivers/i2c/rcar_i2c.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
index 78ef8acacc..8fe0aadffa 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -81,12 +81,13 @@ static int rcar_i2c_finish(struct udevice *dev)
return ret;
}
-static void rcar_i2c_recover(struct udevice *dev)
+static int rcar_i2c_recover(struct udevice *dev)
{
struct rcar_i2c_priv *priv = dev_get_priv(dev);
u32 mcr = RCAR_I2C_ICMCR_MDBS | RCAR_I2C_ICMCR_OBPC;
u32 mcra = mcr | RCAR_I2C_ICMCR_FSDA;
int i;
+ u32 mstat;
/* Send 9 SCL pulses */
for (i = 0; i < 9; i++) {
@@ -106,6 +107,9 @@ static void rcar_i2c_recover(struct udevice *dev)
udelay(5);
writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR);
udelay(5);
+
+ mstat = readl(priv->base + RCAR_I2C_ICMSR);
+ return mstat & RCAR_I2C_ICMCR_FSDA ? -EBUSY : 0;
}
static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
@@ -125,9 +129,7 @@ static int rcar_i2c_set_addr(struct udevice *dev, u8 chip,
u8 read)
ret = wait_for_bit_le32(priv->base + RCAR_I2C_ICMCR,
RCAR_I2C_ICMCR_FSDA, false, 2, true);
if (ret) {
- rcar_i2c_recover(dev);
- val = readl(priv->base + RCAR_I2C_ICMSR);
- if (val & RCAR_I2C_ICMCR_FSDA) {
+ if (rcar_i2c_recover(dev)) {
dev_err(dev, "Bus busy, aborting\n");
return ret;
}
--
2.19.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
