[U-Boot] [PATCH 0/4] meson i2c driver cleanups

Some cleanups for the u-boot meson i2c driver.
Beniamino Galvani (4): i2c: meson: improve Kconfig description i2c: meson: reduce timeout i2c: meson: fix return codes on error i2c: meson: add some comments
drivers/i2c/Kconfig | 7 ++++++- drivers/i2c/meson_i2c.c | 31 ++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 10 deletions(-)

Expand the Kconfig description with hardware features.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com --- drivers/i2c/Kconfig | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 1989f8eb57..a7931aa6d4 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -141,7 +141,12 @@ config SYS_I2C_MESON bool "Amlogic Meson I2C driver" depends on DM_I2C && ARCH_MESON help - Add support for the Amlogic Meson I2C driver. + Add support for the I2C controller available in Amlogic Meson + SoCs. The controller supports programmable bus speed including + standard (100kbits/s) and fast (400kbit/s) speed and allows the + software to define a flexible format of the bit streams. It has an + internal buffer holding up to 8 bytes for transfers and supports + both 7-bit and 10-bit addresses.
config SYS_I2C_MXC bool "NXP i.MX I2C driver"

On 26 November 2017 at 09:40, Beniamino Galvani b.galvani@gmail.com wrote:
Expand the Kconfig description with hardware features.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/Kconfig | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 1989f8eb57..a7931aa6d4 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -141,7 +141,12 @@ config SYS_I2C_MESON bool "Amlogic Meson I2C driver" depends on DM_I2C && ARCH_MESON help
Add support for the Amlogic Meson I2C driver.
Add support for the I2C controller available in Amlogic Meson
SoCs. The controller supports programmable bus speed including
standard (100kbits/s) and fast (400kbit/s) speed and allows the
software to define a flexible format of the bit streams. It has an
internal buffer holding up to 8 bytes for transfers and supports
both 7-bit and 10-bit addresses.
config SYS_I2C_MXC bool "NXP i.MX I2C driver" -- 2.14.3
Great!
Reviewed-by: Simon Glass sjg@chromium.org

Hello Baniamino,
Am 26.11.2017 um 17:40 schrieb Beniamino Galvani:
Expand the Kconfig description with hardware features.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/Kconfig | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Thanks!
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@denx.de

Hello Beniamino,
Am 26.11.2017 um 17:40 schrieb Beniamino Galvani:
Expand the Kconfig description with hardware features.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/Kconfig | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Applied to u-boot-i2c master
Thanks!
bye, Heiko

The datasheet doesn't specify a suggested timeout and 500ms seems very long: reduce it to 100ms.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com --- drivers/i2c/meson_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/meson_i2c.c b/drivers/i2c/meson_i2c.c index 2434d9ed53..84e1997c76 100644 --- a/drivers/i2c/meson_i2c.c +++ b/drivers/i2c/meson_i2c.c @@ -9,7 +9,7 @@ #include <dm.h> #include <i2c.h>
-#define I2C_TIMEOUT_MS 500 +#define I2C_TIMEOUT_MS 100
/* Control register fields */ #define REG_CTRL_START BIT(0)

On 26 November 2017 at 09:40, Beniamino Galvani b.galvani@gmail.com wrote:
The datasheet doesn't specify a suggested timeout and 500ms seems very long: reduce it to 100ms.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/meson_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hello Beniamino,
Am 26.11.2017 um 17:40 schrieb Beniamino Galvani:
The datasheet doesn't specify a suggested timeout and 500ms seems very long: reduce it to 100ms.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/meson_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks!
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko

Hello Beniamino,
Am 26.11.2017 um 17:40 schrieb Beniamino Galvani:
The datasheet doesn't specify a suggested timeout and 500ms seems very long: reduce it to 100ms.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/meson_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to u-boot-i2c master
Thanks!
bye, Heiko

Change meson_i2c_xfer_msg() to return -EREMOTEIO in case of NACK, as done by other drivers. Also, don't change the return error in meson_i2c_xfer().
Signed-off-by: Beniamino Galvani b.galvani@gmail.com --- drivers/i2c/meson_i2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/meson_i2c.c b/drivers/i2c/meson_i2c.c index 84e1997c76..2f39214ad2 100644 --- a/drivers/i2c/meson_i2c.c +++ b/drivers/i2c/meson_i2c.c @@ -178,7 +178,7 @@ static int meson_i2c_xfer_msg(struct meson_i2c *i2c, struct i2c_msg *msg,
if (readl(&i2c->regs->ctrl) & REG_CTRL_ERROR) { debug("meson i2c: error\n"); - return -ENXIO; + return -EREMOTEIO; }
if ((msg->flags & I2C_M_RD) && i2c->count) { @@ -200,7 +200,7 @@ static int meson_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, for (i = 0; i < nmsgs; i++) { ret = meson_i2c_xfer_msg(i2c, msg + i, i == nmsgs - 1); if (ret) - return -EREMOTEIO; + return ret; }
return 0;

On 26 November 2017 at 09:40, Beniamino Galvani b.galvani@gmail.com wrote:
Change meson_i2c_xfer_msg() to return -EREMOTEIO in case of NACK, as done by other drivers. Also, don't change the return error in meson_i2c_xfer().
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/meson_i2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hello Beniamino,
Am 26.11.2017 um 17:40 schrieb Beniamino Galvani:
Change meson_i2c_xfer_msg() to return -EREMOTEIO in case of NACK, as done by other drivers. Also, don't change the return error in meson_i2c_xfer().
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/meson_i2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Thanks!
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko

Hello Beniamino,
Am 26.11.2017 um 17:40 schrieb Beniamino Galvani:
Change meson_i2c_xfer_msg() to return -EREMOTEIO in case of NACK, as done by other drivers. Also, don't change the return error in meson_i2c_xfer().
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/meson_i2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Applied to u-boot-i2c master
Thanks!
bye, Heiko

Add some comment describing the purpose of struct members and functions.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com --- drivers/i2c/meson_i2c.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/meson_i2c.c b/drivers/i2c/meson_i2c.c index 2f39214ad2..4f37d2f316 100644 --- a/drivers/i2c/meson_i2c.c +++ b/drivers/i2c/meson_i2c.c @@ -44,12 +44,12 @@ struct i2c_regs {
struct meson_i2c { struct i2c_regs *regs; - struct i2c_msg *msg; - bool last; - uint count; - uint pos; - u32 tokens[2]; - uint num_tokens; + struct i2c_msg *msg; /* Current I2C message */ + bool last; /* Whether the message is the last */ + uint count; /* Number of bytes in the current transfer */ + uint pos; /* Position of current transfer in message */ + u32 tokens[2]; /* Sequence of tokens to be written */ + uint num_tokens; /* Number of tokens to be written */ };
static void meson_i2c_reset_tokens(struct meson_i2c *i2c) @@ -69,6 +69,10 @@ static void meson_i2c_add_token(struct meson_i2c *i2c, int token) i2c->num_tokens++; }
+/* + * Retrieve data for the current transfer (which can be at most 8 + * bytes) from the device internal buffer. + */ static void meson_i2c_get_data(struct meson_i2c *i2c, u8 *buf, int len) { u32 rdata0, rdata1; @@ -86,6 +90,10 @@ static void meson_i2c_get_data(struct meson_i2c *i2c, u8 *buf, int len) *buf++ = (rdata1 >> (i - 4) * 8) & 0xff; }
+/* + * Write data for the current transfer (which can be at most 8 bytes) + * to the device internal buffer. + */ static void meson_i2c_put_data(struct meson_i2c *i2c, u8 *buf, int len) { u32 wdata0 = 0, wdata1 = 0; @@ -103,6 +111,11 @@ static void meson_i2c_put_data(struct meson_i2c *i2c, u8 *buf, int len) debug("meson i2c: write data %08x %08x len %d\n", wdata0, wdata1, len); }
+/* + * Prepare the next transfer: pick the next 8 bytes in the remaining + * part of message and write tokens and data (if needed) to the + * device. + */ static void meson_i2c_prepare_xfer(struct meson_i2c *i2c) { bool write = !(i2c->msg->flags & I2C_M_RD);

On 26 November 2017 at 09:40, Beniamino Galvani b.galvani@gmail.com wrote:
Add some comment describing the purpose of struct members and functions.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/meson_i2c.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hello Beniammino,
Am 26.11.2017 um 17:40 schrieb Beniamino Galvani:
Add some comment describing the purpose of struct members and functions.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/meson_i2c.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
Thanks!
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko

Hello Beniamino,
Am 26.11.2017 um 17:40 schrieb Beniamino Galvani:
Add some comment describing the purpose of struct members and functions.
Signed-off-by: Beniamino Galvani b.galvani@gmail.com
drivers/i2c/meson_i2c.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
Applied to u-boot-i2c master
Thanks!
bye, Heiko
participants (3)
-
Beniamino Galvani
-
Heiko Schocher
-
Simon Glass