[U-Boot] [PATCH] i2c: mxs: Add I2C multibus support

The i.MX28 has two I2C IP blocks, but the MXS I2C driver is hard-coded to use the I2C block 0 . Add multibus support so we can use both I2C busses as seen fit.
Signed-off-by: Marek Vasut marex@denx.de Cc: Stefano Babic sbabic@denx.de --- drivers/i2c/mxs_i2c.c | 47 +++++++++++++++++++++++++++++++++++++---------- include/configs/mxs.h | 1 + 2 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c index de3b194..989cf07 100644 --- a/drivers/i2c/mxs_i2c.c +++ b/drivers/i2c/mxs_i2c.c @@ -24,9 +24,12 @@
#define MXS_I2C_MAX_TIMEOUT 1000000
+static struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; +static int mxs_i2c_bus; +static int mxs_i2c_speed = CONFIG_SYS_I2C_SPEED; + static void mxs_i2c_reset(void) { - struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; int ret; int speed = i2c_get_bus_speed();
@@ -48,8 +51,6 @@ static void mxs_i2c_reset(void)
static void mxs_i2c_setup_read(uint8_t chip, int len) { - struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; - writel(I2C_QUEUECMD_RETAIN_CLOCK | I2C_QUEUECMD_PRE_SEND_START | I2C_QUEUECMD_MASTER_MODE | I2C_QUEUECMD_DIRECTION | (1 << I2C_QUEUECMD_XFER_COUNT_OFFSET), @@ -67,7 +68,6 @@ static void mxs_i2c_setup_read(uint8_t chip, int len) static int mxs_i2c_write(uchar chip, uint addr, int alen, uchar *buf, int blen, int stop) { - struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; uint32_t data, tmp; int i, remain, off; int timeout = MXS_I2C_MAX_TIMEOUT; @@ -124,7 +124,6 @@ static int mxs_i2c_write(uchar chip, uint addr, int alen,
static int mxs_i2c_wait_for_ack(void) { - struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; uint32_t tmp; int timeout = MXS_I2C_MAX_TIMEOUT;
@@ -162,7 +161,6 @@ err:
int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len) { - struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; uint32_t tmp = 0; int timeout = MXS_I2C_MAX_TIMEOUT; int ret; @@ -237,7 +235,6 @@ int i2c_probe(uchar chip)
int i2c_set_bus_speed(unsigned int speed) { - struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; /* * The timing derivation algorithm. There is no documentation for this * algorithm available, it was derived by using the scope and fiddling @@ -278,7 +275,6 @@ int i2c_set_bus_speed(unsigned int speed)
unsigned int i2c_get_bus_speed(void) { - struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; uint32_t clk = mxc_get_clock(MXC_XTAL_CLK); uint32_t timing0;
@@ -290,10 +286,41 @@ unsigned int i2c_get_bus_speed(void) return clk / ((((timing0 >> 16) - 3) * 2) + 38); }
-void i2c_init(int speed, int slaveadd) +int i2c_set_bus_num(unsigned int bus) { + uint32_t mxs_i2c_regs; + + switch (bus) { + case 0: + mxs_i2c_regs = MXS_I2C0_BASE; + break; +#ifdef CONFIG_MX28 + case 1: + mxs_i2c_regs = MXS_I2C1_BASE; + break; +#endif + default: + printf("Bad bus: %d\n", bus); + return -EINVAL; + } + + mxs_i2c_bus = bus; + i2c_regs = (struct mxs_i2c_regs *)mxs_i2c_regs; + mxs_i2c_reset(); - i2c_set_bus_speed(speed); + i2c_set_bus_speed(mxs_i2c_speed);
+ return 0; +} + +unsigned int i2c_get_bus_num(void) +{ + return mxs_i2c_bus; +} + +void i2c_init(int speed, int slaveadd) +{ + mxs_i2c_speed = speed; + i2c_set_bus_num(0); return; } diff --git a/include/configs/mxs.h b/include/configs/mxs.h index eb96fc1..3936029 100644 --- a/include/configs/mxs.h +++ b/include/configs/mxs.h @@ -149,6 +149,7 @@ /* I2C */ #ifdef CONFIG_CMD_I2C #define CONFIG_I2C_MXS +#define CONFIG_I2C_MULTI_BUS #define CONFIG_HARD_I2C #ifndef CONFIG_SYS_I2C_SPEED #define CONFIG_SYS_I2C_SPEED 400000

Hello Marek,
Am 23.09.2014 13:15, schrieb Marek Vasut:
The i.MX28 has two I2C IP blocks, but the MXS I2C driver is hard-coded to use the I2C block 0 . Add multibus support so we can use both I2C busses as seen fit.
Signed-off-by: Marek Vasutmarex@denx.de Cc: Stefano Babicsbabic@denx.de
drivers/i2c/mxs_i2c.c | 47 +++++++++++++++++++++++++++++++++++++---------- include/configs/mxs.h | 1 + 2 files changed, 38 insertions(+), 10 deletions(-)
Sorry, I have to NACK this patch. Please convert the driver for using the CONFIG_SYS_I2C framework, see for example the drivers/i2c/mxc_i2c.c driver, thanks!
bye, Heiko
diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c index de3b194..989cf07 100644 --- a/drivers/i2c/mxs_i2c.c +++ b/drivers/i2c/mxs_i2c.c @@ -24,9 +24,12 @@
#define MXS_I2C_MAX_TIMEOUT 1000000
+static struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; +static int mxs_i2c_bus; +static int mxs_i2c_speed = CONFIG_SYS_I2C_SPEED;
- static void mxs_i2c_reset(void) {
- struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; int ret; int speed = i2c_get_bus_speed();
@@ -48,8 +51,6 @@ static void mxs_i2c_reset(void)
static void mxs_i2c_setup_read(uint8_t chip, int len) {
- struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE;
- writel(I2C_QUEUECMD_RETAIN_CLOCK | I2C_QUEUECMD_PRE_SEND_START | I2C_QUEUECMD_MASTER_MODE | I2C_QUEUECMD_DIRECTION | (1<< I2C_QUEUECMD_XFER_COUNT_OFFSET),
@@ -67,7 +68,6 @@ static void mxs_i2c_setup_read(uint8_t chip, int len) static int mxs_i2c_write(uchar chip, uint addr, int alen, uchar *buf, int blen, int stop) {
- struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; uint32_t data, tmp; int i, remain, off; int timeout = MXS_I2C_MAX_TIMEOUT;
@@ -124,7 +124,6 @@ static int mxs_i2c_write(uchar chip, uint addr, int alen,
static int mxs_i2c_wait_for_ack(void) {
- struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; uint32_t tmp; int timeout = MXS_I2C_MAX_TIMEOUT;
@@ -162,7 +161,6 @@ err:
int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len) {
- struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; uint32_t tmp = 0; int timeout = MXS_I2C_MAX_TIMEOUT; int ret;
@@ -237,7 +235,6 @@ int i2c_probe(uchar chip)
int i2c_set_bus_speed(unsigned int speed) {
- struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; /*
- The timing derivation algorithm. There is no documentation for this
- algorithm available, it was derived by using the scope and fiddling
@@ -278,7 +275,6 @@ int i2c_set_bus_speed(unsigned int speed)
unsigned int i2c_get_bus_speed(void) {
- struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; uint32_t clk = mxc_get_clock(MXC_XTAL_CLK); uint32_t timing0;
@@ -290,10 +286,41 @@ unsigned int i2c_get_bus_speed(void) return clk / ((((timing0>> 16) - 3) * 2) + 38); }
-void i2c_init(int speed, int slaveadd) +int i2c_set_bus_num(unsigned int bus) {
- uint32_t mxs_i2c_regs;
- switch (bus) {
- case 0:
mxs_i2c_regs = MXS_I2C0_BASE;
break;
+#ifdef CONFIG_MX28
- case 1:
mxs_i2c_regs = MXS_I2C1_BASE;
break;
+#endif
- default:
printf("Bad bus: %d\n", bus);
return -EINVAL;
- }
- mxs_i2c_bus = bus;
- i2c_regs = (struct mxs_i2c_regs *)mxs_i2c_regs;
- mxs_i2c_reset();
- i2c_set_bus_speed(speed);
i2c_set_bus_speed(mxs_i2c_speed);
return 0;
+}
+unsigned int i2c_get_bus_num(void) +{
- return mxs_i2c_bus;
+}
+void i2c_init(int speed, int slaveadd) +{
- mxs_i2c_speed = speed;
- i2c_set_bus_num(0); return; }
diff --git a/include/configs/mxs.h b/include/configs/mxs.h index eb96fc1..3936029 100644 --- a/include/configs/mxs.h +++ b/include/configs/mxs.h @@ -149,6 +149,7 @@ /* I2C */ #ifdef CONFIG_CMD_I2C #define CONFIG_I2C_MXS +#define CONFIG_I2C_MULTI_BUS #define CONFIG_HARD_I2C #ifndef CONFIG_SYS_I2C_SPEED #define CONFIG_SYS_I2C_SPEED 400000

On Tue, Sep 23, 2014 at 8:48 AM, Heiko Schocher hs@denx.de wrote:
Hello Marek,
Am 23.09.2014 13:15, schrieb Marek Vasut:
The i.MX28 has two I2C IP blocks, but the MXS I2C driver is hard-coded to use the I2C block 0 . Add multibus support so we can use both I2C busses as seen fit.
Signed-off-by: Marek Vasutmarex@denx.de Cc: Stefano Babicsbabic@denx.de
drivers/i2c/mxs_i2c.c | 47 +++++++++++++++++++++++++++++++++++++---------- include/configs/mxs.h | 1 + 2 files changed, 38 insertions(+), 10 deletions(-)
Sorry, I have to NACK this patch. Please convert the driver for using the CONFIG_SYS_I2C framework, see for example the drivers/i2c/mxc_i2c.c driver, thanks!
I think the conversion could be a follow up patch, I see no good reason to block this patch as is.

On Tuesday, September 23, 2014 at 03:49:27 PM, Otavio Salvador wrote:
On Tue, Sep 23, 2014 at 8:48 AM, Heiko Schocher hs@denx.de wrote:
Hello Marek,
Am 23.09.2014 13:15, schrieb Marek Vasut:
The i.MX28 has two I2C IP blocks, but the MXS I2C driver is hard-coded to use the I2C block 0 . Add multibus support so we can use both I2C busses as seen fit.
Signed-off-by: Marek Vasutmarex@denx.de Cc: Stefano Babicsbabic@denx.de
drivers/i2c/mxs_i2c.c | 47
+++++++++++++++++++++++++++++++++++++----------
include/configs/mxs.h | 1 + 2 files changed, 38 insertions(+), 10 deletions(-)
Sorry, I have to NACK this patch. Please convert the driver for using the CONFIG_SYS_I2C framework, see for example the drivers/i2c/mxc_i2c.c driver, thanks!
I think the conversion could be a follow up patch, I see no good reason to block this patch as is.
I disagree with you. Accepting this patch as is would only hinder progress toward the conversion and would set a bad example further down the line, leading only to more crap patches. Moreover, this is 2015.01 matter, so there is plenty of time for the rework to take place. Thus, I agree with Heiko.
Best regards, Marek Vasut

Hello Marek,
Am 23.09.2014 16:30, schrieb Marek Vasut:
On Tuesday, September 23, 2014 at 03:49:27 PM, Otavio Salvador wrote:
On Tue, Sep 23, 2014 at 8:48 AM, Heiko Schocherhs@denx.de wrote:
Hello Marek,
Am 23.09.2014 13:15, schrieb Marek Vasut:
The i.MX28 has two I2C IP blocks, but the MXS I2C driver is hard-coded to use the I2C block 0 . Add multibus support so we can use both I2C busses as seen fit.
Signed-off-by: Marek Vasutmarex@denx.de Cc: Stefano Babicsbabic@denx.de
drivers/i2c/mxs_i2c.c | 47
+++++++++++++++++++++++++++++++++++++----------
include/configs/mxs.h | 1 + 2 files changed, 38 insertions(+), 10 deletions(-)
Sorry, I have to NACK this patch. Please convert the driver for using the CONFIG_SYS_I2C framework, see for example the drivers/i2c/mxc_i2c.c driver, thanks!
I think the conversion could be a follow up patch, I see no good reason to block this patch as is.
I disagree with you. Accepting this patch as is would only hinder progress toward the conversion and would set a bad example further down the line, leading
Full ack ...
only to more crap patches. Moreover, this is 2015.01 matter, so there is plenty of time for the rework to take place. Thus, I agree with Heiko.
Ok, perfect, thanks! And reworking this driver should be easy...
bye, Heiko

Hi Marek,
On Tue, Sep 23, 2014 at 8:15 AM, Marek Vasut marex@denx.de wrote:
-void i2c_init(int speed, int slaveadd) +int i2c_set_bus_num(unsigned int bus) {
uint32_t mxs_i2c_regs;
switch (bus) {
case 0:
mxs_i2c_regs = MXS_I2C0_BASE;
break;
+#ifdef CONFIG_MX28
case 1:
mxs_i2c_regs = MXS_I2C1_BASE;
break;
+#endif
default:
printf("Bad bus: %d\n", bus);
Something like "Invalid I2C bus" would be a clearer error message here.

Hello Fabio,
Am 23.09.2014 17:16, schrieb Fabio Estevam:
Hi Marek,
On Tue, Sep 23, 2014 at 8:15 AM, Marek Vasutmarex@denx.de wrote:
-void i2c_init(int speed, int slaveadd) +int i2c_set_bus_num(unsigned int bus) {
uint32_t mxs_i2c_regs;
switch (bus) {
case 0:
mxs_i2c_regs = MXS_I2C0_BASE;
break;
+#ifdef CONFIG_MX28
case 1:
mxs_i2c_regs = MXS_I2C1_BASE;
break;
+#endif
default:
printf("Bad bus: %d\n", bus);
Something like "Invalid I2C bus" would be a clearer error message here.
If Marek convert this driver to the CONFIG_SYS_I2C framework, he can drop this function complete ...
bye, Heiko
participants (4)
-
Fabio Estevam
-
Heiko Schocher
-
Marek Vasut
-
Otavio Salvador