dm: gpio: pca953x: introduce driver model support for pca953x

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 <[email protected]>

Cc Simon Glass <[email protected]>

Cc Masahiro Yamada <[email protected]>

Cc Wenyou Yang <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Purna Chandra Mandal <[email protected]>

Cc Thomas Chou <[email protected]>

Cc Bhuvanchandra DV <[email protected]>

Cc Andrea Scian <[email protected]>

Cc Michal Simek <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Fabio Estevam <[email protected]>

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

Quoted 19 lines · 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:

Quoted 8 lines

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

Quoted 19 lines

=> dm tree:

 i2c         [   ]    |   |   `-- i2c at 021a8000
 gpio        [   ]    |   |       |-- gpio at 30
 gpio        [   ]    |   |       `-- gpio at 32

Signed-off-by Peng Fan <[email protected]>

Cc Simon Glass <[email protected]>

Cc Masahiro Yamada <[email protected]>

Cc Wenyou Yang <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Purna Chandra Mandal <[email protected]>

Cc Thomas Chou <[email protected]>

Cc Bhuvanchandra DV <[email protected]>

Cc Andrea Scian <[email protected]>

Cc Michal Simek <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Fabio Estevam <[email protected]>

Quoted 6 lines

---
 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 <[email protected]>

Quoted 29 lines · Simon Glass

Quoted 27 lines · 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 [ ]

Quoted 3 lines

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.

Quoted 30 lines · Peng Fan

Quoted 29 lines · Simon Glass

Quoted 27 lines · 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 [ ]

Quoted 7 lines

Quoted 3 lines

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

Quoted 43 lines · Michal Simek

Quoted 30 lines · Peng Fan

Quoted 29 lines · Simon Glass

Quoted 27 lines · 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 [ ]

Quoted 7 lines

Quoted 3 lines

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.

Quoted 8 lines

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

common/fdt_support.c +1 -1

index ced119e70d9f..5f5b49c6210b 100644
@@ -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.

drivers/gpio/pca953x_gpio.c +0 -0

index 6b67b079a17b..1eeb4c8f199a 100644

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>

Hi Michal,

Quoted 59 lines · Michal Simek

Quoted 43 lines · Michal Simek

Quoted 30 lines · Peng Fan

Quoted 29 lines · Simon Glass

Quoted 27 lines · 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 [ ]

Quoted 7 lines

Quoted 3 lines

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.

Quoted 8 lines

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

Quoted 3 lines

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.

Quoted 31 lines

common/fdt_support.c +1 -1

index ced119e70d9f..5f5b49c6210b 100644
@@ -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.

drivers/gpio/pca953x_gpio.c +0 -0

index 6b67b079a17b..1eeb4c8f199a 100644
@@ -292,7 +292,7 @@ static int pca953x_probe(struct udevice *dev)
               return ret;
       }
 

drivers/gpio/pca953x_gpio.c static int pca953x_probe(struct udevice *dev) · line 298

Quoted 5 lines

drivers/gpio/pca953x_gpio.c +1 -1

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

Quoted 62 lines · Peng Fan

Hi Michal,

Quoted 59 lines · Michal Simek

Quoted 43 lines · Michal Simek

Quoted 30 lines · Peng Fan

Quoted 29 lines · Simon Glass

Quoted 27 lines · 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 [ ]

Quoted 7 lines

Quoted 3 lines

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.

Quoted 8 lines

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

Quoted 5 lines

Quoted 3 lines

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.

common/fdt_support.c +0 -0

index ced119e70d9f..9c18b312d647 100644
@@ -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));

Quoted 3 lines

Quoted 3 lines

Quoted 4 lines

Quoted 4 lines

common/fdt_support.c +0 -0

index ced119e70d9f..5f5b49c6210b 100644

Quoted 8 lines

common/fdt_support.c void fdt_del_node_and_alias(void *blob, const char · line 946

Quoted 8 lines

common/fdt_support.c +1 -1

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

Quoted 16 lines

Quoted 16 lines

common/fdt_support.c +0 -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.

drivers/gpio/pca953x_gpio.c +0 -0

index 6b67b079a17b..1eeb4c8f199a 100644
@@ -292,7 +292,7 @@ static int pca953x_probe(struct udevice *dev)
               return ret;
       }
 

Quoted 8 lines

drivers/gpio/pca953x_gpio.c static int pca953x_probe(struct udevice *dev) · line 298

Quoted 5 lines

drivers/gpio/pca953x_gpio.c +1 -1

-       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

Hi Michal,

Quoted 93 lines · Michal Simek

Quoted 62 lines · Peng Fan

Hi Michal,

Quoted 59 lines · Michal Simek

Quoted 43 lines · Michal Simek

Quoted 30 lines · Peng Fan

Quoted 29 lines · Simon Glass

Quoted 27 lines · 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 [ ]

Quoted 7 lines

Quoted 3 lines

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.

Quoted 8 lines

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

Quoted 5 lines

Quoted 3 lines

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.

common/fdt_support.c +0 -0

index ced119e70d9f..9c18b312d647 100644
@@ -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));

Quoted 3 lines

Quoted 3 lines

Quoted 12 lines

Quoted 4 lines

Quoted 4 lines

common/fdt_support.c +0 -0

index ced119e70d9f..5f5b49c6210b 100644

Quoted 8 lines

common/fdt_support.c void fdt_del_node_and_alias(void *blob, const char · line 946

Quoted 8 lines

common/fdt_support.c +1 -1

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

Quoted 110 lines · Peng Fan

Hi Michal,

Quoted 93 lines · Michal Simek

Quoted 62 lines · Peng Fan

Hi Michal,

Quoted 59 lines · Michal Simek

Quoted 43 lines · Michal Simek

Quoted 30 lines · Peng Fan

Quoted 29 lines · Simon Glass

Quoted 27 lines · 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 [ ]

Quoted 7 lines

Quoted 3 lines

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.

Quoted 8 lines

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

Quoted 5 lines

Quoted 3 lines

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.

common/fdt_support.c +0 -0

index ced119e70d9f..9c18b312d647 100644
@@ -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));

Quoted 3 lines

Quoted 3 lines

Quoted 12 lines

Quoted 4 lines

Quoted 4 lines

common/fdt_support.c +0 -0

index ced119e70d9f..5f5b49c6210b 100644

Quoted 8 lines

common/fdt_support.c void fdt_del_node_and_alias(void *blob, const char · line 946

Quoted 8 lines

common/fdt_support.c +1 -1

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

Quoted 5 lines

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?

Quoted 34 lines · Peng Fan

Quoted 32 lines · Simon Glass

Quoted 19 lines · 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:

Quoted 8 lines

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

Quoted 3 lines

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

Hi Michal,

Quoted 35 lines · Michal Simek

Quoted 34 lines · Peng Fan

Quoted 32 lines · Simon Glass

Quoted 19 lines · 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:

Quoted 8 lines

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

Quoted 5 lines

Quoted 3 lines

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

"