[U-Boot] [PATCH 1/6] driver:i2c:s3c24x0: adapt driver to new i2c

[Piotr Wilczek]

Hello Heiko,

> -----Original Message-----
> From: Heiko Schocher [mailto:hs at denx.de]
> Sent: Monday, November 18, 2013 7:18 AM
> To: Piotr Wilczek
> Cc: u-boot at lists.denx.de; Minkyu Kang; Kyungmin Park
> Subject: Re: [PATCH 1/6] driver:i2c:s3c24x0: adapt driver to new i2c
> 
> Hello Piotr,
> 
> Am 13.11.2013 15:31, schrieb Piotr Wilczek:
> > The s3c24x0 i2c driver is adapted to new i2c framework.
> >
> > 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>
> > Cc: Heiko Schocher<hs at denx.de>
> > ---
> >   drivers/i2c/Makefile      |    2 +-
> >   drivers/i2c/s3c24x0_i2c.c |  157 +++++++++++++++++++++++++++++-----
> -----------
> >   2 files changed, 101 insertions(+), 58 deletions(-)
> 
> Thanks for your work! Ne real issues with your patch, but can you
> squash your patchset in one patch, as we not want to lost "git bisect"
> functionality.
Ok.

> 
> Beside of this, only minor comments:
> 
> [...]
> > diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
> > index f77a9d1..0051cac 100644
> > --- a/drivers/i2c/s3c24x0_i2c.c
> > +++ b/drivers/i2c/s3c24x0_i2c.c
> > @@ -23,8 +23,6 @@
> >   #include<i2c.h>
> >   #include "s3c24x0_i2c.h"
> >
> > -#ifdef CONFIG_HARD_I2C
> > -
> >   #define	I2C_WRITE	0
> >   #define I2C_READ	1
> >
> > @@ -127,7 +125,6 @@
> >    * For SPL boot some boards need i2c before SDRAM is initialised so
> force
> >    * variables to live in SRAM
> >    */
> > -static unsigned int g_current_bus __attribute__((section(".data")));
> >   static struct s3c24x0_i2c_bus i2c_bus[CONFIG_MAX_I2C_NUM]
> >   			__attribute__((section(".data")));
> >
> > @@ -143,6 +140,7 @@ static struct s3c24x0_i2c_bus *get_bus(unsigned
> int bus_idx)
> >   		struct s3c24x0_i2c_bus *bus;
> >
> >   		bus =&i2c_bus[bus_idx];
> > +
> 
> Why new line here?
I will remove it.

> 
> >   		if (bus->active)
> >   			return bus;
> >   	}
> > @@ -254,17 +252,17 @@ static void ReadWriteByte(struct s3c24x0_i2c
> *i2c)
> >   	writel(readl(&i2c->iiccon)&  ~I2CCON_IRPND,&i2c->iiccon);
> >   }
> >
> > -static struct s3c24x0_i2c *get_base_i2c(void)
> > +static struct s3c24x0_i2c *get_base_i2c(int bus)
> >   {
> >   #ifdef CONFIG_EXYNOS4
> >   	struct s3c24x0_i2c *i2c = (struct s3c24x0_i2c
> *)(samsung_get_base_i2c()
> >   							+
(EXYNOS4_I2C_SPACING
> > -							* g_current_bus));
> > +							* bus));
> >   	return i2c;
> >   #elif defined CONFIG_EXYNOS5
> >   	struct s3c24x0_i2c *i2c = (struct s3c24x0_i2c
> *)(samsung_get_base_i2c()
> >   							+
(EXYNOS5_I2C_SPACING
> > -							* g_current_bus));
> > +							* bus));
> >   	return i2c;
> >   #else
> >   	return s3c24x0_get_base_i2c();
> > @@ -298,7 +296,6 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c,
> int speed, int slaveadd)
> >   	writel(I2C_MODE_MT | I2C_TXRX_ENA,&i2c->iicstat);
> >   }
> >
> > -#ifdef CONFIG_I2C_MULTI_BUS
> >   static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus)
> >   {
> >   	struct exynos5_hsi2c *hsregs = i2c_bus->hsregs; @@ -307,8 +304,10
> > @@ static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus)
> >   	unsigned int i = 0, utemp0 = 0, utemp1 = 0;
> >   	unsigned int t_ftl_cycle;
> >
> > -#if defined CONFIG_EXYNOS5
> > +#if (defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
> >   	clkin = get_i2c_clk();
> > +#else
> > +	clkin = get_PCLK();
> >   #endif
> 
> What has this change to do with the new i2c framework?
Actually it is related to commit shaid: 296a461
Without this change I get the uninitialized variable warning for Exynos 4.
I can make a separate patch fix for this change if you prefer.

> 
> [...]
> 
> 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