[U-Boot] [PATCH v3 1/6] i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed

[U-Boot] [PATCH v3 1/6] i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed

[Ismael Luceno Cortes]

Setting up the delay only needs to be done once; move it to
rcar_i2c_set_speed so it's done at initialization time.

Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
---
 drivers/i2c/rcar_i2c.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
index 10b0f8bad4..a57f72a228 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -116,9 +116,7 @@ static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
 	writel(0, priv->base + RCAR_I2C_ICMSR);
 	writel(priv->icccr, priv->base + RCAR_I2C_ICCCR);
 
-	if (priv->type == RCAR_I2C_TYPE_GEN3)
-		writel(RCAR_I2C_ICFBSCR_TCYC17, priv->base + RCAR_I2C_ICFBSCR);
-
+	/* Wait for the bus */
 	ret = wait_for_bit_le32(priv->base + RCAR_I2C_ICMCR,
 				RCAR_I2C_ICMCR_FSDA, false, 2, true);
 	if (ret) {
@@ -304,6 +302,11 @@ scgd_find:
 	priv->icccr = (scgd << RCAR_I2C_ICCCR_SCGD_OFF) | cdf;
 	writel(priv->icccr, priv->base + RCAR_I2C_ICCCR);
 
+	if (priv->type == RCAR_I2C_TYPE_GEN3) {
+		/* Set SCL/SDA delay */
+		writel(RCAR_I2C_ICFBSCR_TCYC17, priv->base + RCAR_I2C_ICFBSCR);
+	}
+
 	return 0;
 }
 

base-commit: f08023c07d826fbc8e62fdd3367961b2f0b06844
prerequisite-patch-id: 9e5b0458bc15640eb483ccad91dbe85150f9f7be
-- 
2.19.1

[U-Boot] [PATCH v3 2/6] i2c: rcar_i2c: Add comments about registers & values

[Ismael Luceno Cortes]

Document the meaning of macros related to registers and values to be
written to them.

Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
---
 drivers/i2c/rcar_i2c.c | 47 +++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
index a57f72a228..d7b27204cb 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -18,35 +18,40 @@
 #include <asm/io.h>
 #include <wait_bit.h>
 
-#define RCAR_I2C_ICSCR			0x00
-#define RCAR_I2C_ICMCR			0x04
-#define RCAR_I2C_ICMCR_MDBS		BIT(7)
-#define RCAR_I2C_ICMCR_FSCL		BIT(6)
-#define RCAR_I2C_ICMCR_FSDA		BIT(5)
-#define RCAR_I2C_ICMCR_OBPC		BIT(4)
-#define RCAR_I2C_ICMCR_MIE		BIT(3)
+#define RCAR_I2C_ICSCR			0x00 /* slave ctrl */
+#define RCAR_I2C_ICMCR			0x04 /* master ctrl */
+#define RCAR_I2C_ICMCR_MDBS		BIT(7) /* non-fifo mode switch */
+#define RCAR_I2C_ICMCR_FSCL		BIT(6) /* override SCL pin */
+#define RCAR_I2C_ICMCR_FSDA		BIT(5) /* override SDA pin */
+#define RCAR_I2C_ICMCR_OBPC		BIT(4) /* override pins */
+#define RCAR_I2C_ICMCR_MIE		BIT(3) /* master if enable */
 #define RCAR_I2C_ICMCR_TSBE		BIT(2)
-#define RCAR_I2C_ICMCR_FSB		BIT(1)
-#define RCAR_I2C_ICMCR_ESG		BIT(0)
-#define RCAR_I2C_ICSSR			0x08
-#define RCAR_I2C_ICMSR			0x0c
+#define RCAR_I2C_ICMCR_FSB		BIT(1) /* force stop bit */
+#define RCAR_I2C_ICMCR_ESG		BIT(0) /* enable start bit gen */
+#define RCAR_I2C_ICSSR			0x08 /* slave status */
+#define RCAR_I2C_ICMSR			0x0c /* master status */
 #define RCAR_I2C_ICMSR_MASK		0x7f
-#define RCAR_I2C_ICMSR_MNR		BIT(6)
-#define RCAR_I2C_ICMSR_MAL		BIT(5)
-#define RCAR_I2C_ICMSR_MST		BIT(4)
+#define RCAR_I2C_ICMSR_MNR		BIT(6) /* Nack */
+#define RCAR_I2C_ICMSR_MAL		BIT(5) /* Arbitration lost */
+#define RCAR_I2C_ICMSR_MST		BIT(4) /* Stop */
 #define RCAR_I2C_ICMSR_MDE		BIT(3)
 #define RCAR_I2C_ICMSR_MDT		BIT(2)
 #define RCAR_I2C_ICMSR_MDR		BIT(1)
 #define RCAR_I2C_ICMSR_MAT		BIT(0)
-#define RCAR_I2C_ICSIER			0x10
-#define RCAR_I2C_ICMIER			0x14
-#define RCAR_I2C_ICCCR			0x18
+#define RCAR_I2C_ICSIER			0x10 /* slave irq enable */
+#define RCAR_I2C_ICMIER			0x14 /* master irq enable */
+#define RCAR_I2C_ICCCR			0x18 /* clock dividers */
 #define RCAR_I2C_ICCCR_SCGD_OFF		3
-#define RCAR_I2C_ICSAR			0x1c
-#define RCAR_I2C_ICMAR			0x20
-#define RCAR_I2C_ICRXD_ICTXD		0x24
+#define RCAR_I2C_ICSAR			0x1c /* slave address */
+#define RCAR_I2C_ICMAR			0x20 /* master address */
+#define RCAR_I2C_ICRXD_ICTXD		0x24 /* data port */
+/*
+ * First Bit Setup Cycle (Gen3).
+ * Defines 1st bit delay between SDA and SCL.
+ */
 #define RCAR_I2C_ICFBSCR		0x38
-#define RCAR_I2C_ICFBSCR_TCYC17		0x0f
+#define RCAR_I2C_ICFBSCR_TCYC17		0x0f /* 17*Tcyc */
+
 
 enum rcar_i2c_type {
 	RCAR_I2C_TYPE_GEN2,
-- 
2.19.1

[U-Boot] [PATCH v3 3/6] i2c: rcar_i2c: Fix sending of slave addresses

[Ismael Luceno Cortes]

Do the reset before clearing the MSR, otherwise it may result in a read
or write operation instead if the start condition is repeated.

Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
Reviewed-by: Marek Vasut <marek.vasut+renesas@gmail.com>
---
 drivers/i2c/rcar_i2c.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
index d7b27204cb..a88fbcf34f 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -134,9 +134,11 @@ static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
 	}
 
 	writel((chip << 1) | read, priv->base + RCAR_I2C_ICMAR);
-	writel(0, priv->base + RCAR_I2C_ICMSR);
+	/* Reset */
 	writel(RCAR_I2C_ICMCR_MDBS | RCAR_I2C_ICMCR_MIE | RCAR_I2C_ICMCR_ESG,
 	       priv->base + RCAR_I2C_ICMCR);
+	/* Clear Status */
+	writel(0, priv->base + RCAR_I2C_ICMSR);
 
 	ret = wait_for_bit_le32(priv->base + RCAR_I2C_ICMSR, mask,
 				true, 100, true);
-- 
2.19.1

[U-Boot] [PATCH v3 4/6] i2c: rcar_i2c: Don't mask errors with EREMOTEIO at rcar_i2c_xfer

[Ismael Luceno Cortes]

Fix rcar_i2c_xfer return value, previously it was always returning
-EREMOTEIO when dealing with errors from calls to the read/write
functions.

Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
---
 drivers/i2c/rcar_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
index a88fbcf34f..9223eaecbf 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -221,7 +221,7 @@ static int rcar_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
 			ret = rcar_i2c_write_common(dev, msg);
 
 		if (ret)
-			return -EREMOTEIO;
+			return ret;
 	}
 
 	return ret;
-- 
2.19.1

[U-Boot] [PATCH v3 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer

[Ismael Luceno Cortes]

It needs to be done for both reads and writes, so do it at rcar_i2c_xfer
to avoid duplication.

Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
---
 drivers/i2c/rcar_i2c.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
index 9223eaecbf..78ef8acacc 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -158,10 +158,6 @@ static int rcar_i2c_read_common(struct udevice *dev, struct i2c_msg *msg)
 	u32 icmcr = RCAR_I2C_ICMCR_MDBS | RCAR_I2C_ICMCR_MIE;
 	int i, ret = -EREMOTEIO;
 
-	ret = rcar_i2c_set_addr(dev, msg->addr, 1);
-	if (ret)
-		return ret;
-
 	for (i = 0; i < msg->len; i++) {
 		if (msg->len - 1 == i)
 			icmcr |= RCAR_I2C_ICMCR_FSB;
@@ -188,10 +184,6 @@ static int rcar_i2c_write_common(struct udevice *dev, struct i2c_msg *msg)
 	u32 icmcr = RCAR_I2C_ICMCR_MDBS | RCAR_I2C_ICMCR_MIE;
 	int i, ret = -EREMOTEIO;
 
-	ret = rcar_i2c_set_addr(dev, msg->addr, 0);
-	if (ret)
-		return ret;
-
 	for (i = 0; i < msg->len; i++) {
 		writel(msg->buf[i], priv->base + RCAR_I2C_ICRXD_ICTXD);
 		writel(icmcr, priv->base + RCAR_I2C_ICMCR);
@@ -215,6 +207,10 @@ static int rcar_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
 	int ret;
 
 	for (; nmsgs > 0; nmsgs--, msg++) {
+		ret = rcar_i2c_set_addr(dev, msg->addr, 1);
+		if (ret)
+			return ret;
+
 		if (msg->flags & I2C_M_RD)
 			ret = rcar_i2c_read_common(dev, msg);
 		else
@@ -224,7 +220,7 @@ static int rcar_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
 			return ret;
 	}
 
-	return ret;
+	return 0;
 }
 
 static int rcar_i2c_probe_chip(struct udevice *dev, uint addr, uint flags)
-- 
2.19.1

[U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover

[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 <ismael.luceno@silicon-gears.com>
---
 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] [PATCH v3 1/6] i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed

[Marek Vasut]

On 3/6/19 11:46 AM, Ismael Luceno Cortes wrote:
> Setting up the delay only needs to be done once; move it to
> rcar_i2c_set_speed so it's done at initialization time.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>

Reviewed-by: Marek Vasut <marek.vasut+renesas@gmail.com>

> ---
>  drivers/i2c/rcar_i2c.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
> index 10b0f8bad4..a57f72a228 100644
> --- a/drivers/i2c/rcar_i2c.c
> +++ b/drivers/i2c/rcar_i2c.c
> @@ -116,9 +116,7 @@ static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
>  	writel(0, priv->base + RCAR_I2C_ICMSR);
>  	writel(priv->icccr, priv->base + RCAR_I2C_ICCCR);
>  
> -	if (priv->type == RCAR_I2C_TYPE_GEN3)
> -		writel(RCAR_I2C_ICFBSCR_TCYC17, priv->base + RCAR_I2C_ICFBSCR);
> -
> +	/* Wait for the bus */
>  	ret = wait_for_bit_le32(priv->base + RCAR_I2C_ICMCR,
>  				RCAR_I2C_ICMCR_FSDA, false, 2, true);
>  	if (ret) {
> @@ -304,6 +302,11 @@ scgd_find:
>  	priv->icccr = (scgd << RCAR_I2C_ICCCR_SCGD_OFF) | cdf;
>  	writel(priv->icccr, priv->base + RCAR_I2C_ICCCR);
>  
> +	if (priv->type == RCAR_I2C_TYPE_GEN3) {
> +		/* Set SCL/SDA delay */
> +		writel(RCAR_I2C_ICFBSCR_TCYC17, priv->base + RCAR_I2C_ICFBSCR);
> +	}
> +
>  	return 0;
>  }
>  
> 
> base-commit: f08023c07d826fbc8e62fdd3367961b2f0b06844
> prerequisite-patch-id: 9e5b0458bc15640eb483ccad91dbe85150f9f7be
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 2/6] i2c: rcar_i2c: Add comments about registers & values

[Marek Vasut]

On 3/6/19 11:46 AM, Ismael Luceno Cortes wrote:
> Document the meaning of macros related to registers and values to be
> written to them.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>


Reviewed-by: Marek Vasut <marek.vasut+renesas@gmail.com>

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 3/6] i2c: rcar_i2c: Fix sending of slave addresses

[Marek Vasut]

On 3/6/19 11:46 AM, Ismael Luceno Cortes wrote:
> Do the reset before clearing the MSR, otherwise it may result in a read
> or write operation instead if the start condition is repeated.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
> Reviewed-by: Marek Vasut <marek.vasut+renesas@gmail.com>


Reviewed-by: Marek Vasut <marek.vasut+renesas@gmail.com>

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 4/6] i2c: rcar_i2c: Don't mask errors with EREMOTEIO at rcar_i2c_xfer

[Marek Vasut]

On 3/6/19 11:46 AM, Ismael Luceno Cortes wrote:
> Fix rcar_i2c_xfer return value, previously it was always returning
> -EREMOTEIO when dealing with errors from calls to the read/write
> functions.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>

Reviewed-by: Marek Vasut <marek.vasut+renesas@gmail.com>

> ---
>  drivers/i2c/rcar_i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
> index a88fbcf34f..9223eaecbf 100644
> --- a/drivers/i2c/rcar_i2c.c
> +++ b/drivers/i2c/rcar_i2c.c
> @@ -221,7 +221,7 @@ static int rcar_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
>  			ret = rcar_i2c_write_common(dev, msg);
>  
>  		if (ret)
> -			return -EREMOTEIO;
> +			return ret;
>  	}
>  
>  	return ret;
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer

[Marek Vasut]

On 3/6/19 11:46 AM, Ismael Luceno Cortes wrote:
> It needs to be done for both reads and writes, so do it at rcar_i2c_xfer
> to avoid duplication.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>


Reviewed-by: Marek Vasut <marek.vasut+renesas@gmail.com>

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover

[Marek Vasut]

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 <ismael.luceno@silicon-gears.com>

Reviewed-by: Marek Vasut <marek.vasut+renesas@gmail.com>

> ---
>  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] [PATCH v3 1/6] i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed

[Heiko Schocher]

Hello Ismael,

Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
> Setting up the delay only needs to be done once; move it to
> rcar_i2c_set_speed so it's done at initialization time.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
> ---
>   drivers/i2c/rcar_i2c.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

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: hs at denx.de

[U-Boot] [PATCH v3 2/6] i2c: rcar_i2c: Add comments about registers & values

[Heiko Schocher]

Hello Ismael,

Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
> Document the meaning of macros related to registers and values to be
> written to them.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
> ---
>   drivers/i2c/rcar_i2c.c | 47 +++++++++++++++++++++++-------------------
>   1 file changed, 26 insertions(+), 21 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

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: hs at denx.de

[U-Boot] [PATCH v3 3/6] i2c: rcar_i2c: Fix sending of slave addresses

[Heiko Schocher]

Hello Ismael,

Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
> Do the reset before clearing the MSR, otherwise it may result in a read
> or write operation instead if the start condition is repeated.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
> Reviewed-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> ---
>   drivers/i2c/rcar_i2c.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

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: hs at denx.de

[U-Boot] [PATCH v3 4/6] i2c: rcar_i2c: Don't mask errors with EREMOTEIO at rcar_i2c_xfer

[Heiko Schocher]

Hello Ismael,

Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
> Fix rcar_i2c_xfer return value, previously it was always returning
> -EREMOTEIO when dealing with errors from calls to the read/write
> functions.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
> ---
>   drivers/i2c/rcar_i2c.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

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: hs at denx.de

[U-Boot] [PATCH v3 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer

[Heiko Schocher]

Hello Ismael,

Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
> It needs to be done for both reads and writes, so do it at rcar_i2c_xfer
> to avoid duplication.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
> ---
>   drivers/i2c/rcar_i2c.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

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: hs at denx.de

[U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover

[Heiko Schocher]

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 <ismael.luceno@silicon-gears.com>
> ---
>   drivers/i2c/rcar_i2c.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

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: hs at denx.de

[U-Boot] [PATCH v3 1/6] i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed

[Heiko Schocher]

Hello Ismael,

Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
> Setting up the delay only needs to be done once; move it to
> rcar_i2c_set_speed so it's done at initialization time.
> 
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
> ---
>   drivers/i2c/rcar_i2c.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)

Your patch does not apply to

base-commit: f08023c07d826fbc8e62fdd3367961b2f0b06844

I get based on f08023c07d826fbc8e62fdd3367961b2f0b06844

Applying: i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed
error: sha1 information is lacking or useless (drivers/i2c/rcar_i2c.c).
error: could not build fake ancestor
Patch failed at 0001 i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

My base:

pollux:u-boot hs [temp] $ glg
* f08023c07d - (HEAD -> temp, tag: v2019.04-rc3, origin/master, origin/HEAD, master) Prepare 
v2019.04-rc4 (2 days ago) <Tom Rini>
*   e102f74d65 - Merge branch 'master' of git://git.denx.de/u-boot-sh (2 days ago) <Tom Rini>
|\
| * 65eef78cdb - pinctrl: renesas: r8a77990: Reivse USB ID pin name (3 days ago) <Hiroyuki Yokoyama>


How does this work for you ?

 > diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
 > index 10b0f8bad4..a57f72a228 100644
 > --- a/drivers/i2c/rcar_i2c.c
 > +++ b/drivers/i2c/rcar_i2c.c
 > @@ -116,9 +116,7 @@ static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
 >   	writel(0, priv->base + RCAR_I2C_ICMSR);
 >   	writel(priv->icccr, priv->base + RCAR_I2C_ICCCR);
 >
 > -	if (priv->type == RCAR_I2C_TYPE_GEN3)
 > -		writel(RCAR_I2C_ICFBSCR_TCYC17, priv->base + RCAR_I2C_ICFBSCR);
 > -
 > +	/* Wait for the bus */
 >   	ret = wait_for_bit_le32(priv->base + RCAR_I2C_ICMCR,
 >   				RCAR_I2C_ICMCR_FSDA, false, 2, true);
 >   	if (ret) {

I do not find RCAR_I2C_TYPE_GEN3 in current mainline code ... please
check!

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: hs at denx.de

[U-Boot] [PATCH v3 1/6] i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed

[Marek Vasut]

On 3/7/19 6:14 AM, Heiko Schocher wrote:
> Hello Ismael,

Hi,

> Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
>> Setting up the delay only needs to be done once; move it to
>> rcar_i2c_set_speed so it's done at initialization time.
>>
>> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
>> ---
>>   drivers/i2c/rcar_i2c.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> Your patch does not apply to
> 
> base-commit: f08023c07d826fbc8e62fdd3367961b2f0b06844
> 
> I get based on f08023c07d826fbc8e62fdd3367961b2f0b06844
> 
> Applying: i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed
> error: sha1 information is lacking or useless (drivers/i2c/rcar_i2c.c).
> error: could not build fake ancestor
> Patch failed at 0001 i2c: rcar_i2c: Setup SCL/SDA delay at
> rcar_i2c_set_speed
> Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> My base:
> 
> pollux:u-boot hs [temp] $ glg
> * f08023c07d - (HEAD -> temp, tag: v2019.04-rc3, origin/master,
> origin/HEAD, master) Prepare v2019.04-rc4 (2 days ago) <Tom Rini>
> *   e102f74d65 - Merge branch 'master' of git://git.denx.de/u-boot-sh (2
> days ago) <Tom Rini>
> |\
> | * 65eef78cdb - pinctrl: renesas: r8a77990: Reivse USB ID pin name (3
> days ago) <Hiroyuki Yokoyama>
> 
> 
> How does this work for you ?

This series applies in top of

[PATCH] i2c: rcar_i2c: Add Gen3 SoC support

[...]

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 1/6] i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed

[Heiko Schocher]

Hello Marek,

Am 07.03.2019 um 09:04 schrieb Marek Vasut:
> On 3/7/19 6:14 AM, Heiko Schocher wrote:
>> Hello Ismael,
> 
> Hi,
> 
>> Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
>>> Setting up the delay only needs to be done once; move it to
>>> rcar_i2c_set_speed so it's done at initialization time.
>>>
>>> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
>>> ---
>>>    drivers/i2c/rcar_i2c.c | 9 ++++++---
>>>    1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> Your patch does not apply to
>>
>> base-commit: f08023c07d826fbc8e62fdd3367961b2f0b06844
>>
>> I get based on f08023c07d826fbc8e62fdd3367961b2f0b06844
>>
>> Applying: i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed
>> error: sha1 information is lacking or useless (drivers/i2c/rcar_i2c.c).
>> error: could not build fake ancestor
>> Patch failed at 0001 i2c: rcar_i2c: Setup SCL/SDA delay at
>> rcar_i2c_set_speed
>> Use 'git am --show-current-patch' to see the failed patch
>> When you have resolved this problem, run "git am --continue".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>>
>> My base:
>>
>> pollux:u-boot hs [temp] $ glg
>> * f08023c07d - (HEAD -> temp, tag: v2019.04-rc3, origin/master,
>> origin/HEAD, master) Prepare v2019.04-rc4 (2 days ago) <Tom Rini>
>> *   e102f74d65 - Merge branch 'master' of git://git.denx.de/u-boot-sh (2
>> days ago) <Tom Rini>
>> |\
>> | * 65eef78cdb - pinctrl: renesas: r8a77990: Reivse USB ID pin name (3
>> days ago) <Hiroyuki Yokoyama>
>>
>>
>> How does this work for you ?
> 
> This series applies in top of
> 
> [PATCH] i2c: rcar_i2c: Add Gen3 SoC support

Ah! Thanks for the hint, I missed this...

Hmm... I thought the patchseries from Ismael as bugfix patch, so
I thought to pick it up for 2019.04 ... but based on the this patch,
is it okay to pick up this stuff, when merge window is open?

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: hs at denx.de

[U-Boot] [PATCH v3 1/6] i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed

[Marek Vasut]

On 3/7/19 9:17 AM, Heiko Schocher wrote:
> Hello Marek,

Hi,

> Am 07.03.2019 um 09:04 schrieb Marek Vasut:
>> On 3/7/19 6:14 AM, Heiko Schocher wrote:
>>> Hello Ismael,
>>
>> Hi,
>>
>>> Am 06.03.2019 um 11:46 schrieb Ismael Luceno Cortes:
>>>> Setting up the delay only needs to be done once; move it to
>>>> rcar_i2c_set_speed so it's done at initialization time.
>>>>
>>>> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
>>>> ---
>>>>    drivers/i2c/rcar_i2c.c | 9 ++++++---
>>>>    1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> Your patch does not apply to
>>>
>>> base-commit: f08023c07d826fbc8e62fdd3367961b2f0b06844
>>>
>>> I get based on f08023c07d826fbc8e62fdd3367961b2f0b06844
>>>
>>> Applying: i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed
>>> error: sha1 information is lacking or useless (drivers/i2c/rcar_i2c.c).
>>> error: could not build fake ancestor
>>> Patch failed at 0001 i2c: rcar_i2c: Setup SCL/SDA delay at
>>> rcar_i2c_set_speed
>>> Use 'git am --show-current-patch' to see the failed patch
>>> When you have resolved this problem, run "git am --continue".
>>> If you prefer to skip this patch, run "git am --skip" instead.
>>> To restore the original branch and stop patching, run "git am --abort".
>>>
>>> My base:
>>>
>>> pollux:u-boot hs [temp] $ glg
>>> * f08023c07d - (HEAD -> temp, tag: v2019.04-rc3, origin/master,
>>> origin/HEAD, master) Prepare v2019.04-rc4 (2 days ago) <Tom Rini>
>>> *   e102f74d65 - Merge branch 'master' of git://git.denx.de/u-boot-sh (2
>>> days ago) <Tom Rini>
>>> |\
>>> | * 65eef78cdb - pinctrl: renesas: r8a77990: Reivse USB ID pin name (3
>>> days ago) <Hiroyuki Yokoyama>
>>>
>>>
>>> How does this work for you ?
>>
>> This series applies in top of
>>
>> [PATCH] i2c: rcar_i2c: Add Gen3 SoC support
> 
> Ah! Thanks for the hint, I missed this...
> 
> Hmm... I thought the patchseries from Ismael as bugfix patch, so
> I thought to pick it up for 2019.04 ... but based on the this patch,
> is it okay to pick up this stuff, when merge window is open?

The Gen3 support is technically a bugfix too, since it's mostly fixing
build warnings on ARM64 . It should be fine to put it into this release.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover

[Heiko Schocher]

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 <ismael.luceno@silicon-gears.com>
> ---
>   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: hs at denx.de

[U-Boot] [PATCH v3 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover

[Ismael Luceno Cortes]

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.