[U-Boot] [PATCH V2 1/3] dm: spi: soft_spi bug fix

[U-Boot] [PATCH V2 1/3] dm: spi: soft_spi bug fix

[Peng Fan]

When doing xfer, should use device->parent, but not device
When doing bit xfer, should use "!!(tmpdout & 0x80)", but not
"(tmpdout & 0x80)"

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
---

V2:
 Follow Simon's comments, split bug fix into seperate patch.

 drivers/spi/soft_spi.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/soft_spi.c b/drivers/spi/soft_spi.c
index aa4abcc..8cdb520 100644
--- a/drivers/spi/soft_spi.c
+++ b/drivers/spi/soft_spi.c
@@ -34,7 +34,8 @@ struct soft_spi_priv {
 
 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 +44,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 +54,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 +66,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 +104,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,7 +139,7 @@ static int soft_spi_xfer(struct udevice *dev, unsigned int bitlen,
 
 		if (!cpha)
 			soft_spi_scl(dev, 0);
-		soft_spi_sda(dev, tmpdout & 0x80);
+		soft_spi_sda(dev, !!(tmpdout & 0x80));
 		udelay(plat->spi_delay_us);
 		if (cpha)
 			soft_spi_scl(dev, 0);
-- 
2.6.2

[U-Boot] [PATCH V2 2/3] dm: spi: soft_spi: switch to use linux compatible string

[Peng Fan]

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. Introduce SPI_MASTER_NO_RX and SPI_MASTER_NO_TX to handle
   no rx or no tx case.
3. 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>
---

V2:
 Follow Simon's comments, drop uboot compatible string, convert to use
 Linux compatile string, and convert the universal board.

 arch/arm/dts/exynos4210-universal_c210.dts | 10 ++++----
 drivers/spi/soft_spi.c                     | 38 ++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/arch/arm/dts/exynos4210-universal_c210.dts b/arch/arm/dts/exynos4210-universal_c210.dts
index 16948c9..ad3527e 100644
--- a/arch/arm/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/dts/exynos4210-universal_c210.dts
@@ -42,11 +42,11 @@
 	};
 
 	soft-spi {
-		compatible = "u-boot,soft-spi";
-		cs-gpio = <&gpy4 3 0>;
-		sclk-gpio = <&gpy3 1 0>;
-		mosi-gpio = <&gpy3 3 0>;
-		miso-gpio = <&gpy3 0 0>;
+		compatible = "spi-gpio";
+		cs-gpios = <&gpy4 3 0>;
+		gpio-sck = <&gpy3 1 0>;
+		gpio-mosi = <&gpy3 3 0>;
+		gpio-miso = <&gpy3 0 0>;
 		spi-delay-us = <1>;
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/drivers/spi/soft_spi.c b/drivers/spi/soft_spi.c
index 8cdb520..d23dc81 100644
--- a/drivers/spi/soft_spi.c
+++ b/drivers/spi/soft_spi.c
@@ -26,8 +26,12 @@ 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)
+
 struct soft_spi_priv {
 	unsigned int mode;
 };
@@ -139,14 +143,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)
@@ -208,24 +214,36 @@ static int soft_spi_probe(struct udevice *dev)
 	struct spi_slave *slave = dev_get_parent_priv(dev);
 	struct soft_spi_platdata *plat = dev->platdata;
 	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,
+
+	if (gpio_request_by_name(dev, "cs-gpios", 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))
+	    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 = "spi-gpio" },
 	{ }
 };
 
-- 
2.6.2

[U-Boot] [PATCH V2 2/3] dm: spi: soft_spi: switch to use linux compatible string

[Simon Glass]

On 13 April 2016 at 01:18, 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.
> 2. Introduce SPI_MASTER_NO_RX and SPI_MASTER_NO_TX to handle
>    no rx or no tx case.
> 3. 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>
> ---
>
> V2:
>  Follow Simon's comments, drop uboot compatible string, convert to use
>  Linux compatile string, and convert the universal board.
>
>  arch/arm/dts/exynos4210-universal_c210.dts | 10 ++++----
>  drivers/spi/soft_spi.c                     | 38 ++++++++++++++++++++++--------
>  2 files changed, 33 insertions(+), 15 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Please also cc the universal maintainer to this thread.

[U-Boot] [PATCH V2 3/3] dm: gpio: introduce 74x164 driver

[Peng Fan]

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>
---

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

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@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);
+	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

[U-Boot] [PATCH V2 3/3] dm: gpio: introduce 74x164 driver

[Simon Glass]

Hi Peng,

On 13 April 2016 at 01:18, 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>
> ---
>
> 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@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

[U-Boot] [PATCH V2 3/3] dm: gpio: introduce 74x164 driver

[Peng Fan]

Hi Simon,

On Wed, Apr 20, 2016 at 08:40:45AM -0600, Simon Glass wrote:
>Hi Peng,
>
>On 13 April 2016 at 01:18, 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>
>> ---
>>
>> 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@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?

Ok. Will fix in V3.

Thanks,
Peng

>
>> +       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

[U-Boot] [PATCH V2 1/3] dm: spi: soft_spi bug fix

[Simon Glass]

On 13 April 2016 at 01:18, Peng Fan <van.freenix@gmail.com> wrote:
> When doing xfer, should use device->parent, but not device
> When doing bit xfer, should use "!!(tmpdout & 0x80)", but not
> "(tmpdout & 0x80)"
>
> Signed-off-by: Peng Fan <van.freenix@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>
> V2:
>  Follow Simon's comments, split bug fix into seperate patch.
>
>  drivers/spi/soft_spi.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>