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