[U-Boot] [PATCH v2 1/6] i2c: rcar_i2c: Setup SCL/SDA delay at rcar_i2c_set_speed
It only needs to be done once.
Signed-off-by: Ismael Luceno ismael.luceno@silicon-gears.com --- drivers/i2c/rcar_i2c.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c index 10b0f8bad4..74643b085e 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,10 @@ 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
Signed-off-by: Ismael Luceno ismael.luceno@silicon-gears.com ---
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support")
drivers/i2c/rcar_i2c.c | 45 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c index 74643b085e..c1a233b6e9 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -18,35 +18,36 @@ #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_ICFBSCR 0x38 -#define RCAR_I2C_ICFBSCR_TCYC17 0x0f +#define RCAR_I2C_ICSAR 0x1c /* slave address */ +#define RCAR_I2C_ICMAR 0x20 /* master address */ +#define RCAR_I2C_ICRXD_ICTXD 0x24 /* data port */ +#define RCAR_I2C_ICFBSCR 0x38 /* First Bit Setup Cycle (Gen3) */ +#define RCAR_I2C_ICFBSCR_TCYC17 0x0f /* 17*Tcyc delay 1st bit + between SDA and SCL */
enum rcar_i2c_type { RCAR_I2C_TYPE_GEN2,
On 3/5/19 12:16 PM, Ismael Luceno Cortes wrote:
Commit message is missing :(
Signed-off-by: Ismael Luceno ismael.luceno@silicon-gears.com
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support")
Thanks!
drivers/i2c/rcar_i2c.c | 45 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c index 74643b085e..c1a233b6e9 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -18,35 +18,36 @@ #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_ICFBSCR 0x38 -#define RCAR_I2C_ICFBSCR_TCYC17 0x0f +#define RCAR_I2C_ICSAR 0x1c /* slave address */ +#define RCAR_I2C_ICMAR 0x20 /* master address */ +#define RCAR_I2C_ICRXD_ICTXD 0x24 /* data port */ +#define RCAR_I2C_ICFBSCR 0x38 /* First Bit Setup Cycle (Gen3) */ +#define RCAR_I2C_ICFBSCR_TCYC17 0x0f /* 17*Tcyc delay 1st bit
between SDA and SCL */enum rcar_i2c_type { RCAR_I2C_TYPE_GEN2,
Checkpatch is complaining:
WARNING: line over 80 characters #137: FILE: drivers/i2c/rcar_i2c.c:47: +#define RCAR_I2C_ICFBSCR 0x38 /* First Bit Setup Cycle (Gen3) */
WARNING: Block comments use * on subsequent lines #139: FILE: drivers/i2c/rcar_i2c.c:49: +#define RCAR_I2C_ICFBSCR_TCYC17 0x0f /* 17*Tcyc delay 1st bit + between SDA and SCL */
WARNING: Block comments use a trailing */ on a separate line #139: FILE: drivers/i2c/rcar_i2c.c:49: + between SDA and SCL */
total: 1 errors, 3 warnings, 0 checks, 60 lines checked
Please fix and run scripts/checkpatch.pl on the patches before posting them.
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 ---
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support")
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 c1a233b6e9..7131f0c994 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -130,9 +130,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);
On 3/5/19 12:16 PM, 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
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support")
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 c1a233b6e9..7131f0c994 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -130,9 +130,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);
Signed-off-by: Ismael Luceno ismael.luceno@silicon-gears.com ---
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support")
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 7131f0c994..d6b0418290 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -217,7 +217,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;
On 3/5/19 12:16 PM, Ismael Luceno Cortes wrote:
Commit message is missing :-(
Signed-off-by: Ismael Luceno ismael.luceno@silicon-gears.com
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support")
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 7131f0c994..d6b0418290 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -217,7 +217,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;
Signed-off-by: Ismael Luceno ismael.luceno@silicon-gears.com ---
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support") - Fixed masking of return value from rcar_i2c_set_addr
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 d6b0418290..4556b115bd 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -154,10 +154,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; @@ -184,10 +180,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); @@ -211,6 +203,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 @@ -220,7 +216,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)
On 3/5/19 12:16 PM, Ismael Luceno Cortes wrote:
Commit message is missing :-(
The patch itself is fine though. Can you fill the commit message and do a V3 of the series ?
Signed-off-by: Ismael Luceno ismael.luceno@silicon-gears.com
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support") - Fixed masking of return value from rcar_i2c_set_addr
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 d6b0418290..4556b115bd 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -154,10 +154,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;
@@ -184,10 +180,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);
@@ -211,6 +203,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
@@ -220,7 +216,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)
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 ---
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support") - Explained the change - Replaced C99-style variable declaration
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 4556b115bd..21bcf09101 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -77,12 +77,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 ret;
/* Send 9 SCL pulses */ for (i = 0; i < 9; i++) { @@ -102,6 +103,9 @@ static void rcar_i2c_recover(struct udevice *dev) udelay(5); writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR); udelay(5); + + ret = readl(priv->base + RCAR_I2C_ICMSR); + return ret & RCAR_I2C_ICMCR_FSDA; }
static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read) @@ -121,9 +125,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; }
On 3/5/19 12:23 PM, 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
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support") - Explained the change - Replaced C99-style variable declaration
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 4556b115bd..21bcf09101 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -77,12 +77,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 ret;
Can you change this to "reg" or something like that ? $ret is usually a signed int return value, so this could be confusing.
/* Send 9 SCL pulses */ for (i = 0; i < 9; i++) { @@ -102,6 +103,9 @@ static void rcar_i2c_recover(struct udevice *dev) udelay(5); writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR); udelay(5);
- ret = readl(priv->base + RCAR_I2C_ICMSR);
- return ret & RCAR_I2C_ICMCR_FSDA;
What about doing this instead
reg = readl... if (reg & ...FSDA) return -EBUSY;
return 0;
That way, the code will use standard errno.h return values.
}
static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read) @@ -121,9 +125,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;
On 05/Mar/2019 19:32, Marek Vasut wrote:
On 3/5/19 12:23 PM, 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
Notes: Changes since v1: - Rebased on top of patch 1050650 ("i2c: rcar_i2c: Add Gen3 SoC support") - Explained the change - Replaced C99-style variable declaration
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 4556b115bd..21bcf09101 100644 --- a/drivers/i2c/rcar_i2c.c +++ b/drivers/i2c/rcar_i2c.c @@ -77,12 +77,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 ret;
Can you change this to "reg" or something like that ? $ret is usually a signed int return value, so this could be confusing.
Ok.
/* Send 9 SCL pulses */ for (i = 0; i < 9; i++) { @@ -102,6 +103,9 @@ static void rcar_i2c_recover(struct udevice *dev) udelay(5); writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR); udelay(5);
- ret = readl(priv->base + RCAR_I2C_ICMSR);
- return ret & RCAR_I2C_ICMCR_FSDA;
What about doing this instead
reg = readl... if (reg & ...FSDA) return -EBUSY;
return 0;
That way, the code will use standard errno.h return values.
Makes sense.
On 3/5/19 12:16 PM, Ismael Luceno Cortes wrote:
It only needs to be done once.
As much as I hate to do it, I need to ask you to reword the commit message again.
When I look at just the commit message and I read "It only needs to be done once.", I literally have no clue what the change does. I need to look into the patch.
Signed-off-by: Ismael Luceno ismael.luceno@silicon-gears.com
drivers/i2c/rcar_i2c.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c index 10b0f8bad4..74643b085e 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,10 @@ 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);
Please add brackets around the multi-line code, so it's obvious what is in the if conditional and what is not.
return 0; }
base-commit: f08023c07d826fbc8e62fdd3367961b2f0b06844 prerequisite-patch-id: 9e5b0458bc15640eb483ccad91dbe85150f9f7be
participants (2)
-
Ismael Luceno Cortes -
Marek Vasut