[U-Boot] [PATCH 5/6] WIP: i2c: s3c24x0: modify driver for new I2C framework
Piotr Wilczek
p.wilczek at samsung.com
Fri Nov 16 13:27:01 CET 2012
Hello Heiko,
> -----Original Message-----
> From: Heiko Schocher [mailto:hs at denx.de]
> Sent: Friday, November 16, 2012 11:34 AM
> To: Piotr Wilczek
> Cc: u-boot at lists.denx.de; Minkyu Kang; Kyungmin Park; Lukasz Majewski;
> Simon Glass; Stephen Warren; Tom Rini
> Subject: Re: [PATCH 5/6] WIP: i2c: s3c24x0: modify driver for new I2C
> framework
>
> Hello Piotr,
>
> On 15.11.2012 09:15, Piotr Wilczek wrote:
> > This patch modifies s3c24x0 driver for the new I2C framework.
> > Configs for VCMA9.h and smdk5250.h boards are modified.
> > Boards compile successfully but were not tested.
> >
> > Signed-off-by: Piotr Wilczek<p.wilczek at samsung.com>
> > Signed-off-by: Kyungmin Park<kyungmin.park at samsung.com>
> > CC: Minkyu Kang<mk7.kang at samsung.com>
> > ---
> > drivers/i2c/i2c_core.c | 5 ++
> > drivers/i2c/s3c24x0_i2c.c | 129
> ++++++++++++++++++++++++++++++++++++++++---
> > include/configs/VCMA9.h | 4 ++
> > include/configs/smdk5250.h | 4 ++
> > 4 files changed, 133 insertions(+), 9 deletions(-)
> >
> [...]
> > diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
> > index 90d297a..d473eaa 100644
> > --- a/drivers/i2c/s3c24x0_i2c.c
> > +++ b/drivers/i2c/s3c24x0_i2c.c
> > @@ -155,7 +155,6 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c,
> > int speed, int slaveadd)
> >
> > /* set prescaler, divisor according to freq, also set ACKGEN, IRQ
> */
> > writel((div& 0x0F) | 0xA0 | ((pres == 512) ? 0x40 :
> > 0),&i2c->iiccon);
> > -
>
> Why this delete?
>
Unintentional, I fix it.
> > /* init to SLAVE REVEIVE and set slaveaddr */
> > writel(0,&i2c->iicstat);
> > writel(slaveadd,&i2c->iicadd);
> > @@ -168,7 +167,7 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c,
> int speed, int slaveadd)
> > */
> >
> > #ifdef CONFIG_I2C_MULTI_BUS
>
> This define can go completely away.
Right.
>
> > -int i2c_set_bus_num(unsigned int bus)
> > +int s3c24x0_i2c_set_bus_num(unsigned int bus)
>
> all functions are static now, please change.
Yes, I overlooked it.
>
> > {
> > struct s3c24x0_i2c *i2c;
> >
> > @@ -184,13 +183,13 @@ int i2c_set_bus_num(unsigned int bus)
> > return 0;
> > }
> >
> > -unsigned int i2c_get_bus_num(void)
> > +unsigned int s3c24x0_i2c_get_bus_num(void)
>
> No longer needed.
>
> > {
> > return g_current_bus;
> > }
> > #endif
> >
> > -void i2c_init(int speed, int slaveadd)
> > +void s3c24x0_i2c_init(struct i2c_adapter *adap, int speed, int
> > +slaveadd)
>
> static.
>
> > {
> > struct s3c24x0_i2c *i2c;
> > #if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5) @@ -198,8
> > +197,7 @@ void i2c_init(int speed, int slaveadd)
> > #endif
> > int i;
> >
> > - /* By default i2c channel 0 is the current bus */
> > - g_current_bus = 0;
> > + g_current_bus = adap->hwadapnr;
>
> Can we please delete g_current_bus and use adap->hwadapnr only.
> g_current_bus should go away completely.
Ok
>
> > i2c = get_base_i2c();
> >
> > /* wait for some time to give previous transfer a chance to
> finish
> > */ @@ -415,11 +413,13 @@ static int i2c_transfer(struct s3c24x0_i2c
> *i2c,
> > return result;
> > }
> >
> > -int i2c_probe(uchar chip)
> > +int s3c24x0_i2c_probe(struct i2c_adapter *adap, uchar chip)
>
> static.
>
> > {
> > struct s3c24x0_i2c *i2c;
> > uchar buf[1];
> >
> > + g_current_bus = adap->hwadapnr;
>
> Please get rid of g_current_bus, and use adap->hwadapnr only!
>
> > +
> > i2c = get_base_i2c();
> > buf[0] = 0;
> >
> > @@ -431,12 +431,15 @@ int i2c_probe(uchar chip)
> > return i2c_transfer(i2c, I2C_READ, chip<< 1, 0, 0, buf, 1) !=
> I2C_OK;
> > }
> >
> > -int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int
> len)
> > +int s3c24x0_i2c_read(struct i2c_adapter *adap, uchar chip, uint
> addr,
> > + int alen, uchar *buffer, int len)
>
> static.
>
> > {
> > struct s3c24x0_i2c *i2c;
> > uchar xaddr[4];
> > int ret;
> >
> > + g_current_bus = adap->hwadapnr;
> > +
> > if (alen> 4) {
> > debug("I2C read: addr len %d not supported\n", alen);
> > return 1;
> > @@ -475,11 +478,14 @@ int i2c_read(uchar chip, uint addr, int alen,
> uchar *buffer, int len)
> > return 0;
> > }
> >
> > -int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int
> > len)
> > +int s3c24x0_i2c_write(struct i2c_adapter *adap, uchar chip, uint
> addr,
> > + int alen, uchar *buffer, int len)
>
> static
>
> > {
> > struct s3c24x0_i2c *i2c;
> > uchar xaddr[4];
> >
> > + g_current_bus = adap->hwadapnr;
> > +
> > if (alen> 4) {
> > debug("I2C write: addr len %d not supported\n", alen);
> > return 1;
> > @@ -512,4 +518,109 @@ int i2c_write(uchar chip, uint addr, int alen,
> uchar *buffer, int len)
> > (i2c, I2C_WRITE, chip<< 1,&xaddr[4 - alen], alen, buffer,
> > len) != 0);
> > }
> > +
> > +struct i2c_adapter s3c24x0_i2c_adap[] = {
> > + {
> > + .init = s3c24x0_i2c_init,
> > + .probe = s3c24x0_i2c_probe,
> > + .read = s3c24x0_i2c_read,
> > + .write = s3c24x0_i2c_write,
> > + .speed = CONFIG_SYS_I2C_SPEED,
> > + .slaveaddr = CONFIG_SYS_I2C_SLAVE,
>
> Please use a more driver specific name here, for example:
> CONFIG_SYS_I2C_S3C24X0_{SPEED/SLAVE} ...
Ok
>
> > + .init_done = 0,
> > + .hwadapnr = 0,
> > + .name = "s3c24x0-i2c#0"
> > + },
> > +#if CONFIG_MAX_I2C_NUM> 1
>
> Please use a more driver specific name here, for example:
> CONFIG_I2C_S3C24X0_MAX_NUM, thanks!
Ok
>
> > + {
> [...]
> > +#endif
> > +};
> > #endif /* CONFIG_HARD_I2C */
> > diff --git a/include/configs/VCMA9.h b/include/configs/VCMA9.h index
> > fb7d922..f1de21d 100644
> > --- a/include/configs/VCMA9.h
> > +++ b/include/configs/VCMA9.h
> > @@ -95,6 +95,10 @@
> > /* we use the built-in I2C controller */
> > #define CONFIG_DRIVER_S3C24X0_I2C
> ^
> Can we rename this define to CONFIG_SYS_I2C_S3C24X0
>
> > +#define CONFIG_SYS_I2C
> > +#define CONFIG_SYS_I2C_ADAPTERS {&s3c24x0_i2c_adap[0]}
> > +#define CONFIG_SYS_NUM_I2C_ADAPTERS 1
> > +
> > #define CONFIG_SYS_I2C_EEPROM_ADDR 0x50
> > #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 2
> > /* use EEPROM for environment vars */ diff --git
> > a/include/configs/smdk5250.h b/include/configs/smdk5250.h index
> > c0f8622..cb0f449 100644
> > --- a/include/configs/smdk5250.h
> > +++ b/include/configs/smdk5250.h
> > @@ -204,6 +204,10 @@
> > #define CONFIG_MAX_I2C_NUM 8
> ^
> Please rename
> > #define CONFIG_SYS_I2C_SLAVE 0x0
> ^
> Please rename
Ok
>
> > +#define CONFIG_SYS_I2C
> > +#define CONFIG_SYS_I2C_ADAPTERS {&s3c24x0_i2c_adap[0]}
> > +#define CONFIG_SYS_NUM_I2C_ADAPTERS 1
> > +
> > /* Ethernet Controllor Driver */
> > #ifdef CONFIG_CMD_NET
> > #define CONFIG_SMC911X
>
> Thanks!
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Best regards,
Piotr Wilczek
--
Samsung Poland R&D Center | Linux Platform Group
More information about the U-Boot
mailing list