[U-Boot] [PATCH 07/18] dm: i2c: s3c24x0: adjust to dm-i2c api
Heiko Schocher
hs at denx.de
Fri Jan 9 07:31:03 CET 2015
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.marczak at samsung.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Heiko Schocher <hs at denx.de>
> Cc: Minkyu Kang <mk7.kang at 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
More information about the U-Boot
mailing list