| Message ID | 1422285706-22000-8-git-send-email-p.marczak@samsung.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Przemyslaw, On 26 January 2015 at 08:21, Przemyslaw Marczak <p.marczak@samsung.com> wrote: > 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, Odroid U3, Arndale, Odroid XU3 > > Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Heiko Schocher <hs@denx.de> > Cc: Minkyu Kang <mk7.kang@samsung.com> > Tested on snow: Tested-by: Simon Glass <sjg@chromium.org> This looks right to me, but I have a number of nits, mostly code that can be deleted, Please see below. If you can respin this I will pick it up. > --- > Changes v2: > - use consistent return values on errors > - decrease transaction status timeout, because the previous one was too big > --- > drivers/i2c/s3c24x0_i2c.c | 275 +++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 233 insertions(+), 42 deletions(-) > > diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c > index fd328f0..c82640d 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> > @@ -111,9 +112,9 @@ > #define I2C_START_STOP 0x20 /* START / STOP */ > #define I2C_TXRX_ENA 0x10 /* I2C Tx/Rx enable */ > > -#define I2C_TIMEOUT_MS 1000 /* 1 second */ > +#define I2C_TIMEOUT_MS 10 /* 10 ms */ > > -#define HSI2C_TIMEOUT_US 100000 /* 100 ms, finer granularity */ > +#define HSI2C_TIMEOUT_US 10000 /* 10 ms, finer granularity */ Why change the timeouts? In any case that should be a separate patch. > > > /* To support VCMA9 boards and other who dont define max_i2c_num */ > @@ -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) > { > @@ -326,7 +340,7 @@ static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus) > return 0; > } > } > - return -1; > + return -EINVAL; > } > > static void hsi2c_ch_init(struct s3c24x0_i2c_bus *i2c_bus) > @@ -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 > @@ -698,40 +715,40 @@ 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; > > +#ifdef CONFIG_SYS_I2C > i2c_bus = get_bus(adap->hwadapnr); > +#else > + if (!dev) > + return -ENODEV; This can't happen, you can drop this check. > + > + i2c_bus = dev_get_priv(dev); > +#endif > if (!i2c_bus) > - return -1; > + return -EFAULT; This can't happen for driver model, so move this check up into the #ifdef CONFIG_SYS_I2C > > i2c_bus->clock_frequency = speed; > > if (i2c_bus->is_highspeed) { > if (hsi2c_get_clk_details(i2c_bus)) > - return -1; > + return -EFAULT; > 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); Should leave this line alone. > } > > 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 +861,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 -EFAULT; > + Same comments as above. > buf[0] = 0; > > /* > @@ -871,6 +897,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,9 +905,13 @@ 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 -EFAULT; > + > if (alen > 4) { > debug("I2C read: addr len %d not supported\n", alen); > - return 1; > + return -EFBIG; I've been using -EADDRNOTAVAIL > } > > if (alen > 0) { > @@ -906,10 +937,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); > @@ -921,7 +948,7 @@ static int s3c24x0_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, > if (i2c_bus->is_highspeed) > exynos5_i2c_reset(i2c_bus); > debug("I2c read failed %d\n", ret); > - return 1; > + return -EIO; > } > return 0; > } > @@ -933,9 +960,13 @@ 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 -EFAULT; > + > if (alen > 4) { > debug("I2C write: addr len %d not supported\n", alen); > - return 1; > + return -EINVAL; > } > > if (alen > 0) { > @@ -960,10 +991,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 +1037,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 +1060,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) > @@ -1046,7 +1072,7 @@ int i2c_get_bus_num_fdt(int node) > } > > debug("%s: Can't find any matched I2C bus\n", __func__); > - return -1; > + return -EINVAL; > } > > int i2c_reset_port_fdt(const void *blob, int node) > @@ -1057,18 +1083,18 @@ int i2c_reset_port_fdt(const void *blob, int node) > bus = i2c_get_bus_num_fdt(node); > if (bus < 0) { > debug("could not get bus for node %d\n", node); > - return -1; > + return bus; > } > > i2c_bus = get_bus(bus); > if (!i2c_bus) { > - debug("get_bus() failed for node node %d\n", node); > - return -1; > + debug("get_bus() failed for node %d\n", node); > + return -EFAULT; > } > > if (i2c_bus->is_highspeed) { > if (hsi2c_get_clk_details(i2c_bus)) > - return -1; > + return -EINVAL; > hsi2c_ch_init(i2c_bus); > } else { > i2c_ch_init(i2c_bus->regs, i2c_bus->clock_frequency, > @@ -1077,7 +1103,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 +1283,158 @@ 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 -EFAULT; Can't happen, drop this check. > + > + 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 -EFAULT; Can't happen, drop this check. > + > + 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 -EFAULT; Can't happen, drop this check. > + > + 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; > + } Can't happen, drop this check. > + > + i2c_bus = dev_get_priv(dev); > + if (!i2c_bus) { > + error("%s: i2c bus not allocated!", dev->name); > + return -EFAULT; > + } Can't happen, drop this check. > + > + if (!dev->of_id) { > + error("%s: no compat ids!", dev->name); > + return -EINVAL; > + } Can't happen, drop this check. > + i2c_bus->is_highspeed = dev->of_id->data; > + Remove blank line > + 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); You should add a flag to pinmux.h and then do something like: exynos_pinmux_config(i2c_bus->id, i2c_bus->is_highspeed ? PINMUX_FLAG_I2C_HIGH_SPEED : 0); > + > + 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); > +} Not needed, drop this functoin. > + > +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), Not needed, drop this member. > + .child_pre_probe = s3c_i2c_child_pre_probe, Not needed, drop this functoin. > + .priv_auto_alloc_size = sizeof(struct s3c24x0_i2c_bus), > + .ops = &s3c_i2c_ops, > +}; > +#endif /* CONFIG_DM_I2C */ > -- > 1.9.1 > Regards, Simon
Hello Simon, On 01/27/2015 04:13 AM, Simon Glass wrote: > Hi Przemyslaw, > > On 26 January 2015 at 08:21, Przemyslaw Marczak <p.marczak@samsung.com> wrote: >> 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, Odroid U3, Arndale, Odroid XU3 >> >> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> >> Cc: Simon Glass <sjg@chromium.org> >> Cc: Heiko Schocher <hs@denx.de> >> Cc: Minkyu Kang <mk7.kang@samsung.com> >> > > Tested on snow: > > Tested-by: Simon Glass <sjg@chromium.org> > > This looks right to me, but I have a number of nits, mostly code that > can be deleted, Please see below. > > If you can respin this I will pick it up. > Thank you for the review, I will fix the issues and resend this today. Best regards,
diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c index fd328f0..c82640d 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> @@ -111,9 +112,9 @@ #define I2C_START_STOP 0x20 /* START / STOP */ #define I2C_TXRX_ENA 0x10 /* I2C Tx/Rx enable */ -#define I2C_TIMEOUT_MS 1000 /* 1 second */ +#define I2C_TIMEOUT_MS 10 /* 10 ms */ -#define HSI2C_TIMEOUT_US 100000 /* 100 ms, finer granularity */ +#define HSI2C_TIMEOUT_US 10000 /* 10 ms, finer granularity */ /* To support VCMA9 boards and other who dont define max_i2c_num */ @@ -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) { @@ -326,7 +340,7 @@ static int hsi2c_get_clk_details(struct s3c24x0_i2c_bus *i2c_bus) return 0; } } - return -1; + return -EINVAL; } static void hsi2c_ch_init(struct s3c24x0_i2c_bus *i2c_bus) @@ -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 @@ -698,40 +715,40 @@ 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; +#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 -EFAULT; i2c_bus->clock_frequency = speed; if (i2c_bus->is_highspeed) { if (hsi2c_get_clk_details(i2c_bus)) - return -1; + return -EFAULT; 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 +861,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 -EFAULT; + buf[0] = 0; /* @@ -871,6 +897,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,9 +905,13 @@ 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 -EFAULT; + if (alen > 4) { debug("I2C read: addr len %d not supported\n", alen); - return 1; + return -EFBIG; } if (alen > 0) { @@ -906,10 +937,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); @@ -921,7 +948,7 @@ static int s3c24x0_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, if (i2c_bus->is_highspeed) exynos5_i2c_reset(i2c_bus); debug("I2c read failed %d\n", ret); - return 1; + return -EIO; } return 0; } @@ -933,9 +960,13 @@ 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 -EFAULT; + if (alen > 4) { debug("I2C write: addr len %d not supported\n", alen); - return 1; + return -EINVAL; } if (alen > 0) { @@ -960,10 +991,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 +1037,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 +1060,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) @@ -1046,7 +1072,7 @@ int i2c_get_bus_num_fdt(int node) } debug("%s: Can't find any matched I2C bus\n", __func__); - return -1; + return -EINVAL; } int i2c_reset_port_fdt(const void *blob, int node) @@ -1057,18 +1083,18 @@ int i2c_reset_port_fdt(const void *blob, int node) bus = i2c_get_bus_num_fdt(node); if (bus < 0) { debug("could not get bus for node %d\n", node); - return -1; + return bus; } i2c_bus = get_bus(bus); if (!i2c_bus) { - debug("get_bus() failed for node node %d\n", node); - return -1; + debug("get_bus() failed for node %d\n", node); + return -EFAULT; } if (i2c_bus->is_highspeed) { if (hsi2c_get_clk_details(i2c_bus)) - return -1; + return -EINVAL; hsi2c_ch_init(i2c_bus); } else { i2c_ch_init(i2c_bus->regs, i2c_bus->clock_frequency, @@ -1077,7 +1103,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 +1283,158 @@ 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 -EFAULT; + + 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 -EFAULT; + + 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 -EFAULT; + + 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 -EFAULT; + } + + 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 */
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, Odroid U3, Arndale, Odroid XU3 Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> Cc: Simon Glass <sjg@chromium.org> Cc: Heiko Schocher <hs@denx.de> Cc: Minkyu Kang <mk7.kang@samsung.com> --- Changes v2: - use consistent return values on errors - decrease transaction status timeout, because the previous one was too big --- drivers/i2c/s3c24x0_i2c.c | 275 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 233 insertions(+), 42 deletions(-)