[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