| Message ID | dbb9c24189974a6fc157bf1a38cdc82d48be77a5.1460614826.git.michal.simek@xilinx.com |
|---|---|
| State | Rejected |
| Delegated to: | Michal Simek |
| Headers | show |
Hi Michal, On Thu, Apr 14, 2016 at 08:20:28AM +0200, Michal Simek wrote: >Upcomming pca953x gpio driver (tca6416 is compatible with it) >is using for example this DT description (ZynqMP zcu102): >i2c0: i2c@ff020000 { > compatible = "cdns,i2c-r1p10"; > interrupt-parent = <&gic>; > interrupts = <0 17 4>; > reg = <0x0 0xff020000 0x1000>; > #address-cells = <1>; > #size-cells = <0>; > > tca6416_u61: gpio@21 { > compatible = "ti,tca6416"; > reg = <0x21>; > gpio-controller; > #gpio-cells = <2>; > }; >}; > >I2C bus is using #size-cells = <0> which current code detects as >"Bad cell count": >OF: reached root node >OF: ** translation for device gpio@20 ** >__of_translate_address: Bad cell count for gpio@20 >Command 'gpio' failed: Error -19 > >This patch fixes OF_CHECK_COUNTS macro not to report problem when >size-cells is 0 which is also valid bus configuration. > >Signed-off-by: Michal Simek <michal.simek@xilinx.com> Reviewed-by: Peng Fan <van.freenix@gmail.com> Regards, Peng. >--- > > common/fdt_support.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >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 >-- >1.9.1 >
Hello Michal, +CC Stephen Warren On 04/14/2016 08:20 AM, Michal Simek wrote: > Upcomming pca953x gpio driver (tca6416 is compatible with it) > is using for example this DT description (ZynqMP zcu102): > i2c0: i2c@ff020000 { > compatible = "cdns,i2c-r1p10"; > interrupt-parent = <&gic>; > interrupts = <0 17 4>; > reg = <0x0 0xff020000 0x1000>; > #address-cells = <1>; > #size-cells = <0>; > > tca6416_u61: gpio@21 { > compatible = "ti,tca6416"; > reg = <0x21>; > gpio-controller; > #gpio-cells = <2>; > }; > }; > > I2C bus is using #size-cells = <0> which current code detects as > "Bad cell count": > OF: reached root node > OF: ** translation for device gpio@20 ** > __of_translate_address: Bad cell count for gpio@20 > Command 'gpio' failed: Error -19 > > This patch fixes OF_CHECK_COUNTS macro not to report problem when > size-cells is 0 which is also valid bus configuration. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > common/fdt_support.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > 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 > Some time ago I tried to solve GPIO issue on Exynos boards, by proposing change similar to your patch. After some discussion with Stephen Warren and Simon Glass, I was aware, that it was a wrong approach to solve this problem. You can follow it here: http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/247651 https://patchwork.ozlabs.org/patch/564246/ https://patchwork.ozlabs.org/patch/557008/ https://patchwork.ozlabs.org/patch/557010/ https://patchwork.ozlabs.org/patch/557009/ As a conclusion, for MMIO addresses we are using 'reg' property with a range. If you need use this as I2C address or I2C device's register - e.g. 'reg = <0x4>' - it's good until you don't use dev_get_addr() function to get it - because it's a different address space - non-MMIO. I didn't saw your GPIO driver, but I can assume, that you should use fdtdec_get_int() to get the GPIO register addres instead of dev_get_addr(). Please look at function i2c_chip_ofdata_to_platdata() in i2c-uclass.c, the reg is taken by using fdtdec_get_int()... Best regards,
On 14.4.2016 11:45, Przemyslaw Marczak wrote: > Hello Michal, > > +CC Stephen Warren > > On 04/14/2016 08:20 AM, Michal Simek wrote: >> Upcomming pca953x gpio driver (tca6416 is compatible with it) >> is using for example this DT description (ZynqMP zcu102): >> i2c0: i2c@ff020000 { >> compatible = "cdns,i2c-r1p10"; >> interrupt-parent = <&gic>; >> interrupts = <0 17 4>; >> reg = <0x0 0xff020000 0x1000>; >> #address-cells = <1>; >> #size-cells = <0>; >> >> tca6416_u61: gpio@21 { >> compatible = "ti,tca6416"; >> reg = <0x21>; >> gpio-controller; >> #gpio-cells = <2>; >> }; >> }; >> >> I2C bus is using #size-cells = <0> which current code detects as >> "Bad cell count": >> OF: reached root node >> OF: ** translation for device gpio@20 ** >> __of_translate_address: Bad cell count for gpio@20 >> Command 'gpio' failed: Error -19 >> >> This patch fixes OF_CHECK_COUNTS macro not to report problem when >> size-cells is 0 which is also valid bus configuration. >> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> --- >> >> common/fdt_support.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> 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 >> > > Some time ago I tried to solve GPIO issue on Exynos boards, by proposing > change similar to your patch. After some discussion with Stephen Warren > and Simon Glass, I was aware, that it was a wrong approach to solve this > problem. > > You can follow it here: > > http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/247651 > > https://patchwork.ozlabs.org/patch/564246/ > https://patchwork.ozlabs.org/patch/557008/ > https://patchwork.ozlabs.org/patch/557010/ > https://patchwork.ozlabs.org/patch/557009/ > > As a conclusion, for MMIO addresses we are using 'reg' property with a > range. If you need use this as I2C address or I2C device's register - > e.g. 'reg = <0x4>' - it's good until you don't use dev_get_addr() > function to get it - because it's a different address space - non-MMIO. > > I didn't saw your GPIO driver, but I can assume, that you should use > fdtdec_get_int() to get the GPIO register addres instead of dev_get_addr(). > > Please look at function i2c_chip_ofdata_to_platdata() in i2c-uclass.c, > the reg is taken by using fdtdec_get_int()... Thank you so much for this. That explain it. Using fdtdec_get_int() is not a problem. Please ignore this patch. Thanks, Michal
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
Upcomming pca953x gpio driver (tca6416 is compatible with it) is using for example this DT description (ZynqMP zcu102): i2c0: i2c@ff020000 { compatible = "cdns,i2c-r1p10"; interrupt-parent = <&gic>; interrupts = <0 17 4>; reg = <0x0 0xff020000 0x1000>; #address-cells = <1>; #size-cells = <0>; tca6416_u61: gpio@21 { compatible = "ti,tca6416"; reg = <0x21>; gpio-controller; #gpio-cells = <2>; }; }; I2C bus is using #size-cells = <0> which current code detects as "Bad cell count": OF: reached root node OF: ** translation for device gpio@20 ** __of_translate_address: Bad cell count for gpio@20 Command 'gpio' failed: Error -19 This patch fixes OF_CHECK_COUNTS macro not to report problem when size-cells is 0 which is also valid bus configuration. Signed-off-by: Michal Simek <michal.simek@xilinx.com> --- common/fdt_support.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)