Hello Przemyslaw Marczak,

just some nitpick ...

Am 08.01.2015 12:33, schrieb Przemyslaw Marczak:
This commit adjusts the s3c24x0 driver to new i2c api
based on driver-model. The driver supports standard
and high-speed i2c as previous.

Tested on Trats2 and Arndale (also with HS).

Signed-off-by: Przemyslaw Marczak <p.marc...@samsung.com>
Cc: Simon Glass <s...@chromium.org>
Cc: Heiko Schocher <h...@denx.de>
Cc: Minkyu Kang <mk7.k...@samsung.com>
---
  drivers/i2c/s3c24x0_i2c.c | 254 ++++++++++++++++++++++++++++++++++++++++------
  1 file changed, 222 insertions(+), 32 deletions(-)

diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
index fd328f0..c21d479 100644
--- a/drivers/i2c/s3c24x0_i2c.c
+++ b/drivers/i2c/s3c24x0_i2c.c
@@ -9,8 +9,9 @@
   * as they seem to have the same I2C controller inside.
   * The different address mapping is handled by the s3c24xx.h files below.
   */
-
  #include <common.h>
+#include <errno.h>
+#include <dm.h>
  #include <fdtdec.h>
  #if (defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
  #include <asm/arch/clk.h>
@@ -121,13 +122,23 @@
  #define CONFIG_MAX_I2C_NUM 1
  #endif

+DECLARE_GLOBAL_DATA_PTR;
+
  /*
   * For SPL boot some boards need i2c before SDRAM is initialised so force
   * variables to live in SRAM
   */
+#ifdef CONFIG_SYS_I2C
  static struct s3c24x0_i2c_bus i2c_bus[CONFIG_MAX_I2C_NUM]
                        __attribute__((section(".data")));
+#endif

+enum exynos_i2c_type {
+       EXYNOS_I2C_STD,
+       EXYNOS_I2C_HS,
+};
+
+#ifdef CONFIG_SYS_I2C
  /**
   * Get a pointer to the given bus index
   *
@@ -147,6 +158,7 @@ static struct s3c24x0_i2c_bus *get_bus(unsigned int bus_idx)
        debug("Undefined bus: %d\n", bus_idx);
        return NULL;
  }
+#endif

  #if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
  static int GetI2CSDA(void)
@@ -251,6 +263,7 @@ static void ReadWriteByte(struct s3c24x0_i2c *i2c)
        writel(readl(&i2c->iiccon) & ~I2CCON_IRPND, &i2c->iiccon);
  }

+#ifdef CONFIG_SYS_I2C
  static struct s3c24x0_i2c *get_base_i2c(int bus)
  {
  #ifdef CONFIG_EXYNOS4
@@ -267,6 +280,7 @@ static struct s3c24x0_i2c *get_base_i2c(int bus)
        return s3c24x0_get_base_i2c();
  #endif
  }
+#endif

  static void i2c_ch_init(struct s3c24x0_i2c *i2c, int speed, int slaveadd)
  {
@@ -398,18 +412,20 @@ static void exynos5_i2c_reset(struct s3c24x0_i2c_bus 
*i2c_bus)
        hsi2c_ch_init(i2c_bus);
  }

+#ifdef CONFIG_SYS_I2C
  static void s3c24x0_i2c_init(struct i2c_adapter *adap, int speed, int 
slaveadd)
  {
        struct s3c24x0_i2c *i2c;
        struct s3c24x0_i2c_bus *bus;
-
  #if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5)
        struct s3c24x0_gpio *gpio = s3c24x0_get_base_gpio();
  #endif
        ulong start_time = get_timer(0);

-       /* By default i2c channel 0 is the current bus */
        i2c = get_base_i2c(adap->hwadapnr);
+       bus = &i2c_bus[adap->hwadapnr];
+       if (!bus)
+               return;

        /*
         * In case the previous transfer is still going, wait to give it a
@@ -470,12 +486,13 @@ static void s3c24x0_i2c_init(struct i2c_adapter *adap, 
int speed, int slaveadd)
  #endif
        }
  #endif /* #if !(defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5) */
+
        i2c_ch_init(i2c, speed, slaveadd);

-       bus = &i2c_bus[adap->hwadapnr];
        bus->active = true;
        bus->regs = i2c;
  }
+#endif /* CONFIG_SYS_I2C */

  /*
   * Poll the appropriate bit of the fifo status register until the interface is
@@ -545,7 +562,6 @@ static int hsi2c_prepare_transaction(struct exynos5_hsi2c 
*i2c,
                                     bool issue_stop)
  {
        u32 conf;
-

why do you delete this empty line?

        conf = len | HSI2C_MASTER_RUN;

        if (issue_stop)
@@ -698,14 +714,24 @@ static int hsi2c_read(struct exynos5_hsi2c *i2c,
        return rv;
  }

+#ifdef CONFIG_SYS_I2C
  static unsigned int s3c24x0_i2c_set_bus_speed(struct i2c_adapter *adap,
-                                         unsigned int speed)
+                                             unsigned int speed)
+#else
+static int s3c24x0_i2c_set_bus_speed(struct udevice *dev, unsigned int speed)
+#endif
  {
        struct s3c24x0_i2c_bus *i2c_bus;
-

here too ...

+#ifdef CONFIG_SYS_I2C
        i2c_bus = get_bus(adap->hwadapnr);
+#else
+       if (!dev)
+               return -ENODEV;
+
+       i2c_bus = dev_get_priv(dev);
+#endif
        if (!i2c_bus)
-               return -1;
+               return -ENODEV;

        i2c_bus->clock_frequency = speed;

@@ -715,23 +741,12 @@ static unsigned int s3c24x0_i2c_set_bus_speed(struct 
i2c_adapter *adap,
                hsi2c_ch_init(i2c_bus);
        } else {
                i2c_ch_init(i2c_bus->regs, i2c_bus->clock_frequency,
-                           CONFIG_SYS_I2C_S3C24X0_SLAVE);
+                                   CONFIG_SYS_I2C_S3C24X0_SLAVE);
        }

        return 0;
  }

-#ifdef CONFIG_EXYNOS5
-static void exynos_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr)
-{
-       /* This will override the speed selected in the fdt for that port */
-       debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
-       if (i2c_set_bus_speed(speed))
-               printf("i2c_init: failed to init bus %d for speed = %d\n",
-                                               adap->hwadapnr, speed);
-}
-#endif
-
  /*
   * cmd_type is 0 for write, 1 for read.
   *
@@ -844,15 +859,24 @@ bailout:
        return result;
  }

+#ifdef CONFIG_SYS_I2C
  static int s3c24x0_i2c_probe(struct i2c_adapter *adap, uchar chip)
+#else
+static int s3c24x0_i2c_probe(struct udevice *dev, uint chip, uint chip_flags)
+#endif
  {
        struct s3c24x0_i2c_bus *i2c_bus;
        uchar buf[1];
        int ret;

+#ifdef CONFIG_SYS_I2C
        i2c_bus = get_bus(adap->hwadapnr);
+#else
+       i2c_bus = dev_get_priv(dev);
+#endif
        if (!i2c_bus)
-               return -1;
+               return -ENODEV;
+
        buf[0] = 0;

        /*
@@ -871,6 +895,7 @@ static int s3c24x0_i2c_probe(struct i2c_adapter *adap, 
uchar chip)
        return ret != I2C_OK;
  }

+#ifdef CONFIG_SYS_I2C
  static int s3c24x0_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
                            int alen, uchar *buffer, int len)
  {
@@ -878,6 +903,10 @@ static int s3c24x0_i2c_read(struct i2c_adapter *adap, 
uchar chip, uint addr,
        uchar xaddr[4];
        int ret;

+       i2c_bus = get_bus(adap->hwadapnr);
+       if (!i2c_bus)
+               return -1;

above you change "return -1" to "return -ENODEV" ...
Shouldn;t we use here also an errno? Suggestion -EFAULT?

+
        if (alen > 4) {
                debug("I2C read: addr len %d not supported\n", alen);
                return 1;
@@ -906,10 +935,6 @@ static int s3c24x0_i2c_read(struct i2c_adapter *adap, 
uchar chip, uint addr,
                chip |= ((addr >> (alen * 8)) &
                         CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
  #endif
-       i2c_bus = get_bus(adap->hwadapnr);
-       if (!i2c_bus)
-               return -1;
-
        if (i2c_bus->is_highspeed)
                ret = hsi2c_read(i2c_bus->hsregs, chip, &xaddr[4 - alen],
                                 alen, buffer, len);
@@ -933,6 +958,10 @@ static int s3c24x0_i2c_write(struct i2c_adapter *adap, 
uchar chip, uint addr,
        uchar xaddr[4];
        int ret;

+       i2c_bus = get_bus(adap->hwadapnr);
+       if (!i2c_bus)
+               return -1;

here too ...

+
        if (alen > 4) {
                debug("I2C write: addr len %d not supported\n", alen);
                return 1;
@@ -960,10 +989,6 @@ static int s3c24x0_i2c_write(struct i2c_adapter *adap, 
uchar chip, uint addr,
                chip |= ((addr >> (alen * 8)) &
                         CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
  #endif
-       i2c_bus = get_bus(adap->hwadapnr);
-       if (!i2c_bus)
-               return -1;
-
        if (i2c_bus->is_highspeed)
                ret = hsi2c_write(i2c_bus->hsregs, chip, &xaddr[4 - alen],
                                  alen, buffer, len, true);
@@ -1010,7 +1035,7 @@ static void process_nodes(const void *blob, int 
node_list[], int count,
                                                CONFIG_SYS_I2C_S3C24X0_SPEED);
                bus->node = node;
                bus->bus_num = i;
-               exynos_pinmux_config(bus->id, 0);
+               exynos_pinmux_config(PERIPH_ID_I2C0 + bus->id, 0);

                /* Mark position as used */
                node_list[i] = -1;
@@ -1033,7 +1058,6 @@ void board_i2c_init(const void *blob)
                COMPAT_SAMSUNG_EXYNOS5_I2C, node_list,
                CONFIG_MAX_I2C_NUM);
        process_nodes(blob, node_list, count, 1);
-
  }

  int i2c_get_bus_num_fdt(int node)
@@ -1077,7 +1101,17 @@ int i2c_reset_port_fdt(const void *blob, int node)

        return 0;
  }
-#endif
+#endif /* CONFIG_OF_CONTROL */
+
+#ifdef CONFIG_EXYNOS5
+static void exynos_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr)
+{
+       /* This will override the speed selected in the fdt for that port */
+       debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
+       if (i2c_set_bus_speed(speed))
+               error("i2c_init: failed to init bus for speed = %d", speed);
+}
+#endif /* CONFIG_EXYNOS5 */

  /*
   * Register s3c24x0 i2c adapters
@@ -1247,3 +1281,159 @@ U_BOOT_I2C_ADAP_COMPLETE(s3c0, s3c24x0_i2c_init, 
s3c24x0_i2c_probe,
                        CONFIG_SYS_I2C_S3C24X0_SPEED,
                        CONFIG_SYS_I2C_S3C24X0_SLAVE, 0)
  #endif
+#endif /* CONFIG_SYS_I2C */
+
+#ifdef CONFIG_DM_I2C
+static int i2c_write_data(struct s3c24x0_i2c_bus *i2c_bus, uchar chip,
+                         uchar *buffer, int len, bool end_with_repeated_start)
+{
+       int ret;
+
+       if (!i2c_bus)
+               return -1;

and here

+
+       if (i2c_bus->is_highspeed) {
+               ret = hsi2c_write(i2c_bus->hsregs, chip, 0, 0,
+                                 buffer, len, true);
+               if (ret)
+                       exynos5_i2c_reset(i2c_bus);
+       } else {
+               ret = i2c_transfer(i2c_bus->regs, I2C_WRITE,
+                                  chip << 1, 0, 0, buffer, len);
+       }
+
+       return ret != I2C_OK;
+}
+
+static int i2c_read_data(struct s3c24x0_i2c_bus  *i2c_bus, uchar chip,
+                        uchar *buffer, int len)
+{
+       int ret;
+
+       if (!i2c_bus)
+               return -1;

and here

+
+       if (i2c_bus->is_highspeed) {
+               ret = hsi2c_read(i2c_bus->hsregs, chip, 0, 0, buffer, len);
+               if (ret)
+                       exynos5_i2c_reset(i2c_bus);
+       } else {
+               ret = i2c_transfer(i2c_bus->regs, I2C_READ,
+                                  chip << 1, 0, 0, buffer, len);
+       }
+
+       return ret != I2C_OK;
+}
+
+static int s3c24x0_i2c_xfer(struct udevice *dev, struct i2c_msg *msg,
+                           int nmsgs)
+{
+       struct s3c24x0_i2c_bus *i2c_bus;
+       int ret;
+
+       if (!dev)
+               return -ENODEV;
+
+       i2c_bus = dev_get_priv(dev);
+
+       if (!i2c_bus)
+               return -1;

and here...

+
+       for (; nmsgs > 0; nmsgs--, msg++) {
+               bool next_is_read = nmsgs > 1 && (msg[1].flags & I2C_M_RD);
+
+               if (msg->flags & I2C_M_RD) {
+                       ret = i2c_read_data(i2c_bus, msg->addr, msg->buf,
+                                           msg->len);
+               } else {
+                       ret = i2c_write_data(i2c_bus, msg->addr, msg->buf,
+                                            msg->len, next_is_read);
+               }
+               if (ret)
+                       return -EREMOTEIO;
+       }
+
+       return 0;
+}
+
+static int s3c_i2c_ofdata_to_platdata(struct udevice *dev)
+{
+       const void *blob = gd->fdt_blob;
+       struct s3c24x0_i2c_bus *i2c_bus;
+       int node;
+
+       if (!dev) {
+               error("%s: no such device!", dev->name);
+               return -ENODEV;
+       }
+
+       i2c_bus = dev_get_priv(dev);
+       if (!i2c_bus) {
+               error("%s: i2c bus not allocated!", dev->name);
+               return -EINVAL;

ah, here if no bus is found you return -EINVAL ... as EINVAL is used
below again, I prefer to use another errno here ... as suggested EFAULT
or maybe ENOENT? ... but please for all occurrences of "if (!i2c_bus) {"
the same value, thanks!

+       }
+
+       if (!dev->of_id) {
+               error("%s: no compat ids!", dev->name);
+               return -EINVAL;
+       }
+       i2c_bus->is_highspeed = dev->of_id->data;
+
+       node = dev->of_offset;
+
+       if (i2c_bus->is_highspeed) {
+               i2c_bus->hsregs = (struct exynos5_hsi2c *)
+                               fdtdec_get_addr(blob, node, "reg");
+       } else {
+               i2c_bus->regs = (struct s3c24x0_i2c *)
+                               fdtdec_get_addr(blob, node, "reg");
+       }
+
+       i2c_bus->id = pinmux_decode_periph_id(blob, node);
+
+       i2c_bus->clock_frequency = fdtdec_get_int(blob, node,
+                                               "clock-frequency",
+                                               CONFIG_SYS_I2C_S3C24X0_SPEED);
+       i2c_bus->node = node;
+       i2c_bus->bus_num = dev->seq;
+
+       exynos_pinmux_config(i2c_bus->id, i2c_bus->is_highspeed);
+
+       i2c_bus->active = true;
+
+       return 0;
+}
+
+static int s3c_i2c_child_pre_probe(struct udevice *dev)
+{
+       struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
+
+       if (dev->of_offset == -1)
+               return 0;
+       return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
+                                          i2c_chip);
+}
+
+static const struct dm_i2c_ops s3c_i2c_ops = {
+       .xfer           = s3c24x0_i2c_xfer,
+       .probe_chip     = s3c24x0_i2c_probe,
+       .set_bus_speed  = s3c24x0_i2c_set_bus_speed,
+};
+
+static const struct udevice_id s3c_i2c_ids[] = {
+       { .compatible = "samsung,s3c2440-i2c", .data = EXYNOS_I2C_STD },
+       { .compatible = "samsung,exynos5-hsi2c", .data = EXYNOS_I2C_HS },
+       { }
+};
+
+U_BOOT_DRIVER(i2c_s3c) = {
+       .name   = "i2c_s3c",
+       .id     = UCLASS_I2C,
+       .of_match = s3c_i2c_ids,
+       .ofdata_to_platdata = s3c_i2c_ofdata_to_platdata,
+       .per_child_auto_alloc_size = sizeof(struct dm_i2c_chip),
+       .child_pre_probe = s3c_i2c_child_pre_probe,
+       .priv_auto_alloc_size = sizeof(struct s3c24x0_i2c_bus),
+       .ops    = &s3c_i2c_ops,
+};
+#endif /* CONFIG_DM_I2C */


Thanks for your work!

bye,
Heiko
--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to