[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Peng Fan]

Introduce a new driver that supports driver model for pca953x.
The pca953x chips are used as I2C I/O expanders.
This driver is designed to support the following chips:
"
4 bits: pca9536, pca9537
8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
        pca9556, pca9557, pca9574, tca6408, xra1202
16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
         tca6416
24 bits: tca6424
40 bits: pca9505, pca9698
"
But for now this driver only supports max 24 bits and pca953x compatible
chips. pca957x compatible chips are not supported now.
These can be addressed when we need to add such support for the different
chips.
This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
i2c expander using gpio command as following:

=>gpio status -a
Bank gpio at 48:
gpio at 480: input: 1 [ ]
=> gpio clear gpio at 480
gpio: pin gpio at 480 (gpio 224) value is 0
=> gpio status -a
Bank gpio at 48:
gpio at 480: output: 0 [ ]

=> dm tree:
 i2c         [   ]    |   |   `-- i2c at 021a8000
 gpio        [   ]    |   |       |-- gpio at 30
 gpio        [   ]    |   |       `-- gpio at 32

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Wenyou Yang <wenyou.yang@atmel.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
Cc: Purna Chandra Mandal <purna.mandal@microchip.com>
Cc: Thomas Chou <thomas@wytron.com.tw>
Cc: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
Cc: Andrea Scian <andrea.scian@dave.eu>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
---
 drivers/gpio/Kconfig        |  23 +++
 drivers/gpio/Makefile       |   2 +
 drivers/gpio/pca953x_gpio.c | 349 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 374 insertions(+)
 create mode 100644 drivers/gpio/pca953x_gpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 94fabb9..51658f1 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -96,4 +96,27 @@ config PIC32_GPIO
 	help
 	  Say yes here to support Microchip PIC32 GPIOs.
 
+config DM_PCA953X
+	bool "PCA95[357]x, PCA9698, TCA64xx, and MAX7310 I/O ports"
+	depends on DM_GPIO
+	help
+	  Say yes here to provide access to several register-oriented
+	  SMBus I/O expanders, made mostly by NXP or TI.  Compatible
+	  models include:
+
+	  4 bits:	pca9536, pca9537
+
+	  8 bits:	max7310, max7315, pca6107, pca9534, pca9538, pca9554,
+			pca9556, pca9557, pca9574, tca6408, xra1202
+
+	  16 bits:	max7312, max7313, pca9535, pca9539, pca9555, pca9575,
+			tca6416
+
+	  24 bits:	tca6424
+
+	  40 bits:	pca9505, pca9698
+
+	  Now, max 24 bits chips and PCA953X compatible chips are
+	  supported
+
 endmenu
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index ca8c487..bfc67d2 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -11,6 +11,8 @@ obj-$(CONFIG_AXP_GPIO)		+= axp_gpio.o
 endif
 obj-$(CONFIG_DM_GPIO)		+= gpio-uclass.o
 
+obj-$(CONFIG_DM_PCA953X)	+= pca953x_gpio.o
+
 obj-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
 obj-$(CONFIG_ATMEL_PIO4)	+= atmel_pio4.o
 obj-$(CONFIG_INTEL_ICH6_GPIO)	+= intel_ich6_gpio.o
diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
new file mode 100644
index 0000000..6b67b07
--- /dev/null
+++ b/drivers/gpio/pca953x_gpio.c
@@ -0,0 +1,349 @@
+/*
+ * Take linux kernel driver drivers/gpio/gpio-pca953x.c for reference.
+ *
+ * Copyright (C) 2016 Peng Fan <van.freenix@gmail.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ *
+ */
+
+/*
+ * Note:
+ * The driver's compatible table is borrowed from Linux Kernel,
+ * but now max supported gpio pins is 24 and only PCA953X_TYPE
+ * is supported. PCA957X_TYPE is not supported now.
+ * Also the Polarity Inversion feature is not supported now.
+ *
+ * TODO:
+ * 1. Support PCA957X_TYPE
+ * 2. Support max 40 gpio pins
+ * 3. Support Plolarity Inversion
+ */
+
+#include <common.h>
+#include <errno.h>
+#include <dm.h>
+#include <fdtdec.h>
+#include <i2c.h>
+#include <malloc.h>
+#include <asm/gpio.h>
+#include <asm/io.h>
+#include <dt-bindings/gpio/gpio.h>
+
+#define PCA953X_INPUT           0
+#define PCA953X_OUTPUT          1
+#define PCA953X_INVERT          2
+#define PCA953X_DIRECTION       3
+
+#define PCA_GPIO_MASK           0x00FF
+#define PCA_INT                 0x0100
+#define PCA953X_TYPE            0x1000
+#define PCA957X_TYPE            0x2000
+#define PCA_TYPE_MASK           0xF000
+#define PCA_CHIP_TYPE(x)        ((x) & PCA_TYPE_MASK)
+
+enum {
+	PCA953X_DIRECTION_IN,
+	PCA953X_DIRECTION_OUT,
+};
+
+#define MAX_BANK 3
+#define BANK_SZ 8
+
+/*
+ * struct pca953x_info - Data for pca953x
+ *
+ * @dev: udevice structure for the device
+ * @addr: i2c slave address
+ * @invert: Polarity inversion or not
+ * @gpio_count: the number of gpio pins that the device supports
+ * @chip_type: indicate the chip type,PCA953X or PCA957X
+ * @bank_count: the number of banks that the device supports
+ * @reg_output: array to hold the value of output registers
+ * @reg_direction: array to hold the value of direction registers
+ */
+struct pca953x_info {
+	struct udevice *dev;
+	unsigned int addr;
+	unsigned int invert;
+	unsigned int gpio_count;
+	unsigned int chip_type;
+	unsigned int bank_count;
+	u8 reg_output[MAX_BANK];
+	u8 reg_direction[MAX_BANK];
+};
+
+static int pca953x_write_single(struct udevice *dev, int reg, u8 val,
+				int offset)
+{
+	struct pca953x_info *info = dev_get_platdata(dev);
+	int bank_shift = fls((info->gpio_count - 1) / BANK_SZ);
+	int off = offset / BANK_SZ;
+	int ret = 0;
+
+	ret = dm_i2c_write(dev, (reg << bank_shift) + off, &val, 1);
+	if (ret) {
+		dev_err(dev, "%s error\n", __func__);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int pca953x_read_single(struct udevice *dev, int reg, u8 *val,
+			       int offset)
+{
+	struct pca953x_info *info = dev_get_platdata(dev);
+	int bank_shift = fls((info->gpio_count - 1) / BANK_SZ);
+	int off = offset / BANK_SZ;
+	int ret;
+	u8 byte;
+
+	ret = dm_i2c_read(dev, (reg << bank_shift) + off, &byte, 1);
+	if (ret) {
+		dev_err(dev, "%s error\n", __func__);
+		return ret;
+	}
+
+	*val = byte;
+
+	return 0;
+}
+
+static int pca953x_read_regs(struct udevice *dev, int reg, u8 *val)
+{
+	struct pca953x_info *info = dev_get_platdata(dev);
+	int ret = 0;
+
+	if (info->gpio_count <= 8) {
+		ret = dm_i2c_read(dev, reg, val, 1);
+	} else if (info->gpio_count <= 16) {
+		ret = dm_i2c_read(dev, reg << 1, val, info->bank_count);
+	} else {
+		dev_err(dev, "Unsupported now\n");
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int pca953x_is_output(struct udevice *dev, int offset)
+{
+	struct pca953x_info *info = dev_get_platdata(dev);
+
+	int bank = offset / BANK_SZ;
+	int off = offset % BANK_SZ;
+
+	/*0: output; 1: input */
+	return !(info->reg_direction[bank] & (1 << off));
+}
+
+static int pca953x_get_value(struct udevice *dev, unsigned offset)
+{
+	int ret;
+	u8 val = 0;
+
+	ret = pca953x_read_single(dev, PCA953X_INPUT, &val, offset);
+	if (ret)
+		return ret;
+
+	return (val >> offset) & 0x1;
+}
+
+static int pca953x_set_value(struct udevice *dev, unsigned offset,
+			     int value)
+{
+	struct pca953x_info *info = dev_get_platdata(dev);
+	int bank = offset / BANK_SZ;
+	int off = offset % BANK_SZ;
+	u8 val;
+	int ret;
+
+	if (value)
+		val = info->reg_output[bank] | (1 << off);
+	else
+		val = info->reg_output[bank] & ~(1 << off);
+
+	ret = pca953x_write_single(dev, PCA953X_OUTPUT, val, offset);
+	if (ret)
+		return ret;
+
+	info->reg_output[bank] = val;
+
+	return 0;
+}
+
+static int pca953x_set_direction(struct udevice *dev, unsigned offset, int dir)
+{
+	struct pca953x_info *info = dev_get_platdata(dev);
+	int bank = offset / BANK_SZ;
+	int off = offset % BANK_SZ;
+	u8 val;
+	int ret;
+
+	if (dir == PCA953X_DIRECTION_IN)
+		val = info->reg_direction[bank] | (1 << off);
+	else
+		val = info->reg_direction[bank] & ~(1 << off);
+
+	ret = pca953x_write_single(dev, PCA953X_DIRECTION, val, offset);
+	if (ret)
+		return ret;
+
+	info->reg_direction[bank] = val;
+
+	return 0;
+}
+
+static int pca953x_direction_input(struct udevice *dev, unsigned offset)
+{
+	return pca953x_set_direction(dev, offset, PCA953X_DIRECTION_IN);
+}
+
+static int pca953x_direction_output(struct udevice *dev, unsigned offset,
+				    int value)
+{
+	/* Configure output value. */
+	pca953x_set_value(dev, offset, value);
+
+	/* Configure direction as output. */
+	pca953x_set_direction(dev, offset, PCA953X_DIRECTION_OUT);
+
+	return 0;
+}
+
+static int pca953x_get_function(struct udevice *dev, unsigned offset)
+{
+	if (pca953x_is_output(dev, offset))
+		return GPIOF_OUTPUT;
+	else
+		return GPIOF_INPUT;
+}
+
+static int pca953x_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 pca953x_ops = {
+	.direction_input	= pca953x_direction_input,
+	.direction_output	= pca953x_direction_output,
+	.get_value		= pca953x_get_value,
+	.set_value		= pca953x_set_value,
+	.get_function		= pca953x_get_function,
+	.xlate			= pca953x_xlate,
+};
+
+static int pca953x_probe(struct udevice *dev)
+{
+	struct pca953x_info *info = dev_get_platdata(dev);
+	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
+	char name[32], *str;
+	fdt_addr_t addr;
+	ulong driver_data;
+	int ret;
+
+	if (!info) {
+		dev_err(dev, "platdata not ready\n");
+		return -ENOMEM;
+	}
+
+	if (!chip) {
+		dev_err(dev, "i2c not ready\n");
+		return -ENODEV;
+	}
+
+	addr = dev_get_addr(dev);
+	if (addr == FDT_ADDR_T_NONE)
+		return -ENODEV;
+
+	info->addr = (unsigned int)addr;
+
+	driver_data = dev_get_driver_data(dev);
+
+	info->gpio_count = driver_data & PCA_GPIO_MASK;
+	if (info->gpio_count > MAX_BANK * BANK_SZ) {
+		dev_err(dev, "Max support %d pins now\n", MAX_BANK * BANK_SZ);
+		return -EINVAL;
+	}
+
+	info->chip_type = PCA_CHIP_TYPE(driver_data);
+	if (info->chip_type != PCA953X_TYPE) {
+		dev_err(dev, "Only support PCA953X chip type now.\n");
+		return -EINVAL;
+	}
+
+	info->bank_count = DIV_ROUND_UP(info->gpio_count, BANK_SZ);
+
+	ret = pca953x_read_regs(dev, PCA953X_OUTPUT, info->reg_output);
+	if (ret) {
+		dev_err(dev, "Error reading output register\n");
+		return ret;
+	}
+
+	ret = pca953x_read_regs(dev, PCA953X_DIRECTION, info->reg_direction);
+	if (ret) {
+		dev_err(dev, "Error reading direction register\n");
+		return ret;
+	}
+
+	snprintf(name, sizeof(name), "gpio@%u", info->addr);
+	str = strdup(name);
+	if (!str)
+		return -ENOMEM;
+	uc_priv->bank_name = str;
+	uc_priv->gpio_count = info->gpio_count;
+
+	dev_dbg(dev, "%s is ready\n", str);
+
+	return 0;
+}
+
+#define OF_953X(__nrgpio, __int) (ulong)(__nrgpio | PCA953X_TYPE | __int)
+#define OF_957X(__nrgpio, __int) (ulong)(__nrgpio | PCA957X_TYPE | __int)
+
+static const struct udevice_id pca953x_ids[] = {
+	{ .compatible = "nxp,pca9505", .data = OF_953X(40, PCA_INT), },
+	{ .compatible = "nxp,pca9534", .data = OF_953X(8, PCA_INT), },
+	{ .compatible = "nxp,pca9535", .data = OF_953X(16, PCA_INT), },
+	{ .compatible = "nxp,pca9536", .data = OF_953X(4, 0), },
+	{ .compatible = "nxp,pca9537", .data = OF_953X(4, PCA_INT), },
+	{ .compatible = "nxp,pca9538", .data = OF_953X(8, PCA_INT), },
+	{ .compatible = "nxp,pca9539", .data = OF_953X(16, PCA_INT), },
+	{ .compatible = "nxp,pca9554", .data = OF_953X(8, PCA_INT), },
+	{ .compatible = "nxp,pca9555", .data = OF_953X(16, PCA_INT), },
+	{ .compatible = "nxp,pca9556", .data = OF_953X(8, 0), },
+	{ .compatible = "nxp,pca9557", .data = OF_953X(8, 0), },
+	{ .compatible = "nxp,pca9574", .data = OF_957X(8, PCA_INT), },
+	{ .compatible = "nxp,pca9575", .data = OF_957X(16, PCA_INT), },
+	{ .compatible = "nxp,pca9698", .data = OF_953X(40, 0), },
+
+	{ .compatible = "maxim,max7310", .data = OF_953X(8, 0), },
+	{ .compatible = "maxim,max7312", .data = OF_953X(16, PCA_INT), },
+	{ .compatible = "maxim,max7313", .data = OF_953X(16, PCA_INT), },
+	{ .compatible = "maxim,max7315", .data = OF_953X(8, PCA_INT), },
+
+	{ .compatible = "ti,pca6107", .data = OF_953X(8, PCA_INT), },
+	{ .compatible = "ti,tca6408", .data = OF_953X(8, PCA_INT), },
+	{ .compatible = "ti,tca6416", .data = OF_953X(16, PCA_INT), },
+	{ .compatible = "ti,tca6424", .data = OF_953X(24, PCA_INT), },
+
+	{ .compatible = "onsemi,pca9654", .data = OF_953X(8, PCA_INT), },
+
+	{ .compatible = "exar,xra1202", .data = OF_953X(8, 0), },
+	{ }
+};
+
+U_BOOT_DRIVER(pca953x) = {
+	.name		= "pca953x",
+	.id		= UCLASS_GPIO,
+	.ops		= &pca953x_ops,
+	.probe		= pca953x_probe,
+	.platdata_auto_alloc_size = sizeof(struct pca953x_info),
+	.of_match	= pca953x_ids,
+};
-- 
2.6.2

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Simon Glass]

On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
> Introduce a new driver that supports driver model for pca953x.
> The pca953x chips are used as I2C I/O expanders.
> This driver is designed to support the following chips:
> "
> 4 bits: pca9536, pca9537
> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>         pca9556, pca9557, pca9574, tca6408, xra1202
> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>          tca6416
> 24 bits: tca6424
> 40 bits: pca9505, pca9698
> "
> But for now this driver only supports max 24 bits and pca953x compatible
> chips. pca957x compatible chips are not supported now.
> These can be addressed when we need to add such support for the different
> chips.
> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
> i2c expander using gpio command as following:
>
> =>gpio status -a
> Bank gpio at 48:
> gpio at 480: input: 1 [ ]
> => gpio clear gpio at 480
> gpio: pin gpio at 480 (gpio 224) value is 0
> => gpio status -a
> Bank gpio at 48:
> gpio at 480: output: 0 [ ]

Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
the bank name? Also I think you should support a gpio-bank-name
property in the node, to allow a sensible name to be provided.

>
> => dm tree:
>  i2c         [   ]    |   |   `-- i2c at 021a8000
>  gpio        [   ]    |   |       |-- gpio at 30
>  gpio        [   ]    |   |       `-- gpio at 32
>
> Signed-off-by: Peng Fan <van.freenix@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Wenyou Yang <wenyou.yang@atmel.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> Cc: Purna Chandra Mandal <purna.mandal@microchip.com>
> Cc: Thomas Chou <thomas@wytron.com.tw>
> Cc: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> Cc: Andrea Scian <andrea.scian@dave.eu>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  drivers/gpio/Kconfig        |  23 +++
>  drivers/gpio/Makefile       |   2 +
>  drivers/gpio/pca953x_gpio.c | 349 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 374 insertions(+)
>  create mode 100644 drivers/gpio/pca953x_gpio.c

One nit - there is no need to check for chip being NULL as it cannot be.

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

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Peng Fan]

On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>> Introduce a new driver that supports driver model for pca953x.
>> The pca953x chips are used as I2C I/O expanders.
>> This driver is designed to support the following chips:
>> "
>> 4 bits: pca9536, pca9537
>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>         pca9556, pca9557, pca9574, tca6408, xra1202
>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>          tca6416
>> 24 bits: tca6424
>> 40 bits: pca9505, pca9698
>> "
>> But for now this driver only supports max 24 bits and pca953x compatible
>> chips. pca957x compatible chips are not supported now.
>> These can be addressed when we need to add such support for the different
>> chips.
>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>> i2c expander using gpio command as following:
>>
>> =>gpio status -a
>> Bank gpio at 48:
>> gpio at 480: input: 1 [ ]
>> => gpio clear gpio at 480
>> gpio: pin gpio at 480 (gpio 224) value is 0
>> => gpio status -a
>> Bank gpio at 48:
>> gpio at 480: output: 0 [ ]
>
>Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>the bank name? Also I think you should support a gpio-bank-name
>property in the node, to allow a sensible name to be provided.

480 is added by gpio uclass driver I think.
The dts is copied from Linux side. I'd not change the dts, will try to
see how to introudce a sensible name here.

Thanks,
Peng.

>
>>
>> => dm tree:
>>  i2c         [   ]    |   |   `-- i2c at 021a8000
>>  gpio        [   ]    |   |       |-- gpio at 30
>>  gpio        [   ]    |   |       `-- gpio at 32
>>
>> Signed-off-by: Peng Fan <van.freenix@gmail.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Cc: Wenyou Yang <wenyou.yang@atmel.com>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>> Cc: Purna Chandra Mandal <purna.mandal@microchip.com>
>> Cc: Thomas Chou <thomas@wytron.com.tw>
>> Cc: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
>> Cc: Andrea Scian <andrea.scian@dave.eu>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> ---
>>  drivers/gpio/Kconfig        |  23 +++
>>  drivers/gpio/Makefile       |   2 +
>>  drivers/gpio/pca953x_gpio.c | 349 ++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 374 insertions(+)
>>  create mode 100644 drivers/gpio/pca953x_gpio.c
>
>One nit - there is no need to check for chip being NULL as it cannot be.
>
>Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Michal Simek]

On 11.4.2016 07:47, Peng Fan wrote:
> On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>> On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>>> Introduce a new driver that supports driver model for pca953x.
>>> The pca953x chips are used as I2C I/O expanders.
>>> This driver is designed to support the following chips:
>>> "
>>> 4 bits: pca9536, pca9537
>>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>>         pca9556, pca9557, pca9574, tca6408, xra1202
>>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>>          tca6416
>>> 24 bits: tca6424
>>> 40 bits: pca9505, pca9698
>>> "
>>> But for now this driver only supports max 24 bits and pca953x compatible
>>> chips. pca957x compatible chips are not supported now.
>>> These can be addressed when we need to add such support for the different
>>> chips.
>>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>>> i2c expander using gpio command as following:
>>>
>>> =>gpio status -a
>>> Bank gpio at 48:
>>> gpio at 480: input: 1 [ ]
>>> => gpio clear gpio at 480
>>> gpio: pin gpio at 480 (gpio 224) value is 0
>>> => gpio status -a
>>> Bank gpio at 48:
>>> gpio at 480: output: 0 [ ]
>>
>> Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>> the bank name? Also I think you should support a gpio-bank-name
>> property in the node, to allow a sensible name to be provided.
> 
> 480 is added by gpio uclass driver I think.
> The dts is copied from Linux side. I'd not change the dts, will try to
> see how to introudce a sensible name here.

What's the binding you are using?

The part of this patch should be DT binding.

This is my node.
	tca6416_u61: gpio at 21 {
		compatible = "ti,tca6416";
		reg = <0x21>;
		gpio-controller;
		#gpio-cells = <2>;
	};


Thanks,
Michal


ZynqMP> i2c bus
Bus 0:	i2c at ff020000
   20: gpio at 20, offset len 1, flags 0
   21: gpio at 21, offset len 1, flags 0
   75: i2cswitch at 75, offset len 1, flags 0
Bus 750:	i2c at 0
Bus 751:	i2c at 1
Bus 752:	i2c at 2
Bus 1:	i2c at ff030000
   74: i2cswitch at 74, offset len 1, flags 0
   75: i2cswitch at 75, offset len 1, flags 0
Bus 750:	i2c at 0
Bus 751:	i2c at 1
Bus 752:	i2c at 2
Bus 1743:	i2c at 3
Bus 1744:	i2c at 4
Bus 750:	i2c at 0
Bus 751:	i2c at 1
Bus 752:	i2c at 2
Bus 1743:	i2c at 3
Bus 1744:	i2c at 4
Bus 1755:	i2c at 5
Bus 1756:	i2c at 6
Bus 1757:	i2c at 7
ZynqMP> gpio status -a
__of_translate_address: Bad cell count for gpio at 20
Command 'gpio' failed: Error -19
ZynqMP> dm tree
 Class       Probed   Name
----------------------------------------
 root        [ + ]    root_driver
 simple_bus  [   ]    |-- amba_apu
 simple_bus  [ + ]    `-- amba
 eth         [ + ]        |-- ethernet at ff0e0000
 i2c         [ + ]        |-- i2c at ff020000
 gpio        [   ]        |   |-- gpio at 20
 gpio        [   ]        |   |-- gpio at 21
 i2c_mux     [   ]        |   `-- i2cswitch at 75
 i2c         [   ]        |       |-- i2c at 0
 i2c         [   ]        |       |-- i2c at 1
 i2c         [   ]        |       `-- i2c at 2
 i2c         [   ]        |-- i2c at ff030000
 i2c_mux     [   ]        |   |-- i2cswitch at 74
 i2c         [   ]        |   |   |-- i2c at 0
 i2c         [   ]        |   |   |-- i2c at 1
 i2c         [   ]        |   |   |-- i2c at 2
 i2c         [   ]        |   |   |-- i2c at 3
 i2c         [   ]        |   |   `-- i2c at 4
 i2c_mux     [   ]        |   `-- i2cswitch at 75
 i2c         [   ]        |       |-- i2c at 0
 i2c         [   ]        |       |-- i2c at 1
 i2c         [   ]        |       |-- i2c at 2
 i2c         [   ]        |       |-- i2c at 3
 i2c         [   ]        |       |-- i2c at 4
 i2c         [   ]        |       |-- i2c at 5
 i2c         [   ]        |       |-- i2c at 6
 i2c         [   ]        |       `-- i2c at 7
 mmc         [ + ]        |-- sdhci at ff170000
 serial      [ + ]        |-- serial at ff000000
 serial      [   ]        `-- serial at ff010000

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Michal Simek]

On 11.4.2016 14:09, Michal Simek wrote:
> On 11.4.2016 07:47, Peng Fan wrote:
>> On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>>> On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>>>> Introduce a new driver that supports driver model for pca953x.
>>>> The pca953x chips are used as I2C I/O expanders.
>>>> This driver is designed to support the following chips:
>>>> "
>>>> 4 bits: pca9536, pca9537
>>>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>>>         pca9556, pca9557, pca9574, tca6408, xra1202
>>>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>>>          tca6416
>>>> 24 bits: tca6424
>>>> 40 bits: pca9505, pca9698
>>>> "
>>>> But for now this driver only supports max 24 bits and pca953x compatible
>>>> chips. pca957x compatible chips are not supported now.
>>>> These can be addressed when we need to add such support for the different
>>>> chips.
>>>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>>>> i2c expander using gpio command as following:
>>>>
>>>> =>gpio status -a
>>>> Bank gpio at 48:
>>>> gpio at 480: input: 1 [ ]
>>>> => gpio clear gpio at 480
>>>> gpio: pin gpio at 480 (gpio 224) value is 0
>>>> => gpio status -a
>>>> Bank gpio at 48:
>>>> gpio at 480: output: 0 [ ]
>>>
>>> Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>>> the bank name? Also I think you should support a gpio-bank-name
>>> property in the node, to allow a sensible name to be provided.
>>
>> 480 is added by gpio uclass driver I think.
>> The dts is copied from Linux side. I'd not change the dts, will try to
>> see how to introudce a sensible name here.
> 
> What's the binding you are using?
> 
> The part of this patch should be DT binding.
> 
> This is my node.
> 	tca6416_u61: gpio at 21 {
> 		compatible = "ti,tca6416";
> 		reg = <0x21>;
> 		gpio-controller;
> 		#gpio-cells = <2>;
> 	};
> 

Ok. I found where the problem is.

i2c bus has
#address-cells = <1>;
#size-cells = <0>;

And then OF_CHECK_COUNTS thinks that it is incorrect setting for bus
but it should be valid for i2c where only address without size is used.
When I apply patch below driver is probed


diff --git a/common/fdt_support.c b/common/fdt_support.c
index ced119e70d9f..5f5b49c6210b 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -941,7 +941,7 @@ void fdt_del_node_and_alias(void *blob, const char
*alias)
 #define OF_MAX_ADDR_CELLS      4
 #define OF_BAD_ADDR    FDT_ADDR_T_NONE
 #define OF_CHECK_COUNTS(na, ns)        ((na) > 0 && (na) <=
OF_MAX_ADDR_CELLS && \
-                       (ns) > 0)
+                       (ns) >= 0)

 /* Debug utility */
 #ifdef DEBUG

Regarding numbers. It is just hex to int conversion + gpio number.
It means gpio at 20 is 0x20 which is 32. Using hex everywhere make sense
and this is the fix. Maybe better to also use underscore.

diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
index 6b67b079a17b..1eeb4c8f199a 100644
--- a/drivers/gpio/pca953x_gpio.c
+++ b/drivers/gpio/pca953x_gpio.c
@@ -292,7 +292,7 @@ static int pca953x_probe(struct udevice *dev)
                return ret;
        }

-       snprintf(name, sizeof(name), "gpio@%u", info->addr);
+       snprintf(name, sizeof(name), "gpio@%x_", info->addr);
        str = strdup(name);
        if (!str)
                return -ENOMEM;

Output below.

Thanks,
Michal

ZynqMP> dm tree
 Class       Probed   Name
----------------------------------------
 root        [ + ]    root_driver
 simple_bus  [   ]    |-- amba_apu
 simple_bus  [ + ]    `-- amba
 eth         [ + ]        |-- ethernet at ff0e0000
 i2c         [ + ]        |-- i2c at ff020000
 gpio        [ + ]        |   |-- gpio at 20
 gpio        [ + ]        |   |-- gpio at 21
 i2c_mux     [   ]        |   `-- i2cswitch at 75
 i2c         [   ]        |       |-- i2c at 0
 i2c         [   ]        |       |-- i2c at 1
 i2c         [   ]        |       `-- i2c at 2
 i2c         [   ]        |-- i2c at ff030000
 i2c_mux     [   ]        |   |-- i2cswitch at 74
 i2c         [   ]        |   |   |-- i2c at 0
 i2c         [   ]        |   |   |-- i2c at 1
 i2c         [   ]        |   |   |-- i2c at 2
 i2c         [   ]        |   |   |-- i2c at 3
 i2c         [   ]        |   |   `-- i2c at 4
 i2c_mux     [   ]        |   `-- i2cswitch at 75
 i2c         [   ]        |       |-- i2c at 0
 i2c         [   ]        |       |-- i2c at 1
 i2c         [   ]        |       |-- i2c at 2
 i2c         [   ]        |       |-- i2c at 3
 i2c         [   ]        |       |-- i2c at 4
 i2c         [   ]        |       |-- i2c at 5
 i2c         [   ]        |       |-- i2c at 6
 i2c         [   ]        |       `-- i2c at 7
 mmc         [ + ]        |-- sdhci at ff170000
 serial      [ + ]        |-- serial at ff000000
 serial      [   ]        `-- serial at ff010000
ZynqMP> i2c bus
Bus 0:	i2c at ff020000  (active 0)
   20: gpio at 20, offset len 1, flags 0
   21: gpio at 21, offset len 1, flags 0
   75: i2cswitch at 75, offset len 1, flags 0
Bus 750:	i2c at 0
Bus 751:	i2c at 1
Bus 752:	i2c at 2
Bus 1:	i2c at ff030000
   74: i2cswitch at 74, offset len 1, flags 0
   75: i2cswitch at 75, offset len 1, flags 0
Bus 750:	i2c at 0
Bus 751:	i2c at 1
Bus 752:	i2c at 2
Bus 1743:	i2c at 3
Bus 1744:	i2c at 4
Bus 750:	i2c at 0
Bus 751:	i2c at 1
Bus 752:	i2c at 2
Bus 1743:	i2c at 3
Bus 1744:	i2c at 4
Bus 1755:	i2c at 5
Bus 1756:	i2c at 6
Bus 1757:	i2c at 7
ZynqMP> gpio status -a
Bank gpio at 20_:
gpio at 20_0: output: 1 [ ]
gpio at 20_1: output: 1 [ ]
gpio at 20_2: output: 1 [ ]
gpio at 20_3: output: 1 [ ]
gpio at 20_4: output: 0 [ ]
gpio at 20_5: output: 1 [ ]
gpio at 20_6: output: 1 [ ]
gpio at 20_7: output: 1 [ ]
gpio at 20_8: output: 0 [ ]
gpio at 20_9: output: 0 [ ]
gpio at 20_10: output: 0 [ ]
gpio at 20_11: output: 0 [ ]
gpio at 20_12: output: 0 [ ]
gpio at 20_13: output: 0 [ ]
gpio at 20_14: output: 0 [ ]
gpio at 20_15: output: 0 [ ]

Bank gpio at 21_:
gpio at 21_0: input: 1 [ ]
gpio at 21_1: input: 1 [ ]
gpio at 21_2: input: 1 [ ]
gpio at 21_3: input: 1 [ ]
gpio at 21_4: input: 1 [ ]
gpio at 21_5: input: 0 [ ]
gpio at 21_6: input: 0 [ ]
gpio at 21_7: input: 0 [ ]
gpio at 21_8: input: 0 [ ]
gpio at 21_9: input: 0 [ ]
gpio at 21_10: input: 0 [ ]
gpio at 21_11: input: 0 [ ]
gpio at 21_12: input: 0 [ ]
gpio at 21_13: input: 0 [ ]
gpio at 21_14: input: 0 [ ]
gpio at 21_15: input: 0 [ ]
ZynqMP>

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Peng Fan]

Hi Michal,
On Mon, Apr 11, 2016 at 02:09:22PM +0200, Michal Simek wrote:
>On 11.4.2016 07:47, Peng Fan wrote:
>> On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>>> On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>>>> Introduce a new driver that supports driver model for pca953x.
>>>> The pca953x chips are used as I2C I/O expanders.
>>>> This driver is designed to support the following chips:
>>>> "
>>>> 4 bits: pca9536, pca9537
>>>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>>>         pca9556, pca9557, pca9574, tca6408, xra1202
>>>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>>>          tca6416
>>>> 24 bits: tca6424
>>>> 40 bits: pca9505, pca9698
>>>> "
>>>> But for now this driver only supports max 24 bits and pca953x compatible
>>>> chips. pca957x compatible chips are not supported now.
>>>> These can be addressed when we need to add such support for the different
>>>> chips.
>>>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>>>> i2c expander using gpio command as following:
>>>>
>>>> =>gpio status -a
>>>> Bank gpio at 48:
>>>> gpio at 480: input: 1 [ ]
>>>> => gpio clear gpio at 480
>>>> gpio: pin gpio at 480 (gpio 224) value is 0
>>>> => gpio status -a
>>>> Bank gpio at 48:
>>>> gpio at 480: output: 0 [ ]
>>>
>>> Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>>> the bank name? Also I think you should support a gpio-bank-name
>>> property in the node, to allow a sensible name to be provided.
>> 
>> 480 is added by gpio uclass driver I think.
>> The dts is copied from Linux side. I'd not change the dts, will try to
>> see how to introudce a sensible name here.
>
>What's the binding you are using?

I use 
"
	max7310_a: gpio at 30 {
		compatible = "maxim,max7310";
		reg = <0x30>;
		gpio-controller;
		#gpio-cells = <2>;
		resets = <&max7310_reset>;
	};

	max7310_b: gpio at 32 {
		compatible = "maxim,max7310";
		reg = <0x32>;
		gpio-controller;
		#gpio-cells = <2>;
		resets = <&max7310_reset>;
	};
"

Regards,
Peng.
>
>The part of this patch should be DT binding.
>
>This is my node.
>	tca6416_u61: gpio at 21 {
>		compatible = "ti,tca6416";
>		reg = <0x21>;
>		gpio-controller;
>		#gpio-cells = <2>;
>	};
>
>
>Thanks,
>Michal
>
>
>ZynqMP> i2c bus
>Bus 0:	i2c at ff020000
>   20: gpio at 20, offset len 1, flags 0
>   21: gpio at 21, offset len 1, flags 0
>   75: i2cswitch at 75, offset len 1, flags 0
>Bus 750:	i2c at 0
>Bus 751:	i2c at 1
>Bus 752:	i2c at 2
>Bus 1:	i2c at ff030000
>   74: i2cswitch at 74, offset len 1, flags 0
>   75: i2cswitch at 75, offset len 1, flags 0
>Bus 750:	i2c at 0
>Bus 751:	i2c at 1
>Bus 752:	i2c at 2
>Bus 1743:	i2c at 3
>Bus 1744:	i2c at 4
>Bus 750:	i2c at 0
>Bus 751:	i2c at 1
>Bus 752:	i2c at 2
>Bus 1743:	i2c at 3
>Bus 1744:	i2c at 4
>Bus 1755:	i2c at 5
>Bus 1756:	i2c at 6
>Bus 1757:	i2c at 7
>ZynqMP> gpio status -a
>__of_translate_address: Bad cell count for gpio at 20
>Command 'gpio' failed: Error -19
>ZynqMP> dm tree
> Class       Probed   Name
>----------------------------------------
> root        [ + ]    root_driver
> simple_bus  [   ]    |-- amba_apu
> simple_bus  [ + ]    `-- amba
> eth         [ + ]        |-- ethernet at ff0e0000
> i2c         [ + ]        |-- i2c at ff020000
> gpio        [   ]        |   |-- gpio at 20
> gpio        [   ]        |   |-- gpio at 21
> i2c_mux     [   ]        |   `-- i2cswitch at 75
> i2c         [   ]        |       |-- i2c at 0
> i2c         [   ]        |       |-- i2c at 1
> i2c         [   ]        |       `-- i2c at 2
> i2c         [   ]        |-- i2c at ff030000
> i2c_mux     [   ]        |   |-- i2cswitch at 74
> i2c         [   ]        |   |   |-- i2c at 0
> i2c         [   ]        |   |   |-- i2c at 1
> i2c         [   ]        |   |   |-- i2c at 2
> i2c         [   ]        |   |   |-- i2c at 3
> i2c         [   ]        |   |   `-- i2c at 4
> i2c_mux     [   ]        |   `-- i2cswitch at 75
> i2c         [   ]        |       |-- i2c at 0
> i2c         [   ]        |       |-- i2c at 1
> i2c         [   ]        |       |-- i2c at 2
> i2c         [   ]        |       |-- i2c at 3
> i2c         [   ]        |       |-- i2c at 4
> i2c         [   ]        |       |-- i2c at 5
> i2c         [   ]        |       |-- i2c at 6
> i2c         [   ]        |       `-- i2c at 7
> mmc         [ + ]        |-- sdhci at ff170000
> serial      [ + ]        |-- serial at ff000000
> serial      [   ]        `-- serial at ff010000
>

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Peng Fan]

Hi Michal,

On Mon, Apr 11, 2016 at 02:40:06PM +0200, Michal Simek wrote:
>On 11.4.2016 14:09, Michal Simek wrote:
>> On 11.4.2016 07:47, Peng Fan wrote:
>>> On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>>>> On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>>>>> Introduce a new driver that supports driver model for pca953x.
>>>>> The pca953x chips are used as I2C I/O expanders.
>>>>> This driver is designed to support the following chips:
>>>>> "
>>>>> 4 bits: pca9536, pca9537
>>>>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>>>>         pca9556, pca9557, pca9574, tca6408, xra1202
>>>>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>>>>          tca6416
>>>>> 24 bits: tca6424
>>>>> 40 bits: pca9505, pca9698
>>>>> "
>>>>> But for now this driver only supports max 24 bits and pca953x compatible
>>>>> chips. pca957x compatible chips are not supported now.
>>>>> These can be addressed when we need to add such support for the different
>>>>> chips.
>>>>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>>>>> i2c expander using gpio command as following:
>>>>>
>>>>> =>gpio status -a
>>>>> Bank gpio at 48:
>>>>> gpio at 480: input: 1 [ ]
>>>>> => gpio clear gpio at 480
>>>>> gpio: pin gpio at 480 (gpio 224) value is 0
>>>>> => gpio status -a
>>>>> Bank gpio at 48:
>>>>> gpio at 480: output: 0 [ ]
>>>>
>>>> Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>>>> the bank name? Also I think you should support a gpio-bank-name
>>>> property in the node, to allow a sensible name to be provided.
>>>
>>> 480 is added by gpio uclass driver I think.
>>> The dts is copied from Linux side. I'd not change the dts, will try to
>>> see how to introudce a sensible name here.
>> 
>> What's the binding you are using?
>> 
>> The part of this patch should be DT binding.
>> 
>> This is my node.
>> 	tca6416_u61: gpio at 21 {
>> 		compatible = "ti,tca6416";
>> 		reg = <0x21>;
>> 		gpio-controller;
>> 		#gpio-cells = <2>;
>> 	};
>> 
>
>Ok. I found where the problem is.
>
>i2c bus has
>#address-cells = <1>;
>#size-cells = <0>;
>
>And then OF_CHECK_COUNTS thinks that it is incorrect setting for bus
>but it should be valid for i2c where only address without size is used.
>When I apply patch below driver is probed

I did not met this issue.

>
>
>diff --git a/common/fdt_support.c b/common/fdt_support.c
>index ced119e70d9f..5f5b49c6210b 100644
>--- a/common/fdt_support.c
>+++ b/common/fdt_support.c
>@@ -941,7 +941,7 @@ void fdt_del_node_and_alias(void *blob, const char
>*alias)
> #define OF_MAX_ADDR_CELLS      4
> #define OF_BAD_ADDR    FDT_ADDR_T_NONE
> #define OF_CHECK_COUNTS(na, ns)        ((na) > 0 && (na) <=
>OF_MAX_ADDR_CELLS && \
>-                       (ns) > 0)
>+                       (ns) >= 0)
>
> /* Debug utility */
> #ifdef DEBUG
>
>Regarding numbers. It is just hex to int conversion + gpio number.
>It means gpio at 20 is 0x20 which is 32. Using hex everywhere make sense
>and this is the fix. Maybe better to also use underscore.
>
>diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
>index 6b67b079a17b..1eeb4c8f199a 100644
>--- a/drivers/gpio/pca953x_gpio.c
>+++ b/drivers/gpio/pca953x_gpio.c
>@@ -292,7 +292,7 @@ static int pca953x_probe(struct udevice *dev)
>                return ret;
>        }
>
>-       snprintf(name, sizeof(name), "gpio@%u", info->addr);
>+       snprintf(name, sizeof(name), "gpio@%x_", info->addr);
>        str = strdup(name);
>        if (!str)
>                return -ENOMEM;

Thanks for this.
Also Would you mind I add your Tested-By in V2?

Thanks,
Peng.

>
>Output below.
>
>Thanks,
>Michal
>
>ZynqMP> dm tree
> Class       Probed   Name
>----------------------------------------
> root        [ + ]    root_driver
> simple_bus  [   ]    |-- amba_apu
> simple_bus  [ + ]    `-- amba
> eth         [ + ]        |-- ethernet at ff0e0000
> i2c         [ + ]        |-- i2c at ff020000
> gpio        [ + ]        |   |-- gpio at 20
> gpio        [ + ]        |   |-- gpio at 21
> i2c_mux     [   ]        |   `-- i2cswitch at 75
> i2c         [   ]        |       |-- i2c at 0
> i2c         [   ]        |       |-- i2c at 1
> i2c         [   ]        |       `-- i2c at 2
> i2c         [   ]        |-- i2c at ff030000
> i2c_mux     [   ]        |   |-- i2cswitch at 74
> i2c         [   ]        |   |   |-- i2c at 0
> i2c         [   ]        |   |   |-- i2c at 1
> i2c         [   ]        |   |   |-- i2c at 2
> i2c         [   ]        |   |   |-- i2c at 3
> i2c         [   ]        |   |   `-- i2c at 4
> i2c_mux     [   ]        |   `-- i2cswitch at 75
> i2c         [   ]        |       |-- i2c at 0
> i2c         [   ]        |       |-- i2c at 1
> i2c         [   ]        |       |-- i2c at 2
> i2c         [   ]        |       |-- i2c at 3
> i2c         [   ]        |       |-- i2c at 4
> i2c         [   ]        |       |-- i2c at 5
> i2c         [   ]        |       |-- i2c at 6
> i2c         [   ]        |       `-- i2c at 7
> mmc         [ + ]        |-- sdhci at ff170000
> serial      [ + ]        |-- serial at ff000000
> serial      [   ]        `-- serial at ff010000
>ZynqMP> i2c bus
>Bus 0:	i2c at ff020000  (active 0)
>   20: gpio at 20, offset len 1, flags 0
>   21: gpio at 21, offset len 1, flags 0
>   75: i2cswitch at 75, offset len 1, flags 0
>Bus 750:	i2c at 0
>Bus 751:	i2c at 1
>Bus 752:	i2c at 2
>Bus 1:	i2c at ff030000
>   74: i2cswitch at 74, offset len 1, flags 0
>   75: i2cswitch at 75, offset len 1, flags 0
>Bus 750:	i2c at 0
>Bus 751:	i2c at 1
>Bus 752:	i2c at 2
>Bus 1743:	i2c at 3
>Bus 1744:	i2c at 4
>Bus 750:	i2c at 0
>Bus 751:	i2c at 1
>Bus 752:	i2c at 2
>Bus 1743:	i2c at 3
>Bus 1744:	i2c at 4
>Bus 1755:	i2c at 5
>Bus 1756:	i2c at 6
>Bus 1757:	i2c at 7
>ZynqMP> gpio status -a
>Bank gpio at 20_:
>gpio at 20_0: output: 1 [ ]
>gpio at 20_1: output: 1 [ ]
>gpio at 20_2: output: 1 [ ]
>gpio at 20_3: output: 1 [ ]
>gpio at 20_4: output: 0 [ ]
>gpio at 20_5: output: 1 [ ]
>gpio at 20_6: output: 1 [ ]
>gpio at 20_7: output: 1 [ ]
>gpio at 20_8: output: 0 [ ]
>gpio at 20_9: output: 0 [ ]
>gpio at 20_10: output: 0 [ ]
>gpio at 20_11: output: 0 [ ]
>gpio at 20_12: output: 0 [ ]
>gpio at 20_13: output: 0 [ ]
>gpio at 20_14: output: 0 [ ]
>gpio at 20_15: output: 0 [ ]
>
>Bank gpio at 21_:
>gpio at 21_0: input: 1 [ ]
>gpio at 21_1: input: 1 [ ]
>gpio at 21_2: input: 1 [ ]
>gpio at 21_3: input: 1 [ ]
>gpio at 21_4: input: 1 [ ]
>gpio at 21_5: input: 0 [ ]
>gpio at 21_6: input: 0 [ ]
>gpio at 21_7: input: 0 [ ]
>gpio at 21_8: input: 0 [ ]
>gpio at 21_9: input: 0 [ ]
>gpio at 21_10: input: 0 [ ]
>gpio at 21_11: input: 0 [ ]
>gpio at 21_12: input: 0 [ ]
>gpio at 21_13: input: 0 [ ]
>gpio at 21_14: input: 0 [ ]
>gpio at 21_15: input: 0 [ ]
>ZynqMP>
>

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Michal Simek]

On 12.4.2016 03:22, Peng Fan wrote:
> Hi Michal,
> On Mon, Apr 11, 2016 at 02:09:22PM +0200, Michal Simek wrote:
>> On 11.4.2016 07:47, Peng Fan wrote:
>>> On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>>>> On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>>>>> Introduce a new driver that supports driver model for pca953x.
>>>>> The pca953x chips are used as I2C I/O expanders.
>>>>> This driver is designed to support the following chips:
>>>>> "
>>>>> 4 bits: pca9536, pca9537
>>>>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>>>>         pca9556, pca9557, pca9574, tca6408, xra1202
>>>>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>>>>          tca6416
>>>>> 24 bits: tca6424
>>>>> 40 bits: pca9505, pca9698
>>>>> "
>>>>> But for now this driver only supports max 24 bits and pca953x compatible
>>>>> chips. pca957x compatible chips are not supported now.
>>>>> These can be addressed when we need to add such support for the different
>>>>> chips.
>>>>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>>>>> i2c expander using gpio command as following:
>>>>>
>>>>> =>gpio status -a
>>>>> Bank gpio at 48:
>>>>> gpio at 480: input: 1 [ ]
>>>>> => gpio clear gpio at 480
>>>>> gpio: pin gpio at 480 (gpio 224) value is 0
>>>>> => gpio status -a
>>>>> Bank gpio at 48:
>>>>> gpio at 480: output: 0 [ ]
>>>>
>>>> Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>>>> the bank name? Also I think you should support a gpio-bank-name
>>>> property in the node, to allow a sensible name to be provided.
>>>
>>> 480 is added by gpio uclass driver I think.
>>> The dts is copied from Linux side. I'd not change the dts, will try to
>>> see how to introudce a sensible name here.
>>
>> What's the binding you are using?
> 
> I use 
> "
> 	max7310_a: gpio at 30 {
> 		compatible = "maxim,max7310";
> 		reg = <0x30>;
> 		gpio-controller;
> 		#gpio-cells = <2>;
> 		resets = <&max7310_reset>;
> 	};
> 
> 	max7310_b: gpio at 32 {
> 		compatible = "maxim,max7310";
> 		reg = <0x32>;
> 		gpio-controller;
> 		#gpio-cells = <2>;
> 		resets = <&max7310_reset>;
> 	};
> "
> 

What's this board? I can't see this in u-boot repo.
imx6q from Linux repo?

Thanks,
Michal

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Michal Simek]

On 12.4.2016 03:25, Peng Fan wrote:
> Hi Michal,
> 
> On Mon, Apr 11, 2016 at 02:40:06PM +0200, Michal Simek wrote:
>> On 11.4.2016 14:09, Michal Simek wrote:
>>> On 11.4.2016 07:47, Peng Fan wrote:
>>>> On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>>>>> On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>>>>>> Introduce a new driver that supports driver model for pca953x.
>>>>>> The pca953x chips are used as I2C I/O expanders.
>>>>>> This driver is designed to support the following chips:
>>>>>> "
>>>>>> 4 bits: pca9536, pca9537
>>>>>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>>>>>         pca9556, pca9557, pca9574, tca6408, xra1202
>>>>>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>>>>>          tca6416
>>>>>> 24 bits: tca6424
>>>>>> 40 bits: pca9505, pca9698
>>>>>> "
>>>>>> But for now this driver only supports max 24 bits and pca953x compatible
>>>>>> chips. pca957x compatible chips are not supported now.
>>>>>> These can be addressed when we need to add such support for the different
>>>>>> chips.
>>>>>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>>>>>> i2c expander using gpio command as following:
>>>>>>
>>>>>> =>gpio status -a
>>>>>> Bank gpio at 48:
>>>>>> gpio at 480: input: 1 [ ]
>>>>>> => gpio clear gpio at 480
>>>>>> gpio: pin gpio at 480 (gpio 224) value is 0
>>>>>> => gpio status -a
>>>>>> Bank gpio at 48:
>>>>>> gpio at 480: output: 0 [ ]
>>>>>
>>>>> Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>>>>> the bank name? Also I think you should support a gpio-bank-name
>>>>> property in the node, to allow a sensible name to be provided.
>>>>
>>>> 480 is added by gpio uclass driver I think.
>>>> The dts is copied from Linux side. I'd not change the dts, will try to
>>>> see how to introudce a sensible name here.
>>>
>>> What's the binding you are using?
>>>
>>> The part of this patch should be DT binding.
>>>
>>> This is my node.
>>> 	tca6416_u61: gpio at 21 {
>>> 		compatible = "ti,tca6416";
>>> 		reg = <0x21>;
>>> 		gpio-controller;
>>> 		#gpio-cells = <2>;
>>> 	};
>>>
>>
>> Ok. I found where the problem is.
>>
>> i2c bus has
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> And then OF_CHECK_COUNTS thinks that it is incorrect setting for bus
>> but it should be valid for i2c where only address without size is used.
>> When I apply patch below driver is probed
> 
> I did not met this issue.

That's interesting. Can you please add

Can you please apply this and look at na and ns values?
If you are on imx6q then you should reach the same problem as I if this
code is called that's why I would like to confirm this.

diff --git a/common/fdt_support.c b/common/fdt_support.c
index ced119e70d9f..9c18b312d647 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -1109,6 +1109,7 @@ static u64 __of_translate_address(void *blob, int
node_offset, const fdt32_t *in

        /* Cound address cells & copy address locally */
        bus->count_cells(blob, parent, &na, &ns);
+       printf("addr cells %d, size cells %d\n", na, ns);
        if (!OF_CHECK_COUNTS(na, ns)) {
                printf("%s: Bad cell count for %s\n", __FUNCTION__,
                       fdt_get_name(blob, node_offset, NULL));


> 
>>
>>
>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>> index ced119e70d9f..5f5b49c6210b 100644
>> --- a/common/fdt_support.c
>> +++ b/common/fdt_support.c
>> @@ -941,7 +941,7 @@ void fdt_del_node_and_alias(void *blob, const char
>> *alias)
>> #define OF_MAX_ADDR_CELLS      4
>> #define OF_BAD_ADDR    FDT_ADDR_T_NONE
>> #define OF_CHECK_COUNTS(na, ns)        ((na) > 0 && (na) <=
>> OF_MAX_ADDR_CELLS && \
>> -                       (ns) > 0)
>> +                       (ns) >= 0)
>>
>> /* Debug utility */
>> #ifdef DEBUG
>>
>> Regarding numbers. It is just hex to int conversion + gpio number.
>> It means gpio at 20 is 0x20 which is 32. Using hex everywhere make sense
>> and this is the fix. Maybe better to also use underscore.
>>
>> diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
>> index 6b67b079a17b..1eeb4c8f199a 100644
>> --- a/drivers/gpio/pca953x_gpio.c
>> +++ b/drivers/gpio/pca953x_gpio.c
>> @@ -292,7 +292,7 @@ static int pca953x_probe(struct udevice *dev)
>>                return ret;
>>        }
>>
>> -       snprintf(name, sizeof(name), "gpio@%u", info->addr);
>> +       snprintf(name, sizeof(name), "gpio@%x_", info->addr);
>>        str = strdup(name);
>>        if (!str)
>>                return -ENOMEM;
> 
> Thanks for this.
> Also Would you mind I add your Tested-By in V2?

I will when this is fixed and we know more about that issue above.

Thanks,
Michal

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Peng Fan]

Hi Michal,
On Tue, Apr 12, 2016 at 07:17:55AM +0200, Michal Simek wrote:
>On 12.4.2016 03:25, Peng Fan wrote:
>> Hi Michal,
>> 
>> On Mon, Apr 11, 2016 at 02:40:06PM +0200, Michal Simek wrote:
>>> On 11.4.2016 14:09, Michal Simek wrote:
>>>> On 11.4.2016 07:47, Peng Fan wrote:
>>>>> On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>>>>>> On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>>>>>>> Introduce a new driver that supports driver model for pca953x.
>>>>>>> The pca953x chips are used as I2C I/O expanders.
>>>>>>> This driver is designed to support the following chips:
>>>>>>> "
>>>>>>> 4 bits: pca9536, pca9537
>>>>>>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>>>>>>         pca9556, pca9557, pca9574, tca6408, xra1202
>>>>>>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>>>>>>          tca6416
>>>>>>> 24 bits: tca6424
>>>>>>> 40 bits: pca9505, pca9698
>>>>>>> "
>>>>>>> But for now this driver only supports max 24 bits and pca953x compatible
>>>>>>> chips. pca957x compatible chips are not supported now.
>>>>>>> These can be addressed when we need to add such support for the different
>>>>>>> chips.
>>>>>>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>>>>>>> i2c expander using gpio command as following:
>>>>>>>
>>>>>>> =>gpio status -a
>>>>>>> Bank gpio at 48:
>>>>>>> gpio at 480: input: 1 [ ]
>>>>>>> => gpio clear gpio at 480
>>>>>>> gpio: pin gpio at 480 (gpio 224) value is 0
>>>>>>> => gpio status -a
>>>>>>> Bank gpio at 48:
>>>>>>> gpio at 480: output: 0 [ ]
>>>>>>
>>>>>> Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>>>>>> the bank name? Also I think you should support a gpio-bank-name
>>>>>> property in the node, to allow a sensible name to be provided.
>>>>>
>>>>> 480 is added by gpio uclass driver I think.
>>>>> The dts is copied from Linux side. I'd not change the dts, will try to
>>>>> see how to introudce a sensible name here.
>>>>
>>>> What's the binding you are using?
>>>>
>>>> The part of this patch should be DT binding.
>>>>
>>>> This is my node.
>>>> 	tca6416_u61: gpio at 21 {
>>>> 		compatible = "ti,tca6416";
>>>> 		reg = <0x21>;
>>>> 		gpio-controller;
>>>> 		#gpio-cells = <2>;
>>>> 	};
>>>>
>>>
>>> Ok. I found where the problem is.
>>>
>>> i2c bus has
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> And then OF_CHECK_COUNTS thinks that it is incorrect setting for bus
>>> but it should be valid for i2c where only address without size is used.
>>> When I apply patch below driver is probed
>> 
>> I did not met this issue.
>
>That's interesting. Can you please add
>
>Can you please apply this and look at na and ns values?
>If you are on imx6q then you should reach the same problem as I if this
>code is called that's why I would like to confirm this.
>
>diff --git a/common/fdt_support.c b/common/fdt_support.c
>index ced119e70d9f..9c18b312d647 100644
>--- a/common/fdt_support.c
>+++ b/common/fdt_support.c
>@@ -1109,6 +1109,7 @@ static u64 __of_translate_address(void *blob, int
>node_offset, const fdt32_t *in
>
>        /* Cound address cells & copy address locally */
>        bus->count_cells(blob, parent, &na, &ns);
>+       printf("addr cells %d, size cells %d\n", na, ns);
>        if (!OF_CHECK_COUNTS(na, ns)) {
>                printf("%s: Bad cell count for %s\n", __FUNCTION__,
>                       fdt_get_name(blob, node_offset, NULL));
>
>
>> 
>>>
>>>
>>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>>> index ced119e70d9f..5f5b49c6210b 100644
>>> --- a/common/fdt_support.c
>>> +++ b/common/fdt_support.c
>>> @@ -941,7 +941,7 @@ void fdt_del_node_and_alias(void *blob, const char
>>> *alias)
>>> #define OF_MAX_ADDR_CELLS      4
>>> #define OF_BAD_ADDR    FDT_ADDR_T_NONE
>>> #define OF_CHECK_COUNTS(na, ns)        ((na) > 0 && (na) <=
>>> OF_MAX_ADDR_CELLS && \
>>> -                       (ns) > 0)
>>> +                       (ns) >= 0)

You are correct.

I have no idea why I first test my patch, I did not met the issue as you.
After applying your patch as above, gpio status -a can correctly detect
max7310.

The dts I used is for mx6sxsabreauto board, not in U-Boot now.

Thanks,
Peng.

>>>
>>> /* Debug utility */
>>> #ifdef DEBUG
>>>
>>> Regarding numbers. It is just hex to int conversion + gpio number.
>>> It means gpio at 20 is 0x20 which is 32. Using hex everywhere make sense
>>> and this is the fix. Maybe better to also use underscore.
>>>
>>> diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
>>> index 6b67b079a17b..1eeb4c8f199a 100644
>>> --- a/drivers/gpio/pca953x_gpio.c
>>> +++ b/drivers/gpio/pca953x_gpio.c
>>> @@ -292,7 +292,7 @@ static int pca953x_probe(struct udevice *dev)
>>>                return ret;
>>>        }
>>>
>>> -       snprintf(name, sizeof(name), "gpio@%u", info->addr);
>>> +       snprintf(name, sizeof(name), "gpio@%x_", info->addr);
>>>        str = strdup(name);
>>>        if (!str)
>>>                return -ENOMEM;
>> 
>> Thanks for this.
>> Also Would you mind I add your Tested-By in V2?
>
>I will when this is fixed and we know more about that issue above.
>
>Thanks,
>Michal
>
>

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Michal Simek]

On 13.4.2016 07:50, Peng Fan wrote:
> Hi Michal,
> On Tue, Apr 12, 2016 at 07:17:55AM +0200, Michal Simek wrote:
>> On 12.4.2016 03:25, Peng Fan wrote:
>>> Hi Michal,
>>>
>>> On Mon, Apr 11, 2016 at 02:40:06PM +0200, Michal Simek wrote:
>>>> On 11.4.2016 14:09, Michal Simek wrote:
>>>>> On 11.4.2016 07:47, Peng Fan wrote:
>>>>>> On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>>>>>>> On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>>>>>>>> Introduce a new driver that supports driver model for pca953x.
>>>>>>>> The pca953x chips are used as I2C I/O expanders.
>>>>>>>> This driver is designed to support the following chips:
>>>>>>>> "
>>>>>>>> 4 bits: pca9536, pca9537
>>>>>>>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>>>>>>>         pca9556, pca9557, pca9574, tca6408, xra1202
>>>>>>>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>>>>>>>          tca6416
>>>>>>>> 24 bits: tca6424
>>>>>>>> 40 bits: pca9505, pca9698
>>>>>>>> "
>>>>>>>> But for now this driver only supports max 24 bits and pca953x compatible
>>>>>>>> chips. pca957x compatible chips are not supported now.
>>>>>>>> These can be addressed when we need to add such support for the different
>>>>>>>> chips.
>>>>>>>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>>>>>>>> i2c expander using gpio command as following:
>>>>>>>>
>>>>>>>> =>gpio status -a
>>>>>>>> Bank gpio at 48:
>>>>>>>> gpio at 480: input: 1 [ ]
>>>>>>>> => gpio clear gpio at 480
>>>>>>>> gpio: pin gpio at 480 (gpio 224) value is 0
>>>>>>>> => gpio status -a
>>>>>>>> Bank gpio at 48:
>>>>>>>> gpio at 480: output: 0 [ ]
>>>>>>>
>>>>>>> Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>>>>>>> the bank name? Also I think you should support a gpio-bank-name
>>>>>>> property in the node, to allow a sensible name to be provided.
>>>>>>
>>>>>> 480 is added by gpio uclass driver I think.
>>>>>> The dts is copied from Linux side. I'd not change the dts, will try to
>>>>>> see how to introudce a sensible name here.
>>>>>
>>>>> What's the binding you are using?
>>>>>
>>>>> The part of this patch should be DT binding.
>>>>>
>>>>> This is my node.
>>>>> 	tca6416_u61: gpio at 21 {
>>>>> 		compatible = "ti,tca6416";
>>>>> 		reg = <0x21>;
>>>>> 		gpio-controller;
>>>>> 		#gpio-cells = <2>;
>>>>> 	};
>>>>>
>>>>
>>>> Ok. I found where the problem is.
>>>>
>>>> i2c bus has
>>>> #address-cells = <1>;
>>>> #size-cells = <0>;
>>>>
>>>> And then OF_CHECK_COUNTS thinks that it is incorrect setting for bus
>>>> but it should be valid for i2c where only address without size is used.
>>>> When I apply patch below driver is probed
>>>
>>> I did not met this issue.
>>
>> That's interesting. Can you please add
>>
>> Can you please apply this and look at na and ns values?
>> If you are on imx6q then you should reach the same problem as I if this
>> code is called that's why I would like to confirm this.
>>
>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>> index ced119e70d9f..9c18b312d647 100644
>> --- a/common/fdt_support.c
>> +++ b/common/fdt_support.c
>> @@ -1109,6 +1109,7 @@ static u64 __of_translate_address(void *blob, int
>> node_offset, const fdt32_t *in
>>
>>        /* Cound address cells & copy address locally */
>>        bus->count_cells(blob, parent, &na, &ns);
>> +       printf("addr cells %d, size cells %d\n", na, ns);
>>        if (!OF_CHECK_COUNTS(na, ns)) {
>>                printf("%s: Bad cell count for %s\n", __FUNCTION__,
>>                       fdt_get_name(blob, node_offset, NULL));
>>
>>
>>>
>>>>
>>>>
>>>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>>>> index ced119e70d9f..5f5b49c6210b 100644
>>>> --- a/common/fdt_support.c
>>>> +++ b/common/fdt_support.c
>>>> @@ -941,7 +941,7 @@ void fdt_del_node_and_alias(void *blob, const char
>>>> *alias)
>>>> #define OF_MAX_ADDR_CELLS      4
>>>> #define OF_BAD_ADDR    FDT_ADDR_T_NONE
>>>> #define OF_CHECK_COUNTS(na, ns)        ((na) > 0 && (na) <=
>>>> OF_MAX_ADDR_CELLS && \
>>>> -                       (ns) > 0)
>>>> +                       (ns) >= 0)
> 
> You are correct.
> 
> I have no idea why I first test my patch, I did not met the issue as you.
> After applying your patch as above, gpio status -a can correctly detect
> max7310.
> 
> The dts I used is for mx6sxsabreauto board, not in U-Boot now.

good. Do you want me to create this patch or do you want to create it
and add it as 1/2 before this one?

Thanks,
Michal

[U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x

[Michal Simek]

On 13.4.2016 08:04, Michal Simek wrote:
> On 13.4.2016 07:50, Peng Fan wrote:
>> Hi Michal,
>> On Tue, Apr 12, 2016 at 07:17:55AM +0200, Michal Simek wrote:
>>> On 12.4.2016 03:25, Peng Fan wrote:
>>>> Hi Michal,
>>>>
>>>> On Mon, Apr 11, 2016 at 02:40:06PM +0200, Michal Simek wrote:
>>>>> On 11.4.2016 14:09, Michal Simek wrote:
>>>>>> On 11.4.2016 07:47, Peng Fan wrote:
>>>>>>> On Sat, Apr 09, 2016 at 12:33:34PM -0600, Simon Glass wrote:
>>>>>>>> On 18 March 2016 at 03:54, Peng Fan <van.freenix@gmail.com> wrote:
>>>>>>>>> Introduce a new driver that supports driver model for pca953x.
>>>>>>>>> The pca953x chips are used as I2C I/O expanders.
>>>>>>>>> This driver is designed to support the following chips:
>>>>>>>>> "
>>>>>>>>> 4 bits: pca9536, pca9537
>>>>>>>>> 8 bits: max7310, max7315, pca6107, pca9534, pca9538, pca9554,
>>>>>>>>>         pca9556, pca9557, pca9574, tca6408, xra1202
>>>>>>>>> 16 bits: max7312, max7313, pca9535, pca9539, pca9555, pca9575,
>>>>>>>>>          tca6416
>>>>>>>>> 24 bits: tca6424
>>>>>>>>> 40 bits: pca9505, pca9698
>>>>>>>>> "
>>>>>>>>> But for now this driver only supports max 24 bits and pca953x compatible
>>>>>>>>> chips. pca957x compatible chips are not supported now.
>>>>>>>>> These can be addressed when we need to add such support for the different
>>>>>>>>> chips.
>>>>>>>>> This driver has been tested on i.MX6 SoloX Sabreauto board with max7310
>>>>>>>>> i2c expander using gpio command as following:
>>>>>>>>>
>>>>>>>>> =>gpio status -a
>>>>>>>>> Bank gpio at 48:
>>>>>>>>> gpio at 480: input: 1 [ ]
>>>>>>>>> => gpio clear gpio at 480
>>>>>>>>> gpio: pin gpio at 480 (gpio 224) value is 0
>>>>>>>>> => gpio status -a
>>>>>>>>> Bank gpio at 48:
>>>>>>>>> gpio at 480: output: 0 [ ]
>>>>>>>>
>>>>>>>> Don't you think 480 is confusing? Perhaps you should have gpio at 48_ as
>>>>>>>> the bank name? Also I think you should support a gpio-bank-name
>>>>>>>> property in the node, to allow a sensible name to be provided.
>>>>>>>
>>>>>>> 480 is added by gpio uclass driver I think.
>>>>>>> The dts is copied from Linux side. I'd not change the dts, will try to
>>>>>>> see how to introudce a sensible name here.
>>>>>>
>>>>>> What's the binding you are using?
>>>>>>
>>>>>> The part of this patch should be DT binding.
>>>>>>
>>>>>> This is my node.
>>>>>> 	tca6416_u61: gpio at 21 {
>>>>>> 		compatible = "ti,tca6416";
>>>>>> 		reg = <0x21>;
>>>>>> 		gpio-controller;
>>>>>> 		#gpio-cells = <2>;
>>>>>> 	};
>>>>>>
>>>>>
>>>>> Ok. I found where the problem is.
>>>>>
>>>>> i2c bus has
>>>>> #address-cells = <1>;
>>>>> #size-cells = <0>;
>>>>>
>>>>> And then OF_CHECK_COUNTS thinks that it is incorrect setting for bus
>>>>> but it should be valid for i2c where only address without size is used.
>>>>> When I apply patch below driver is probed
>>>>
>>>> I did not met this issue.
>>>
>>> That's interesting. Can you please add
>>>
>>> Can you please apply this and look at na and ns values?
>>> If you are on imx6q then you should reach the same problem as I if this
>>> code is called that's why I would like to confirm this.
>>>
>>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>>> index ced119e70d9f..9c18b312d647 100644
>>> --- a/common/fdt_support.c
>>> +++ b/common/fdt_support.c
>>> @@ -1109,6 +1109,7 @@ static u64 __of_translate_address(void *blob, int
>>> node_offset, const fdt32_t *in
>>>
>>>        /* Cound address cells & copy address locally */
>>>        bus->count_cells(blob, parent, &na, &ns);
>>> +       printf("addr cells %d, size cells %d\n", na, ns);
>>>        if (!OF_CHECK_COUNTS(na, ns)) {
>>>                printf("%s: Bad cell count for %s\n", __FUNCTION__,
>>>                       fdt_get_name(blob, node_offset, NULL));
>>>
>>>
>>>>
>>>>>
>>>>>
>>>>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>>>>> index ced119e70d9f..5f5b49c6210b 100644
>>>>> --- a/common/fdt_support.c
>>>>> +++ b/common/fdt_support.c
>>>>> @@ -941,7 +941,7 @@ void fdt_del_node_and_alias(void *blob, const char
>>>>> *alias)
>>>>> #define OF_MAX_ADDR_CELLS      4
>>>>> #define OF_BAD_ADDR    FDT_ADDR_T_NONE
>>>>> #define OF_CHECK_COUNTS(na, ns)        ((na) > 0 && (na) <=
>>>>> OF_MAX_ADDR_CELLS && \
>>>>> -                       (ns) > 0)
>>>>> +                       (ns) >= 0)
>>
>> You are correct.
>>
>> I have no idea why I first test my patch, I did not met the issue as you.
>> After applying your patch as above, gpio status -a can correctly detect
>> max7310.
>>
>> The dts I used is for mx6sxsabreauto board, not in U-Boot now.
> 
> good. Do you want me to create this patch or do you want to create it
> and add it as 1/2 before this one?
> 

I have sent a patch for it and you are in CC.

Thanks,
Michal