Patchwork [U-Boot] i2c: s3c24xx: add hsi2c controller support

login
register
mail settings
Submitter Naveen Krishna Ch
Date March 12, 2013, 10:58 a.m.
Message ID <1363085897-19814-1-git-send-email-ch.naveen@samsung.com>
Download mbox | patch
Permalink /patch/227012/
State Superseded
Delegated to: Heiko Schocher
Headers show

Comments

Naveen Krishna Ch - March 12, 2013, 10:58 a.m.
Add support for hsi2c controller available on exynos5420.

Note: driver currently supports only fast speed mode 100kbps

Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
Signed-off-by: R. Chandrasekar <rc.sekar@samsung.com>
---
 drivers/i2c/s3c24x0_i2c.c |  389 ++++++++++++++++++++++++++++++++++++++++++---
 drivers/i2c/s3c24x0_i2c.h |   33 ++++
 2 files changed, 397 insertions(+), 25 deletions(-)
Simon Glass - March 12, 2013, 1:11 p.m.
Hi Naveen,

On Tue, Mar 12, 2013 at 3:58 AM, Naveen Krishna Chatradhi <
ch.naveen@samsung.com> wrote:

> Add support for hsi2c controller available on exynos5420.
>
> Note: driver currently supports only fast speed mode 100kbps
>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> Signed-off-by: R. Chandrasekar <rc.sekar@samsung.com>
>

I just commented on the Linux driver, so some comments may apply here also.


> ---
>  drivers/i2c/s3c24x0_i2c.c |  389
> ++++++++++++++++++++++++++++++++++++++++++---
>  drivers/i2c/s3c24x0_i2c.h |   33 ++++
>  2 files changed, 397 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
> index 769a2ba..3117ab7 100644
> --- a/drivers/i2c/s3c24x0_i2c.c
> +++ b/drivers/i2c/s3c24x0_i2c.c
> @@ -32,6 +32,7 @@
>  #include <asm/arch/clk.h>
>  #include <asm/arch/cpu.h>
>  #include <asm/arch/pinmux.h>
> +#include <asm/arch/clock.h>
>  #else
>  #include <asm/arch/s3c24x0_cpu.h>
>  #endif
> @@ -50,6 +51,60 @@
>  #define I2C_NOK_LA     3       /* Lost arbitration */
>  #define I2C_NOK_TOUT   4       /* time out */
>
> +/* HSI2C specific register description */
> +
> +/* I2C_CTL Register bits */
> +#define HSI2C_FUNC_MODE_I2C            (1u << 0)
> +#define HSI2C_MASTER                   (1u << 3)
> +#define HSI2C_RXCHON                   (1u << 6)       /* Write/Send */
> +#define HSI2C_TXCHON                   (1u << 7)       /* Read/Receive */
> +#define HSI2C_SW_RST                   (1u << 31)
> +
> +/* I2C_FIFO_CTL Register bits */
> +#define HSI2C_RXFIFO_EN                        (1u << 0)
> +#define HSI2C_TXFIFO_EN                        (1u << 1)
> +#define HSI2C_TXFIFO_TRIGGER_LEVEL     (0x20 << 16)
> +#define HSI2C_RXFIFO_TRIGGER_LEVEL     (0x20 << 4)
> +
> +/* I2C_TRAILING_CTL Register bits */
> +#define HSI2C_TRAILING_COUNT           (0xff)
> +
> +/* I2C_INT_EN Register bits */
> +#define HSI2C_INT_TX_ALMOSTEMPTY_EN    (1u << 0)
> +#define HSI2C_INT_RX_ALMOSTFULL_EN     (1u << 1)
> +#define HSI2C_INT_TRAILING_EN          (1u << 6)
> +#define HSI2C_INT_I2C_EN               (1u << 9)
> +
> +/* I2C_CONF Register bits */
> +#define HSI2C_AUTO_MODE                        (1u << 31)
> +#define HSI2C_10BIT_ADDR_MODE          (1u << 30)
> +#define HSI2C_HS_MODE                  (1u << 29)
> +
> +/* I2C_AUTO_CONF Register bits */
> +#define HSI2C_READ_WRITE               (1u << 16)
> +#define HSI2C_STOP_AFTER_TRANS         (1u << 17)
> +#define HSI2C_MASTER_RUN               (1u << 31)
> +
> +/* I2C_TIMEOUT Register bits */
> +#define HSI2C_TIMEOUT_EN               (1u << 31)
> +
> +/* I2C_TRANS_STATUS register bits */
> +#define HSI2C_MASTER_BUSY              (1u << 17)
> +#define HSI2C_SLAVE_BUSY               (1u << 16)
> +#define HSI2C_NO_DEV                   (1u << 3)
> +#define HSI2C_NO_DEV_ACK               (1u << 2)
> +#define HSI2C_TRANS_ABORT              (1u << 1)
> +#define HSI2C_TRANS_DONE               (1u << 0)
> +#define HSI2C_TIMEOUT_AUTO             (0u << 0)
> +
> +#define HSI2C_SLV_ADDR_MAS(x)          ((x & 0x3ff) << 10)
> +
> +/* Controller operating frequency, timing values for operation
> + * are calculated against this frequency
> + */
> +#define HSI2C_FS_TX_CLOCK              1000000
> +
> +/* S3C I2C Controller bits */
>  #define I2CSTAT_BSY    0x20    /* Busy bit */
>  #define I2CSTAT_NACK   0x01    /* Nack bit */
>  #define I2CCON_ACKGEN  0x80    /* Acknowledge generation */
> @@ -61,6 +116,7 @@
>
>  #define I2C_TIMEOUT 1          /* 1 second */
>
> +#define        HSI2C_TIMEOUT   100
>
>  /*
>   * For SPL boot some boards need i2c before SDRAM is initialised so force
> @@ -120,9 +176,23 @@ static int WaitForXfer(struct s3c24x0_i2c *i2c)
>         return (readl(&i2c->iiccon) & I2CCON_IRPND) ? I2C_OK :
> I2C_NOK_TOUT;
>  }
>
> -static int IsACK(struct s3c24x0_i2c *i2c)
> +static int hsi2c_wait_for_irq(struct exynos5_hsi2c *i2c)
>  {
> -       return !(readl(&i2c->iicstat) & I2CSTAT_NACK);
> +       int i = HSI2C_TIMEOUT * 10;
> +       int ret = I2C_NOK_TOUT;
> +
> +       while (i > 0) {
> +               /* wait for a while and retry */
> +               udelay(50);
> +               if (readl(&i2c->usi_int_stat) &
> +                       (HSI2C_INT_I2C_EN | HSI2C_INT_TX_ALMOSTEMPTY_EN)) {
> +                       ret = I2C_OK;
> +                       break;
> +               }
> +               i--;
>

Please can we use the get_timer() method for timeouts instead of udelay()?


> +       }
> +
> +       return ret;
>  }
>
>  static void ReadWriteByte(struct s3c24x0_i2c *i2c)
> @@ -130,6 +200,22 @@ static void ReadWriteByte(struct s3c24x0_i2c *i2c)
>         writel(readl(&i2c->iiccon) & ~I2CCON_IRPND, &i2c->iiccon);
>  }
>
> +static void hsi2c_clear_irqpd(struct exynos5_hsi2c *i2c)
> +{
> +       writel(readl(&i2c->usi_int_stat), &i2c->usi_int_stat);
> +}
> +
> +static int hsi2c_isack(struct exynos5_hsi2c *i2c)
> +{
> +       return readl(&i2c->usi_trans_status) &
> +                       (HSI2C_NO_DEV | HSI2C_NO_DEV_ACK);
> +}
> +
> +static int IsACK(struct s3c24x0_i2c *i2c)
>

There are two functions - hsi2c_isack() and IsAck(). What is the difference?


> +{
> +       return !(readl(&i2c->iicstat) & I2CSTAT_NACK);
> +}
> +
>  static struct s3c24x0_i2c *get_base_i2c(void)
>  {
>  #ifdef CONFIG_EXYNOS4
> @@ -147,6 +233,18 @@ static struct s3c24x0_i2c *get_base_i2c(void)
>  #endif
>  }
>
> +static struct exynos5_hsi2c *get_base_hsi2c(void)
> +{
> +       struct exynos5_hsi2c *i2c = NULL;
> +       int bus = g_current_bus;
> +
> +       if (proid_is_exynos5420())
> +               i2c = (struct exynos5_hsi2c *)(EXYNOS5420_I2C_BASE +
> +                                       ((bus) * EXYNOS5_I2C_SPACING));
> +
> +       return i2c;
> +}
>

This should come from the device tree information that you are decoding
(the ->reg property).


> +
>  static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd)
>  {
>         ulong freq, pres = 16, div;
> @@ -174,6 +272,74 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c, int
> speed, int slaveadd)
>         writel(I2C_MODE_MT | I2C_TXRX_ENA, &i2c->iicstat);
>  }
>
> +static void hsi2c_ch_init(struct exynos5_hsi2c *i2c, int speed)
> +{
> +       u32 i2c_timeout;
> +       ulong clkin = get_i2c_clk();
> +       u32 i2c_timing_s1;
> +       u32 i2c_timing_s2;
> +       u32 i2c_timing_s3;
> +       u32 i2c_timing_sla;
> +       unsigned int op_clk = HSI2C_FS_TX_CLOCK;
> +       unsigned int n_clkdiv;
> +       unsigned int t_start_su, t_start_hd;
> +       unsigned int t_stop_su;
> +       unsigned int t_data_su, t_data_hd;
> +       unsigned int t_scl_l, t_scl_h;
> +       unsigned int t_sr_release;
> +       unsigned int t_ftl_cycle;
> +       unsigned int i = 0, utemp0 = 0, utemp1 = 0, utemp2 = 0;
> +
> +       /* FPCLK / FI2C =
> +        * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + 2 * FLT_CYCLE
> +        * uTemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2)
> +        * uTemp1 = (TSCLK_L + TSCLK_H + 2)
> +        * uTemp2 = TSCLK_L + TSCLK_H
> +        */
> +       t_ftl_cycle = (readl(&i2c->usi_conf) >> 16) & 0x7;
> +       utemp0 = (clkin / op_clk) - 8 - 2 * t_ftl_cycle;
> +
> +       /* CLK_DIV max is 256 */
> +       for (i = 0; i < 256; i++) {
> +               utemp1 = utemp0 / (i + 1);
> +               /* SCLK_L/H max is 256 / 2 */
> +               if (utemp1 < 128) {
> +                       utemp2 = utemp1 - 2;
> +                       break;
> +               }
> +       }
> +
> +       n_clkdiv = i;
> +       t_scl_l = utemp2 / 2;
> +       t_scl_h = utemp2 / 2;
> +       t_start_su = t_scl_l;
> +       t_start_hd = t_scl_l;
> +       t_stop_su = t_scl_l;
> +       t_data_su = t_scl_l / 2;
> +       t_data_hd = t_scl_l / 2;
> +       t_sr_release = utemp2;
> +
> +       i2c_timing_s1 = t_start_su << 24 | t_start_hd << 16 | t_stop_su <<
> 8;
> +       i2c_timing_s2 = t_data_su << 24 | t_scl_l << 8 | t_scl_h << 0;
> +       i2c_timing_s3 = n_clkdiv << 16 | t_sr_release << 0;
> +       i2c_timing_sla = t_data_hd << 0;
> +
> +       writel(HSI2C_TRAILING_COUNT, &i2c->usi_trailing_ctl);
> +
> +       /* Clear to enable Timeout */
> +       i2c_timeout = readl(&i2c->usi_timeout);
> +       i2c_timeout &= ~HSI2C_TIMEOUT_EN;
> +       writel(i2c_timeout, &i2c->usi_timeout);
> +
> +       writel(readl(&i2c->usi_conf) | HSI2C_AUTO_MODE, &i2c->usi_conf);
> +
> +       /* Currently operating in Fast speed mode. */
> +       writel(i2c_timing_s1, &i2c->usi_timing_fs1);
> +       writel(i2c_timing_s2, &i2c->usi_timing_fs2);
> +       writel(i2c_timing_s3, &i2c->usi_timing_fs3);
> +       writel(i2c_timing_sla, &i2c->usi_timing_sla);
> +}
> +
>  /*
>   * MULTI BUS I2C support
>   */
> @@ -181,16 +347,18 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c, int
> speed, int slaveadd)
>  #ifdef CONFIG_I2C_MULTI_BUS
>  int i2c_set_bus_num(unsigned int bus)
>  {
> -       struct s3c24x0_i2c *i2c;
> -
>         if ((bus < 0) || (bus >= CONFIG_MAX_I2C_NUM)) {
>                 debug("Bad bus: %d\n", bus);
>                 return -1;
>         }
>
>         g_current_bus = bus;
> -       i2c = get_base_i2c();
> -       i2c_ch_init(i2c, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> +       if (proid_is_exynos5420() && (bus > 3)) {
> +               hsi2c_ch_init(get_base_hsi2c(),
> +                                               CONFIG_SYS_I2C_SPEED);
> +       } else
> +               i2c_ch_init(get_base_i2c(), CONFIG_SYS_I2C_SPEED,
> +                                               CONFIG_SYS_I2C_SLAVE);
>
>         return 0;
>  }
> @@ -269,24 +437,182 @@ void i2c_init(int speed, int slaveadd)
>  }
>
>  /*
> + * Send a STOP event and wait for it to have completed
> + *
> + * @param mode If it is a master transmitter or receiver
> + * @return I2C_OK if the line became idle before timeout I2C_NOK_TOUT
> otherwise
> + */
> +static int hsi2c_send_stop(struct exynos5_hsi2c *i2c, int result)
> +{
> +       int timeout;
> +       int ret = I2C_NOK_TOUT;
> +
> +       /* Wait for the STOP to send and the bus to go idle */
> +       for (timeout = HSI2C_TIMEOUT; timeout > 0; timeout -= 5) {
> +               if (!(readl(&i2c->usi_trans_status) & HSI2C_MASTER_BUSY)) {
> +                       ret = I2C_OK;
> +                       goto out;
> +               }
> +               udelay(5);
> +       }
> + out:
> +       /* Setting the STOP event to fire */
> +       writel(HSI2C_FUNC_MODE_I2C, &i2c->usi_ctl);
> +       writel(0x0, &i2c->usi_int_en);
> +
> +       return (result == I2C_OK) ? ret : result;
> +}
> +
> +static int hsi2c_write(unsigned char chip,
> +                       unsigned char addr[],
> +                       unsigned char alen,
> +                       unsigned char data[],
> +                       unsigned short len)
> +{
> +       struct exynos5_hsi2c *i2c = get_base_hsi2c();
> +       int i = 0, result = I2C_OK;
> +       u32 i2c_auto_conf;
> +       u32 stat;
> +
> +       /* Check I2C bus idle */
> +       i = HSI2C_TIMEOUT * 20;
> +       while ((readl(&i2c->usi_trans_status) & HSI2C_MASTER_BUSY)
> +                                                       && (i > 0)) {
> +               udelay(50);
> +               i--;
> +       }
>

It seems like perhaps you should have a function to wait until not busy and
call it from the above function, and here.


> +
> +       stat = readl(&i2c->usi_trans_status);
> +
> +       if (stat & HSI2C_MASTER_BUSY) {
> +               debug("%s: bus busy\n", __func__);
> +               return I2C_NOK_TOUT;
> +       }
> +       /* Disable TXFIFO and RXFIFO */
> +       writel(0, &i2c->usi_fifo_ctl);
> +
> +       /* chip address */
> +       writel(HSI2C_SLV_ADDR_MAS(chip), &i2c->i2c_addr);
> +
> +       /* Enable interrupts */
> +       writel((HSI2C_INT_I2C_EN | HSI2C_INT_TX_ALMOSTEMPTY_EN),
> +                                               &i2c->usi_int_en);
> +
> +       /* usi_ctl enable i2c func, master write configure */
> +       writel((HSI2C_TXCHON | HSI2C_FUNC_MODE_I2C | HSI2C_MASTER),
> +                                                       &i2c->usi_ctl);
> +
> +       /* i2c_conf configure */
> +       writel(readl(&i2c->usi_conf) | HSI2C_AUTO_MODE, &i2c->usi_conf);
> +
> +       /* auto_conf for write length and stop configure */
> +       i2c_auto_conf = ((len + alen) | HSI2C_STOP_AFTER_TRANS);
> +       i2c_auto_conf &= ~HSI2C_READ_WRITE;
> +       writel(i2c_auto_conf, &i2c->usi_auto_conf);
> +
> +       /* Master run, start xfer */
> +       writel(readl(&i2c->usi_auto_conf) | HSI2C_MASTER_RUN,
> +                                               &i2c->usi_auto_conf);
> +
> +       result = hsi2c_wait_for_irq(i2c);
> +       if ((result == I2C_OK) && hsi2c_isack(i2c)) {
> +               result = I2C_NACK;
> +               goto err;
> +       }
> +
> +       for (i = 0; i < alen && (result == I2C_OK); i++) {
> +               writel(addr[i], &i2c->usi_txdata);
> +               result = hsi2c_wait_for_irq(i2c);
> +       }
> +
> +       for (i = 0; i < len && (result == I2C_OK); i++) {
> +               writel(data[i], &i2c->usi_txdata);
> +               result = hsi2c_wait_for_irq(i2c);
>

Do we have to wait for an irq after every tx byte?


> +       }
> +
> + err:
> +       hsi2c_clear_irqpd(i2c);
> +       return hsi2c_send_stop(i2c, result);
> +}
> +
> +static int hsi2c_read(unsigned char chip,
> +                       unsigned char addr[],
> +                       unsigned char alen,
> +                       unsigned char data[],
> +                       unsigned short len,
> +                       int check)
>

This seems to have a lot of common code with the above. Should they be
joined up?


> +{
> +       struct exynos5_hsi2c *i2c = get_base_hsi2c();
> +       int i, result;
> +       u32 i2c_auto_conf;
> +
> +       if (!check) {
> +               result =  hsi2c_write(chip, addr, alen, data, 0);
> +               if (result != I2C_OK) {
> +                       debug("write failed Result = %d\n", result);
> +                       return result;
> +               }
> +       }
> +
> +       /* start read */
> +       /* Disable TXFIFO and RXFIFO */
> +       writel(0, &i2c->usi_fifo_ctl);
> +
> +       /* chip address */
> +       writel(HSI2C_SLV_ADDR_MAS(chip), &i2c->i2c_addr);
> +
> +       /* Enable interrupts */
> +       writel(HSI2C_INT_I2C_EN, &i2c->usi_int_en);
> +
> +               /* i2c_conf configure */
>

outdent


> +       writel(readl(&i2c->usi_conf) | HSI2C_AUTO_MODE, &i2c->usi_conf);
> +
> +       /* auto_conf, length and stop configure */
> +       i2c_auto_conf = (len | HSI2C_STOP_AFTER_TRANS | HSI2C_READ_WRITE);
> +       writel(i2c_auto_conf, &i2c->usi_auto_conf);
> +
> +       /* usi_ctl enable i2c func, master WRITE configure */
> +       writel((HSI2C_RXCHON | HSI2C_FUNC_MODE_I2C | HSI2C_MASTER),
> +                                                       &i2c->usi_ctl);
> +
> +       /* Master run, start xfer */
> +       writel(readl(&i2c->usi_auto_conf) | HSI2C_MASTER_RUN,
> +                                               &i2c->usi_auto_conf);
> +
> +       result = hsi2c_wait_for_irq(i2c);
> +       if ((result == I2C_OK) && hsi2c_isack(i2c)) {
> +               result = I2C_NACK;
> +               goto err;
> +       }
> +
> +       for (i = 0; i < len && (result == I2C_OK); i++) {
> +               result = hsi2c_wait_for_irq(i2c);
> +               data[i] = readl(&i2c->usi_rxdata);
> +       }
>

I suppose the FIFOs are not used here. How larger are the FIFOs?


> + err:
> +       /* Stop and quit */
> +       hsi2c_clear_irqpd(i2c);
> +       return hsi2c_send_stop(i2c, result);
> +}
> +
> +/*
>   * cmd_type is 0 for write, 1 for read.
>   *
>   * addr_len can take any value from 0-255, it is only limited
>   * by the char, we could make it larger if needed. If it is
>   * 0 we skip the address write cycle.
>   */
> -static int i2c_transfer(struct s3c24x0_i2c *i2c,
> -                       unsigned char cmd_type,
> +static int i2c_transfer(unsigned char cmd_type,
>                         unsigned char chip,
>                         unsigned char addr[],
>                         unsigned char addr_len,
>                         unsigned char data[],
>                         unsigned short data_len)
>  {
> +       struct s3c24x0_i2c *i2c = get_base_i2c();
>         int i, result;
>
>         if (data == 0 || data_len == 0) {
> -               /*Don't support data transfer of no length or to address 0
> */
>                 debug("i2c_transfer: bad call\n");
>                 return I2C_NOK;
>         }
> @@ -428,10 +754,8 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
>
>  int i2c_probe(uchar chip)
>  {
> -       struct s3c24x0_i2c *i2c;
>         uchar buf[1];
>
> -       i2c = get_base_i2c();
>         buf[0] = 0;
>
>         /*
> @@ -439,12 +763,15 @@ int i2c_probe(uchar chip)
>          * address was <ACK>ed (i.e. there was a chip at that address which
>          * drove the data line low).
>          */
> -       return i2c_transfer(i2c, I2C_READ, chip << 1, 0, 0, buf, 1) !=
> I2C_OK;
> +       if (proid_is_exynos5420() && (g_current_bus > 3))
> +               return (hsi2c_read(chip, 0, 1, buf, 1, 1) != I2C_OK);
>

I think the type of i2c port should be set by the FDT, not hard-coded
according to the chip type.


> +       else
> +               return i2c_transfer(
> +                               I2C_READ, chip << 1, 0, 0, buf, 1) !=
> I2C_OK;
>  }
>
>  int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
>  {
> -       struct s3c24x0_i2c *i2c;
>         uchar xaddr[4];
>         int ret;
>
> @@ -476,9 +803,13 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
> *buffer, int len)
>                 chip |= ((addr >> (alen * 8)) &
>                          CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
>  #endif
> -       i2c = get_base_i2c();
> -       ret = i2c_transfer(i2c, I2C_READ, chip << 1, &xaddr[4 - alen],
> alen,
> -                       buffer, len);
> +       if (proid_is_exynos5420() && (g_current_bus > 3))
> +               ret = hsi2c_read(chip, &xaddr[4 - alen],
> +                                               alen, buffer, len, 0);

+       else
> +               ret = i2c_transfer(I2C_READ, chip << 1,
> +                               &xaddr[4 - alen], alen, buffer, len);
> +
>         if (ret != 0) {
>                 debug("I2c read: failed %d\n", ret);
>                 return 1;
> @@ -488,7 +819,6 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
> *buffer, int len)
>
>  int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
>  {
> -       struct s3c24x0_i2c *i2c;
>         uchar xaddr[4];
>
>         if (alen > 4) {
> @@ -518,31 +848,36 @@ int i2c_write(uchar chip, uint addr, int alen, uchar
> *buffer, int len)
>                 chip |= ((addr >> (alen * 8)) &
>                          CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
>  #endif
> -       i2c = get_base_i2c();
> -       return (i2c_transfer
> -               (i2c, I2C_WRITE, chip << 1, &xaddr[4 - alen], alen, buffer,
> -                len) != 0);
> +       if (proid_is_exynos5420() && (g_current_bus > 3))
> +               return (hsi2c_write(chip, &xaddr[4 - alen],
> +                                       alen, buffer, len) != 0);
> +       else
> +               return (i2c_transfer(I2C_WRITE, chip << 1,
> +                               &xaddr[4 - alen], alen, buffer, len) != 0);
>  }
>
>  #ifdef CONFIG_OF_CONTROL
>  void board_i2c_init(const void *blob)
>  {
> +       struct s3c24x0_i2c_bus *bus;
>         int node_list[CONFIG_MAX_I2C_NUM];
>         int count, i;
>
>         count = fdtdec_find_aliases_for_id(blob, "i2c",
>                 COMPAT_SAMSUNG_S3C2440_I2C, node_list,
>                 CONFIG_MAX_I2C_NUM);
> -
>         for (i = 0; i < count; i++) {
> -               struct s3c24x0_i2c_bus *bus;
>                 int node = node_list[i];
>
>                 if (node <= 0)
>                         continue;
>                 bus = &i2c_bus[i];
> +
>                 bus->regs = (struct s3c24x0_i2c *)
> -                       fdtdec_get_addr(blob, node, "reg");
> +                               fdtdec_get_addr(blob, node, "reg");
>

Here you have bus->regs so you should use it in the driver.


> +               bus->hsregs = (struct exynos5_hsi2c *)
> +                               fdtdec_get_addr(blob, node, "reg");
> +
>

All of the ports here will be COMPAT_SAMSUNG_S3C2440_I2C. I think you need
to find ports of a different type for the high speed i2c. Perhaps look at
how tegra i2c handles having two different node types in the same driver.

The type of a port needs to be stored in the bus structure, and I think
this should be done even in the non-FDT case, to simplify the code.


>                 bus->id = pinmux_decode_periph_id(blob, node);
>                 bus->node = node;
>                 bus->bus_num = i2c_busses++;
> @@ -589,7 +924,11 @@ int i2c_reset_port_fdt(const void *blob, int node)
>                 return -1;
>         }
>
> -       i2c_ch_init(i2c->regs, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> +       if (proid_is_exynos5420() && (bus > 3))
> +               hsi2c_ch_init(i2c->hsregs, CONFIG_SYS_I2C_SPEED);
> +       else
> +               i2c_ch_init(i2c->regs, CONFIG_SYS_I2C_SPEED,
> +                                               CONFIG_SYS_I2C_SLAVE);
>
>         return 0;
>  }
> diff --git a/drivers/i2c/s3c24x0_i2c.h b/drivers/i2c/s3c24x0_i2c.h
> index a56d749..81ed19c 100644
> --- a/drivers/i2c/s3c24x0_i2c.h
> +++ b/drivers/i2c/s3c24x0_i2c.h
> @@ -31,10 +31,43 @@ struct s3c24x0_i2c {
>         u32     iiclc;
>  };
>
> +struct exynos5_hsi2c {
> +       u32     usi_ctl;
> +       u32     usi_fifo_ctl;
> +       u32     usi_trailing_ctl;
> +       u32     usi_clk_ctl;
> +       u32     usi_clk_slot;
> +       u32     spi_ctl;
> +       u32     uart_ctl;
> +       u32     res1;
> +       u32     usi_int_en;
> +       u32     usi_int_stat;
> +       u32     usi_modem_stat;
> +       u32     usi_error_stat;
> +       u32     usi_fifo_stat;
> +       u32     usi_txdata;
> +       u32     usi_rxdata;
> +       u32     res2;
> +       u32     usi_conf;
> +       u32     usi_auto_conf;
> +       u32     usi_timeout;
> +       u32     usi_manual_cmd;
> +       u32     usi_trans_status;
> +       u32     usi_timing_hs1;
> +       u32     usi_timing_hs2;
> +       u32     usi_timing_hs3;
> +       u32     usi_timing_fs1;
> +       u32     usi_timing_fs2;
> +       u32     usi_timing_fs3;
> +       u32     usi_timing_sla;
> +       u32     i2c_addr;
> +};
> +
>  struct s3c24x0_i2c_bus {
>         int node;       /* device tree node */
>         int bus_num;    /* i2c bus number */
>         struct s3c24x0_i2c *regs;
> +       struct exynos5_hsi2c *hsregs;
>         int id;
>  };
>  #endif /* _S3C24X0_I2C_H */
> --
> 1.7.9.5
>
> Regards,
Simon
naveen krishna chatradhi - March 12, 2013, 2:03 p.m.
On 12 March 2013 22:11, Simon Glass <sjg@chromium.org> wrote:
> Hi Naveen,
>
> On Tue, Mar 12, 2013 at 3:58 AM, Naveen Krishna Chatradhi <
> ch.naveen@samsung.com> wrote:
>
>> Add support for hsi2c controller available on exynos5420.
>>
>> Note: driver currently supports only fast speed mode 100kbps
>>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>> Signed-off-by: R. Chandrasekar <rc.sekar@samsung.com>
>>
>
> I just commented on the Linux driver, so some comments may apply here also.
>
>
>> ---
>>  drivers/i2c/s3c24x0_i2c.c |  389
>> ++++++++++++++++++++++++++++++++++++++++++---
>>  drivers/i2c/s3c24x0_i2c.h |   33 ++++
>>  2 files changed, 397 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
>> index 769a2ba..3117ab7 100644
>> --- a/drivers/i2c/s3c24x0_i2c.c
>> +++ b/drivers/i2c/s3c24x0_i2c.c
>> @@ -32,6 +32,7 @@
>>  #include <asm/arch/clk.h>
>>  #include <asm/arch/cpu.h>
>>  #include <asm/arch/pinmux.h>
>> +#include <asm/arch/clock.h>
>>  #else
>>  #include <asm/arch/s3c24x0_cpu.h>
>>  #endif
>> @@ -50,6 +51,60 @@
>>  #define I2C_NOK_LA     3       /* Lost arbitration */
>>  #define I2C_NOK_TOUT   4       /* time out */
>>
>> +/* HSI2C specific register description */
>> +
>> +/* I2C_CTL Register bits */
>> +#define HSI2C_FUNC_MODE_I2C            (1u << 0)
>> +#define HSI2C_MASTER                   (1u << 3)
>> +#define HSI2C_RXCHON                   (1u << 6)       /* Write/Send */
>> +#define HSI2C_TXCHON                   (1u << 7)       /* Read/Receive */
>> +#define HSI2C_SW_RST                   (1u << 31)
>> +
>> +/* I2C_FIFO_CTL Register bits */
>> +#define HSI2C_RXFIFO_EN                        (1u << 0)
>> +#define HSI2C_TXFIFO_EN                        (1u << 1)
>> +#define HSI2C_TXFIFO_TRIGGER_LEVEL     (0x20 << 16)
>> +#define HSI2C_RXFIFO_TRIGGER_LEVEL     (0x20 << 4)
>> +
>> +/* I2C_TRAILING_CTL Register bits */
>> +#define HSI2C_TRAILING_COUNT           (0xff)
>> +
>> +/* I2C_INT_EN Register bits */
>> +#define HSI2C_INT_TX_ALMOSTEMPTY_EN    (1u << 0)
>> +#define HSI2C_INT_RX_ALMOSTFULL_EN     (1u << 1)
>> +#define HSI2C_INT_TRAILING_EN          (1u << 6)
>> +#define HSI2C_INT_I2C_EN               (1u << 9)
>> +
>> +/* I2C_CONF Register bits */
>> +#define HSI2C_AUTO_MODE                        (1u << 31)
>> +#define HSI2C_10BIT_ADDR_MODE          (1u << 30)
>> +#define HSI2C_HS_MODE                  (1u << 29)
>> +
>> +/* I2C_AUTO_CONF Register bits */
>> +#define HSI2C_READ_WRITE               (1u << 16)
>> +#define HSI2C_STOP_AFTER_TRANS         (1u << 17)
>> +#define HSI2C_MASTER_RUN               (1u << 31)
>> +
>> +/* I2C_TIMEOUT Register bits */
>> +#define HSI2C_TIMEOUT_EN               (1u << 31)
>> +
>> +/* I2C_TRANS_STATUS register bits */
>> +#define HSI2C_MASTER_BUSY              (1u << 17)
>> +#define HSI2C_SLAVE_BUSY               (1u << 16)
>> +#define HSI2C_NO_DEV                   (1u << 3)
>> +#define HSI2C_NO_DEV_ACK               (1u << 2)
>> +#define HSI2C_TRANS_ABORT              (1u << 1)
>> +#define HSI2C_TRANS_DONE               (1u << 0)
>> +#define HSI2C_TIMEOUT_AUTO             (0u << 0)
>> +
>> +#define HSI2C_SLV_ADDR_MAS(x)          ((x & 0x3ff) << 10)
>> +
>> +/* Controller operating frequency, timing values for operation
>> + * are calculated against this frequency
>> + */
>> +#define HSI2C_FS_TX_CLOCK              1000000
>> +
>> +/* S3C I2C Controller bits */
>>  #define I2CSTAT_BSY    0x20    /* Busy bit */
>>  #define I2CSTAT_NACK   0x01    /* Nack bit */
>>  #define I2CCON_ACKGEN  0x80    /* Acknowledge generation */
>> @@ -61,6 +116,7 @@
>>
>>  #define I2C_TIMEOUT 1          /* 1 second */
>>
>> +#define        HSI2C_TIMEOUT   100
>>
>>  /*
>>   * For SPL boot some boards need i2c before SDRAM is initialised so force
>> @@ -120,9 +176,23 @@ static int WaitForXfer(struct s3c24x0_i2c *i2c)
>>         return (readl(&i2c->iiccon) & I2CCON_IRPND) ? I2C_OK :
>> I2C_NOK_TOUT;
>>  }
>>
>> -static int IsACK(struct s3c24x0_i2c *i2c)
>> +static int hsi2c_wait_for_irq(struct exynos5_hsi2c *i2c)
>>  {
>> -       return !(readl(&i2c->iicstat) & I2CSTAT_NACK);
>> +       int i = HSI2C_TIMEOUT * 10;
>> +       int ret = I2C_NOK_TOUT;
>> +
>> +       while (i > 0) {
>> +               /* wait for a while and retry */
>> +               udelay(50);
>> +               if (readl(&i2c->usi_int_stat) &
>> +                       (HSI2C_INT_I2C_EN | HSI2C_INT_TX_ALMOSTEMPTY_EN)) {
>> +                       ret = I2C_OK;
>> +                       break;
>> +               }
>> +               i--;
>>
>
> Please can we use the get_timer() method for timeouts instead of udelay()?
Sure thing
>
>
>> +       }
>> +
>> +       return ret;
>>  }
>>
>>  static void ReadWriteByte(struct s3c24x0_i2c *i2c)
>> @@ -130,6 +200,22 @@ static void ReadWriteByte(struct s3c24x0_i2c *i2c)
>>         writel(readl(&i2c->iiccon) & ~I2CCON_IRPND, &i2c->iiccon);
>>  }
>>
>> +static void hsi2c_clear_irqpd(struct exynos5_hsi2c *i2c)
>> +{
>> +       writel(readl(&i2c->usi_int_stat), &i2c->usi_int_stat);
>> +}
>> +
>> +static int hsi2c_isack(struct exynos5_hsi2c *i2c)
>> +{
>> +       return readl(&i2c->usi_trans_status) &
>> +                       (HSI2C_NO_DEV | HSI2C_NO_DEV_ACK);
>> +}
>> +
>> +static int IsACK(struct s3c24x0_i2c *i2c)
>>
>
> There are two functions - hsi2c_isack() and IsAck(). What is the difference?
There are 3 similar functions re-written for I2C and HSI2C. May be i
can use one function
once the DT is completely implemented in this driver
>
>
>> +{
>> +       return !(readl(&i2c->iicstat) & I2CSTAT_NACK);
>> +}
>> +
>>  static struct s3c24x0_i2c *get_base_i2c(void)
>>  {
>>  #ifdef CONFIG_EXYNOS4
>> @@ -147,6 +233,18 @@ static struct s3c24x0_i2c *get_base_i2c(void)
>>  #endif
>>  }
>>
>> +static struct exynos5_hsi2c *get_base_hsi2c(void)
>> +{
>> +       struct exynos5_hsi2c *i2c = NULL;
>> +       int bus = g_current_bus;
>> +
>> +       if (proid_is_exynos5420())
>> +               i2c = (struct exynos5_hsi2c *)(EXYNOS5420_I2C_BASE +
>> +                                       ((bus) * EXYNOS5_I2C_SPACING));
>> +
>> +       return i2c;
>> +}
>>
>
> This should come from the device tree information that you are decoding
> (the ->reg property).
I've not used the DT yet. I just followed the legacy code in the driver.
Will implement it and send v2 soon.
>
>
>> +
>>  static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd)
>>  {
>>         ulong freq, pres = 16, div;
>> @@ -174,6 +272,74 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c, int
>> speed, int slaveadd)
>>         writel(I2C_MODE_MT | I2C_TXRX_ENA, &i2c->iicstat);
>>  }
>>
>> +static void hsi2c_ch_init(struct exynos5_hsi2c *i2c, int speed)
>> +{
>> +       u32 i2c_timeout;
>> +       ulong clkin = get_i2c_clk();
>> +       u32 i2c_timing_s1;
>> +       u32 i2c_timing_s2;
>> +       u32 i2c_timing_s3;
>> +       u32 i2c_timing_sla;
>> +       unsigned int op_clk = HSI2C_FS_TX_CLOCK;
>> +       unsigned int n_clkdiv;
>> +       unsigned int t_start_su, t_start_hd;
>> +       unsigned int t_stop_su;
>> +       unsigned int t_data_su, t_data_hd;
>> +       unsigned int t_scl_l, t_scl_h;
>> +       unsigned int t_sr_release;
>> +       unsigned int t_ftl_cycle;
>> +       unsigned int i = 0, utemp0 = 0, utemp1 = 0, utemp2 = 0;
>> +
>> +       /* FPCLK / FI2C =
>> +        * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + 2 * FLT_CYCLE
>> +        * uTemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2)
>> +        * uTemp1 = (TSCLK_L + TSCLK_H + 2)
>> +        * uTemp2 = TSCLK_L + TSCLK_H
>> +        */
>> +       t_ftl_cycle = (readl(&i2c->usi_conf) >> 16) & 0x7;
>> +       utemp0 = (clkin / op_clk) - 8 - 2 * t_ftl_cycle;
>> +
>> +       /* CLK_DIV max is 256 */
>> +       for (i = 0; i < 256; i++) {
>> +               utemp1 = utemp0 / (i + 1);
>> +               /* SCLK_L/H max is 256 / 2 */
>> +               if (utemp1 < 128) {
>> +                       utemp2 = utemp1 - 2;
>> +                       break;
>> +               }
>> +       }
>> +
>> +       n_clkdiv = i;
>> +       t_scl_l = utemp2 / 2;
>> +       t_scl_h = utemp2 / 2;
>> +       t_start_su = t_scl_l;
>> +       t_start_hd = t_scl_l;
>> +       t_stop_su = t_scl_l;
>> +       t_data_su = t_scl_l / 2;
>> +       t_data_hd = t_scl_l / 2;
>> +       t_sr_release = utemp2;
>> +
>> +       i2c_timing_s1 = t_start_su << 24 | t_start_hd << 16 | t_stop_su <<
>> 8;
>> +       i2c_timing_s2 = t_data_su << 24 | t_scl_l << 8 | t_scl_h << 0;
>> +       i2c_timing_s3 = n_clkdiv << 16 | t_sr_release << 0;
>> +       i2c_timing_sla = t_data_hd << 0;
>> +
>> +       writel(HSI2C_TRAILING_COUNT, &i2c->usi_trailing_ctl);
>> +
>> +       /* Clear to enable Timeout */
>> +       i2c_timeout = readl(&i2c->usi_timeout);
>> +       i2c_timeout &= ~HSI2C_TIMEOUT_EN;
>> +       writel(i2c_timeout, &i2c->usi_timeout);
>> +
>> +       writel(readl(&i2c->usi_conf) | HSI2C_AUTO_MODE, &i2c->usi_conf);
>> +
>> +       /* Currently operating in Fast speed mode. */
>> +       writel(i2c_timing_s1, &i2c->usi_timing_fs1);
>> +       writel(i2c_timing_s2, &i2c->usi_timing_fs2);
>> +       writel(i2c_timing_s3, &i2c->usi_timing_fs3);
>> +       writel(i2c_timing_sla, &i2c->usi_timing_sla);
>> +}
>> +
>>  /*
>>   * MULTI BUS I2C support
>>   */
>> @@ -181,16 +347,18 @@ static void i2c_ch_init(struct s3c24x0_i2c *i2c, int
>> speed, int slaveadd)
>>  #ifdef CONFIG_I2C_MULTI_BUS
>>  int i2c_set_bus_num(unsigned int bus)
>>  {
>> -       struct s3c24x0_i2c *i2c;
>> -
>>         if ((bus < 0) || (bus >= CONFIG_MAX_I2C_NUM)) {
>>                 debug("Bad bus: %d\n", bus);
>>                 return -1;
>>         }
>>
>>         g_current_bus = bus;
>> -       i2c = get_base_i2c();
>> -       i2c_ch_init(i2c, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>> +       if (proid_is_exynos5420() && (bus > 3)) {
>> +               hsi2c_ch_init(get_base_hsi2c(),
>> +                                               CONFIG_SYS_I2C_SPEED);
>> +       } else
>> +               i2c_ch_init(get_base_i2c(), CONFIG_SYS_I2C_SPEED,
>> +                                               CONFIG_SYS_I2C_SLAVE);
>>
>>         return 0;
>>  }
>> @@ -269,24 +437,182 @@ void i2c_init(int speed, int slaveadd)
>>  }
>>
>>  /*
>> + * Send a STOP event and wait for it to have completed
>> + *
>> + * @param mode If it is a master transmitter or receiver
>> + * @return I2C_OK if the line became idle before timeout I2C_NOK_TOUT
>> otherwise
>> + */
>> +static int hsi2c_send_stop(struct exynos5_hsi2c *i2c, int result)
>> +{
>> +       int timeout;
>> +       int ret = I2C_NOK_TOUT;
>> +
>> +       /* Wait for the STOP to send and the bus to go idle */
>> +       for (timeout = HSI2C_TIMEOUT; timeout > 0; timeout -= 5) {
>> +               if (!(readl(&i2c->usi_trans_status) & HSI2C_MASTER_BUSY)) {
>> +                       ret = I2C_OK;
>> +                       goto out;
>> +               }
>> +               udelay(5);
>> +       }
>> + out:
>> +       /* Setting the STOP event to fire */
>> +       writel(HSI2C_FUNC_MODE_I2C, &i2c->usi_ctl);
>> +       writel(0x0, &i2c->usi_int_en);
>> +
>> +       return (result == I2C_OK) ? ret : result;
>> +}
>> +
>> +static int hsi2c_write(unsigned char chip,
>> +                       unsigned char addr[],
>> +                       unsigned char alen,
>> +                       unsigned char data[],
>> +                       unsigned short len)
>> +{
>> +       struct exynos5_hsi2c *i2c = get_base_hsi2c();
>> +       int i = 0, result = I2C_OK;
>> +       u32 i2c_auto_conf;
>> +       u32 stat;
>> +
>> +       /* Check I2C bus idle */
>> +       i = HSI2C_TIMEOUT * 20;
>> +       while ((readl(&i2c->usi_trans_status) & HSI2C_MASTER_BUSY)
>> +                                                       && (i > 0)) {
>> +               udelay(50);
>> +               i--;
>> +       }
>>
>
> It seems like perhaps you should have a function to wait until not busy and
> call it from the above function, and here.
Sure.
>
>
>> +
>> +       stat = readl(&i2c->usi_trans_status);
>> +
>> +       if (stat & HSI2C_MASTER_BUSY) {
>> +               debug("%s: bus busy\n", __func__);
>> +               return I2C_NOK_TOUT;
>> +       }
>> +       /* Disable TXFIFO and RXFIFO */
>> +       writel(0, &i2c->usi_fifo_ctl);
>> +
>> +       /* chip address */
>> +       writel(HSI2C_SLV_ADDR_MAS(chip), &i2c->i2c_addr);
>> +
>> +       /* Enable interrupts */
>> +       writel((HSI2C_INT_I2C_EN | HSI2C_INT_TX_ALMOSTEMPTY_EN),
>> +                                               &i2c->usi_int_en);
>> +
>> +       /* usi_ctl enable i2c func, master write configure */
>> +       writel((HSI2C_TXCHON | HSI2C_FUNC_MODE_I2C | HSI2C_MASTER),
>> +                                                       &i2c->usi_ctl);
>> +
>> +       /* i2c_conf configure */
>> +       writel(readl(&i2c->usi_conf) | HSI2C_AUTO_MODE, &i2c->usi_conf);
>> +
>> +       /* auto_conf for write length and stop configure */
>> +       i2c_auto_conf = ((len + alen) | HSI2C_STOP_AFTER_TRANS);
>> +       i2c_auto_conf &= ~HSI2C_READ_WRITE;
>> +       writel(i2c_auto_conf, &i2c->usi_auto_conf);
>> +
>> +       /* Master run, start xfer */
>> +       writel(readl(&i2c->usi_auto_conf) | HSI2C_MASTER_RUN,
>> +                                               &i2c->usi_auto_conf);
>> +
>> +       result = hsi2c_wait_for_irq(i2c);
>> +       if ((result == I2C_OK) && hsi2c_isack(i2c)) {
>> +               result = I2C_NACK;
>> +               goto err;
>> +       }
>> +
>> +       for (i = 0; i < alen && (result == I2C_OK); i++) {
>> +               writel(addr[i], &i2c->usi_txdata);
>> +               result = hsi2c_wait_for_irq(i2c);
>> +       }
>> +
>> +       for (i = 0; i < len && (result == I2C_OK); i++) {
>> +               writel(data[i], &i2c->usi_txdata);
>> +               result = hsi2c_wait_for_irq(i2c);
>>
>
> Do we have to wait for an irq after every tx byte?
Actually, FIFOs were not implemented at all in the u-boot driver.
FIFOs are 64 depth and of variable sizes. Will test with them enabled
>
>
>> +       }
>> +
>> + err:
>> +       hsi2c_clear_irqpd(i2c);
>> +       return hsi2c_send_stop(i2c, result);
>> +}
>> +
>> +static int hsi2c_read(unsigned char chip,
>> +                       unsigned char addr[],
>> +                       unsigned char alen,
>> +                       unsigned char data[],
>> +                       unsigned short len,
>> +                       int check)
>>
>
> This seems to have a lot of common code with the above. Should they be
> joined up?
Common initialization code can be split out.
>
>
>> +{
>> +       struct exynos5_hsi2c *i2c = get_base_hsi2c();
>> +       int i, result;
>> +       u32 i2c_auto_conf;
>> +
>> +       if (!check) {
>> +               result =  hsi2c_write(chip, addr, alen, data, 0);
>> +               if (result != I2C_OK) {
>> +                       debug("write failed Result = %d\n", result);
>> +                       return result;
>> +               }
>> +       }
>> +
>> +       /* start read */
>> +       /* Disable TXFIFO and RXFIFO */
>> +       writel(0, &i2c->usi_fifo_ctl);
>> +
>> +       /* chip address */
>> +       writel(HSI2C_SLV_ADDR_MAS(chip), &i2c->i2c_addr);
>> +
>> +       /* Enable interrupts */
>> +       writel(HSI2C_INT_I2C_EN, &i2c->usi_int_en);
>> +
>> +               /* i2c_conf configure */
>>
>
> outdent
>
>
>> +       writel(readl(&i2c->usi_conf) | HSI2C_AUTO_MODE, &i2c->usi_conf);
>> +
>> +       /* auto_conf, length and stop configure */
>> +       i2c_auto_conf = (len | HSI2C_STOP_AFTER_TRANS | HSI2C_READ_WRITE);
>> +       writel(i2c_auto_conf, &i2c->usi_auto_conf);
>> +
>> +       /* usi_ctl enable i2c func, master WRITE configure */
>> +       writel((HSI2C_RXCHON | HSI2C_FUNC_MODE_I2C | HSI2C_MASTER),
>> +                                                       &i2c->usi_ctl);
>> +
>> +       /* Master run, start xfer */
>> +       writel(readl(&i2c->usi_auto_conf) | HSI2C_MASTER_RUN,
>> +                                               &i2c->usi_auto_conf);
>> +
>> +       result = hsi2c_wait_for_irq(i2c);
>> +       if ((result == I2C_OK) && hsi2c_isack(i2c)) {
>> +               result = I2C_NACK;
>> +               goto err;
>> +       }
>> +
>> +       for (i = 0; i < len && (result == I2C_OK); i++) {
>> +               result = hsi2c_wait_for_irq(i2c);
>> +               data[i] = readl(&i2c->usi_rxdata);
>> +       }
>>
>
> I suppose the FIFOs are not used here. How larger are the FIFOs?
>
>
>> + err:
>> +       /* Stop and quit */
>> +       hsi2c_clear_irqpd(i2c);
>> +       return hsi2c_send_stop(i2c, result);
>> +}
>> +
>> +/*
>>   * cmd_type is 0 for write, 1 for read.
>>   *
>>   * addr_len can take any value from 0-255, it is only limited
>>   * by the char, we could make it larger if needed. If it is
>>   * 0 we skip the address write cycle.
>>   */
>> -static int i2c_transfer(struct s3c24x0_i2c *i2c,
>> -                       unsigned char cmd_type,
>> +static int i2c_transfer(unsigned char cmd_type,
>>                         unsigned char chip,
>>                         unsigned char addr[],
>>                         unsigned char addr_len,
>>                         unsigned char data[],
>>                         unsigned short data_len)
>>  {
>> +       struct s3c24x0_i2c *i2c = get_base_i2c();
>>         int i, result;
>>
>>         if (data == 0 || data_len == 0) {
>> -               /*Don't support data transfer of no length or to address 0
>> */
>>                 debug("i2c_transfer: bad call\n");
>>                 return I2C_NOK;
>>         }
>> @@ -428,10 +754,8 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
>>
>>  int i2c_probe(uchar chip)
>>  {
>> -       struct s3c24x0_i2c *i2c;
>>         uchar buf[1];
>>
>> -       i2c = get_base_i2c();
>>         buf[0] = 0;
>>
>>         /*
>> @@ -439,12 +763,15 @@ int i2c_probe(uchar chip)
>>          * address was <ACK>ed (i.e. there was a chip at that address which
>>          * drove the data line low).
>>          */
>> -       return i2c_transfer(i2c, I2C_READ, chip << 1, 0, 0, buf, 1) !=
>> I2C_OK;
>> +       if (proid_is_exynos5420() && (g_current_bus > 3))
>> +               return (hsi2c_read(chip, 0, 1, buf, 1, 1) != I2C_OK);
>>
>
> I think the type of i2c port should be set by the FDT, not hard-coded
> according to the chip type.
Will implement it, that would simplify a lot of checks for future aswell.
>
>
>> +       else
>> +               return i2c_transfer(
>> +                               I2C_READ, chip << 1, 0, 0, buf, 1) !=
>> I2C_OK;
>>  }
>>
>>  int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
>>  {
>> -       struct s3c24x0_i2c *i2c;
>>         uchar xaddr[4];
>>         int ret;
>>
>> @@ -476,9 +803,13 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
>> *buffer, int len)
>>                 chip |= ((addr >> (alen * 8)) &
>>                          CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
>>  #endif
>> -       i2c = get_base_i2c();
>> -       ret = i2c_transfer(i2c, I2C_READ, chip << 1, &xaddr[4 - alen],
>> alen,
>> -                       buffer, len);
>> +       if (proid_is_exynos5420() && (g_current_bus > 3))
>> +               ret = hsi2c_read(chip, &xaddr[4 - alen],
>> +                                               alen, buffer, len, 0);
>
> +       else
>> +               ret = i2c_transfer(I2C_READ, chip << 1,
>> +                               &xaddr[4 - alen], alen, buffer, len);
>> +
>>         if (ret != 0) {
>>                 debug("I2c read: failed %d\n", ret);
>>                 return 1;
>> @@ -488,7 +819,6 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
>> *buffer, int len)
>>
>>  int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
>>  {
>> -       struct s3c24x0_i2c *i2c;
>>         uchar xaddr[4];
>>
>>         if (alen > 4) {
>> @@ -518,31 +848,36 @@ int i2c_write(uchar chip, uint addr, int alen, uchar
>> *buffer, int len)
>>                 chip |= ((addr >> (alen * 8)) &
>>                          CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
>>  #endif
>> -       i2c = get_base_i2c();
>> -       return (i2c_transfer
>> -               (i2c, I2C_WRITE, chip << 1, &xaddr[4 - alen], alen, buffer,
>> -                len) != 0);
>> +       if (proid_is_exynos5420() && (g_current_bus > 3))
>> +               return (hsi2c_write(chip, &xaddr[4 - alen],
>> +                                       alen, buffer, len) != 0);
>> +       else
>> +               return (i2c_transfer(I2C_WRITE, chip << 1,
>> +                               &xaddr[4 - alen], alen, buffer, len) != 0);
>>  }
>>
>>  #ifdef CONFIG_OF_CONTROL
>>  void board_i2c_init(const void *blob)
>>  {
>> +       struct s3c24x0_i2c_bus *bus;
>>         int node_list[CONFIG_MAX_I2C_NUM];
>>         int count, i;
>>
>>         count = fdtdec_find_aliases_for_id(blob, "i2c",
>>                 COMPAT_SAMSUNG_S3C2440_I2C, node_list,
>>                 CONFIG_MAX_I2C_NUM);
>> -
>>         for (i = 0; i < count; i++) {
>> -               struct s3c24x0_i2c_bus *bus;
>>                 int node = node_list[i];
>>
>>                 if (node <= 0)
>>                         continue;
>>                 bus = &i2c_bus[i];
>> +
>>                 bus->regs = (struct s3c24x0_i2c *)
>> -                       fdtdec_get_addr(blob, node, "reg");
>> +                               fdtdec_get_addr(blob, node, "reg");
>>
>
> Here you have bus->regs so you should use it in the driver.
>
>
>> +               bus->hsregs = (struct exynos5_hsi2c *)
>> +                               fdtdec_get_addr(blob, node, "reg");
>> +
>>
>
> All of the ports here will be COMPAT_SAMSUNG_S3C2440_I2C. I think you need
> to find ports of a different type for the high speed i2c. Perhaps look at
> how tegra i2c handles having two different node types in the same driver.
This would help Simon, Thanks.
>
> The type of a port needs to be stored in the bus structure, and I think
> this should be done even in the non-FDT case, to simplify the code.
Right, Will go through the references and submit a v2 with the DT changes fixed.
>
>
>>                 bus->id = pinmux_decode_periph_id(blob, node);
>>                 bus->node = node;
>>                 bus->bus_num = i2c_busses++;
>> @@ -589,7 +924,11 @@ int i2c_reset_port_fdt(const void *blob, int node)
>>                 return -1;
>>         }
>>
>> -       i2c_ch_init(i2c->regs, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>> +       if (proid_is_exynos5420() && (bus > 3))
>> +               hsi2c_ch_init(i2c->hsregs, CONFIG_SYS_I2C_SPEED);
>> +       else
>> +               i2c_ch_init(i2c->regs, CONFIG_SYS_I2C_SPEED,
>> +                                               CONFIG_SYS_I2C_SLAVE);
>>
>>         return 0;
>>  }
>> diff --git a/drivers/i2c/s3c24x0_i2c.h b/drivers/i2c/s3c24x0_i2c.h
>> index a56d749..81ed19c 100644
>> --- a/drivers/i2c/s3c24x0_i2c.h
>> +++ b/drivers/i2c/s3c24x0_i2c.h
>> @@ -31,10 +31,43 @@ struct s3c24x0_i2c {
>>         u32     iiclc;
>>  };
>>
>> +struct exynos5_hsi2c {
>> +       u32     usi_ctl;
>> +       u32     usi_fifo_ctl;
>> +       u32     usi_trailing_ctl;
>> +       u32     usi_clk_ctl;
>> +       u32     usi_clk_slot;
>> +       u32     spi_ctl;
>> +       u32     uart_ctl;
>> +       u32     res1;
>> +       u32     usi_int_en;
>> +       u32     usi_int_stat;
>> +       u32     usi_modem_stat;
>> +       u32     usi_error_stat;
>> +       u32     usi_fifo_stat;
>> +       u32     usi_txdata;
>> +       u32     usi_rxdata;
>> +       u32     res2;
>> +       u32     usi_conf;
>> +       u32     usi_auto_conf;
>> +       u32     usi_timeout;
>> +       u32     usi_manual_cmd;
>> +       u32     usi_trans_status;
>> +       u32     usi_timing_hs1;
>> +       u32     usi_timing_hs2;
>> +       u32     usi_timing_hs3;
>> +       u32     usi_timing_fs1;
>> +       u32     usi_timing_fs2;
>> +       u32     usi_timing_fs3;
>> +       u32     usi_timing_sla;
>> +       u32     i2c_addr;
>> +};
>> +
>>  struct s3c24x0_i2c_bus {
>>         int node;       /* device tree node */
>>         int bus_num;    /* i2c bus number */
>>         struct s3c24x0_i2c *regs;
>> +       struct exynos5_hsi2c *hsregs;
>>         int id;
>>  };
>>  #endif /* _S3C24X0_I2C_H */
>> --
>> 1.7.9.5
>>
>> Regards,
> Simon
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Naveen Krishna Ch - Sept. 30, 2013, 6:58 a.m.
This patchset fixes few bugs in the existing s3c24x0.c code (standard i2c)
and add support for High-speed i2c bus controller available on Exynos5 SoCs
from Samsung.

Exynos5250 channels [0 ~ 3] can configured for I2C contoller or
           new HSI2C controller
           channels [3 ~ 7] are standard I2C controller channels
Exynos5420 channels [0 ~ 3] are standard I2C controller channels and
           channels [4 ~ 10] are High-speed controller channels

Patchset:
1.  exynos: i2c: Fix i2c driver to handle NACKs properly
    Improvements and fixes from Vadim Bendebury for standard i2c calls

2.  exynos: i2c: Change FDT bus setup code to enumerate ports correctly
    FDT bus setup code from Simon Glass

3.  PATCH v5: i2c: s3c24xx: add hsi2c controller support
    High-speed controller register description and defines new i2c read/write,
    probe and set_bus calls.

    This is been reviewed earlier at
    http://lists.denx.de/pipermail/u-boot/2013-May/153245.html
    Thanks for review and improvements from Vadim Bendebury.

Question:
4. RFC: samsung: i2c: Enable new CONFIG_SYS_I2C framework
I've tried to implement the new I2C multi bus framework in u-boot tree,
taking tegra-i2c.c as reference.

a). We have different number of channels on Exynos5250 and Exynos5420
   (as explained above). How are we supposed to define the U_BOOT_I2C_ADAP_COMPLETE
   in such cases. Can i use #ifdef EXYNOS5420 or EXYNOS5250
b). When i define 11 buses as in the case of Exynos5420, the "i2c bus" lists them
 SMDK5420 # i2c bus
 Bus 0:  s3c0
 Bus 1:  s3c1
 Bus 2:  s3c10
 Bus 3:  s3c2
 Bus 4:  s3c3
 Bus 5:  s3c4
 Bus 6:  s3c5
 Bus 7:  s3c6
 Bus 8:  s3c7
 Bus 9:  s3c8
 Bus 10: s3c9
 or  (If i change the name to hsi2c)
 SMDK5420 # i2c bus
 Bus 0:  hsi2c10
 Bus 1:  hsi2c4
 Bus 2:  hsi2c5
 Bus 3:  hsi2c6
 Bus 4:  hsi2c7
 Bus 5:  hsi2c8
 Bus 6:  hsi2c9
 Bus 7:  s3c0
 Bus 8:  s3c1
 Bus 9:  s3c2
 Bus 10: s3c3

Whats the expected behaviour. If the above result is correct, I need to changei
the strings to get them in the correct order.

c). What's the alternative for the
   board_i2c_init(), i2c_get_bus_num_fdt(), i2c_reset_port_fdt().
  "Functions to get the I2C bus number and reset I2C bus using FDT node"

   I think, these functions are still needed.

Kindly, help me with the above questions.
I'm willing to implement the new i2c frame work.

Thanks in advance.

Naveen Krishna Chatradhi (3):
  exynos: i2c: Fix i2c driver to handle NACKs properly
  i2c: s3c24xx: add hsi2c controller support
  RFC: samsung: i2c: Enable new CONFIG_SYS_I2C framework

Simon Glass (1):
  exynos: i2c: Change FDT bus setup code to enumerate ports correctly

 drivers/i2c/Makefile      |    2 +-
 drivers/i2c/s3c24x0_i2c.c |  912 ++++++++++++++++++++++++++++++++++++---------
 drivers/i2c/s3c24x0_i2c.h |   38 ++
 3 files changed, 771 insertions(+), 181 deletions(-)
Heiko Schocher - Sept. 30, 2013, 8:05 a.m.
Hello Naveen,

Am 30.09.2013 08:58, schrieb Naveen Krishna Chatradhi:
> This patchset fixes few bugs in the existing s3c24x0.c code (standard i2c)
> and add support for High-speed i2c bus controller available on Exynos5 SoCs
> from Samsung.
>
> Exynos5250 channels [0 ~ 3] can configured for I2C contoller or
>             new HSI2C controller
>             channels [3 ~ 7] are standard I2C controller channels
> Exynos5420 channels [0 ~ 3] are standard I2C controller channels and
>             channels [4 ~ 10] are High-speed controller channels
>
> Patchset:
> 1.  exynos: i2c: Fix i2c driver to handle NACKs properly
>      Improvements and fixes from Vadim Bendebury for standard i2c calls
>
> 2.  exynos: i2c: Change FDT bus setup code to enumerate ports correctly
>      FDT bus setup code from Simon Glass
>
> 3.  PATCH v5: i2c: s3c24xx: add hsi2c controller support
>      High-speed controller register description and defines new i2c read/write,
>      probe and set_bus calls.
>
>      This is been reviewed earlier at
>      http://lists.denx.de/pipermail/u-boot/2013-May/153245.html
>      Thanks for review and improvements from Vadim Bendebury.
>
> Question:
> 4. RFC: samsung: i2c: Enable new CONFIG_SYS_I2C framework
> I've tried to implement the new I2C multi bus framework in u-boot tree,
> taking tegra-i2c.c as reference.
>
> a). We have different number of channels on Exynos5250 and Exynos5420
>     (as explained above). How are we supposed to define the U_BOOT_I2C_ADAP_COMPLETE
>     in such cases. Can i use #ifdef EXYNOS5420 or EXYNOS5250

 From my side, Yes.

> b). When i define 11 buses as in the case of Exynos5420, the "i2c bus" lists them
>   SMDK5420 # i2c bus
>   Bus 0:  s3c0
>   Bus 1:  s3c1
>   Bus 2:  s3c10
>   Bus 3:  s3c2
>   Bus 4:  s3c3
>   Bus 5:  s3c4
>   Bus 6:  s3c5
>   Bus 7:  s3c6
>   Bus 8:  s3c7
>   Bus 9:  s3c8
>   Bus 10: s3c9
>   or  (If i change the name to hsi2c)
>   SMDK5420 # i2c bus
>   Bus 0:  hsi2c10
>   Bus 1:  hsi2c4
>   Bus 2:  hsi2c5
>   Bus 3:  hsi2c6
>   Bus 4:  hsi2c7
>   Bus 5:  hsi2c8
>   Bus 6:  hsi2c9
>   Bus 7:  s3c0
>   Bus 8:  s3c1
>   Bus 9:  s3c2
>   Bus 10: s3c3
>
> Whats the expected behaviour. If the above result is correct, I need to changei
> the strings to get them in the correct order.

What, if you use two digits:

   Bus 0:  hsi2c01
   Bus 1:  hsi2c02
   [...]
   Bus 7:  s3c00
   Bus 8:  s3c01
   [...]

?

Or with one digit:

   Bus 0:  hsi2c1
   Bus 1:  hsi2c2
   [...]
   Bus 7:  s3c0
   Bus 8:  s3c1
   [...]

> c). What's the alternative for the
>     board_i2c_init(), i2c_get_bus_num_fdt(), i2c_reset_port_fdt().
>    "Functions to get the I2C bus number and reset I2C bus using FDT node"
>
>     I think, these functions are still needed.

Hmm.. that needs a general discussion, how to use fdt and i2c I think.

I would prefer a way (not really nowing, if it is possible for all
configurations) where, if using fdt, the DT gets parsed and the
availiable i2c buses gets created ... After that, "normal" i2c access
with i2c_set_bus_num(), i2c_read/write should be possible ... this is
currently not possible with the i2c framework, but should not be so hard
to do. Except the restriction, that it would not work in SPL case, or
before relocation for i2c buses announced through dt

Define CONFIG_SYS_I2C_MAX_HOPS -> CONFIG_SYS_I2C_DIRECT_BUS is not defined
so i2c_bus[] is used in drivers/i2c/i2c_core.c. Define the fix i2c buses in
the board config file with CONFIG_SYS_I2C_BUSES (if you have no fix buses,
let this empty) and add a function in drivers/i2c/i2c_core.c, which adds
new i2c buses to i2c_bus[] after the fix buses and call this new function,
from where you interpret the fdt ...

int i2c_get_bus_num_fdt(int node) should be integrated to drivers/i2c/i2c_core.c

For what purpose is i2c_reset_port_fdt() ?

bye,
Heiko
naveen krishna chatradhi - Sept. 30, 2013, 10:03 a.m.
Helo Heiko,

Thanks for timely reply.

On 30 September 2013 13:35, Heiko Schocher <hs@denx.de> wrote:
> Hello Naveen,
>
> Am 30.09.2013 08:58, schrieb Naveen Krishna Chatradhi:
>
>> This patchset fixes few bugs in the existing s3c24x0.c code (standard i2c)
>> and add support for High-speed i2c bus controller available on Exynos5
>> SoCs
>> from Samsung.
>>
>> Exynos5250 channels [0 ~ 3] can configured for I2C contoller or
>>             new HSI2C controller
>>             channels [3 ~ 7] are standard I2C controller channels
>> Exynos5420 channels [0 ~ 3] are standard I2C controller channels and
>>             channels [4 ~ 10] are High-speed controller channels
>>
>> Patchset:
>> 1.  exynos: i2c: Fix i2c driver to handle NACKs properly
>>      Improvements and fixes from Vadim Bendebury for standard i2c calls
>>
>> 2.  exynos: i2c: Change FDT bus setup code to enumerate ports correctly
>>      FDT bus setup code from Simon Glass
>>
>> 3.  PATCH v5: i2c: s3c24xx: add hsi2c controller support
>>      High-speed controller register description and defines new i2c
>> read/write,
>>      probe and set_bus calls.
>>
>>      This is been reviewed earlier at
>>      http://lists.denx.de/pipermail/u-boot/2013-May/153245.html
>>      Thanks for review and improvements from Vadim Bendebury.
>>
>> Question:
>> 4. RFC: samsung: i2c: Enable new CONFIG_SYS_I2C framework
>> I've tried to implement the new I2C multi bus framework in u-boot tree,
>> taking tegra-i2c.c as reference.
>>
>> a). We have different number of channels on Exynos5250 and Exynos5420
>>     (as explained above). How are we supposed to define the
>> U_BOOT_I2C_ADAP_COMPLETE
>>     in such cases. Can i use #ifdef EXYNOS5420 or EXYNOS5250
>
>
> From my side, Yes.
Ok, Will do accordingly.
But, I would prefer a way of integrating it to FDT or passing as
platform data (board files)
instead of keeping it in the driver.

>
>
>> b). When i define 11 buses as in the case of Exynos5420, the "i2c bus"
>> lists them
>>   SMDK5420 # i2c bus
>>   Bus 0:  s3c0
>>   Bus 1:  s3c1
>>   Bus 2:  s3c10
>>   Bus 3:  s3c2
>>   Bus 4:  s3c3
>>   Bus 5:  s3c4
>>   Bus 6:  s3c5
>>   Bus 7:  s3c6
>>   Bus 8:  s3c7
>>   Bus 9:  s3c8
>>   Bus 10: s3c9
>>   or  (If i change the name to hsi2c)
>>   SMDK5420 # i2c bus
>>   Bus 0:  hsi2c10
>>   Bus 1:  hsi2c4
>>   Bus 2:  hsi2c5
>>   Bus 3:  hsi2c6
>>   Bus 4:  hsi2c7
>>   Bus 5:  hsi2c8
>>   Bus 6:  hsi2c9
>>   Bus 7:  s3c0
>>   Bus 8:  s3c1
>>   Bus 9:  s3c2
>>   Bus 10: s3c3
>>
>> Whats the expected behaviour. If the above result is correct, I need to
>> changei
>> the strings to get them in the correct order.
>
>
> What, if you use two digits:
>
>   Bus 0:  hsi2c01
>   Bus 1:  hsi2c02
>   [...]
>   Bus 7:  s3c00
>   Bus 8:  s3c01
>   [...]
>
> ?
>
> Or with one digit:
>
>   Bus 0:  hsi2c1
>   Bus 1:  hsi2c2
>   [...]
>
>   Bus 7:  s3c0
>   Bus 8:  s3c1
>   [...]
In the Exynos5420 hardware
channels are as below

channel:  --0----1----2----3------4--------5---------6-------7--------8-------9-------10
controller: i2c, i2c, i2c, i2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c.
But the "i2c bus" command is showing the bus number according to the "name"
string comparison.  Which seems wrong. Isn't it ??

>
>
>> c). What's the alternative for the
>>     board_i2c_init(), i2c_get_bus_num_fdt(), i2c_reset_port_fdt().
>>    "Functions to get the I2C bus number and reset I2C bus using FDT node"
>>
>>     I think, these functions are still needed.
>
>
> Hmm.. that needs a general discussion, how to use fdt and i2c I think.
>
> I would prefer a way (not really nowing, if it is possible for all
> configurations) where, if using fdt, the DT gets parsed and the
> availiable i2c buses gets created ... After that, "normal" i2c access
> with i2c_set_bus_num(), i2c_read/write should be possible ... this is
> currently not possible with the i2c framework, but should not be so hard
> to do. Except the restriction, that it would not work in SPL case, or
> before relocation for i2c buses announced through dt
once i2c_init_board() is done board_i2c_init() is not quite needed
using i2c_init_board we can avoid the relocation problem aswell.

by the definition of i2c_get_bus_num() in drivers/i2c/i2c_core.c

unsigned int i2c_get_bus_num(void)
{
        return gd->cur_i2c_bus;
}

we don't need a special function i2c_get_bus_num_fdt()

IMHO, i2c_reset_port_fdt() is the only function to be discussed
>
> Define CONFIG_SYS_I2C_MAX_HOPS -> CONFIG_SYS_I2C_DIRECT_BUS is not defined
> so i2c_bus[] is used in drivers/i2c/i2c_core.c. Define the fix i2c buses in
> the board config file with CONFIG_SYS_I2C_BUSES (if you have no fix buses,
> let this empty) and add a function in drivers/i2c/i2c_core.c, which adds
> new i2c buses to i2c_bus[] after the fix buses and call this new function,
> from where you interpret the fdt ...
I din't quite understood this.
Can you point me to some readme or Doc or discussion Please.
>
> int i2c_get_bus_num_fdt(int node) should be integrated to
> drivers/i2c/i2c_core.c
>
> For what purpose is i2c_reset_port_fdt() ?
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Thanks,
Heiko Schocher - Oct. 1, 2013, 6:05 a.m.
Hello Naveen,

Am 30.09.2013 12:03, schrieb Naveen Krishna Ch:
> Helo Heiko,
>
> Thanks for timely reply.
>
> On 30 September 2013 13:35, Heiko Schocher<hs@denx.de>  wrote:
>> Hello Naveen,
>>
>> Am 30.09.2013 08:58, schrieb Naveen Krishna Chatradhi:
>>
>>> This patchset fixes few bugs in the existing s3c24x0.c code (standard i2c)
>>> and add support for High-speed i2c bus controller available on Exynos5
>>> SoCs
>>> from Samsung.
>>>
>>> Exynos5250 channels [0 ~ 3] can configured for I2C contoller or
>>>              new HSI2C controller
>>>              channels [3 ~ 7] are standard I2C controller channels
>>> Exynos5420 channels [0 ~ 3] are standard I2C controller channels and
>>>              channels [4 ~ 10] are High-speed controller channels
>>>
>>> Patchset:
>>> 1.  exynos: i2c: Fix i2c driver to handle NACKs properly
>>>       Improvements and fixes from Vadim Bendebury for standard i2c calls
>>>
>>> 2.  exynos: i2c: Change FDT bus setup code to enumerate ports correctly
>>>       FDT bus setup code from Simon Glass
>>>
>>> 3.  PATCH v5: i2c: s3c24xx: add hsi2c controller support
>>>       High-speed controller register description and defines new i2c
>>> read/write,
>>>       probe and set_bus calls.
>>>
>>>       This is been reviewed earlier at
>>>       http://lists.denx.de/pipermail/u-boot/2013-May/153245.html
>>>       Thanks for review and improvements from Vadim Bendebury.
>>>
>>> Question:
>>> 4. RFC: samsung: i2c: Enable new CONFIG_SYS_I2C framework
>>> I've tried to implement the new I2C multi bus framework in u-boot tree,
>>> taking tegra-i2c.c as reference.
[...]
>>> b). When i define 11 buses as in the case of Exynos5420, the "i2c bus"
>>> lists them
>>>    SMDK5420 # i2c bus
>>>    Bus 0:  s3c0
>>>    Bus 1:  s3c1
>>>    Bus 2:  s3c10
>>>    Bus 3:  s3c2
>>>    Bus 4:  s3c3
>>>    Bus 5:  s3c4
>>>    Bus 6:  s3c5
>>>    Bus 7:  s3c6
>>>    Bus 8:  s3c7
>>>    Bus 9:  s3c8
>>>    Bus 10: s3c9
>>>    or  (If i change the name to hsi2c)
>>>    SMDK5420 # i2c bus
>>>    Bus 0:  hsi2c10
>>>    Bus 1:  hsi2c4
>>>    Bus 2:  hsi2c5
>>>    Bus 3:  hsi2c6
>>>    Bus 4:  hsi2c7
>>>    Bus 5:  hsi2c8
>>>    Bus 6:  hsi2c9
>>>    Bus 7:  s3c0
>>>    Bus 8:  s3c1
>>>    Bus 9:  s3c2
>>>    Bus 10: s3c3
>>>
>>> Whats the expected behaviour. If the above result is correct, I need to
>>> changei
>>> the strings to get them in the correct order.
>>
>>
>> What, if you use two digits:
>>
>>    Bus 0:  hsi2c01
>>    Bus 1:  hsi2c02
>>    [...]
>>    Bus 7:  s3c00
>>    Bus 8:  s3c01
>>    [...]
>>
>> ?
>>
>> Or with one digit:
>>
>>    Bus 0:  hsi2c1
>>    Bus 1:  hsi2c2
>>    [...]
>>
>>    Bus 7:  s3c0
>>    Bus 8:  s3c1
>>    [...]
> In the Exynos5420 hardware
> channels are as below
>
> channel:  --0----1----2----3------4--------5---------6-------7--------8-------9-------10
> controller: i2c, i2c, i2c, i2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c.
> But the "i2c bus" command is showing the bus number according to the "name"
> string comparison.  Which seems wrong. Isn't it ??

Hmm.. you are right, seems that the compiler sorts them alphabetical ...
So, two ways:
- use another name, saying first a two digit for the channel ?
- or, use CONFIG_SYS_NUM_I2C_BUSES, CONFIG_SYS_I2C_DIRECT_BUS
   and CONFIG_SYS_I2C_BUSES as described in the README (grep
   for CONFIG_SYS_I2C_BUSES in include/configs and you will find
   some examples ...) and you can define, which adapter is on which
   i2c_bus number ...

>>> c). What's the alternative for the
>>>      board_i2c_init(), i2c_get_bus_num_fdt(), i2c_reset_port_fdt().
>>>     "Functions to get the I2C bus number and reset I2C bus using FDT node"
>>>
>>>      I think, these functions are still needed.
>>
>>
>> Hmm.. that needs a general discussion, how to use fdt and i2c I think.
>>
>> I would prefer a way (not really nowing, if it is possible for all
>> configurations) where, if using fdt, the DT gets parsed and the
>> availiable i2c buses gets created ... After that, "normal" i2c access
>> with i2c_set_bus_num(), i2c_read/write should be possible ... this is
>> currently not possible with the i2c framework, but should not be so hard
>> to do. Except the restriction, that it would not work in SPL case, or
>> before relocation for i2c buses announced through dt
> once i2c_init_board() is done board_i2c_init() is not quite needed
> using i2c_init_board we can avoid the relocation problem aswell.
>
> by the definition of i2c_get_bus_num() in drivers/i2c/i2c_core.c
>
> unsigned int i2c_get_bus_num(void)
> {
>          return gd->cur_i2c_bus;
> }
>
> we don't need a special function i2c_get_bus_num_fdt()

Ah, ok!

> IMHO, i2c_reset_port_fdt() is the only function to be discussed

And why is i2c_init() not good? Why must we have here a new function?

>> Define CONFIG_SYS_I2C_MAX_HOPS ->  CONFIG_SYS_I2C_DIRECT_BUS is not defined
>> so i2c_bus[] is used in drivers/i2c/i2c_core.c. Define the fix i2c buses in
>> the board config file with CONFIG_SYS_I2C_BUSES (if you have no fix buses,
>> let this empty) and add a function in drivers/i2c/i2c_core.c, which adds
>> new i2c buses to i2c_bus[] after the fix buses and call this new function,
>> from where you interpret the fdt ...
> I din't quite understood this.
> Can you point me to some readme or Doc or discussion Please.

just the U-Boot README ... The above was just a fast idea, how it
is possible to add i2c buses from the info in the fdt ...

Here some theoretical code, how it could look like:

in the board config file add:

#define CONFIG_SYS_I2C_DIRECT_BUS       1
#define CONFIG_SYS_I2C_MAX_HOPS         0
#define CONFIG_SYS_NUM_I2C_BUSES	10 /* maximum possible i2c buses used on the hw */
#define CONFIG_SYS_FIX_I2C_BUSES	1 /* just as an example, here with one fix i2c bus */
#define CONFIG_SYS_I2C_BUSES    {       {0}, /* adapter 0 is fix on i2c_bus number 0 */
                                 }

in drivers/i2c/i2c_core.c:

static int i2c_fix_bus = CONFIG_SYS_FIX_I2C_BUSES;

/* add one i2c bus without muxes ... ToDo: how to add i2c buses with muxes */
static int i2c_add_one_bus(char *adapter_name)
{
	struct i2c_bus_hose *tmp;

	if (i2c_fix_bus >= CONFIG_SYS_NUM_I2C_BUSES)
		return -ENOMEM;

	/* search adapter with name */
	adap = i2c_search_adapter_name(name); /* Code this function */
	if (adap < 0)
		return -ENODEV;

	tmp = kalloc(sizeof(struct i2c_bus_hose));
	tmp->adapter = adap;
	/* if found add it into i2c_bus */
	memcpy(&i2c_bus[i2c_fix_bus], tmp, sizeof(struct i2c_bus_hose));
	i2c_fix_bus++;
}

And maybe here and there some adaptions for getting this running...
Thinking of i2c_set_bus_num(), there must be now a check for
i2c_fix_bus I think ...

Adapt the README ...

And then, from the place where you interpret the fdt, call if
you have the information for one i2c bus, i2c_add_one_bus() ...

Not sure, if I overlooked here something ...

bye,
Heiko
naveen krishna chatradhi - Oct. 1, 2013, 6:19 a.m.
Hello Heiko,

On 1 October 2013 11:35, Heiko Schocher <hs@denx.de> wrote:
> Hello Naveen,
>
> Am 30.09.2013 12:03, schrieb Naveen Krishna Ch:
>
>> Helo Heiko,
>>
>> Thanks for timely reply.
>>
>> On 30 September 2013 13:35, Heiko Schocher<hs@denx.de>  wrote:
>>>
>>> Hello Naveen,
>>>
>>> Am 30.09.2013 08:58, schrieb Naveen Krishna Chatradhi:
>>>
>>>> This patchset fixes few bugs in the existing s3c24x0.c code (standard
>>>> i2c)
>>>> and add support for High-speed i2c bus controller available on Exynos5
>>>> SoCs
>>>> from Samsung.
>>>>
>>>> Exynos5250 channels [0 ~ 3] can configured for I2C contoller or
>>>>              new HSI2C controller
>>>>              channels [3 ~ 7] are standard I2C controller channels
>>>> Exynos5420 channels [0 ~ 3] are standard I2C controller channels and
>>>>              channels [4 ~ 10] are High-speed controller channels
>>>>
>>>> Patchset:
>>>> 1.  exynos: i2c: Fix i2c driver to handle NACKs properly
>>>>       Improvements and fixes from Vadim Bendebury for standard i2c calls
>>>>
>>>> 2.  exynos: i2c: Change FDT bus setup code to enumerate ports correctly
>>>>       FDT bus setup code from Simon Glass
>>>>
>>>> 3.  PATCH v5: i2c: s3c24xx: add hsi2c controller support
>>>>       High-speed controller register description and defines new i2c
>>>> read/write,
>>>>       probe and set_bus calls.
>>>>
>>>>       This is been reviewed earlier at
>>>>       http://lists.denx.de/pipermail/u-boot/2013-May/153245.html
>>>>       Thanks for review and improvements from Vadim Bendebury.
>>>>
>>>> Question:
>>>> 4. RFC: samsung: i2c: Enable new CONFIG_SYS_I2C framework
>>>> I've tried to implement the new I2C multi bus framework in u-boot tree,
>>>> taking tegra-i2c.c as reference.
>
> [...]
>
>>>> b). When i define 11 buses as in the case of Exynos5420, the "i2c bus"
>>>> lists them
>>>>    SMDK5420 # i2c bus
>>>>    Bus 0:  s3c0
>>>>    Bus 1:  s3c1
>>>>    Bus 2:  s3c10
>>>>    Bus 3:  s3c2
>>>>    Bus 4:  s3c3
>>>>    Bus 5:  s3c4
>>>>    Bus 6:  s3c5
>>>>    Bus 7:  s3c6
>>>>    Bus 8:  s3c7
>>>>    Bus 9:  s3c8
>>>>    Bus 10: s3c9
>>>>    or  (If i change the name to hsi2c)
>>>>    SMDK5420 # i2c bus
>>>>    Bus 0:  hsi2c10
>>>>    Bus 1:  hsi2c4
>>>>    Bus 2:  hsi2c5
>>>>    Bus 3:  hsi2c6
>>>>    Bus 4:  hsi2c7
>>>>    Bus 5:  hsi2c8
>>>>    Bus 6:  hsi2c9
>>>>    Bus 7:  s3c0
>>>>    Bus 8:  s3c1
>>>>    Bus 9:  s3c2
>>>>    Bus 10: s3c3
>>>>
>>>> Whats the expected behaviour. If the above result is correct, I need to
>>>> changei
>>>> the strings to get them in the correct order.
>>>
>>>
>>>
>>> What, if you use two digits:
>>>
>>>    Bus 0:  hsi2c01
>>>    Bus 1:  hsi2c02
>>>    [...]
>>>    Bus 7:  s3c00
>>>    Bus 8:  s3c01
>>>    [...]
>>>
>>> ?
>>>
>>> Or with one digit:
>>>
>>>    Bus 0:  hsi2c1
>>>    Bus 1:  hsi2c2
>>>    [...]
>>>
>>>    Bus 7:  s3c0
>>>    Bus 8:  s3c1
>>>    [...]
>>
>> In the Exynos5420 hardware
>> channels are as below
>>
>> channel:
>> --0----1----2----3------4--------5---------6-------7--------8-------9-------10
>> controller: i2c, i2c, i2c, i2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c,
>> hsi2c.
>> But the "i2c bus" command is showing the bus number according to the
>> "name"
>> string comparison.  Which seems wrong. Isn't it ??
>
>
> Hmm.. you are right, seems that the compiler sorts them alphabetical ...
> So, two ways:
> - use another name, saying first a two digit for the channel ?
> - or, use CONFIG_SYS_NUM_I2C_BUSES, CONFIG_SYS_I2C_DIRECT_BUS
>   and CONFIG_SYS_I2C_BUSES as described in the README (grep
>   for CONFIG_SYS_I2C_BUSES in include/configs and you will find
>   some examples ...) and you can define, which adapter is on which
>   i2c_bus number ...
Will try and implement it this way.
>
>
>>>> c). What's the alternative for the
>>>>      board_i2c_init(), i2c_get_bus_num_fdt(), i2c_reset_port_fdt().
>>>>     "Functions to get the I2C bus number and reset I2C bus using FDT
>>>> node"
>>>>
>>>>      I think, these functions are still needed.
>>>
>>>
>>>
>>> Hmm.. that needs a general discussion, how to use fdt and i2c I think.
>>>
>>> I would prefer a way (not really nowing, if it is possible for all
>>> configurations) where, if using fdt, the DT gets parsed and the
>>> availiable i2c buses gets created ... After that, "normal" i2c access
>>> with i2c_set_bus_num(), i2c_read/write should be possible ... this is
>>> currently not possible with the i2c framework, but should not be so hard
>>> to do. Except the restriction, that it would not work in SPL case, or
>>> before relocation for i2c buses announced through dt
>>
>> once i2c_init_board() is done board_i2c_init() is not quite needed
>> using i2c_init_board we can avoid the relocation problem aswell.
>>
>> by the definition of i2c_get_bus_num() in drivers/i2c/i2c_core.c
>>
>> unsigned int i2c_get_bus_num(void)
>> {
>>          return gd->cur_i2c_bus;
>> }
>>
>> we don't need a special function i2c_get_bus_num_fdt()
>
>
> Ah, ok!
>
>
>> IMHO, i2c_reset_port_fdt() is the only function to be discussed
>
>
> And why is i2c_init() not good? Why must we have here a new function?
That's right, even if there is a need for  i2c_reset_port_fdt().
it must be a i2c-core function instead of being in a driver.
>
>
>>> Define CONFIG_SYS_I2C_MAX_HOPS ->  CONFIG_SYS_I2C_DIRECT_BUS is not
>>> defined
>>> so i2c_bus[] is used in drivers/i2c/i2c_core.c. Define the fix i2c buses
>>> in
>>> the board config file with CONFIG_SYS_I2C_BUSES (if you have no fix
>>> buses,
>>> let this empty) and add a function in drivers/i2c/i2c_core.c, which adds
>>> new i2c buses to i2c_bus[] after the fix buses and call this new
>>> function,
>>> from where you interpret the fdt ...
>>
>> I din't quite understood this.
>> Can you point me to some readme or Doc or discussion Please.
>
>
> just the U-Boot README ... The above was just a fast idea, how it
> is possible to add i2c buses from the info in the fdt ...
>
> Here some theoretical code, how it could look like:
>
> in the board config file add:
>
> #define CONFIG_SYS_I2C_DIRECT_BUS       1
> #define CONFIG_SYS_I2C_MAX_HOPS         0
> #define CONFIG_SYS_NUM_I2C_BUSES        10 /* maximum possible i2c buses
> used on the hw */
> #define CONFIG_SYS_FIX_I2C_BUSES        1 /* just as an example, here with
> one fix i2c bus */
> #define CONFIG_SYS_I2C_BUSES    {       {0}, /* adapter 0 is fix on i2c_bus
> number 0 */
>                                 }
>
> in drivers/i2c/i2c_core.c:
>
> static int i2c_fix_bus = CONFIG_SYS_FIX_I2C_BUSES;
>
> /* add one i2c bus without muxes ... ToDo: how to add i2c buses with muxes
> */
> static int i2c_add_one_bus(char *adapter_name)
> {
>         struct i2c_bus_hose *tmp;
>
>         if (i2c_fix_bus >= CONFIG_SYS_NUM_I2C_BUSES)
>                 return -ENOMEM;
>
>         /* search adapter with name */
>         adap = i2c_search_adapter_name(name); /* Code this function */
>         if (adap < 0)
>                 return -ENODEV;
>
>         tmp = kalloc(sizeof(struct i2c_bus_hose));
>         tmp->adapter = adap;
>         /* if found add it into i2c_bus */
>         memcpy(&i2c_bus[i2c_fix_bus], tmp, sizeof(struct i2c_bus_hose));
>         i2c_fix_bus++;
> }
Thanks for this explanation.
>
> And maybe here and there some adaptions for getting this running...
> Thinking of i2c_set_bus_num(), there must be now a check for
> i2c_fix_bus I think ...
>
> Adapt the README ...
>
> And then, from the place where you interpret the fdt, call if
> you have the information for one i2c bus, i2c_add_one_bus() ...
>
> Not sure, if I overlooked here something ...

Will try and do this.
Mean while can we get the other 3 patches reviewed.
Patch 1: http://patchwork.ozlabs.org/patch/278933/
Patch 2: http://patchwork.ozlabs.org/patch/278931/
Patch 3: http://patchwork.ozlabs.org/patch/278930/
They were tested without the change to the new i2c framework.
>
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Heiko Schocher - Oct. 1, 2013, 6:36 a.m.
Hello Naveen,

Am 01.10.2013 08:19, schrieb Naveen Krishna Ch:
> Hello Heiko,
>
> On 1 October 2013 11:35, Heiko Schocher<hs@denx.de>  wrote:
>> Hello Naveen,
>>
>> Am 30.09.2013 12:03, schrieb Naveen Krishna Ch:
>>
>>> Helo Heiko,
>>>
>>> Thanks for timely reply.
>>>
>>> On 30 September 2013 13:35, Heiko Schocher<hs@denx.de>   wrote:
>>>>
>>>> Hello Naveen,
>>>>
>>>> Am 30.09.2013 08:58, schrieb Naveen Krishna Chatradhi:
>>>>
>>>>> This patchset fixes few bugs in the existing s3c24x0.c code (standard
>>>>> i2c)
>>>>> and add support for High-speed i2c bus controller available on Exynos5
>>>>> SoCs
>>>>> from Samsung.
>>>>>
>>>>> Exynos5250 channels [0 ~ 3] can configured for I2C contoller or
>>>>>               new HSI2C controller
>>>>>               channels [3 ~ 7] are standard I2C controller channels
>>>>> Exynos5420 channels [0 ~ 3] are standard I2C controller channels and
>>>>>               channels [4 ~ 10] are High-speed controller channels
>>>>>
>>>>> Patchset:
>>>>> 1.  exynos: i2c: Fix i2c driver to handle NACKs properly
>>>>>        Improvements and fixes from Vadim Bendebury for standard i2c calls
>>>>>
>>>>> 2.  exynos: i2c: Change FDT bus setup code to enumerate ports correctly
>>>>>        FDT bus setup code from Simon Glass
>>>>>
>>>>> 3.  PATCH v5: i2c: s3c24xx: add hsi2c controller support
>>>>>        High-speed controller register description and defines new i2c
>>>>> read/write,
>>>>>        probe and set_bus calls.
>>>>>
>>>>>        This is been reviewed earlier at
>>>>>        http://lists.denx.de/pipermail/u-boot/2013-May/153245.html
>>>>>        Thanks for review and improvements from Vadim Bendebury.
>>>>>
>>>>> Question:
>>>>> 4. RFC: samsung: i2c: Enable new CONFIG_SYS_I2C framework
>>>>> I've tried to implement the new I2C multi bus framework in u-boot tree,
>>>>> taking tegra-i2c.c as reference.
[...]
>>> channel:
>>> --0----1----2----3------4--------5---------6-------7--------8-------9-------10
>>> controller: i2c, i2c, i2c, i2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c,
>>> hsi2c.
>>> But the "i2c bus" command is showing the bus number according to the
>>> "name"
>>> string comparison.  Which seems wrong. Isn't it ??
>>
>>
>> Hmm.. you are right, seems that the compiler sorts them alphabetical ...
>> So, two ways:
>> - use another name, saying first a two digit for the channel ?
>> - or, use CONFIG_SYS_NUM_I2C_BUSES, CONFIG_SYS_I2C_DIRECT_BUS
>>    and CONFIG_SYS_I2C_BUSES as described in the README (grep
>>    for CONFIG_SYS_I2C_BUSES in include/configs and you will find
>>    some examples ...) and you can define, which adapter is on which
>>    i2c_bus number ...
> Will try and implement it this way.

Ok.

>>>>> c). What's the alternative for the
>>>>>       board_i2c_init(), i2c_get_bus_num_fdt(), i2c_reset_port_fdt().
>>>>>      "Functions to get the I2C bus number and reset I2C bus using FDT
>>>>> node"
>>>>>
>>>>>       I think, these functions are still needed.
>>>>
>>>>
>>>>
>>>> Hmm.. that needs a general discussion, how to use fdt and i2c I think.
>>>>
>>>> I would prefer a way (not really nowing, if it is possible for all
>>>> configurations) where, if using fdt, the DT gets parsed and the
>>>> availiable i2c buses gets created ... After that, "normal" i2c access
>>>> with i2c_set_bus_num(), i2c_read/write should be possible ... this is
>>>> currently not possible with the i2c framework, but should not be so hard
>>>> to do. Except the restriction, that it would not work in SPL case, or
>>>> before relocation for i2c buses announced through dt
>>>
>>> once i2c_init_board() is done board_i2c_init() is not quite needed
>>> using i2c_init_board we can avoid the relocation problem aswell.
>>>
>>> by the definition of i2c_get_bus_num() in drivers/i2c/i2c_core.c
>>>
>>> unsigned int i2c_get_bus_num(void)
>>> {
>>>           return gd->cur_i2c_bus;
>>> }
>>>
>>> we don't need a special function i2c_get_bus_num_fdt()
>>
>>
>> Ah, ok!
>>
>>
>>> IMHO, i2c_reset_port_fdt() is the only function to be discussed
>>
>>
>> And why is i2c_init() not good? Why must we have here a new function?
> That's right, even if there is a need for  i2c_reset_port_fdt().
> it must be a i2c-core function instead of being in a driver.
>>
>>
>>>> Define CONFIG_SYS_I2C_MAX_HOPS ->   CONFIG_SYS_I2C_DIRECT_BUS is not
>>>> defined
>>>> so i2c_bus[] is used in drivers/i2c/i2c_core.c. Define the fix i2c buses
>>>> in
>>>> the board config file with CONFIG_SYS_I2C_BUSES (if you have no fix
>>>> buses,
>>>> let this empty) and add a function in drivers/i2c/i2c_core.c, which adds
>>>> new i2c buses to i2c_bus[] after the fix buses and call this new
>>>> function,
>>>> from where you interpret the fdt ...
>>>
>>> I din't quite understood this.
>>> Can you point me to some readme or Doc or discussion Please.
>>
>>
>> just the U-Boot README ... The above was just a fast idea, how it
>> is possible to add i2c buses from the info in the fdt ...
>>
>> Here some theoretical code, how it could look like:
[...]
> Thanks for this explanation.

! It is just theoretical ... you must try it ;-)

>> And maybe here and there some adaptions for getting this running...
>> Thinking of i2c_set_bus_num(), there must be now a check for
>> i2c_fix_bus I think ...
>>
>> Adapt the README ...
>>
>> And then, from the place where you interpret the fdt, call if
>> you have the information for one i2c bus, i2c_add_one_bus() ...
>>
>> Not sure, if I overlooked here something ...
>
> Will try and do this.
> Mean while can we get the other 3 patches reviewed.
> Patch 1: http://patchwork.ozlabs.org/patch/278933/

This patch seems good to me, added to u-boot-i2c.git
as it seems it is a bugfix.

> Patch 2: http://patchwork.ozlabs.org/patch/278931/
> Patch 3: http://patchwork.ozlabs.org/patch/278930/

Think, they are OK, but they go after the next release to mainline,
as we are close to the next release ...

> They were tested without the change to the new i2c framework.

Thanks for your work!

bye,
Heiko

Patch

diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
index 769a2ba..3117ab7 100644
--- a/drivers/i2c/s3c24x0_i2c.c
+++ b/drivers/i2c/s3c24x0_i2c.c
@@ -32,6 +32,7 @@ 
 #include <asm/arch/clk.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/pinmux.h>
+#include <asm/arch/clock.h>
 #else
 #include <asm/arch/s3c24x0_cpu.h>
 #endif
@@ -50,6 +51,60 @@ 
 #define I2C_NOK_LA	3	/* Lost arbitration */
 #define I2C_NOK_TOUT	4	/* time out */
 
+/* HSI2C specific register description */
+
+/* I2C_CTL Register bits */
+#define HSI2C_FUNC_MODE_I2C		(1u << 0)
+#define HSI2C_MASTER			(1u << 3)
+#define HSI2C_RXCHON			(1u << 6)	/* Write/Send */
+#define HSI2C_TXCHON			(1u << 7)	/* Read/Receive */
+#define HSI2C_SW_RST			(1u << 31)
+
+/* I2C_FIFO_CTL Register bits */
+#define HSI2C_RXFIFO_EN			(1u << 0)
+#define HSI2C_TXFIFO_EN			(1u << 1)
+#define HSI2C_TXFIFO_TRIGGER_LEVEL	(0x20 << 16)
+#define HSI2C_RXFIFO_TRIGGER_LEVEL	(0x20 << 4)
+
+/* I2C_TRAILING_CTL Register bits */
+#define HSI2C_TRAILING_COUNT		(0xff)
+
+/* I2C_INT_EN Register bits */
+#define HSI2C_INT_TX_ALMOSTEMPTY_EN	(1u << 0)
+#define HSI2C_INT_RX_ALMOSTFULL_EN	(1u << 1)
+#define HSI2C_INT_TRAILING_EN		(1u << 6)
+#define HSI2C_INT_I2C_EN		(1u << 9)
+
+/* I2C_CONF Register bits */
+#define HSI2C_AUTO_MODE			(1u << 31)
+#define HSI2C_10BIT_ADDR_MODE		(1u << 30)
+#define HSI2C_HS_MODE			(1u << 29)
+
+/* I2C_AUTO_CONF Register bits */
+#define HSI2C_READ_WRITE		(1u << 16)
+#define HSI2C_STOP_AFTER_TRANS		(1u << 17)
+#define HSI2C_MASTER_RUN		(1u << 31)
+
+/* I2C_TIMEOUT Register bits */
+#define HSI2C_TIMEOUT_EN		(1u << 31)
+
+/* I2C_TRANS_STATUS register bits */
+#define HSI2C_MASTER_BUSY		(1u << 17)
+#define HSI2C_SLAVE_BUSY		(1u << 16)
+#define HSI2C_NO_DEV			(1u << 3)
+#define HSI2C_NO_DEV_ACK		(1u << 2)
+#define HSI2C_TRANS_ABORT		(1u << 1)
+#define HSI2C_TRANS_DONE		(1u << 0)
+#define HSI2C_TIMEOUT_AUTO		(0u << 0)
+
+#define HSI2C_SLV_ADDR_MAS(x)		((x & 0x3ff) << 10)
+
+/* Controller operating frequency, timing values for operation
+ * are calculated against this frequency
+ */
+#define HSI2C_FS_TX_CLOCK		1000000
+
+/* S3C I2C Controller bits */
 #define I2CSTAT_BSY	0x20	/* Busy bit */
 #define I2CSTAT_NACK	0x01	/* Nack bit */
 #define I2CCON_ACKGEN	0x80	/* Acknowledge generation */
@@ -61,6 +116,7 @@ 
 
 #define I2C_TIMEOUT 1		/* 1 second */
 
+#define	HSI2C_TIMEOUT	100
 
 /*
  * For SPL boot some boards need i2c before SDRAM is initialised so force
@@ -120,9 +176,23 @@  static int WaitForXfer(struct s3c24x0_i2c *i2c)
 	return (readl(&i2c->iiccon) & I2CCON_IRPND) ? I2C_OK : I2C_NOK_TOUT;
 }
 
-static int IsACK(struct s3c24x0_i2c *i2c)
+static int hsi2c_wait_for_irq(struct exynos5_hsi2c *i2c)
 {
-	return !(readl(&i2c->iicstat) & I2CSTAT_NACK);
+	int i = HSI2C_TIMEOUT * 10;
+	int ret = I2C_NOK_TOUT;
+
+	while (i > 0) {
+		/* wait for a while and retry */
+		udelay(50);
+		if (readl(&i2c->usi_int_stat) &
+			(HSI2C_INT_I2C_EN | HSI2C_INT_TX_ALMOSTEMPTY_EN)) {
+			ret = I2C_OK;
+			break;
+		}
+		i--;
+	}
+
+	return ret;
 }
 
 static void ReadWriteByte(struct s3c24x0_i2c *i2c)
@@ -130,6 +200,22 @@  static void ReadWriteByte(struct s3c24x0_i2c *i2c)
 	writel(readl(&i2c->iiccon) & ~I2CCON_IRPND, &i2c->iiccon);
 }
 
+static void hsi2c_clear_irqpd(struct exynos5_hsi2c *i2c)
+{
+	writel(readl(&i2c->usi_int_stat), &i2c->usi_int_stat);
+}
+
+static int hsi2c_isack(struct exynos5_hsi2c *i2c)
+{
+	return readl(&i2c->usi_trans_status) &
+			(HSI2C_NO_DEV | HSI2C_NO_DEV_ACK);
+}
+
+static int IsACK(struct s3c24x0_i2c *i2c)
+{
+	return !(readl(&i2c->iicstat) & I2CSTAT_NACK);
+}
+
 static struct s3c24x0_i2c *get_base_i2c(void)
 {
 #ifdef CONFIG_EXYNOS4
@@ -147,6 +233,18 @@  static struct s3c24x0_i2c *get_base_i2c(void)
 #endif
 }
 
+static struct exynos5_hsi2c *get_base_hsi2c(void)
+{
+	struct exynos5_hsi2c *i2c = NULL;
+	int bus = g_current_bus;
+
+	if (proid_is_exynos5420())
+		i2c = (struct exynos5_hsi2c *)(EXYNOS5420_I2C_BASE +
+					((bus) * EXYNOS5_I2C_SPACING));
+
+	return i2c;
+}
+
 static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd)
 {
 	ulong freq, pres = 16, div;
@@ -174,6 +272,74 @@  static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd)
 	writel(I2C_MODE_MT | I2C_TXRX_ENA, &i2c->iicstat);
 }
 
+static void hsi2c_ch_init(struct exynos5_hsi2c *i2c, int speed)
+{
+	u32 i2c_timeout;
+	ulong clkin = get_i2c_clk();
+	u32 i2c_timing_s1;
+	u32 i2c_timing_s2;
+	u32 i2c_timing_s3;
+	u32 i2c_timing_sla;
+	unsigned int op_clk = HSI2C_FS_TX_CLOCK;
+	unsigned int n_clkdiv;
+	unsigned int t_start_su, t_start_hd;
+	unsigned int t_stop_su;
+	unsigned int t_data_su, t_data_hd;
+	unsigned int t_scl_l, t_scl_h;
+	unsigned int t_sr_release;
+	unsigned int t_ftl_cycle;
+	unsigned int i = 0, utemp0 = 0, utemp1 = 0, utemp2 = 0;
+
+	/* FPCLK / FI2C =
+	 * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + 2 * FLT_CYCLE
+	 * uTemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2)
+	 * uTemp1 = (TSCLK_L + TSCLK_H + 2)
+	 * uTemp2 = TSCLK_L + TSCLK_H
+	 */
+	t_ftl_cycle = (readl(&i2c->usi_conf) >> 16) & 0x7;
+	utemp0 = (clkin / op_clk) - 8 - 2 * t_ftl_cycle;
+
+	/* CLK_DIV max is 256 */
+	for (i = 0; i < 256; i++) {
+		utemp1 = utemp0 / (i + 1);
+		/* SCLK_L/H max is 256 / 2 */
+		if (utemp1 < 128) {
+			utemp2 = utemp1 - 2;
+			break;
+		}
+	}
+
+	n_clkdiv = i;
+	t_scl_l = utemp2 / 2;
+	t_scl_h = utemp2 / 2;
+	t_start_su = t_scl_l;
+	t_start_hd = t_scl_l;
+	t_stop_su = t_scl_l;
+	t_data_su = t_scl_l / 2;
+	t_data_hd = t_scl_l / 2;
+	t_sr_release = utemp2;
+
+	i2c_timing_s1 = t_start_su << 24 | t_start_hd << 16 | t_stop_su << 8;
+	i2c_timing_s2 = t_data_su << 24 | t_scl_l << 8 | t_scl_h << 0;
+	i2c_timing_s3 = n_clkdiv << 16 | t_sr_release << 0;
+	i2c_timing_sla = t_data_hd << 0;
+
+	writel(HSI2C_TRAILING_COUNT, &i2c->usi_trailing_ctl);
+
+	/* Clear to enable Timeout */
+	i2c_timeout = readl(&i2c->usi_timeout);
+	i2c_timeout &= ~HSI2C_TIMEOUT_EN;
+	writel(i2c_timeout, &i2c->usi_timeout);
+
+	writel(readl(&i2c->usi_conf) | HSI2C_AUTO_MODE, &i2c->usi_conf);
+
+	/* Currently operating in Fast speed mode. */
+	writel(i2c_timing_s1, &i2c->usi_timing_fs1);
+	writel(i2c_timing_s2, &i2c->usi_timing_fs2);
+	writel(i2c_timing_s3, &i2c->usi_timing_fs3);
+	writel(i2c_timing_sla, &i2c->usi_timing_sla);
+}
+
 /*
  * MULTI BUS I2C support
  */
@@ -181,16 +347,18 @@  static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd)
 #ifdef CONFIG_I2C_MULTI_BUS
 int i2c_set_bus_num(unsigned int bus)
 {
-	struct s3c24x0_i2c *i2c;
-
 	if ((bus < 0) || (bus >= CONFIG_MAX_I2C_NUM)) {
 		debug("Bad bus: %d\n", bus);
 		return -1;
 	}
 
 	g_current_bus = bus;
-	i2c = get_base_i2c();
-	i2c_ch_init(i2c, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
+	if (proid_is_exynos5420() && (bus > 3)) {
+		hsi2c_ch_init(get_base_hsi2c(),
+						CONFIG_SYS_I2C_SPEED);
+	} else
+		i2c_ch_init(get_base_i2c(), CONFIG_SYS_I2C_SPEED,
+						CONFIG_SYS_I2C_SLAVE);
 
 	return 0;
 }
@@ -269,24 +437,182 @@  void i2c_init(int speed, int slaveadd)
 }
 
 /*
+ * Send a STOP event and wait for it to have completed
+ *
+ * @param mode	If it is a master transmitter or receiver
+ * @return I2C_OK if the line became idle before timeout I2C_NOK_TOUT otherwise
+ */
+static int hsi2c_send_stop(struct exynos5_hsi2c *i2c, int result)
+{
+	int timeout;
+	int ret = I2C_NOK_TOUT;
+
+	/* Wait for the STOP to send and the bus to go idle */
+	for (timeout = HSI2C_TIMEOUT; timeout > 0; timeout -= 5) {
+		if (!(readl(&i2c->usi_trans_status) & HSI2C_MASTER_BUSY)) {
+			ret = I2C_OK;
+			goto out;
+		}
+		udelay(5);
+	}
+ out:
+	/* Setting the STOP event to fire */
+	writel(HSI2C_FUNC_MODE_I2C, &i2c->usi_ctl);
+	writel(0x0, &i2c->usi_int_en);
+
+	return (result == I2C_OK) ? ret : result;
+}
+
+static int hsi2c_write(unsigned char chip,
+			unsigned char addr[],
+			unsigned char alen,
+			unsigned char data[],
+			unsigned short len)
+{
+	struct exynos5_hsi2c *i2c = get_base_hsi2c();
+	int i = 0, result = I2C_OK;
+	u32 i2c_auto_conf;
+	u32 stat;
+
+	/* Check I2C bus idle */
+	i = HSI2C_TIMEOUT * 20;
+	while ((readl(&i2c->usi_trans_status) & HSI2C_MASTER_BUSY)
+							&& (i > 0)) {
+		udelay(50);
+		i--;
+	}
+
+	stat = readl(&i2c->usi_trans_status);
+
+	if (stat & HSI2C_MASTER_BUSY) {
+		debug("%s: bus busy\n", __func__);
+		return I2C_NOK_TOUT;
+	}
+	/* Disable TXFIFO and RXFIFO */
+	writel(0, &i2c->usi_fifo_ctl);
+
+	/* chip address */
+	writel(HSI2C_SLV_ADDR_MAS(chip), &i2c->i2c_addr);
+
+	/* Enable interrupts */
+	writel((HSI2C_INT_I2C_EN | HSI2C_INT_TX_ALMOSTEMPTY_EN),
+						&i2c->usi_int_en);
+
+	/* usi_ctl enable i2c func, master write configure */
+	writel((HSI2C_TXCHON | HSI2C_FUNC_MODE_I2C | HSI2C_MASTER),
+							&i2c->usi_ctl);
+
+	/* i2c_conf configure */
+	writel(readl(&i2c->usi_conf) | HSI2C_AUTO_MODE, &i2c->usi_conf);
+
+	/* auto_conf for write length and stop configure */
+	i2c_auto_conf = ((len + alen) | HSI2C_STOP_AFTER_TRANS);
+	i2c_auto_conf &= ~HSI2C_READ_WRITE;
+	writel(i2c_auto_conf, &i2c->usi_auto_conf);
+
+	/* Master run, start xfer */
+	writel(readl(&i2c->usi_auto_conf) | HSI2C_MASTER_RUN,
+						&i2c->usi_auto_conf);
+
+	result = hsi2c_wait_for_irq(i2c);
+	if ((result == I2C_OK) && hsi2c_isack(i2c)) {
+		result = I2C_NACK;
+		goto err;
+	}
+
+	for (i = 0; i < alen && (result == I2C_OK); i++) {
+		writel(addr[i], &i2c->usi_txdata);
+		result = hsi2c_wait_for_irq(i2c);
+	}
+
+	for (i = 0; i < len && (result == I2C_OK); i++) {
+		writel(data[i], &i2c->usi_txdata);
+		result = hsi2c_wait_for_irq(i2c);
+	}
+
+ err:
+	hsi2c_clear_irqpd(i2c);
+	return hsi2c_send_stop(i2c, result);
+}
+
+static int hsi2c_read(unsigned char chip,
+			unsigned char addr[],
+			unsigned char alen,
+			unsigned char data[],
+			unsigned short len,
+			int check)
+{
+	struct exynos5_hsi2c *i2c = get_base_hsi2c();
+	int i, result;
+	u32 i2c_auto_conf;
+
+	if (!check) {
+		result =  hsi2c_write(chip, addr, alen, data, 0);
+		if (result != I2C_OK) {
+			debug("write failed Result = %d\n", result);
+			return result;
+		}
+	}
+
+	/* start read */
+	/* Disable TXFIFO and RXFIFO */
+	writel(0, &i2c->usi_fifo_ctl);
+
+	/* chip address */
+	writel(HSI2C_SLV_ADDR_MAS(chip), &i2c->i2c_addr);
+
+	/* Enable interrupts */
+	writel(HSI2C_INT_I2C_EN, &i2c->usi_int_en);
+
+		/* i2c_conf configure */
+	writel(readl(&i2c->usi_conf) | HSI2C_AUTO_MODE, &i2c->usi_conf);
+
+	/* auto_conf, length and stop configure */
+	i2c_auto_conf = (len | HSI2C_STOP_AFTER_TRANS | HSI2C_READ_WRITE);
+	writel(i2c_auto_conf, &i2c->usi_auto_conf);
+
+	/* usi_ctl enable i2c func, master WRITE configure */
+	writel((HSI2C_RXCHON | HSI2C_FUNC_MODE_I2C | HSI2C_MASTER),
+							&i2c->usi_ctl);
+
+	/* Master run, start xfer */
+	writel(readl(&i2c->usi_auto_conf) | HSI2C_MASTER_RUN,
+						&i2c->usi_auto_conf);
+
+	result = hsi2c_wait_for_irq(i2c);
+	if ((result == I2C_OK) && hsi2c_isack(i2c)) {
+		result = I2C_NACK;
+		goto err;
+	}
+
+	for (i = 0; i < len && (result == I2C_OK); i++) {
+		result = hsi2c_wait_for_irq(i2c);
+		data[i] = readl(&i2c->usi_rxdata);
+	}
+ err:
+	/* Stop and quit */
+	hsi2c_clear_irqpd(i2c);
+	return hsi2c_send_stop(i2c, result);
+}
+
+/*
  * cmd_type is 0 for write, 1 for read.
  *
  * addr_len can take any value from 0-255, it is only limited
  * by the char, we could make it larger if needed. If it is
  * 0 we skip the address write cycle.
  */
-static int i2c_transfer(struct s3c24x0_i2c *i2c,
-			unsigned char cmd_type,
+static int i2c_transfer(unsigned char cmd_type,
 			unsigned char chip,
 			unsigned char addr[],
 			unsigned char addr_len,
 			unsigned char data[],
 			unsigned short data_len)
 {
+	struct s3c24x0_i2c *i2c = get_base_i2c();
 	int i, result;
 
 	if (data == 0 || data_len == 0) {
-		/*Don't support data transfer of no length or to address 0 */
 		debug("i2c_transfer: bad call\n");
 		return I2C_NOK;
 	}
@@ -428,10 +754,8 @@  static int i2c_transfer(struct s3c24x0_i2c *i2c,
 
 int i2c_probe(uchar chip)
 {
-	struct s3c24x0_i2c *i2c;
 	uchar buf[1];
 
-	i2c = get_base_i2c();
 	buf[0] = 0;
 
 	/*
@@ -439,12 +763,15 @@  int i2c_probe(uchar chip)
 	 * address was <ACK>ed (i.e. there was a chip at that address which
 	 * drove the data line low).
 	 */
-	return i2c_transfer(i2c, I2C_READ, chip << 1, 0, 0, buf, 1) != I2C_OK;
+	if (proid_is_exynos5420() && (g_current_bus > 3))
+		return (hsi2c_read(chip, 0, 1, buf, 1, 1) != I2C_OK);
+	else
+		return i2c_transfer(
+				I2C_READ, chip << 1, 0, 0, buf, 1) != I2C_OK;
 }
 
 int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
 {
-	struct s3c24x0_i2c *i2c;
 	uchar xaddr[4];
 	int ret;
 
@@ -476,9 +803,13 @@  int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
 		chip |= ((addr >> (alen * 8)) &
 			 CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
 #endif
-	i2c = get_base_i2c();
-	ret = i2c_transfer(i2c, I2C_READ, chip << 1, &xaddr[4 - alen], alen,
-			buffer, len);
+	if (proid_is_exynos5420() && (g_current_bus > 3))
+		ret = hsi2c_read(chip, &xaddr[4 - alen],
+						alen, buffer, len, 0);
+	else
+		ret = i2c_transfer(I2C_READ, chip << 1,
+				&xaddr[4 - alen], alen, buffer, len);
+
 	if (ret != 0) {
 		debug("I2c read: failed %d\n", ret);
 		return 1;
@@ -488,7 +819,6 @@  int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
 
 int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
 {
-	struct s3c24x0_i2c *i2c;
 	uchar xaddr[4];
 
 	if (alen > 4) {
@@ -518,31 +848,36 @@  int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
 		chip |= ((addr >> (alen * 8)) &
 			 CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
 #endif
-	i2c = get_base_i2c();
-	return (i2c_transfer
-		(i2c, I2C_WRITE, chip << 1, &xaddr[4 - alen], alen, buffer,
-		 len) != 0);
+	if (proid_is_exynos5420() && (g_current_bus > 3))
+		return (hsi2c_write(chip, &xaddr[4 - alen],
+					alen, buffer, len) != 0);
+	else
+		return (i2c_transfer(I2C_WRITE, chip << 1,
+				&xaddr[4 - alen], alen, buffer, len) != 0);
 }
 
 #ifdef CONFIG_OF_CONTROL
 void board_i2c_init(const void *blob)
 {
+	struct s3c24x0_i2c_bus *bus;
 	int node_list[CONFIG_MAX_I2C_NUM];
 	int count, i;
 
 	count = fdtdec_find_aliases_for_id(blob, "i2c",
 		COMPAT_SAMSUNG_S3C2440_I2C, node_list,
 		CONFIG_MAX_I2C_NUM);
-
 	for (i = 0; i < count; i++) {
-		struct s3c24x0_i2c_bus *bus;
 		int node = node_list[i];
 
 		if (node <= 0)
 			continue;
 		bus = &i2c_bus[i];
+
 		bus->regs = (struct s3c24x0_i2c *)
-			fdtdec_get_addr(blob, node, "reg");
+				fdtdec_get_addr(blob, node, "reg");
+		bus->hsregs = (struct exynos5_hsi2c *)
+				fdtdec_get_addr(blob, node, "reg");
+
 		bus->id = pinmux_decode_periph_id(blob, node);
 		bus->node = node;
 		bus->bus_num = i2c_busses++;
@@ -589,7 +924,11 @@  int i2c_reset_port_fdt(const void *blob, int node)
 		return -1;
 	}
 
-	i2c_ch_init(i2c->regs, CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
+	if (proid_is_exynos5420() && (bus > 3))
+		hsi2c_ch_init(i2c->hsregs, CONFIG_SYS_I2C_SPEED);
+	else
+		i2c_ch_init(i2c->regs, CONFIG_SYS_I2C_SPEED,
+						CONFIG_SYS_I2C_SLAVE);
 
 	return 0;
 }
diff --git a/drivers/i2c/s3c24x0_i2c.h b/drivers/i2c/s3c24x0_i2c.h
index a56d749..81ed19c 100644
--- a/drivers/i2c/s3c24x0_i2c.h
+++ b/drivers/i2c/s3c24x0_i2c.h
@@ -31,10 +31,43 @@  struct s3c24x0_i2c {
 	u32	iiclc;
 };
 
+struct exynos5_hsi2c {
+	u32	usi_ctl;
+	u32	usi_fifo_ctl;
+	u32	usi_trailing_ctl;
+	u32	usi_clk_ctl;
+	u32	usi_clk_slot;
+	u32	spi_ctl;
+	u32	uart_ctl;
+	u32	res1;
+	u32	usi_int_en;
+	u32	usi_int_stat;
+	u32	usi_modem_stat;
+	u32	usi_error_stat;
+	u32	usi_fifo_stat;
+	u32	usi_txdata;
+	u32	usi_rxdata;
+	u32	res2;
+	u32	usi_conf;
+	u32	usi_auto_conf;
+	u32	usi_timeout;
+	u32	usi_manual_cmd;
+	u32	usi_trans_status;
+	u32	usi_timing_hs1;
+	u32	usi_timing_hs2;
+	u32	usi_timing_hs3;
+	u32	usi_timing_fs1;
+	u32	usi_timing_fs2;
+	u32	usi_timing_fs3;
+	u32	usi_timing_sla;
+	u32	i2c_addr;
+};
+
 struct s3c24x0_i2c_bus {
 	int node;	/* device tree node */
 	int bus_num;	/* i2c bus number */
 	struct s3c24x0_i2c *regs;
+	struct exynos5_hsi2c *hsregs;
 	int id;
 };
 #endif /* _S3C24X0_I2C_H */