* [U-Boot] [PATCH 2/6] i2c: rcar_i2c: Add comments about registers & values
2019-03-04 14:19 [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings Ismael Luceno Cortes
@ 2019-03-04 14:19 ` Ismael Luceno Cortes
2019-03-04 14:19 ` [U-Boot] [PATCH 3/6] i2c: rcar_i2c: Fix sending of slave addresses Ismael Luceno Cortes
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Ismael Luceno Cortes @ 2019-03-04 14:19 UTC (permalink / raw)
To: u-boot
Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
---
drivers/i2c/rcar_i2c.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
index bd7c37a207..7212587234 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -18,33 +18,33 @@
#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 ((u32)BIT(3))
#define RCAR_I2C_ICMSR_MDT BIT(2)
#define RCAR_I2C_ICMSR_MDR ((u32)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 */
struct rcar_i2c_priv {
void __iomem *base;
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 3/6] i2c: rcar_i2c: Fix sending of slave addresses
2019-03-04 14:19 [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings Ismael Luceno Cortes
2019-03-04 14:19 ` [U-Boot] [PATCH 2/6] i2c: rcar_i2c: Add comments about registers & values Ismael Luceno Cortes
@ 2019-03-04 14:19 ` Ismael Luceno Cortes
2019-03-04 14:19 ` [U-Boot] [PATCH 4/6] i2c: rcar_i2c: Don't mask errors with EREMOTEIO at rcar_i2c_xfer Ismael Luceno Cortes
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Ismael Luceno Cortes @ 2019-03-04 14:19 UTC (permalink / raw)
To: u-boot
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>
---
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 7212587234..8cdd37c006 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -120,9 +120,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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 4/6] i2c: rcar_i2c: Don't mask errors with EREMOTEIO at rcar_i2c_xfer
2019-03-04 14:19 [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings Ismael Luceno Cortes
2019-03-04 14:19 ` [U-Boot] [PATCH 2/6] i2c: rcar_i2c: Add comments about registers & values Ismael Luceno Cortes
2019-03-04 14:19 ` [U-Boot] [PATCH 3/6] i2c: rcar_i2c: Fix sending of slave addresses Ismael Luceno Cortes
@ 2019-03-04 14:19 ` Ismael Luceno Cortes
2019-03-04 14:19 ` [U-Boot] [PATCH 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer Ismael Luceno Cortes
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Ismael Luceno Cortes @ 2019-03-04 14:19 UTC (permalink / raw)
To: u-boot
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 8cdd37c006..b4cc0c55b1 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -207,7 +207,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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer
2019-03-04 14:19 [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings Ismael Luceno Cortes
` (2 preceding siblings ...)
2019-03-04 14:19 ` [U-Boot] [PATCH 4/6] i2c: rcar_i2c: Don't mask errors with EREMOTEIO at rcar_i2c_xfer Ismael Luceno Cortes
@ 2019-03-04 14:19 ` Ismael Luceno Cortes
2019-03-04 19:46 ` Marek Vasut
2019-03-04 14:19 ` [U-Boot] [PATCH 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover Ismael Luceno Cortes
2019-03-04 19:45 ` [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings Marek Vasut
5 siblings, 1 reply; 15+ messages in thread
From: Ismael Luceno Cortes @ 2019-03-04 14:19 UTC (permalink / raw)
To: u-boot
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 b4cc0c55b1..5e04b68d95 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -144,10 +144,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;
@@ -174,10 +170,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);
@@ -201,6 +193,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 -EREMOTEIO;
+
if (msg->flags & I2C_M_RD)
ret = rcar_i2c_read_common(dev, msg);
else
@@ -210,7 +206,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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer
2019-03-04 14:19 ` [U-Boot] [PATCH 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer Ismael Luceno Cortes
@ 2019-03-04 19:46 ` Marek Vasut
2019-03-05 10:46 ` Ismael Luceno Cortes
0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2019-03-04 19:46 UTC (permalink / raw)
To: u-boot
On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
> 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 b4cc0c55b1..5e04b68d95 100644
> --- a/drivers/i2c/rcar_i2c.c
> +++ b/drivers/i2c/rcar_i2c.c
> @@ -144,10 +144,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;
> @@ -174,10 +170,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);
> @@ -201,6 +193,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 -EREMOTEIO;
Shouldn't you return ret here ? It seems like you fixed similar issue in
4/6.
> +
> if (msg->flags & I2C_M_RD)
> ret = rcar_i2c_read_common(dev, msg);
> else
> @@ -210,7 +206,7 @@ static int rcar_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
> return ret;
> }
>
> - return ret;
> + return 0;
Is this change valid ?
> }
>
> static int rcar_i2c_probe_chip(struct udevice *dev, uint addr, uint flags)
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer
2019-03-04 19:46 ` Marek Vasut
@ 2019-03-05 10:46 ` Ismael Luceno Cortes
2019-03-05 10:55 ` Marek Vasut
0 siblings, 1 reply; 15+ messages in thread
From: Ismael Luceno Cortes @ 2019-03-05 10:46 UTC (permalink / raw)
To: u-boot
On 04/Mar/2019 20:46, Marek Vasut wrote:
> On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
> > 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 b4cc0c55b1..5e04b68d95 100644
> > --- a/drivers/i2c/rcar_i2c.c
> > +++ b/drivers/i2c/rcar_i2c.c
> > @@ -144,10 +144,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;
> > @@ -174,10 +170,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);
> > @@ -201,6 +193,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 -EREMOTEIO;
>
> Shouldn't you return ret here ? It seems like you fixed similar issue in
> 4/6.
Correct. Thanks.
> > if (msg->flags & I2C_M_RD)
> > ret = rcar_i2c_read_common(dev, msg);
> > else
> > @@ -210,7 +206,7 @@ static int rcar_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
> > return ret;
> > }
> >
> > - return ret;
> > + return 0;
>
> Is this change valid ?
Yes, ret is always checked, so it can only be 0.
^ permalink raw reply [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer
2019-03-05 10:46 ` Ismael Luceno Cortes
@ 2019-03-05 10:55 ` Marek Vasut
0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2019-03-05 10:55 UTC (permalink / raw)
To: u-boot
On 3/5/19 11:46 AM, Ismael Luceno Cortes wrote:
> On 04/Mar/2019 20:46, Marek Vasut wrote:
>> On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
>>> 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 b4cc0c55b1..5e04b68d95 100644
>>> --- a/drivers/i2c/rcar_i2c.c
>>> +++ b/drivers/i2c/rcar_i2c.c
>>> @@ -144,10 +144,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;
>>> @@ -174,10 +170,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);
>>> @@ -201,6 +193,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 -EREMOTEIO;
>>
>> Shouldn't you return ret here ? It seems like you fixed similar issue in
>> 4/6.
>
> Correct. Thanks.
>
>>> if (msg->flags & I2C_M_RD)
>>> ret = rcar_i2c_read_common(dev, msg);
>>> else
>>> @@ -210,7 +206,7 @@ static int rcar_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
>>> return ret;
>>> }
>>>
>>> - return ret;
>>> + return 0;
>>
>> Is this change valid ?
>
> Yes, ret is always checked, so it can only be 0.
OK
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover
2019-03-04 14:19 [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings Ismael Luceno Cortes
` (3 preceding siblings ...)
2019-03-04 14:19 ` [U-Boot] [PATCH 5/6] i2c: rcar_i2c: Set the slave address from rcar_i2c_xfer Ismael Luceno Cortes
@ 2019-03-04 14:19 ` Ismael Luceno Cortes
2019-03-04 19:47 ` Marek Vasut
2019-03-04 19:45 ` [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings Marek Vasut
5 siblings, 1 reply; 15+ messages in thread
From: Ismael Luceno Cortes @ 2019-03-04 14:19 UTC (permalink / raw)
To: u-boot
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 5e04b68d95..b829e8b20d 100644
--- a/drivers/i2c/rcar_i2c.c
+++ b/drivers/i2c/rcar_i2c.c
@@ -68,7 +68,7 @@ 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;
@@ -93,6 +93,9 @@ static void rcar_i2c_recover(struct udevice *dev)
udelay(5);
writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR);
udelay(5);
+
+ u32 val = readl(priv->base + RCAR_I2C_ICMSR);
+ return val & RCAR_I2C_ICMCR_FSDA;
}
static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
@@ -108,12 +111,11 @@ 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);
+ /* Wait for the bus */
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover
2019-03-04 14:19 ` [U-Boot] [PATCH 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover Ismael Luceno Cortes
@ 2019-03-04 19:47 ` Marek Vasut
2019-03-05 10:03 ` Ismael Luceno Cortes
0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2019-03-04 19:47 UTC (permalink / raw)
To: u-boot
On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
Why is this change needed ? A commit message explaining it would help a lot.
> 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 5e04b68d95..b829e8b20d 100644
> --- a/drivers/i2c/rcar_i2c.c
> +++ b/drivers/i2c/rcar_i2c.c
> @@ -68,7 +68,7 @@ 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;
> @@ -93,6 +93,9 @@ static void rcar_i2c_recover(struct udevice *dev)
> udelay(5);
> writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR);
> udelay(5);
> +
> + u32 val = readl(priv->base + RCAR_I2C_ICMSR);
> + return val & RCAR_I2C_ICMCR_FSDA;
Declare all your variables at the beginning of the function, I believe
the compiler would even complain about this.
> }
>
> static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
> @@ -108,12 +111,11 @@ 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);
>
> + /* Wait for the bus */
> 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;
> }
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover
2019-03-04 19:47 ` Marek Vasut
@ 2019-03-05 10:03 ` Ismael Luceno Cortes
2019-03-05 10:56 ` Marek Vasut
0 siblings, 1 reply; 15+ messages in thread
From: Ismael Luceno Cortes @ 2019-03-05 10:03 UTC (permalink / raw)
To: u-boot
On 04/Mar/2019 20:47, Marek Vasut wrote:
> On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
>
> Why is this change needed ? A commit message explaining it would help a lot.
It's cosmetic. Any call to the recover function would need to do the
same check afterwards, so it' 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 5e04b68d95..b829e8b20d 100644
> > --- a/drivers/i2c/rcar_i2c.c
> > +++ b/drivers/i2c/rcar_i2c.c
> > @@ -68,7 +68,7 @@ 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;
> > @@ -93,6 +93,9 @@ static void rcar_i2c_recover(struct udevice *dev)
> > udelay(5);
> > writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR);
> > udelay(5);
> > +
> > + u32 val = readl(priv->base + RCAR_I2C_ICMSR);
> > + return val & RCAR_I2C_ICMCR_FSDA;
>
> Declare all your variables at the beginning of the function, I believe
> the compiler would even complain about this.
The default mode is -std=gnu11 since GCC 5, and the Makefile defines
CSTD_FLAG to that too.
> > }
> >
> > static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
> > @@ -108,12 +111,11 @@ 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);
> >
> > + /* Wait for the bus */
> > 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;
> > }
^ permalink raw reply [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover
2019-03-05 10:03 ` Ismael Luceno Cortes
@ 2019-03-05 10:56 ` Marek Vasut
0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2019-03-05 10:56 UTC (permalink / raw)
To: u-boot
On 3/5/19 11:03 AM, Ismael Luceno Cortes wrote:
> On 04/Mar/2019 20:47, Marek Vasut wrote:
>> On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
>>
>> Why is this change needed ? A commit message explaining it would help a lot.
>
> It's cosmetic. Any call to the recover function would need to do the
> same check afterwards, so it' sensible to make it part of the function.
This needs to be in the commit message then.
>>> 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 5e04b68d95..b829e8b20d 100644
>>> --- a/drivers/i2c/rcar_i2c.c
>>> +++ b/drivers/i2c/rcar_i2c.c
>>> @@ -68,7 +68,7 @@ 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;
>>> @@ -93,6 +93,9 @@ static void rcar_i2c_recover(struct udevice *dev)
>>> udelay(5);
>>> writel(mcra | RCAR_I2C_ICMCR_FSCL, priv->base + RCAR_I2C_ICMCR);
>>> udelay(5);
>>> +
>>> + u32 val = readl(priv->base + RCAR_I2C_ICMSR);
>>> + return val & RCAR_I2C_ICMCR_FSDA;
>>
>> Declare all your variables at the beginning of the function, I believe
>> the compiler would even complain about this.
>
> The default mode is -std=gnu11 since GCC 5, and the Makefile defines
> CSTD_FLAG to that too.
>
>>> }
>>>
>>> static int rcar_i2c_set_addr(struct udevice *dev, u8 chip, u8 read)
>>> @@ -108,12 +111,11 @@ 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);
>>>
>>> + /* Wait for the bus */
>>> 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;
>>> }
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings
2019-03-04 14:19 [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings Ismael Luceno Cortes
` (4 preceding siblings ...)
2019-03-04 14:19 ` [U-Boot] [PATCH 6/6] i2c: rcar_i2c: Move FSDA check to rcar_i2c_recover Ismael Luceno Cortes
@ 2019-03-04 19:45 ` Marek Vasut
2019-03-05 10:23 ` Ismael Luceno Cortes
5 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2019-03-04 19:45 UTC (permalink / raw)
To: u-boot
On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
Hello Ismael,
the patch is missing commit message, please fix globally.
I sent this patch [1] two days ago, I believe it fixes the same issue,
can you rebase the series on top of it ?
[1] http://patchwork.ozlabs.org/patch/1050650/
> Signed-off-by: Ismael Luceno <ismael.luceno@silicon-gears.com>
> ---
> drivers/i2c/rcar_i2c.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/rcar_i2c.c b/drivers/i2c/rcar_i2c.c
> index 8d87c73713..bd7c37a207 100644
> --- a/drivers/i2c/rcar_i2c.c
> +++ b/drivers/i2c/rcar_i2c.c
> @@ -34,9 +34,9 @@
> #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_MDE BIT(3)
> +#define RCAR_I2C_ICMSR_MDE ((u32)BIT(3))
> #define RCAR_I2C_ICMSR_MDT BIT(2)
> -#define RCAR_I2C_ICMSR_MDR BIT(1)
> +#define RCAR_I2C_ICMSR_MDR ((u32)BIT(1))
> #define RCAR_I2C_ICMSR_MAT BIT(0)
> #define RCAR_I2C_ICSIER 0x10
> #define RCAR_I2C_ICMIER 0x14
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings
2019-03-04 19:45 ` [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings Marek Vasut
@ 2019-03-05 10:23 ` Ismael Luceno Cortes
2019-03-05 10:57 ` Marek Vasut
0 siblings, 1 reply; 15+ messages in thread
From: Ismael Luceno Cortes @ 2019-03-05 10:23 UTC (permalink / raw)
To: u-boot
On 04/Mar/2019 20:45, Marek Vasut wrote:
> On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
>
> Hello Ismael,
>
> the patch is missing commit message, please fix globally.
>
> I sent this patch [1] two days ago, I believe it fixes the same issue,
> can you rebase the series on top of it ?
>
> [1] http://patchwork.ozlabs.org/patch/1050650/
I'll rebase.
BTW, if the ICFBSCR only needs to be written to once, it would make
sense to do it at the end of rcar_i2c_set_speed.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/6] i2c: rcar_i2c: Fix u32 cast warnings
2019-03-05 10:23 ` Ismael Luceno Cortes
@ 2019-03-05 10:57 ` Marek Vasut
0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2019-03-05 10:57 UTC (permalink / raw)
To: u-boot
On 3/5/19 11:23 AM, Ismael Luceno Cortes wrote:
> On 04/Mar/2019 20:45, Marek Vasut wrote:
>> On 3/4/19 3:19 PM, Ismael Luceno Cortes wrote:
>>
>> Hello Ismael,
>>
>> the patch is missing commit message, please fix globally.
>>
>> I sent this patch [1] two days ago, I believe it fixes the same issue,
>> can you rebase the series on top of it ?
>>
>> [1] http://patchwork.ozlabs.org/patch/1050650/
>
> I'll rebase.
>
> BTW, if the ICFBSCR only needs to be written to once, it would make
> sense to do it at the end of rcar_i2c_set_speed.
Can you prepare a patch please ?
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 15+ messages in thread