From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Mon, 11 Apr 2016 14:40:06 +0200 Subject: [U-Boot] [PATCH] dm: gpio: pca953x: introduce driver model support for pca953x In-Reply-To: <570B93F2.2020204@xilinx.com> References: <1458294896-19795-1-git-send-email-van.freenix@gmail.com> <20160411054754.GE8379@linux-7smt.suse> <570B93F2.2020204@xilinx.com> Message-ID: <570B9B26.9080106@xilinx.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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>