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 <[email protected]> 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
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@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@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> > _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
