* [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix
@ 2016-03-21 8:29 Peng Fan
2016-03-21 8:29 ` [U-Boot] [PATCH 2/2] dm: gpio: introduce 74x164 driver Peng Fan
2016-04-09 18:33 ` [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix Simon Glass
0 siblings, 2 replies; 6+ messages in thread
From: Peng Fan @ 2016-03-21 8:29 UTC (permalink / raw)
To: u-boot
1. Support compatible string "spi-gpio" which is used by Linux
Linux use different bindings, so use UBOOT_COMPAT and
LINUX_COMPAT to differentiate them.
2. Bug fix
When doing xfer, should use device->parent, but not device
When doing bit xfer, should use "!!(tmpdout & 0x80)", but not
"(tmpdout & 0x80)"
3. Introduce SPI_MASTER_NO_RX and SPI_MASTER_NO_TX to handle
no rx or no tx case.
4. Tested on i.MX6 UltraLite board with 74LV595 spi-gpio chip.
Signed-off-by: Peng Fan <van.freenix@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Jagan Teki <jteki@openedev.com>
Cc: Stefano Babic <sbabic@denx.de>
---
drivers/spi/soft_spi.c | 75 ++++++++++++++++++++++++++++++++++++++------------
1 file changed, 57 insertions(+), 18 deletions(-)
diff --git a/drivers/spi/soft_spi.c b/drivers/spi/soft_spi.c
index aa4abcc..9a2b5f2 100644
--- a/drivers/spi/soft_spi.c
+++ b/drivers/spi/soft_spi.c
@@ -26,15 +26,23 @@ struct soft_spi_platdata {
struct gpio_desc mosi;
struct gpio_desc miso;
int spi_delay_us;
+ int flags;
};
+#define SPI_MASTER_NO_RX BIT(0)
+#define SPI_MASTER_NO_TX BIT(1)
+
+#define UBOOT_COMPAT BIT(0)
+#define LINUX_COMPAT BIT(1)
+
struct soft_spi_priv {
unsigned int mode;
};
static int soft_spi_scl(struct udevice *dev, int bit)
{
- struct soft_spi_platdata *plat = dev->platdata;
+ struct udevice *bus = dev_get_parent(dev);
+ struct soft_spi_platdata *plat = dev_get_platdata(bus);
dm_gpio_set_value(&plat->sclk, bit);
@@ -43,7 +51,8 @@ static int soft_spi_scl(struct udevice *dev, int bit)
static int soft_spi_sda(struct udevice *dev, int bit)
{
- struct soft_spi_platdata *plat = dev->platdata;
+ struct udevice *bus = dev_get_parent(dev);
+ struct soft_spi_platdata *plat = dev_get_platdata(bus);
dm_gpio_set_value(&plat->mosi, bit);
@@ -52,7 +61,8 @@ static int soft_spi_sda(struct udevice *dev, int bit)
static int soft_spi_cs_activate(struct udevice *dev)
{
- struct soft_spi_platdata *plat = dev->platdata;
+ struct udevice *bus = dev_get_parent(dev);
+ struct soft_spi_platdata *plat = dev_get_platdata(bus);
dm_gpio_set_value(&plat->cs, 0);
dm_gpio_set_value(&plat->sclk, 0);
@@ -63,7 +73,8 @@ static int soft_spi_cs_activate(struct udevice *dev)
static int soft_spi_cs_deactivate(struct udevice *dev)
{
- struct soft_spi_platdata *plat = dev->platdata;
+ struct udevice *bus = dev_get_parent(dev);
+ struct soft_spi_platdata *plat = dev_get_platdata(bus);
dm_gpio_set_value(&plat->cs, 0);
@@ -100,8 +111,9 @@ static int soft_spi_release_bus(struct udevice *dev)
static int soft_spi_xfer(struct udevice *dev, unsigned int bitlen,
const void *dout, void *din, unsigned long flags)
{
- struct soft_spi_priv *priv = dev_get_priv(dev);
- struct soft_spi_platdata *plat = dev->platdata;
+ struct udevice *bus = dev_get_parent(dev);
+ struct soft_spi_priv *priv = dev_get_priv(bus);
+ struct soft_spi_platdata *plat = dev_get_platdata(bus);
uchar tmpdin = 0;
uchar tmpdout = 0;
const u8 *txd = dout;
@@ -134,14 +146,16 @@ static int soft_spi_xfer(struct udevice *dev, unsigned int bitlen,
if (!cpha)
soft_spi_scl(dev, 0);
- soft_spi_sda(dev, tmpdout & 0x80);
+ if ((plat->flags & SPI_MASTER_NO_TX) == 0)
+ soft_spi_sda(dev, !!(tmpdout & 0x80));
udelay(plat->spi_delay_us);
if (cpha)
soft_spi_scl(dev, 0);
else
soft_spi_scl(dev, 1);
tmpdin <<= 1;
- tmpdin |= dm_gpio_get_value(&plat->miso);
+ if ((plat->flags & SPI_MASTER_NO_RX) == 0)
+ tmpdin |= dm_gpio_get_value(&plat->miso);
tmpdout <<= 1;
udelay(plat->spi_delay_us);
if (cpha)
@@ -202,25 +216,50 @@ static int soft_spi_probe(struct udevice *dev)
{
struct spi_slave *slave = dev_get_parent_priv(dev);
struct soft_spi_platdata *plat = dev->platdata;
+ ulong driver_data = dev_get_driver_data(dev);
int cs_flags, clk_flags;
+ int ret;
cs_flags = (slave->mode & SPI_CS_HIGH) ? 0 : GPIOD_ACTIVE_LOW;
clk_flags = (slave->mode & SPI_CPOL) ? GPIOD_ACTIVE_LOW : 0;
- if (gpio_request_by_name(dev, "cs-gpio", 0, &plat->cs,
- GPIOD_IS_OUT | cs_flags) ||
- gpio_request_by_name(dev, "sclk-gpio", 0, &plat->sclk,
- GPIOD_IS_OUT | clk_flags) ||
- gpio_request_by_name(dev, "mosi-gpio", 0, &plat->mosi,
- GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE) ||
- gpio_request_by_name(dev, "miso-gpio", 0, &plat->miso,
- GPIOD_IS_IN))
- return -EINVAL;
+ if (driver_data & UBOOT_COMPAT) {
+ if (gpio_request_by_name(dev, "cs-gpio", 0, &plat->cs,
+ GPIOD_IS_OUT | cs_flags) ||
+ gpio_request_by_name(dev, "sclk-gpio", 0, &plat->sclk,
+ GPIOD_IS_OUT | clk_flags) ||
+ gpio_request_by_name(dev, "mosi-gpio", 0, &plat->mosi,
+ GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE) ||
+ gpio_request_by_name(dev, "miso-gpio", 0, &plat->miso,
+ GPIOD_IS_IN))
+ return -EINVAL;
+ } else if (driver_data & LINUX_COMPAT) {
+ if (gpio_request_by_name(dev, "cs-gpios", 0, &plat->cs,
+ GPIOD_IS_OUT | cs_flags) ||
+ gpio_request_by_name(dev, "gpio-sck", 0, &plat->sclk,
+ GPIOD_IS_OUT | clk_flags))
+ return -EINVAL;
+
+ ret = gpio_request_by_name(dev, "gpio-mosi", 0, &plat->mosi,
+ GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
+ if (ret)
+ plat->flags |= SPI_MASTER_NO_TX;
+
+ ret = gpio_request_by_name(dev, "gpio-miso", 0, &plat->miso,
+ GPIOD_IS_IN);
+ if (ret)
+ plat->flags |= SPI_MASTER_NO_RX;
+
+ if ((plat->flags & (SPI_MASTER_NO_RX | SPI_MASTER_NO_TX)) ==
+ (SPI_MASTER_NO_RX | SPI_MASTER_NO_TX))
+ return -EINVAL;
+ }
return 0;
}
static const struct udevice_id soft_spi_ids[] = {
- { .compatible = "u-boot,soft-spi" },
+ { .compatible = "u-boot,soft-spi", .data = UBOOT_COMPAT },
+ { .compatible = "spi-gpio", .data = LINUX_COMPAT },
{ }
};
--
2.6.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] dm: gpio: introduce 74x164 driver
2016-03-21 8:29 [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix Peng Fan
@ 2016-03-21 8:29 ` Peng Fan
2016-04-09 18:33 ` Simon Glass
2016-04-09 18:33 ` [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix Simon Glass
1 sibling, 1 reply; 6+ messages in thread
From: Peng Fan @ 2016-03-21 8:29 UTC (permalink / raw)
To: u-boot
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@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
+ * @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 {
+ struct udevice *dev;
+ 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;
+ int ret;
+
+ if (value)
+ info->buffer[bank] |= 1 << pin;
+ else
+ info->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;
+}
+
+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);
+ info->buffer = calloc(info->nregs, sizeof(u8));
+ if (!info->buffer) {
+ ret = -ENOMEM;
+ goto free_str;
+ }
+
+ 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;
+ }
+
+ 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);
+ 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;
+
+ 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),
+ .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_GPIO
+ help
+ Driver for 74x164 compatible serial-in/parallel-out 8-outputs
+ shift registers. 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 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix
2016-03-21 8:29 [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix Peng Fan
2016-03-21 8:29 ` [U-Boot] [PATCH 2/2] dm: gpio: introduce 74x164 driver Peng Fan
@ 2016-04-09 18:33 ` Simon Glass
2016-04-11 5:45 ` Peng Fan
1 sibling, 1 reply; 6+ messages in thread
From: Simon Glass @ 2016-04-09 18:33 UTC (permalink / raw)
To: u-boot
Hi Peng,
On 21 March 2016 at 02:29, Peng Fan <van.freenix@gmail.com> wrote:
> 1. Support compatible string "spi-gpio" which is used by Linux
> Linux use different bindings, so use UBOOT_COMPAT and
> LINUX_COMPAT to differentiate them.
There is only one user - instead of this, can you please drop the
U-Boot binding and move the universal board over to use your new
binding? There should be only one.
> 2. Bug fix
> When doing xfer, should use device->parent, but not device
> When doing bit xfer, should use "!!(tmpdout & 0x80)", but not
> "(tmpdout & 0x80)"
> 3. Introduce SPI_MASTER_NO_RX and SPI_MASTER_NO_TX to handle
> no rx or no tx case.
Perhaps put these two in separate patches?
> 4. Tested on i.MX6 UltraLite board with 74LV595 spi-gpio chip.
>
> Signed-off-by: Peng Fan <van.freenix@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Jagan Teki <jteki@openedev.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
> drivers/spi/soft_spi.c | 75 ++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 57 insertions(+), 18 deletions(-)
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] dm: gpio: introduce 74x164 driver
2016-03-21 8:29 ` [U-Boot] [PATCH 2/2] dm: gpio: introduce 74x164 driver Peng Fan
@ 2016-04-09 18:33 ` Simon Glass
2016-04-11 5:44 ` Peng Fan
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2016-04-09 18:33 UTC (permalink / raw)
To: u-boot
Hi Peng,
On 21 March 2016 at 02:29, Peng Fan <van.freenix@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@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;
> + else
> + info->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_GPIO
> + help
> + Driver 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 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 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] dm: gpio: introduce 74x164 driver
2016-04-09 18:33 ` Simon Glass
@ 2016-04-11 5:44 ` Peng Fan
0 siblings, 0 replies; 6+ messages in thread
From: Peng Fan @ 2016-04-11 5:44 UTC (permalink / raw)
To: u-boot
Hi Simon,
Thanks for reviewing.
On Sat, Apr 09, 2016 at 12:33:40PM -0600, Simon Glass wrote:
>Hi Peng,
>
>On 21 March 2016 at 02:29, Peng Fan <van.freenix@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@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
Will drop it in V2.
>
>> + 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
Fix in V2.
>
>> + int ret;
>> +
>> + if (value)
>> + info->buffer[bank] |= 1 << pin;
>> + else
>> + info->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
Fix in V2.
>
>> +}
>> +
>> +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?
In Kernel:
Documentation/devicetree/bindings/gpio/gpio-74x164.txt
>
>> + 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.
In general usage, only 1, but follow kernel, designed to be chained.
So we do not have fixed value.
>
>> +
>> + 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.
Will fix in V2.
>
>> +
>> + 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?
Oh. Should be used for spi_get_bus_and_cs, will fix in V2.
>
>> + 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);
Ok. Will try this. Fix in V2.
>
>> +
>> + 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.
Fix in V2.
>
>> + .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_GPIO
>> + help
>> + Driver 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.
Will fix in V2.
>
>> + shift registers. 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 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix
2016-04-09 18:33 ` [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix Simon Glass
@ 2016-04-11 5:45 ` Peng Fan
0 siblings, 0 replies; 6+ messages in thread
From: Peng Fan @ 2016-04-11 5:45 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Sat, Apr 09, 2016 at 12:33:37PM -0600, Simon Glass wrote:
>Hi Peng,
>
>On 21 March 2016 at 02:29, Peng Fan <van.freenix@gmail.com> wrote:
>> 1. Support compatible string "spi-gpio" which is used by Linux
>> Linux use different bindings, so use UBOOT_COMPAT and
>> LINUX_COMPAT to differentiate them.
>
>There is only one user - instead of this, can you please drop the
>U-Boot binding and move the universal board over to use your new
>binding? There should be only one.
Ok. Will drop the U-Boot binding.
>
>> 2. Bug fix
>> When doing xfer, should use device->parent, but not device
>> When doing bit xfer, should use "!!(tmpdout & 0x80)", but not
>> "(tmpdout & 0x80)"
>> 3. Introduce SPI_MASTER_NO_RX and SPI_MASTER_NO_TX to handle
>> no rx or no tx case.
>
>Perhaps put these two in separate patches?
Will try it.
Thanks,
Peng.
>
>> 4. Tested on i.MX6 UltraLite board with 74LV595 spi-gpio chip.
>>
>> Signed-off-by: Peng Fan <van.freenix@gmail.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Jagan Teki <jteki@openedev.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> ---
>> drivers/spi/soft_spi.c | 75 ++++++++++++++++++++++++++++++++++++++------------
>> 1 file changed, 57 insertions(+), 18 deletions(-)
>
>Regards,
>Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-11 5:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-21 8:29 [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix Peng Fan
2016-03-21 8:29 ` [U-Boot] [PATCH 2/2] dm: gpio: introduce 74x164 driver Peng Fan
2016-04-09 18:33 ` Simon Glass
2016-04-11 5:44 ` Peng Fan
2016-04-09 18:33 ` [U-Boot] [PATCH 1/2] dm: spi: soft_spi: support linux way and bug fix Simon Glass
2016-04-11 5:45 ` Peng Fan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.