From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Fri, 09 Jan 2015 07:31:03 +0100 Subject: [U-Boot] [PATCH 07/18] dm: i2c: s3c24x0: adjust to dm-i2c api In-Reply-To: <1420716809-16276-7-git-send-email-p.marczak@samsung.com> References: <1420716524-15969-1-git-send-email-p.marczak@samsung.com> <1420716809-16276-1-git-send-email-p.marczak@samsung.com> <1420716809-16276-7-git-send-email-p.marczak@samsung.com> Message-ID: <54AF75A7.1040801@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > Cc: Simon Glass > Cc: Heiko Schocher > Cc: Minkyu Kang > --- > 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 > +#include > +#include > #include > #if (defined CONFIG_EXYNOS4 || defined CONFIG_EXYNOS5) > #include > @@ -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