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@48: gpio@480: input: 1 [ ] => gpio clear gpio@480 gpio: pin gpio@480 (gpio 224) value is 0 => gpio status -a Bank gpio@48: gpio@480: output: 0 [ ]
Don't you think 480 is confusing? Perhaps you should have gpio@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@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@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@ff0e0000 i2c [ + ] |-- i2c@ff020000 gpio [ + ] | |-- gpio@20 gpio [ + ] | |-- gpio@21 i2c_mux [ ] | `-- i2cswitch@75 i2c [ ] | |-- i2c@0 i2c [ ] | |-- i2c@1 i2c [ ] | `-- i2c@2 i2c [ ] |-- i2c@ff030000 i2c_mux [ ] | |-- i2cswitch@74 i2c [ ] | | |-- i2c@0 i2c [ ] | | |-- i2c@1 i2c [ ] | | |-- i2c@2 i2c [ ] | | |-- i2c@3 i2c [ ] | | `-- i2c@4 i2c_mux [ ] | `-- i2cswitch@75 i2c [ ] | |-- i2c@0 i2c [ ] | |-- i2c@1 i2c [ ] | |-- i2c@2 i2c [ ] | |-- i2c@3 i2c [ ] | |-- i2c@4 i2c [ ] | |-- i2c@5 i2c [ ] | |-- i2c@6 i2c [ ] | `-- i2c@7 mmc [ + ] |-- sdhci@ff170000 serial [ + ] |-- serial@ff000000 serial [ ] `-- serial@ff010000 ZynqMP> i2c bus Bus 0: i2c@ff020000 (active 0) 20: gpio@20, offset len 1, flags 0 21: gpio@21, offset len 1, flags 0 75: i2cswitch@75, offset len 1, flags 0 Bus 750: i2c@0 Bus 751: i2c@1 Bus 752: i2c@2 Bus 1: i2c@ff030000 74: i2cswitch@74, offset len 1, flags 0 75: i2cswitch@75, offset len 1, flags 0 Bus 750: i2c@0 Bus 751: i2c@1 Bus 752: i2c@2 Bus 1743: i2c@3 Bus 1744: i2c@4 Bus 750: i2c@0 Bus 751: i2c@1 Bus 752: i2c@2 Bus 1743: i2c@3 Bus 1744: i2c@4 Bus 1755: i2c@5 Bus 1756: i2c@6 Bus 1757: i2c@7 ZynqMP> gpio status -a Bank gpio@20_: gpio@20_0: output: 1 [ ] gpio@20_1: output: 1 [ ] gpio@20_2: output: 1 [ ] gpio@20_3: output: 1 [ ] gpio@20_4: output: 0 [ ] gpio@20_5: output: 1 [ ] gpio@20_6: output: 1 [ ] gpio@20_7: output: 1 [ ] gpio@20_8: output: 0 [ ] gpio@20_9: output: 0 [ ] gpio@20_10: output: 0 [ ] gpio@20_11: output: 0 [ ] gpio@20_12: output: 0 [ ] gpio@20_13: output: 0 [ ] gpio@20_14: output: 0 [ ] gpio@20_15: output: 0 [ ]
Bank gpio@21_: gpio@21_0: input: 1 [ ] gpio@21_1: input: 1 [ ] gpio@21_2: input: 1 [ ] gpio@21_3: input: 1 [ ] gpio@21_4: input: 1 [ ] gpio@21_5: input: 0 [ ] gpio@21_6: input: 0 [ ] gpio@21_7: input: 0 [ ] gpio@21_8: input: 0 [ ] gpio@21_9: input: 0 [ ] gpio@21_10: input: 0 [ ] gpio@21_11: input: 0 [ ] gpio@21_12: input: 0 [ ] gpio@21_13: input: 0 [ ] gpio@21_14: input: 0 [ ] gpio@21_15: input: 0 [ ] ZynqMP>