Hi,
On 25 July 2016 at 07:10, Vignesh R <vigne...@ti.com> wrote:
> TI's PCF8575 is a 16-bit I2C GPIO expander.The device features a
> 16-bit quasi-bidirectional I/O ports. Each quasi-bidirectional I/O can
> be used as an input or output without the use of a data-direction
> control signal. The I/Os should be high before being used as inputs.
> Read the device documentation for more details[1].
>
> This driver is based on pcf857x driver available in Linux v4.7 kernel.
> It supports basic reading and writing of gpio pins.
>
> [1] http://www.ti.com/lit/ds/symlink/pcf8575.pdf
>
> Signed-off-by: Vignesh R <vigne...@ti.com>
> ---
> doc/device-tree-bindings/gpio/gpio-pcf857x.txt | 71 +++++++++
> drivers/gpio/Kconfig | 7 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/pcf8575_gpio.c | 199
> +++++++++++++++++++++++++
> 4 files changed, 278 insertions(+)
> create mode 100644 doc/device-tree-bindings/gpio/gpio-pcf857x.txt
> create mode 100644 drivers/gpio/pcf8575_gpio.c
Reviewed-by: Simon Glass <s...@chromium.org>
A few nits below.
>
> diff --git a/doc/device-tree-bindings/gpio/gpio-pcf857x.txt
> b/doc/device-tree-bindings/gpio/gpio-pcf857x.txt
> new file mode 100644
> index 000000000000..ada4e2973323
> --- /dev/null
> +++ b/doc/device-tree-bindings/gpio/gpio-pcf857x.txt
> @@ -0,0 +1,71 @@
> +* PCF857x-compatible I/O expanders
> +
> +The PCF857x-compatible chips have "quasi-bidirectional" I/O lines that can be
> +driven high by a pull-up current source or driven low to ground. This
> combines
> +the direction and output level into a single bit per line, which can't be
> read
> +back. We can't actually know at initialization time whether a line is
> configured
> +(a) as output and driving the signal low/high, or (b) as input and reporting
> a
> +low/high value, without knowing the last value written since the chip came
> out
> +of reset (if any). The only reliable solution for setting up line direction
> is
> +thus to do it explicitly.
> +
> +Required Properties:
> +
> + - compatible: should be one of the following.
> + - "maxim,max7328": For the Maxim MAX7378
> + - "maxim,max7329": For the Maxim MAX7329
> + - "nxp,pca8574": For the NXP PCA8574
> + - "nxp,pca8575": For the NXP PCA8575
> + - "nxp,pca9670": For the NXP PCA9670
> + - "nxp,pca9671": For the NXP PCA9671
> + - "nxp,pca9672": For the NXP PCA9672
> + - "nxp,pca9673": For the NXP PCA9673
> + - "nxp,pca9674": For the NXP PCA9674
> + - "nxp,pca9675": For the NXP PCA9675
> + - "nxp,pcf8574": For the NXP PCF8574
> + - "nxp,pcf8574a": For the NXP PCF8574A
> + - "nxp,pcf8575": For the NXP PCF8575
> + - "ti,tca9554": For the TI TCA9554
> +
> + - reg: I2C slave address.
> +
> + - gpio-controller: Marks the device node as a gpio controller.
> + - #gpio-cells: Should be 2. The first cell is the GPIO number and the
> second
> + cell specifies GPIO flags, as defined in <dt-bindings/gpio/gpio.h>. Only
> the
> + GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
> +
> +Optional Properties:
> +
> + - lines-initial-states: Bitmask that specifies the initial state of each
> + line. When a bit is set to zero, the corresponding line will be
> initialized to
> + the input (pulled-up) state. When the bit is set to one, the line will be
> + initialized the low-level output state. If the property is not specified
> + all lines will be initialized to the input state.
> +
> + The I/O expander can detect input state changes, and thus optionally act as
> + an interrupt controller. When the expander interrupt line is connected all
> the
> + following properties must be set. For more information please see the
> + interrupt controller device tree bindings documentation available at
> + Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
> +
> + - interrupt-controller: Identifies the node as an interrupt controller.
> + - #interrupt-cells: Number of cells to encode an interrupt source, shall
> be 2.
> + - interrupt-parent: phandle of the parent interrupt controller.
> + - interrupts: Interrupt specifier for the controllers interrupt.
> +
> +
> +Please refer to gpio.txt in this directory for details of the common GPIO
> +bindings used by client devices.
> +
> +Example: PCF8575 I/O expander node
> +
> + pcf8575: gpio@20 {
> + compatible = "nxp,pcf8575";
> + reg = <0x20>;
> + interrupt-parent = <&irqpin2>;
> + interrupts = <3 0>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 73b862dc0b21..1af05358ec76 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -79,6 +79,13 @@ config PM8916_GPIO
> Power and reset buttons are placed in "pm8916_key" bank and
> have gpio numbers 0 and 1 respectively.
>
> +config PCF8575_GPIO
> + bool "PCF8575 I2C GPIO Expander driver"
> + depends on DM_GPIO && DM_I2C
> + help
> + Support for PCF8575 I2C 16 bit GPIO expander. Most of these
16-bit
> + chips are from NXP and TI.
You mention a single chip and then say 'most of these chips'. Is there
a series of chips, or are there lot of different chips that are
compatible? Can you update the help to be a bit more specific?
Can you briefly summarise the features of the driver? E.g. can set
each to input or output (I think). Anything else?
> +
> config ROCKCHIP_GPIO
> bool "Rockchip GPIO driver"
> depends on DM_GPIO
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 792d19186aad..8f426c82cdca 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -57,3 +57,4 @@ obj-$(CONFIG_PIC32_GPIO) += pic32_gpio.o
> obj-$(CONFIG_MVEBU_GPIO) += mvebu_gpio.o
> obj-$(CONFIG_MSM_GPIO) += msm_gpio.o
> obj-$(CONFIG_PM8916_GPIO) += pm8916_gpio.o
> +obj-$(CONFIG_PCF8575_GPIO) += pcf8575_gpio.o
Can you move that up one line to maintain order?
> diff --git a/drivers/gpio/pcf8575_gpio.c b/drivers/gpio/pcf8575_gpio.c
> new file mode 100644
> index 000000000000..3ca97eee11db
> --- /dev/null
> +++ b/drivers/gpio/pcf8575_gpio.c
> @@ -0,0 +1,199 @@
> +/*
> + * PCF8575 I2C GPIO EXPANDER DRIVER
> + *
> + * Copyright (C) 2016 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * Vignesh R <vigne...@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + *
> + *
> + * Driver for TI PCF-8575 16 bit I2C gpio expander. Based on
> + * gpio-pcf857x Linux Kernel(v4.7) driver.
> + *
> + * Copyright (C) 2007 David Brownell
> + *
> + */
> +
> +/*
> + * NOTE: The driver and devicetree bindings are borrowed from Linux
> + * Kernel, but driver does not support all PCF857x devices. It currently
> + * supports PCF8575 16Bit expander by TI and NXP.
> + *
> + * TODO:
TODO(email)
> + * Support 8 bit PCF857x compatible expanders.
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <i2c.h>
> +#include <asm-generic/gpio.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct pcf8575_chip {
> + int gpio_count; /* No GPIOs supported by the chip */
s/No/Number of/
or s/No/No./
> + unsigned int out; /* software latch */
Can you explain that one a bit more?
> + const char *bank_name; /* Name of the expander bank */
> +};
> +
> +/* Read/Write to 16-bit I/O expander */
> +
> +static int pcf8575_i2c_write_le16(struct udevice *dev, unsigned int word)
> +{
> + struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
> + struct i2c_msg msg;
> + u8 buf[2] = { word & 0xff, word >> 8, };
> + int ret;
> +
> + msg.addr = chip->chip_addr;
> + msg.buf = buf;
> + msg.flags = 0;
> + msg.len = 2;
> + ret = dm_i2c_xfer(dev, &msg, 1);
Can you use dm_i2c_write()?
> +
> + if (ret)
> + printf("%s i2c write failed to addr %x\n", __func__,
> msg.addr);
> +
> + return ret;
> +}
> +
> +static int pcf8575_i2c_read_le16(struct udevice *dev)
> +{
> + struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
> + struct i2c_msg msg;
> + u8 buf[2];
> + int ret;
> +
> + msg.addr = chip->chip_addr;
> + msg.buf = buf;
> + msg.flags = I2C_M_RD;
> + msg.len = 2;
> +
> + ret = dm_i2c_xfer(dev, &msg, 1);
> + if (ret) {
> + printf("%s i2c read failed to addr %x\n", __func__, msg.addr);
> + return ret;
> + }
> +
> + return (msg.buf[1] << 8) | msg.buf[0];
Does dm_i2c_read() work instead of calling dm_i2c_xfer()?
> +}
> +
> +static int pcf8575_direction_input(struct udevice *dev, unsigned offset)
> +{
> + struct pcf8575_chip *pc = dev_get_platdata(dev);
> + int status;
> +
> + pc->out |= BIT(offset);
> + status = pcf8575_i2c_write_le16(dev, pc->out);
> +
> + return status;
> +}
> +
> +static int pcf8575_direction_output(struct udevice *dev,
> + unsigned int offset, int value)
> +{
> + struct pcf8575_chip *pc = dev_get_platdata(dev);
> + int ret;
> +
> + if (value)
> + pc->out |= BIT(offset);
> + else
> + pc->out &= ~BIT(offset);
> +
> + ret = pcf8575_i2c_write_le16(dev, pc->out);
> +
> + return ret;
> +}
> +
> +static int pcf8575_get_value(struct udevice *dev, unsigned int offset)
> +{
> + int value;
> +
> + value = pcf8575_i2c_read_le16(dev);
> +
> + return (value < 0) ? value : ((value & BIT(offset)) >> offset);
> +}
> +
> +static int pcf8575_set_value(struct udevice *dev, unsigned int offset,
> + int value)
> +{
> + return pcf8575_direction_output(dev, offset, value);
> +}
> +
> +static int pcf8575_ofdata_platdata(struct udevice *dev)
> +{
> + struct pcf8575_chip *pc = dev_get_platdata(dev);
s/pc/plat/
(it's what most drivers use)
> + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +
> + int n_latch;
> +
> + uc_priv->gpio_count = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> + "gpio-count", 16);
> + uc_priv->bank_name = fdt_getprop(gd->fdt_blob, dev->of_offset,
> + "gpio-bank-name", NULL);
> + if (!uc_priv->bank_name)
> + uc_priv->bank_name = fdt_get_name(gd->fdt_blob,
> + dev->of_offset, NULL);
> +
> + /* NOTE: these chips have strange "quasi-bidirectional" I/O pins.
> + * We can't actually know whether a pin is configured (a) as output
> + * and driving the signal low, or (b) as input and reporting a low
> + * value ... without knowing the last value written since the chip
> + * came out of reset (if any). We can't read the latched output.
> + * In short, the only reliable solution for setting up pin direction
> + * is to do it explicitly.
> + *
> + * Using n_latch avoids that trouble. When left initialized to zero,
> + * our software copy of the "latch" then matches the chip's all-ones
> + * reset state. Otherwise it flags pins to be driven low.
> + */
> +
> + n_latch = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
> + "lines-initial-states", 0);
> + pc->out = ~n_latch;
> +
> + return 0;
> +}
> +
> +static int pcf8575_gpio_probe(struct udevice *dev)
> +{
> + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +
> + debug("%s GPIO controller with %d gpios probed\n",
> + uc_priv->bank_name, uc_priv->gpio_count);
> +
> + return 0;
> +}
> +
> +static const struct dm_gpio_ops pcf8575_gpio_ops = {
> + .direction_input = pcf8575_direction_input,
> + .direction_output = pcf8575_direction_output,
> + .get_value = pcf8575_get_value,
> + .set_value = pcf8575_set_value,
> +};
> +
> +static const struct udevice_id pcf8575_gpio_ids[] = {
> + { .compatible = "nxp,pcf8575" },
> + { .compatible = "ti,pcf8575" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(gpio_pcf8575) = {
> + .name = "gpio_pcf8575",
> + .id = UCLASS_GPIO,
> + .ops = &pcf8575_gpio_ops,
> + .of_match = pcf8575_gpio_ids,
> + .ofdata_to_platdata = pcf8575_ofdata_platdata,
> + .probe = pcf8575_gpio_probe,
> + .platdata_auto_alloc_size = sizeof(struct pcf8575_chip),
> +};
> --
> 2.9.2
>
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
