[U-Boot] [PATCH] i2c: imx: bus arbitration fix when reading block data
Heiko Schocher
hs at denx.de
Tue Jan 16 05:29:36 UTC 2018
Hello Jim,
Am 27.12.2017 um 02:05 schrieb Jim Brennan:
> From e643f91ccfa544b7e3153a7721fba66e0e494759 Mon Sep 17 00:00:00 2001
> From: Jim Brennan <jbrennan at impinj.com>
> 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 <jbrennan at impinj.com>
>
> ---
>
> 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> ("<title line>")' - 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 <stdbool.h>^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 <dm.h>
> #include <dm/pinctrl.h>
> #include <fdtdec.h>
> +#include <stdbool.h>
>
> 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 = (msg->flags & I2C_M_RD) ? true : false;
> bool next_is_read = nmsgs > 1 && (msg[1].flags & I2C_M_RD);
> debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
> - if (msg->flags & I2C_M_RD)
> + if (current_is_read)
> ret = i2c_read_data(i2c_bus, msg->addr, msg->buf,
> msg->len);
> else {
> ret = i2c_write_data(i2c_bus, msg->addr, msg->buf,
> msg->len);
> - if (ret)
> + }
> + if (ret)
> + break;
> +
> + if (nmsgs > 1 && (current_is_read ^ next_is_read)) {
> + u8 addr = msg->addr << 1;
> +
> + /* Reuse ret */
> + ret = readb(base + (I2CR << reg_shift));
> + ret |= I2CR_RSTA;
> + writeb(ret, base + (I2CR << reg_shift));
> +
> + if (next_is_read)
> + addr |= 1;
> +
> + ret = tx_byte(i2c_bus, addr);
> + if (ret < 0)
> break;
> - if (next_is_read) {
> - /* Reuse ret */
> - ret = readb(base + (I2CR << reg_shift));
> - ret |= I2CR_RSTA;
> - writeb(ret, base + (I2CR << reg_shift));
> -
> - ret = tx_byte(i2c_bus, (msg->addr << 1) | 1);
> - if (ret < 0) {
> - i2c_imx_stop(i2c_bus);
> - break;
> - }
> - }
> }
> }
>
>
--
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
More information about the U-Boot
mailing list