Skip to contentSkip to conversation

fdt: Do not report bad cell count when #size-cells = <0>

PATCH New Michal Simek 3 participants 4 messages Apr 14, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fdt: Do not report bad cell count when #size-cells = <0>

Upcomming pca953x gpio driver (tca6416 is compatible with it) is using for example this DT description (ZynqMP zcu102):

i2c0: i2c at 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 at 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 at 20 ** __of_translate_address: Bad cell count for gpio at 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.

---

 common/fdt_support.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common/fdt_support.c +1 -1
index ced119e70d9f..5f5b49c6210b 100644
@@ -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
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 fdt: Do not report bad cell count when #size-cells = <0>

Hi Michal,

Quoted 25 lines · Michal Simek

Upcomming pca953x gpio driver (tca6416 is compatible with it) is using for example this DT description (ZynqMP zcu102):

i2c0: i2c at 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 at 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 at 20 ** __of_translate_address: Bad cell count for gpio at 20 Command 'gpio' failed: Error -19

Quoted 4 lines

This patch fixes OF_CHECK_COUNTS macro not to report problem when size-cells is 0 which is also valid bus configuration.

Regards,
Peng.

Quoted 21 lines

---

common/fdt_support.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
common/fdt_support.c +1 -1
index ced119e70d9f..5f5b49c6210b 100644
@@ -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
Signature · 1 line
1.9.1
#2 lore raw

PATCH 1/1 fdt: Do not report bad cell count when #size-cells = <0>

In reply to Michal Simek

Hello Michal,

+CC Stephen Warren

Quoted 40 lines · Michal Simek

Upcomming pca953x gpio driver (tca6416 is compatible with it) is using for example this DT description (ZynqMP zcu102):

i2c0: i2c at 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 at 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 at 20 ** __of_translate_address: Bad cell count for gpio at 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.

---

  common/fdt_support.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
common/fdt_support.c +0 -0
index ced119e70d9f..5f5b49c6210b 100644

common/fdt_support.c void fdt_del_node_and_alias(void *blob, const char *alias) · line 947

Quoted 8 lines
common/fdt_support.c +1 -1
@@ -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,

Signature · 4 lines
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
#3 lore raw

PATCH 1/1 fdt: Do not report bad cell count when #size-cells = <0>

Quoted 75 lines · Przemyslaw Marczak

Hello Michal,

+CC Stephen Warren

Quoted 47 lines · Michal Simek

Upcomming pca953x gpio driver (tca6416 is compatible with it) is using for example this DT description (ZynqMP zcu102):

i2c0: i2c at 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 at 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 at 20 ** __of_translate_address: Bad cell count for gpio at 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.

---

  common/fdt_support.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
common/fdt_support.c +1 -1
index ced119e70d9f..5f5b49c6210b 100644
@@ -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)
 
Quoted 3 lines
  /* 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.

Quoted 5 lines

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