Hi Peng,
On 21 March 2016 at 02:29, Peng Fan van.freenix@gmail.com wrote:
Introduce driver to support "fairchild,74hc595" devices.
- Take linux drivers/drivers/gpio/gpio-74x164.c as reference.
- Following the naming used in Linux driver with gen_7x164 as the prefix.
- Enable CONFIG_DM_74X164 to use this driver.
- Follow Documentation/devicetree/bindings/gpio/gpio-74x164.txt to add device nodes
- Tested on i.MX6 UltraLite with 74LV595 using gpio command and oscillograph.
Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Chin Liang See clsee@altera.com Cc: Bhuvanchandra DV bhuvanchandra.dv@toradex.com Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Stefano Babic sbabic@denx.de
drivers/gpio/74x164_gpio.c | 220 +++++++++++++++++++++++++++++++++++++++++++++ drivers/gpio/Kconfig | 8 ++ drivers/gpio/Makefile | 1 + 3 files changed, 229 insertions(+) create mode 100644 drivers/gpio/74x164_gpio.c
diff --git a/drivers/gpio/74x164_gpio.c b/drivers/gpio/74x164_gpio.c new file mode 100644 index 0000000..5d12a9a --- /dev/null +++ b/drivers/gpio/74x164_gpio.c @@ -0,0 +1,220 @@ +/*
- 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@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_chip - Data for 74Hx164
- @dev: udevice structure for the device
This doesn't appear to be u
- @slave: spi slave
- @oe: OE pin
- @nregs: number of registers
- @buffer: buffer for chained chips
- */
+#define GEN_74X164_NUMBER_GPIOS 8
+struct gen_74x164_info {
How about gen_74x164_priv?
struct udevice *dev;
This doesn't seem to be used
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_info *info = dev_get_platdata(dev);struct spi_slave *slave = info->slave;int ret;ret = spi_claim_bus(slave);if (ret)return ret;ret = spi_xfer(slave, info->nregs * 8, info->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_info *info = dev_get_platdata(dev);u8 bank = info->nregs - 1 - offset / 8;u8 pin = offset % 8;return (info->buffer[bank] >> pin) & 0x1;+}
+static int gen_74x164_set_value(struct udevice *dev, unsigned offset,
int value)+{
struct gen_74x164_info *info = dev_get_platdata(dev);u8 bank = info->nregs - 1 - offset / 8;u8 pin = offset % 8;
uint should be good enough for these
int ret;if (value)info->buffer[bank] |= 1 << pin;elseinfo->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 -EINVAL;
-ENOSYS if it is not supported
+}
+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_info *info = dev_get_platdata(dev);struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);char *str, *str1;int ret;const void *fdt = gd->fdt_blob;int node = dev->of_offset;struct udevice *bus;int busnum;char name[30];str = strdup(dev->name);if (!str)return -ENOMEM;info->nregs = fdtdec_get_int(fdt, node, "registers-number", 1);
Do you have a device tree binding file you can add for this?
info->buffer = calloc(info->nregs, sizeof(u8));if (!info->buffer) {ret = -ENOMEM;goto free_str;}
What's the largest number of registers you can have? If it is small, like 32, consider making buffer a simple fixed-length array.
ret = fdtdec_get_byte_array(fdt, node, "registers-default",info->buffer, info->nregs);if (ret)dev_dbg(dev, "No registers-default property\n");ret = gpio_request_by_name(dev, "oe-gpios", 0, &info->oe,GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);if (ret) {dev_err(dev, "No oe-pins property\n");goto free_buf;}/* output enable */ret = dm_gpio_set_value(&info->oe, 0);if (ret) {dev_err(dev, "Set oe-pins value failure\n");goto free_buf;}
The GPIOD_IS_OUT_ACTIVE above will already activate this GPIO.
uc_priv->bank_name = str;uc_priv->gpio_count = info->nregs * 8;bus = dev_get_parent(dev);busnum = bus->seq;snprintf(name, sizeof(name), "generic_%d:%d", busnum, 0);str1 = strdup(name);
What is str1 used for?
if (!str1) {ret = -ENOMEM;goto free_buf;}ret = spi_get_bus_and_cs(busnum, 0, 1000000, SPI_MODE_0,"spi_generic_drv", str, &bus, &info->slave);if (ret)goto free_str1;
Why do you need this? The parent device is the SPI bus. See drivers/misc/cros_ec_spi.c for how it probes. There really shouldn't be much to do.
I'd suggest:
infp->slave = dev_get_parent_priv(dev);
ret = gen_74x164_write_conf(dev);if (ret)goto free_str1;dev_dbg(dev, "%s is ready\n", dev->name);return 0;+free_str1:
free(str1);+free_buf:
free(info->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,.platdata_auto_alloc_size = sizeof(struct gen_74x164_info),
It does not seem to be used before probe(), so I think this should be priv_auto_alloc_size.
.of_match = gen_74x164_ids,+}; diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 51658f1..43fc9c4 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -96,6 +96,14 @@ config PIC32_GPIO help Say yes here to support Microchip PIC32 GPIOs.
+config DM_74X164
bool "74x164 serial-in/parallel-out 8-bits shift register"depends on DM_GPIOhelpDriver for 74x164 compatible serial-in/parallel-out 8-outputs
74x164-compatible
Should you mention the oriiginal manufacturer. Also you could mention that it uses SPI.
shift registers. This driver can be used to provide accessto 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 bfc67d2..393b148 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