| Message ID | 1340338339-11626-16-git-send-email-troy.kisky@boundarydevices.com |
|---|---|
| State | Superseded |
| Delegated to: | Heiko Schocher |
| Headers | show |
Dear Troy Kisky, > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> > --- > drivers/i2c/mxc_i2c.c | 121 > ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 100 > insertions(+), 21 deletions(-) > > diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c > index cb061f7..ec05798 100644 > --- a/drivers/i2c/mxc_i2c.c > +++ b/drivers/i2c/mxc_i2c.c > @@ -58,9 +58,7 @@ struct mxc_i2c_regs { > #define I2SR_IIF (1 << 1) > #define I2SR_RX_NO_AK (1 << 0) > > -#ifdef CONFIG_SYS_I2C_BASE > -#define I2C_BASE CONFIG_SYS_I2C_BASE > -#else > +#if defined(CONFIG_HARD_I2C) && !defined(CONFIG_SYS_I2C_BASE) > #error "define CONFIG_SYS_I2C_BASE to use the mxc_i2c driver" > #endif > > @@ -114,11 +112,11 @@ static uint8_t i2c_imx_get_clk(unsigned int rate) > } > > /* > - * Init I2C Bus > + * Set I2C Bus speed > */ > -void i2c_init(int speed, int unused) > +int bus_i2c_set_bus_speed(void *base, int speed) > { > - struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; > + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base; > u8 clk_idx = i2c_imx_get_clk(speed); > u8 idx = i2c_clk_div[clk_idx][1]; > > @@ -128,23 +126,15 @@ void i2c_init(int speed, int unused) > /* Reset module */ > writeb(0, &i2c_regs->i2cr); > writeb(0, &i2c_regs->i2sr); > -} > - > -/* > - * Set I2C Speed > - */ > -int i2c_set_bus_speed(unsigned int speed) > -{ > - i2c_init(speed, 0); > return 0; > } > > /* > * Get I2C Speed > */ > -unsigned int i2c_get_bus_speed(void) > +unsigned int bus_i2c_get_bus_speed(void *base) > { > - struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; > + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base; > u8 clk_idx = readb(&i2c_regs->ifdr); > u8 clk_div; > > @@ -282,12 +272,13 @@ static int i2c_init_transfer(struct mxc_i2c_regs > *i2c_regs, /* > * Read data from I2C device > */ > -int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) > +int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf, > + int len) > { > - struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; > int ret; > unsigned int temp; > int i; > + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base; > > ret = i2c_init_transfer(i2c_regs, chip, addr, alen); > if (ret) > @@ -338,11 +329,12 @@ int i2c_read(uchar chip, uint addr, int alen, uchar > *buf, int len) /* > * Write data to I2C device > */ > -int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) > +int bus_i2c_write(void *base, uchar chip, uint addr, int alen, uchar *buf, > + int len) > { > - struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; > int ret; > int i; > + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base; > > ret = i2c_init_transfer(i2c_regs, chip, addr, alen); > if (ret) > @@ -359,10 +351,97 @@ int i2c_write(uchar chip, uint addr, int alen, uchar > *buf, int len) return ret; > } > > +typedef void (*toggle_i2c_fn)(void *p); I really don't like this kind of stuff ... who's supposed to look for this typedefs. It makes the code not obviously clear. > +#ifdef CONFIG_I2C_MULTI_BUS > +static unsigned g_bus; > +#else > +#define g_bus 0 > +#endif > + > +struct i2c_parms { > + void *base; > + void *toggle_data; > + toggle_i2c_fn toggle_fn; > +}; > + > +struct i2c_parms g_parms[3]; > + > +void *get_base(void) static? Or this is part of i2c api? > +{ > +#ifndef CONFIG_SYS_I2C_BASE > + return g_parms[g_bus].base; > +#elif defined(CONFIG_I2C_MULTI_BUS) > + void *ret = g_parms[g_bus].base; > + if (!ret) > + ret = (void *)CONFIG_SYS_I2C_BASE; > + return ret; > +#else > + return (void *)CONFIG_SYS_I2C_BASE; > +#endif > +} > + > +int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) > +{ > + return bus_i2c_read(get_base(), chip, addr, alen, buf, len); > +} > + > +int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) > +{ > + return bus_i2c_write(get_base(), chip, addr, alen, buf, len); > +} > /* > * Try if a chip add given address responds (probe the chip) > */ > int i2c_probe(uchar chip) > { > - return i2c_write(chip, 0, 0, NULL, 0); > + return bus_i2c_write(get_base(), chip, 0, 0, NULL, 0); > +} > + > +void bus_i2c_init(void *base, int speed, int unused, > + toggle_i2c_fn toggle_fn, void *toggle_data) > +{ > + int i = 0; > + struct i2c_parms *p = g_parms; > + if (!base) > + return; > + for (;;) { > + if (!p->base || (p->base == base)) { > + p->base = base; > + if (toggle_data) { > + p->toggle_data = toggle_data; > + p->toggle_fn = toggle_fn; > + } > + break; > + } > + p++; > + i++; > + if (i >= ARRAY_SIZE(g_parms)) > + return; > + } > + bus_i2c_set_bus_speed(base, speed); > +} > + > +/* > + * Init I2C Bus > + */ > +void i2c_init(int speed, int unused) > +{ > + bus_i2c_init(get_base(), speed, unused, NULL, NULL); > +} > + > +/* > + * Set I2C Speed > + */ > +int i2c_set_bus_speed(unsigned int speed) > +{ > + return bus_i2c_set_bus_speed(get_base(), speed); > +} > + > +/* > + * Get I2C Speed > + */ > +unsigned int i2c_get_bus_speed(void) > +{ > + return bus_i2c_get_bus_speed(get_base()); > }
Hello Troy, On 22.06.2012 06:12, Troy Kisky wrote: > Signed-off-by: Troy Kisky<troy.kisky@boundarydevices.com> > --- > drivers/i2c/mxc_i2c.c | 121 ++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 100 insertions(+), 21 deletions(-) > > diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c > index cb061f7..ec05798 100644 > --- a/drivers/i2c/mxc_i2c.c > +++ b/drivers/i2c/mxc_i2c.c [...] > @@ -359,10 +351,97 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) > return ret; > } > > +typedef void (*toggle_i2c_fn)(void *p); > + > +#ifdef CONFIG_I2C_MULTI_BUS > +static unsigned g_bus; This is only valid after relocation ... and i2c is maybe used before relocation. Try something (not tested) like that: static unsigned int __attribute__((section (".data"))) g_bus = 0; > +#else > +#define g_bus 0 > +#endif > + > +struct i2c_parms { > + void *base; > + void *toggle_data; > + toggle_i2c_fn toggle_fn; For what is this toggle_* needed? bye, Heiko
Dear Heiko Schocher, > Hello Troy, > > On 22.06.2012 06:12, Troy Kisky wrote: > > Signed-off-by: Troy Kisky<troy.kisky@boundarydevices.com> > > --- > > > > drivers/i2c/mxc_i2c.c | 121 > > ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 100 > > insertions(+), 21 deletions(-) > > > > diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c > > index cb061f7..ec05798 100644 > > --- a/drivers/i2c/mxc_i2c.c > > +++ b/drivers/i2c/mxc_i2c.c > > [...] > > > @@ -359,10 +351,97 @@ int i2c_write(uchar chip, uint addr, int alen, > > uchar *buf, int len) > > > > return ret; > > > > } > > > > +typedef void (*toggle_i2c_fn)(void *p); > > + > > +#ifdef CONFIG_I2C_MULTI_BUS > > +static unsigned g_bus; > > This is only valid after relocation ... and i2c is maybe used before > relocation. > > Try something (not tested) like that: > > static unsigned int __attribute__((section (".data"))) g_bus = 0; This is a good knowledge to have, adding it amongst important emails :) > > > +#else > > +#define g_bus 0 > > +#endif > > + > > +struct i2c_parms { > > + void *base; > > + void *toggle_data; > > + toggle_i2c_fn toggle_fn; > > For what is this toggle_* needed? > > bye, > Heiko Best regards, Marek Vasut
Dear Marek Vasut, In message <201206242208.51305.marek.vasut@gmail.com> you wrote: > > > static unsigned int __attribute__((section (".data"))) g_bus = 0; > > This is a good knowledge to have, adding it amongst important emails :) Please add a note to it that whenever youneed doing this, chances are >95% that you are doing something fundamally wrong. Best regards, Wolfgang Denk
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index cb061f7..ec05798 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -58,9 +58,7 @@ struct mxc_i2c_regs { #define I2SR_IIF (1 << 1) #define I2SR_RX_NO_AK (1 << 0) -#ifdef CONFIG_SYS_I2C_BASE -#define I2C_BASE CONFIG_SYS_I2C_BASE -#else +#if defined(CONFIG_HARD_I2C) && !defined(CONFIG_SYS_I2C_BASE) #error "define CONFIG_SYS_I2C_BASE to use the mxc_i2c driver" #endif @@ -114,11 +112,11 @@ static uint8_t i2c_imx_get_clk(unsigned int rate) } /* - * Init I2C Bus + * Set I2C Bus speed */ -void i2c_init(int speed, int unused) +int bus_i2c_set_bus_speed(void *base, int speed) { - struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base; u8 clk_idx = i2c_imx_get_clk(speed); u8 idx = i2c_clk_div[clk_idx][1]; @@ -128,23 +126,15 @@ void i2c_init(int speed, int unused) /* Reset module */ writeb(0, &i2c_regs->i2cr); writeb(0, &i2c_regs->i2sr); -} - -/* - * Set I2C Speed - */ -int i2c_set_bus_speed(unsigned int speed) -{ - i2c_init(speed, 0); return 0; } /* * Get I2C Speed */ -unsigned int i2c_get_bus_speed(void) +unsigned int bus_i2c_get_bus_speed(void *base) { - struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base; u8 clk_idx = readb(&i2c_regs->ifdr); u8 clk_div; @@ -282,12 +272,13 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs, /* * Read data from I2C device */ -int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) +int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf, + int len) { - struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; int ret; unsigned int temp; int i; + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base; ret = i2c_init_transfer(i2c_regs, chip, addr, alen); if (ret) @@ -338,11 +329,12 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) /* * Write data to I2C device */ -int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) +int bus_i2c_write(void *base, uchar chip, uint addr, int alen, uchar *buf, + int len) { - struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; int ret; int i; + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base; ret = i2c_init_transfer(i2c_regs, chip, addr, alen); if (ret) @@ -359,10 +351,97 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) return ret; } +typedef void (*toggle_i2c_fn)(void *p); + +#ifdef CONFIG_I2C_MULTI_BUS +static unsigned g_bus; +#else +#define g_bus 0 +#endif + +struct i2c_parms { + void *base; + void *toggle_data; + toggle_i2c_fn toggle_fn; +}; + +struct i2c_parms g_parms[3]; + +void *get_base(void) +{ +#ifndef CONFIG_SYS_I2C_BASE + return g_parms[g_bus].base; +#elif defined(CONFIG_I2C_MULTI_BUS) + void *ret = g_parms[g_bus].base; + if (!ret) + ret = (void *)CONFIG_SYS_I2C_BASE; + return ret; +#else + return (void *)CONFIG_SYS_I2C_BASE; +#endif +} + +int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) +{ + return bus_i2c_read(get_base(), chip, addr, alen, buf, len); +} + +int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) +{ + return bus_i2c_write(get_base(), chip, addr, alen, buf, len); +} /* * Try if a chip add given address responds (probe the chip) */ int i2c_probe(uchar chip) { - return i2c_write(chip, 0, 0, NULL, 0); + return bus_i2c_write(get_base(), chip, 0, 0, NULL, 0); +} + +void bus_i2c_init(void *base, int speed, int unused, + toggle_i2c_fn toggle_fn, void *toggle_data) +{ + int i = 0; + struct i2c_parms *p = g_parms; + if (!base) + return; + for (;;) { + if (!p->base || (p->base == base)) { + p->base = base; + if (toggle_data) { + p->toggle_data = toggle_data; + p->toggle_fn = toggle_fn; + } + break; + } + p++; + i++; + if (i >= ARRAY_SIZE(g_parms)) + return; + } + bus_i2c_set_bus_speed(base, speed); +} + +/* + * Init I2C Bus + */ +void i2c_init(int speed, int unused) +{ + bus_i2c_init(get_base(), speed, unused, NULL, NULL); +} + +/* + * Set I2C Speed + */ +int i2c_set_bus_speed(unsigned int speed) +{ + return bus_i2c_set_bus_speed(get_base(), speed); +} + +/* + * Get I2C Speed + */ +unsigned int i2c_get_bus_speed(void) +{ + return bus_i2c_get_bus_speed(get_base()); }
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> --- drivers/i2c/mxc_i2c.c | 121 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 100 insertions(+), 21 deletions(-)