[U-Boot] [PATCH V2 3/3] dm: i2c: add i2c-gpio driver

Simon Glass sjg at chromium.org
Sat Mar 28 16:08:28 CET 2015


Hi Przemyslaw,

On 27 March 2015 at 11:33, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
> This commit adds driver model support to software emulated i2c bus driver.
> The device-tree node is compatible with the kernel i2c-gpio driver. It means,
> that boards device-tree "i2c-gpio" node, should be the same as in the kernel.
> For operating, it requires proper compatible and gpio pins dts description.
> Added:
> - Config: CONFIG_DM_I2C_GPIO
> - File: drivers/i2c/i2c-gpio.c
> - File: doc/device-tree-bindings/i2c/i2c-gpio.txt
>
> Driver base code is taken from: drivers/i2c/soft-i2c.c, changes:
> - update comments style,
> - move preprocesor macros into functions,
> - add device-tree support,
> - add driver model i2c support.
> - add Kconfig entry
>
> Signed-off-by: Przemyslaw Marczak <p.marczak at samsung.com>
> CC: Masahiro Yamada <yamada.masahiro at socionext.com>
> Cc: Lukasz Majewski <l.majewski at samsung.com>
> Cc: Mike Frysinger <vapier at gentoo.org>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Heiko Schocher <hs at denx.de>
>
> Changes V2:
> - new file for software i2c driver: i2c-gpio.c
> - update driver naming: use of "i2c-gpio"
> - add full compatibility with the kernels device-tree "i2c-gpio" node
> - fix Kconfig entry

Sorry I still have a few more comments.

> ---
>  doc/device-tree-bindings/i2c/i2c-gpio.txt |  37 ++++
>  drivers/i2c/Kconfig                       |   9 +
>  drivers/i2c/Makefile                      |   1 +
>  drivers/i2c/i2c-gpio.c                    | 353 ++++++++++++++++++++++++++++++
>  4 files changed, 400 insertions(+)
>  create mode 100644 doc/device-tree-bindings/i2c/i2c-gpio.txt
>  create mode 100644 drivers/i2c/i2c-gpio.c
>
> diff --git a/doc/device-tree-bindings/i2c/i2c-gpio.txt b/doc/device-tree-bindings/i2c/i2c-gpio.txt
> new file mode 100644
> index 0000000..3978381
> --- /dev/null
> +++ b/doc/device-tree-bindings/i2c/i2c-gpio.txt
> @@ -0,0 +1,37 @@
> +I2C gpio device binding
> +=======================
> +
> +Driver:
> +- drivers/i2c/i2c-gpio.c
> +
> +Software i2c device-tree node properties:
> +Required:
> +* #address-cells = <1>;
> +* #size-cells = <0>;
> +* compatible = "i2c-gpio";
> +* gpios = <sda ...>, <scl ...>;
> +
> +Optional:
> +* i2c-gpio,delay-us = <5>;
> +   The resulting transfer speed can be adjusted by setting the delay[us]
> +   between gpio-toggle operations. Speed [Hz] = 1us / 4 * udelay,
> +   It not defined, then default is 5us (~50KHz).
> +
> +Example:
> +
> +i2c-gpio at 1 {
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +
> +       compatible = "i2c-gpio";
> +       gpios = <&gpd1 0 GPIO_ACTIVE_HIGH>, /* SDA */
> +               <&gpd1 1 GPIO_ACTIVE_HIGH>; /* CLK */
> +
> +       i2c-gpio,delay-us = <5>;
> +
> +       some_device at 5 {
> +               compatible = "some_device";
> +               reg = <0x5>;
> +               ...
> +       };
> +};
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index 979522f..22e4a7c 100644
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -19,6 +19,15 @@ config DM_I2C_COMPAT
>           to convert all code for a board in a single commit. It should not
>           be enabled for any board in an official release.
>
> +config DM_I2C_GPIO
> +       bool "Enable Driver Model for software emulated I2C bus driver"
> +       depends on DM_I2C && DM_GPIO
> +       help
> +         Enable the i2c bus driver emulation by using the GPIO.

by using GPIOs.

> +         The bus gpio configuration is given by the device-tree.

GPIO

device tree
(no hypen)

> +         Kernel style device-tree node for i2c-gpio is supported.

Kernel-style device tree bindings are supported

> +         Binding info: doc/device-tree-bindings/i2c/i2c-gpio.txt
> +
>  config SYS_I2C_UNIPHIER
>         bool "UniPhier I2C driver"
>         depends on ARCH_UNIPHIER && DM_I2C
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index 774bc94..d9e9f3a 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -6,6 +6,7 @@
>  #
>  obj-$(CONFIG_DM_I2C) += i2c-uclass.o
>  obj-$(CONFIG_DM_I2C_COMPAT) += i2c-uclass-compat.o
> +obj-$(CONFIG_DM_I2C_GPIO) += i2c-gpio.o
>
>  obj-$(CONFIG_SYS_I2C_ADI) += adi_i2c.o
>  obj-$(CONFIG_I2C_MV) += mv_i2c.o
> diff --git a/drivers/i2c/i2c-gpio.c b/drivers/i2c/i2c-gpio.c
> new file mode 100644
> index 0000000..8e9ed6b
> --- /dev/null
> +++ b/drivers/i2c/i2c-gpio.c
> @@ -0,0 +1,353 @@
> +/*
> + * (C) Copyright 2015, Samsung Electronics
> + * Przemyslaw Marczak <p.marczak at samsung.com>
> + * Add driver-model support as a separated driver file
> + *
> + * (C) Copyright 2009
> + * Heiko Schocher, DENX Software Engineering, hs at denx.de.
> + * Changes for multibus/multiadapter I2C support.
> + *
> + * (C) Copyright 2001, 2002
> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + *
> + * This has been changed substantially by Gerald Van Baren, Custom IDEAS,
> + * vanbaren at cideas.com.  It was heavily influenced by LiMon, written by
> + * Neil Russell.
> + */
> +#include <common.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <i2c.h>
> +#include <div64.h>
> +#include <asm/gpio.h>
> +
> +#define DEFAULT_UDELAY 5
> +#define RETRIES                0
> +#define I2C_ACK                0
> +#define I2C_NOACK      1
> +
> +#ifdef DEBUG_I2C
> +#define PRINTD(fmt, args...)   do {    \
> +               printf (fmt, ##args);   \
> +       } while (0)
> +#else
> +#define PRINTD(fmt, args...)
> +#endif

I don't see any point in this - how about just using debug() instead?

> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +enum {
> +       PIN_SDA = 0,
> +       PIN_SCL,

PIN_COUNT

> +};
> +

Document these members - speed is in HzHz, udelay is the delay for what?

> +struct i2c_gpio_bus {
> +       unsigned int speed;
> +       unsigned long udelay;
> +       struct gpio_desc gpios[2]; /* sda, scl */

gpios[PIN_COUNT]

> +};
> +
> +/* Local functions */
> +static void send_reset(struct gpio_desc *, struct gpio_desc *, int);
> +static void send_start(struct gpio_desc *, struct gpio_desc *, int);
> +static void send_stop(struct gpio_desc *, struct gpio_desc *, int);
> +static void send_ack(struct gpio_desc *, struct gpio_desc *, int, int);
> +static int write_byte(struct gpio_desc *, struct gpio_desc *, int, uchar);
> +static uchar read_byte(struct gpio_desc *, struct gpio_desc *, int, int);

If you move send_reset() down a bit can you drop these declarations?

> +
> +static int I2C_READ(struct gpio_desc *sda)

Lower case - how about gpio_i2c_read()? Same for others.

> +{
> +       return dm_gpio_get_value(sda);
> +}
> +
> +static void I2C_SDA(struct gpio_desc *sda, int bit)
> +{
> +       if (bit) {
> +               dm_gpio_set_dir_flags(sda, GPIOD_IS_IN);

I assume the polarity is never set, so this should be OK.

> +       } else {
> +               dm_gpio_set_dir_flags(sda, GPIOD_IS_OUT);
> +               dm_gpio_set_value(sda, 0);
> +       }
> +}
> +
> +static void I2C_SCL(struct gpio_desc *scl, int bit)
> +{
> +       dm_gpio_set_dir_flags(scl, GPIOD_IS_OUT);
> +       dm_gpio_set_value(scl, bit);
> +}
> +
> +static void I2C_DELAY(unsigned long us)
> +{
> +       udelay(us);     /* 1/4 I2C clock duration */
> +}
> +
> +/**
> + * Send a reset sequence consisting of 9 clocks with the data signal high
> + * to clock any confused device back into an idle state.  Also send a
> + * <stop> at the end of the sequence for belts & suspenders.
> + */
> +static void send_reset(struct gpio_desc *scl, struct gpio_desc *sda, int delay)
> +{
> +       int j;
> +
> +       I2C_SCL(scl, 1);
> +       I2C_SDA(sda, 1);
> +
> +       for (j = 0; j < 9; j++) {
> +               I2C_SCL(scl, 0);
> +               I2C_DELAY(delay);
> +               I2C_DELAY(delay);
> +               I2C_SCL(scl, 1);
> +               I2C_DELAY(delay);
> +               I2C_DELAY(delay);

I wonder why we don't do one call with delay * 2?

> +       }
> +       send_stop(scl, sda, delay);
> +}
> +
> +/* START: High -> Low on SDA while SCL is High */
> +static void send_start(struct gpio_desc *scl, struct gpio_desc *sda, int delay)
> +{
> +       I2C_DELAY(delay);
> +       I2C_SDA(sda, 1);
> +       I2C_DELAY(delay);
> +       I2C_SCL(scl, 1);
> +       I2C_DELAY(delay);
> +       I2C_SDA(sda, 0);
> +       I2C_DELAY(delay);
> +}
> +
> +/* STOP: Low -> High on SDA while SCL is High */
> +static void send_stop(struct gpio_desc *scl, struct gpio_desc *sda, int delay)
> +{
> +       I2C_SCL(scl, 0);
> +       I2C_DELAY(delay);
> +       I2C_SDA(sda, 0);
> +       I2C_DELAY(delay);
> +       I2C_SCL(scl, 1);
> +       I2C_DELAY(delay);
> +       I2C_SDA(sda, 1);
> +       I2C_DELAY(delay);
> +}
> +
> +/* ack should be I2C_ACK or I2C_NOACK */
> +static void send_ack(struct gpio_desc *scl, struct gpio_desc *sda,
> +                    int delay, int ack)
> +{
> +       I2C_SCL(scl, 0);
> +       I2C_DELAY(delay);
> +       I2C_SDA(sda, ack);
> +       I2C_DELAY(delay);
> +       I2C_SCL(scl, 1);
> +       I2C_DELAY(delay);
> +       I2C_DELAY(delay);
> +       I2C_SCL(scl, 0);
> +       I2C_DELAY(delay);
> +}
> +
> +/* Send 8 bits and look for an acknowledgement */
> +static int write_byte(struct gpio_desc *scl, struct gpio_desc *sda,
> +                     int delay, uchar data)
> +{
> +       int j;
> +       int nack;
> +
> +       for (j = 0; j < 8; j++) {
> +               I2C_SCL(scl, 0);
> +               I2C_DELAY(delay);
> +               I2C_SDA(sda, data & 0x80);
> +               I2C_DELAY(delay);
> +               I2C_SCL(scl, 1);
> +               I2C_DELAY(delay);
> +               I2C_DELAY(delay);

This sequence of 7 calls appears a lot in this code. Could it go in a
function and be called from various places?

> +
> +               data <<= 1;
> +       }
> +
> +       /* Look for an <ACK>(negative logic) and return it */
> +       I2C_SCL(scl, 0);
> +       I2C_DELAY(delay);
> +       I2C_SDA(sda, 1);
> +       I2C_DELAY(delay);
> +       I2C_SCL(scl, 1);
> +       I2C_DELAY(delay);
> +       I2C_DELAY(delay);
> +       nack = I2C_READ(sda);
> +       I2C_SCL(scl, 0);
> +       I2C_DELAY(delay);
> +
> +       return nack;    /* not a nack is an ack */
> +}
> +
> +/**
> + * if ack == I2C_ACK, ACK the byte so can continue reading, else
> + * send I2C_NOACK to end the read.
> + */
> +static uchar read_byte(struct gpio_desc *scl, struct gpio_desc *sda,
> +                      int delay, int ack)
> +{
> +       int  data;
> +       int  j;
> +
> +       /* Read 8 bits, MSB first */
> +       I2C_SDA(sda, 1);
> +       data = 0;
> +       for (j = 0; j < 8; j++) {
> +               I2C_SCL(scl, 0);
> +               I2C_DELAY(delay);
> +               I2C_SCL(scl, 1);
> +               I2C_DELAY(delay);
> +               data <<= 1;
> +               data |= I2C_READ(sda);
> +               I2C_DELAY(delay);
> +       }
> +       send_ack(scl, sda, delay, ack);
> +
> +       return data;
> +}
> +
> +static int i2c_write_data(struct i2c_gpio_bus *bus, uchar chip,
> +                         uchar *buffer, int len, bool end_with_repeated_start)
> +{
> +       struct gpio_desc *scl = &bus->gpios[PIN_SCL];
> +       struct gpio_desc *sda = &bus->gpios[PIN_SDA];
> +       unsigned int delay = bus->udelay;
> +       int failures = 0;
> +
> +       PRINTD("%s: chip %x buffer %p len %d\n", __func__, chip, buffer, len);
> +
> +       send_start(scl, sda, delay);
> +       if (write_byte(scl, sda, delay, chip << 1)) {
> +               send_stop(scl, sda, delay);
> +               PRINTD("i2c_write, no chip responded %02X\n", chip);
> +               return -EIO;
> +       }
> +
> +       while (len-- > 0) {
> +               if (write_byte(scl, sda, delay, *buffer++))
> +                       failures++;
> +       }
> +
> +       send_stop(scl, sda, delay);
> +
> +       return failures;
> +}
> +
> +static int i2c_read_data(struct i2c_gpio_bus *bus, uchar chip,
> +                        uchar *buffer, int len)
> +{
> +       struct gpio_desc *scl = &bus->gpios[PIN_SCL];
> +       struct gpio_desc *sda = &bus->gpios[PIN_SDA];
> +       unsigned int delay = bus->udelay;
> +
> +       PRINTD("%s: chip %x buffer: %x len %d\n", __func__, chip, buffer, len);
> +
> +       send_start(scl, sda, delay);
> +       write_byte(scl, sda, delay, (chip << 1) | 1);   /* read cycle */
> +
> +       while (len-- > 0)
> +               *buffer++ = read_byte(scl, sda, delay, len == 0);
> +
> +       send_stop(scl, sda, delay);
> +
> +       return 0;
> +}
> +
> +static int i2c_gpio_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
> +{
> +       struct i2c_gpio_bus *bus = dev_get_priv(dev);
> +       int ret;
> +
> +       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(bus, msg->addr, msg->buf, msg->len);
> +               else
> +                       ret = i2c_write_data(bus, msg->addr, msg->buf, msg->len,
> +                                            next_is_read);
> +
> +               if (ret)
> +                       return -EREMOTEIO;
> +       }
> +
> +       return 0;
> +}
> +
> +static int i2c_gpio_probe(struct udevice *dev, uint chip, uint chip_flags)
> +{
> +       struct i2c_gpio_bus *bus = dev_get_priv(dev);
> +       struct gpio_desc *scl = &bus->gpios[PIN_SCL];
> +       struct gpio_desc *sda = &bus->gpios[PIN_SDA];
> +       unsigned int delay = bus->udelay;
> +       int ret;
> +
> +       send_start(scl, sda, delay);
> +       ret = write_byte(scl, sda, delay, (chip << 1) | 0);
> +       send_stop(scl, sda, delay);
> +
> +       PRINTD("%s: bus: %d (%s) chip: %x flags: %x ret: %d\n",
> +              __func__, dev->seq, dev->name, chip, chip_flags, ret);
> +
> +       return ret;
> +}
> +
> +static int i2c_gpio_set_bus_speed(struct udevice *dev, unsigned int speed)
> +{
> +       struct i2c_gpio_bus *bus = dev_get_priv(dev);
> +       struct gpio_desc *scl = &bus->gpios[PIN_SCL];
> +       struct gpio_desc *sda = &bus->gpios[PIN_SDA];
> +
> +       bus->speed = speed;
> +       bus->udelay = lldiv(1000000, speed << 2);

Why lldiv() if the value can never be >100,000? Can we just use normal division?

> +
> +       send_reset(scl, sda, bus->udelay);
> +
> +       PRINTD("%s: bus: %d (%s) speed: %u Hz (1/4 of period: %lu us)\n",
> +              __func__, dev->seq, dev->name, speed, bus->udelay);
> +
> +       return 0;
> +}
> +
> +static int i2c_gpio_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct i2c_gpio_bus *bus = dev_get_priv(dev);
> +       const void *blob = gd->fdt_blob;
> +       int node = dev->of_offset;
> +       int ret;
> +
> +       ret = gpio_request_list_by_name(dev, "gpios", bus->gpios,
> +                                       ARRAY_SIZE(bus->gpios), 0);
> +       if (ret < 0)
> +               goto error;
> +
> +       bus->udelay = fdtdec_get_int(blob, node, "i2c-gpio,delay-us",
> +                                    DEFAULT_UDELAY);
> +
> +       PRINTD("%s: bus: %d (%s) fdt node ok\n", __func__, dev->seq, dev->name);
> +
> +       return 0;
> +error:
> +       error("Can't get %s gpios! Error: %d", dev->name, ret);
> +       return ret;
> +}
> +
> +static const struct dm_i2c_ops i2c_gpio_ops = {
> +       .xfer           = i2c_gpio_xfer,
> +       .probe_chip     = i2c_gpio_probe,
> +       .set_bus_speed  = i2c_gpio_set_bus_speed,
> +};
> +
> +static const struct udevice_id i2c_gpio_ids[] = {
> +       { .compatible = "i2c-gpio" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(i2c_gpio) = {
> +       .name   = "i2c-gpio",
> +       .id     = UCLASS_I2C,
> +       .of_match = i2c_gpio_ids,
> +       .ofdata_to_platdata = i2c_gpio_ofdata_to_platdata,
> +       .priv_auto_alloc_size = sizeof(struct i2c_gpio_bus),
> +       .ops    = &i2c_gpio_ops,
> +};
> --
> 1.9.1
>

Regards,
Simon


More information about the U-Boot mailing list