Re: [U-Boot] [PATCH] i2c: imx: bus arbitration fix when reading block data

[Heiko Schocher]

Hello Breno,

Am 16.01.2018 um 15:36 schrieb Breno Matheus Lima:

Hi Jim,

2018-01-16 10:47 GMT-02:00 Heiko Schocher :

Hello Fabio,

Am 16.01.2018 um 12:50 schrieb Fabio Estevam:


Hi Heiko,

On Tue, Jan 16, 2018 at 3:29 AM, Heiko Schocher  wrote:


And some Tested-by would be helpful ...

@Stefano, @Fabio: Any chance to test this change? Thanks!



Breno on Cc will do a quick test on this one.


I tested your patch on a mx6sabresd board and U-Boot is hanging in
power_init_board():

U-Boot SPL 2018.01-39204-g3244ca3 (Jan 16 2018 - 12:13:25)
Trying to boot from MMC1


U-Boot 2018.01-39204-g3244ca3 (Jan 16 2018 - 12:13:25 -0200)

CPU:   Freescale i.MX6QP rev1.0 996 MHz (running at 792 MHz)
CPU:   Automotive temperature grade (-40C to 125C) at 25C
Reset cause: POR
Board: MX6-SabreSD
I2C:   ready
DRAM:  1 GiB


With DEBUG mode enabled I'm getting the following output:

U-Boot 2018.01-39204-g3244ca3-dirty (Jan 16 2018 - 12:24:34 -0200)

CPU:   Freescale i.MX6QP rev1.0 996 MHz (running at 792 MHz)
CPU:   Automotive temperature grade (-40C to 125C) at 36C
Reset cause: POR
Board: MX6-SabreSD
I2C:   ready
DRAM:  1 GiB
pmic_alloc: new pmic struct: 0x4f56e1e8
pmic_get: pmic PFUZE100 -> 0x4f56e1e8
i2c_write_data: chip=0x8, len=0x0
write_data:

I also tested on a mx7dsabresd and on this board your patch is working
fine. If you need more details please let me know.


Many thanks for testing!

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] i2c: imx: bus arbitration fix when reading block data

[Fabio Estevam]

Hi Breno,

On Tue, Jan 16, 2018 at 12:36 PM, Breno Matheus Lima
 wrote:

> I tested your patch on a mx6sabresd board and U-Boot is hanging in
> power_init_board():

Thanks for testing!

Seems like this patch needs a rework then.

Thanks
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] i2c: imx: bus arbitration fix when reading block data

[Breno Matheus Lima]

Hi Jim,

2018-01-16 10:47 GMT-02:00 Heiko Schocher :
> Hello Fabio,
>
> Am 16.01.2018 um 12:50 schrieb Fabio Estevam:
>>
>> Hi Heiko,
>>
>> On Tue, Jan 16, 2018 at 3:29 AM, Heiko Schocher  wrote:
>>
>>> And some Tested-by would be helpful ...
>>>
>>> @Stefano, @Fabio: Any chance to test this change? Thanks!
>>
>>
>> Breno on Cc will do a quick test on this one.

I tested your patch on a mx6sabresd board and U-Boot is hanging in
power_init_board():

U-Boot SPL 2018.01-39204-g3244ca3 (Jan 16 2018 - 12:13:25)
Trying to boot from MMC1


U-Boot 2018.01-39204-g3244ca3 (Jan 16 2018 - 12:13:25 -0200)

CPU:   Freescale i.MX6QP rev1.0 996 MHz (running at 792 MHz)
CPU:   Automotive temperature grade (-40C to 125C) at 25C
Reset cause: POR
Board: MX6-SabreSD
I2C:   ready
DRAM:  1 GiB


With DEBUG mode enabled I'm getting the following output:

U-Boot 2018.01-39204-g3244ca3-dirty (Jan 16 2018 - 12:24:34 -0200)

CPU:   Freescale i.MX6QP rev1.0 996 MHz (running at 792 MHz)
CPU:   Automotive temperature grade (-40C to 125C) at 36C
Reset cause: POR
Board: MX6-SabreSD
I2C:   ready
DRAM:  1 GiB
pmic_alloc: new pmic struct: 0x4f56e1e8
pmic_get: pmic PFUZE100 -> 0x4f56e1e8
i2c_write_data: chip=0x8, len=0x0
write_data:

I also tested on a mx7dsabresd and on this board your patch is working
fine. If you need more details please let me know.

Thanks,
Breno Lima
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] i2c: imx: bus arbitration fix when reading block data

[Heiko Schocher]

Hello Fabio,

Am 16.01.2018 um 12:50 schrieb Fabio Estevam:

Hi Heiko,

On Tue, Jan 16, 2018 at 3:29 AM, Heiko Schocher  wrote:


And some Tested-by would be helpful ...

@Stefano, @Fabio: Any chance to test this change? Thanks!


Breno on Cc will do a quick test on this one.


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] i2c: imx: bus arbitration fix when reading block data

[Fabio Estevam]

Hi Heiko,

On Tue, Jan 16, 2018 at 3:29 AM, Heiko Schocher  wrote:

> And some Tested-by would be helpful ...
>
> @Stefano, @Fabio: Any chance to test this change? Thanks!

Breno on Cc will do a quick test on this one.

Regards,

Fabio Estevam
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] i2c: imx: bus arbitration fix when reading block data

[Heiko Schocher]

Hello Jim,

Am 27.12.2017 um 02:05 schrieb Jim Brennan:

 From e643f91ccfa544b7e3153a7721fba66e0e494759 Mon Sep 17 00:00:00 2001
From: Jim Brennan 
Date: Wed, 13 Dec 2017 13:44:02 -0800
Subject: [PATCH] i2c: imx: bus arbitration fix when reading block data

Fixes arbitration failure on imx platform due to incorrect
chip address use when reading a block of data.  Add support
for both reading or writing a block of data or any combination.

Signed-off-by: Jim Brennan 

---

  drivers/i2c/mxc_i2c.c | 65 ---
  1 file changed, 41 insertions(+), 24 deletions(-)


Your patch drops a lot of checkpatch warnings, see:

http://xeidos.ddns.net/tbot/id_590/tbot.txt

and search for "2018-01-16 02:41:58" to see the wget cmd, to get your patch 
from aptchwork
and search for "2018-01-16 02:42:00" to see the checkpatch.pl command.

ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("")' - ie: 
'commit fatal: bad o ("721fba66e0e494759")'
#15: 

>From e643f91ccfa544b7e3153a7721fba66e0e494759 Mon Sep 17 00:00:00 2001 



ERROR: DOS line endings
#42: FILE: drivers/i2c/mxc_i2c.c:28:
+#include ^M$
[...]

Please fix this, and resend your patch, thanks!

And some Tested-by would be helpful ...

@Stefano, @Fabio: Any chance to test this change? Thanks!

bye,
Heiko


diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 205274e947..bd2a39ebe6 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -25,6 +25,7 @@
  #include 
  #include 
  #include 
+#include 
  
  DECLARE_GLOBAL_DATA_PTR;
  
@@ -275,7 +276,7 @@ static void i2c_imx_stop(struct mxc_i2c_bus *i2c_bus)

   * write register address
   */
  static int i2c_init_transfer_(struct mxc_i2c_bus *i2c_bus, u8 chip,
- u32 addr, int alen)
+ u32 addr, int alen, bool read)
  {
unsigned int temp;
int ret;
@@ -317,9 +318,17 @@ static int i2c_init_transfer_(struct mxc_i2c_bus *i2c_bus, 
u8 chip,
temp |= I2CR_MTX | I2CR_TX_NO_AK;
writeb(temp, base + (I2CR << reg_shift));
  
-	if (alen >= 0)	{

-   /* write slave address */
-   ret = tx_byte(i2c_bus, chip << 1);
+   /* write slave address */
+   u8 slave_address = (chip << 1);
+
+   if (read)
+   slave_address |= 1;
+   ret = tx_byte(i2c_bus, slave_address);
+   if (ret < 0)
+   return ret;
+
+   while (alen--) {
+   ret = tx_byte(i2c_bus, (addr >> (alen * 8)) & 0xff);
if (ret < 0)
return ret;
  
@@ -413,7 +422,7 @@ exit:

  #endif
  
  static int i2c_init_transfer(struct mxc_i2c_bus *i2c_bus, u8 chip,

-u32 addr, int alen)
+u32 addr, int alen, bool read)
  {
int retry;
int ret;
@@ -424,7 +433,7 @@ static int i2c_init_transfer(struct mxc_i2c_bus *i2c_bus, 
u8 chip,
return -EINVAL;
  
  	for (retry = 0; retry < 3; retry++) {

-   ret = i2c_init_transfer_(i2c_bus, chip, addr, alen);
+   ret = i2c_init_transfer_(i2c_bus, chip, addr, alen, read);
if (ret >= 0)
return 0;
i2c_imx_stop(i2c_bus);
@@ -536,7 +545,7 @@ static int bus_i2c_read(struct mxc_i2c_bus *i2c_bus, u8 
chip, u32 addr,
VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
ulong base = i2c_bus->base;
  
-	ret = i2c_init_transfer(i2c_bus, chip, addr, alen);

+   ret = i2c_init_transfer(i2c_bus, chip, addr, alen, false);
if (ret < 0)
return ret;
  
@@ -566,7 +575,7 @@ static int bus_i2c_write(struct mxc_i2c_bus *i2c_bus, u8 chip, u32 addr,

  {
int ret = 0;
  
-	ret = i2c_init_transfer(i2c_bus, chip, addr, alen);

+   ret = i2c_init_transfer(i2c_bus, chip, addr, alen, false);
if (ret < 0)
return ret;
  
@@ -817,7 +826,7 @@ static int mxc_i2c_probe_chip(struct udevice *bus, u32 chip_addr,

int ret;
struct mxc_i2c_bus *i2c_bus = dev_get_priv(bus);
  
-	ret = i2c_init_transfer(i2c_bus, chip_addr, 0, 0);

+   ret = i2c_init_transfer(i2c_bus, chip_addr, 0, 0, false);
if (ret < 0) {
debug("%s failed, ret = %d\n", __func__, ret);
return ret;
@@ -841,35 +850,43 @@ static int mxc_i2c_xfer(struct udevice *bus, struct 
i2c_msg *msg, int nmsgs)
 * because here we only want to send out chip address. The register
 * address is wrapped in msg.
 */
-   ret = i2c_init_transfer(i2c_bus, msg->addr, 0, 0);
+
+   bool read = (msg->flags & I2C_M_RD) ? true : false;
+
+   ret = i2c_init_transfer(i2c_bus, msg->addr, 0, 0, read);
if (ret < 0) {
debug("i2c_init_transfer error: %d\n", ret);
return ret;
}
  
  	for (; nmsgs > 0; nmsgs--, msg++) {

+   bool current_is_read =