[U-Boot] [PATCH V2 3/3] dm: gpio: introduce 74x164 driver
Simon Glass
sjg at chromium.org
Wed Apr 20 16:40:45 CEST 2016
Hi Peng,
On 13 April 2016 at 01:18, Peng Fan <van.freenix at gmail.com> wrote:
> Introduce driver to support "fairchild,74hc595" devices.
> 1. Take linux drivers/drivers/gpio/gpio-74x164.c as reference.
> 2. Following the naming used in Linux driver with gen_7x164 as the prefix.
> 3. Enable CONFIG_DM_74X164 to use this driver.
> 4. Follow Documentation/devicetree/bindings/gpio/gpio-74x164.txt to add device
> nodes
> 5. Tested on i.MX6 UltraLite with 74LV595 using gpio command and oscillograph.
>
> Signed-off-by: Peng Fan <van.freenix at gmail.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
> Cc: Chin Liang See <clsee at altera.com>
> Cc: Bhuvanchandra DV <bhuvanchandra.dv at toradex.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
> Cc: Fabio Estevam <fabio.estevam at nxp.com>
> Cc: Stefano Babic <sbabic at denx.de>
> ---
>
> V2:
> Address Simon's comments:
> Drop unused dev entry of gen_74x164_priv
> Rename gen_74x164_info to gen_74x164_priv
> Change u8 to uint
> Change EINVAL to ENOSYS
> Drop dm_gpio_set_value when probe, since GPIOD_IS_OUT_ACTIVE already
> active the gpio.
> Drop complicated spi_get_bus_and_cs, use dev_get_parent_priv.
> To Kconfig help msg, I add 74lv165 and 75hc595, actually, the linux
> one does not contain manufacture info.
>
> drivers/gpio/74x164_gpio.c | 198 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/gpio/Kconfig | 8 ++
> drivers/gpio/Makefile | 1 +
> 3 files changed, 207 insertions(+)
> create mode 100644 drivers/gpio/74x164_gpio.c
This looks good, but can you please adjust as below?
>
> diff --git a/drivers/gpio/74x164_gpio.c b/drivers/gpio/74x164_gpio.c
> new file mode 100644
> index 0000000..1791dc8
> --- /dev/null
> +++ b/drivers/gpio/74x164_gpio.c
> @@ -0,0 +1,198 @@
> +/*
> + * Take drivers/gpio/gpio-74x164.c as reference.
> + *
> + * 74Hx164 - Generic serial-in/parallel-out 8-bits shift register GPIO driver
> + *
> + * Copyright (C) 2016 Peng Fan <van.freenix at gmail.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + *
> + */
> +
> +#include <common.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <malloc.h>
> +#include <asm/gpio.h>
> +#include <asm/io.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <spi.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/*
> + * struct gen_74x164_priv - Data for 74Hx164
> + *
> + * @slave: spi slave
> + * @oe: OE pin
> + * @nregs: number of registers
> + * @buffer: buffer for chained chips
> + */
> +#define GEN_74X164_NUMBER_GPIOS 8
> +
> +struct gen_74x164_priv {
> + struct spi_slave *slave;
> + struct gpio_desc oe;
> + u32 nregs;
> + /*
> + * Since the nregs are chained, every byte sent will make
> + * the previous byte shift to the next register in the
> + * chain. Thus, the first byte sent will end up in the last
> + * register at the end of the transfer. So, to have a logical
> + * numbering, store the bytes in reverse order.
> + */
> + u8 *buffer;
> +};
> +
> +static int gen_74x164_write_conf(struct udevice *dev)
> +{
> + struct gen_74x164_priv *priv = dev_get_priv(dev);
> + struct spi_slave *slave = priv->slave;
> + int ret;
> +
> + ret = spi_claim_bus(slave);
We should have a call like dm_claim_bus(struct udevice *dev)
Then you can avoid this priv business. Similarly with
dm_spi_release_bus() and dm_spi_xfer(). They should be easy to add.
Can you do a new patch?
> + if (ret)
> + return ret;
> +
> + ret = spi_xfer(slave, priv->nregs * 8, priv->buffer, NULL,
> + SPI_XFER_BEGIN | SPI_XFER_END);
> +
> + spi_release_bus(slave);
> +
> + return ret;
> +}
> +
> +static int gen_74x164_get_value(struct udevice *dev, unsigned offset)
> +{
> + struct gen_74x164_priv *priv = dev_get_priv(dev);
> + uint bank = priv->nregs - 1 - offset / 8;
> + uint pin = offset % 8;
> +
> + return (priv->buffer[bank] >> pin) & 0x1;
> +}
> +
> +static int gen_74x164_set_value(struct udevice *dev, unsigned offset,
> + int value)
> +{
> + struct gen_74x164_priv *priv = dev_get_priv(dev);
> + uint bank = priv->nregs - 1 - offset / 8;
> + uint pin = offset % 8;
> + int ret;
> +
> + if (value)
> + priv->buffer[bank] |= 1 << pin;
> + else
> + priv->buffer[bank] &= ~(1 << pin);
> +
> + ret = gen_74x164_write_conf(dev);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int gen_74x164_direction_input(struct udevice *dev, unsigned offset)
> +{
> + return -ENOSYS;
> +}
> +
> +static int gen_74x164_direction_output(struct udevice *dev, unsigned offset,
> + int value)
> +{
> + return gen_74x164_set_value(dev, offset, value);
> +}
> +
> +static int gen_74x164_get_function(struct udevice *dev, unsigned offset)
> +{
> + return GPIOF_OUTPUT;
> +}
> +
> +static int gen_74x164_xlate(struct udevice *dev, struct gpio_desc *desc,
> + struct fdtdec_phandle_args *args)
> +{
> + desc->offset = args->args[0];
> + desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
> +
> + return 0;
> +}
> +
> +static const struct dm_gpio_ops gen_74x164_ops = {
> + .direction_input = gen_74x164_direction_input,
> + .direction_output = gen_74x164_direction_output,
> + .get_value = gen_74x164_get_value,
> + .set_value = gen_74x164_set_value,
> + .get_function = gen_74x164_get_function,
> + .xlate = gen_74x164_xlate,
> +};
> +
> +static int gen_74x164_probe(struct udevice *dev)
> +{
> + struct gen_74x164_priv *priv = dev_get_priv(dev);
> + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> + char *str, name[32];
> + int ret;
> + const void *fdt = gd->fdt_blob;
> + int node = dev->of_offset;
> +
> + snprintf(name, sizeof(name), "%s_", dev->name);
> + str = strdup(name);
> + if (!str)
> + return -ENOMEM;
> +
> + /*
> + * See Linux kernel:
> + * Documentation/devicetree/bindings/gpio/gpio-74x164.txt
> + */
> + priv->nregs = fdtdec_get_int(fdt, node, "registers-number", 1);
> + priv->buffer = calloc(priv->nregs, sizeof(u8));
> + if (!priv->buffer) {
> + ret = -ENOMEM;
> + goto free_str;
> + }
> +
> + ret = fdtdec_get_byte_array(fdt, node, "registers-default",
> + priv->buffer, priv->nregs);
> + if (ret)
> + dev_dbg(dev, "No registers-default property\n");
> +
> + ret = gpio_request_by_name(dev, "oe-gpios", 0, &priv->oe,
> + GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
> + if (ret) {
> + dev_err(dev, "No oe-pins property\n");
> + goto free_buf;
> + }
> +
> + uc_priv->bank_name = str;
> + uc_priv->gpio_count = priv->nregs * 8;
> +
> + priv->slave = dev_get_parent_priv(dev);
> +
> + ret = gen_74x164_write_conf(dev);
> + if (ret)
> + goto free_buf;
> +
> + dev_dbg(dev, "%s is ready\n", dev->name);
> +
> + return 0;
> +
> +free_buf:
> + free(priv->buffer);
> +free_str:
> + free(str);
> + return ret;
> +}
> +
> +static const struct udevice_id gen_74x164_ids[] = {
> + { .compatible = "fairchild,74hc595" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(74x164) = {
> + .name = "74x164",
> + .id = UCLASS_GPIO,
> + .ops = &gen_74x164_ops,
> + .probe = gen_74x164_probe,
> + .priv_auto_alloc_size = sizeof(struct gen_74x164_priv),
> + .of_match = gen_74x164_ids,
> +};
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 8ff657c..7aecd13 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -112,6 +112,14 @@ config MVEBU_GPIO
> help
> Say yes here to support Marvell MVEBU (Armada XP/38x) GPIOs.
>
> +config DM_74X164
> + bool "74x164 serial-in/parallel-out 8-bits shift register"
> + depends on DM_GPIO
> + help
> + Driver for 74x164 compatible serial-in/parallel-out 8-outputs
> + shift registers, such as 74lv165, 74hc595.
> + This driver can be used to provide access to more gpio outputs.
> +
> config DM_PCA953X
> bool "PCA95[357]x, PCA9698, TCA64xx, and MAX7310 I/O ports"
> depends on DM_GPIO
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index a7a466c..cca5f39 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -12,6 +12,7 @@ endif
> obj-$(CONFIG_DM_GPIO) += gpio-uclass.o
>
> obj-$(CONFIG_DM_PCA953X) += pca953x_gpio.o
> +obj-$(CONFIG_DM_74X164) += 74x164_gpio.o
>
> obj-$(CONFIG_AT91_GPIO) += at91_gpio.o
> obj-$(CONFIG_ATMEL_PIO4) += atmel_pio4.o
> --
> 2.6.2
>
Regards,
Simon
More information about the U-Boot
mailing list