[PATCH 0/3] Support reading an u32 from a multi-value property
The main part of the series consists of adding the dev_read_u32_index and dev_read_u32_index_default functions. During implementation and testing I noticed the lack of testing for the 64-bit data access functions. I decided to add the 64-bit test patch to the series to avoid merge conflicts that could have occurred with the submission of a single separated patch. In fact, it also contains changes to the test/dm/test-fdt.c file.
If this series is accepted, it will be possible to evaluate the addition of functions for indexed access also for the dev_read_s32 and dev_read_u32u functions. Maybe to add in a second version of this series.
Dario Binacchi (3): dm: test: add test case for dev_read_u64 function dm: core: support reading a single indexed u32 value dm: core: refactor functions reading an u32 from dt
arch/sandbox/dts/test.dts | 2 ++ drivers/core/of_access.c | 38 ++++++++++++++++++------------ drivers/core/ofnode.c | 49 ++++++++++++++++++++++++++++----------- drivers/core/read.c | 13 +++++++++++ include/dm/of_access.h | 19 +++++++++++++++ include/dm/ofnode.h | 25 ++++++++++++++++++++ include/dm/read.h | 40 ++++++++++++++++++++++++++++++++ include/test/ut.h | 16 +++++++++++++ test/dm/test-fdt.c | 39 +++++++++++++++++++++++++++++++ 9 files changed, 212 insertions(+), 29 deletions(-)
Add test case to cover dev_read_u64 and dev_read_u64_default functions.
Signed-off-by: Dario Binacchi dariobin@libero.it ---
arch/sandbox/dts/test.dts | 1 + include/test/ut.h | 16 ++++++++++++++++ test/dm/test-fdt.c | 10 ++++++++++ 3 files changed, 27 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 4a277934a7..6664adb385 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -93,6 +93,7 @@ <&gpio_b 9 0xc 3 2 1>; int-value = <1234>; uint-value = <(-1234)>; + int64-value = /bits/ 64 <0x1111222233334444>; interrupts-extended = <&irq 3 0>; };
diff --git a/include/test/ut.h b/include/test/ut.h index 04df8ba3af..ab861588a8 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -104,6 +104,22 @@ int ut_check_console_dump(struct unit_test_state *uts, int total_bytes); } \ }
+/* Assert that two 64 int expressions are equal */ +#define ut_asserteq_64(expr1, expr2) { \ + u64 _val1 = (expr1), _val2 = (expr2); \ + \ + if (_val1 != _val2) { \ + ut_failf(uts, __FILE__, __LINE__, __func__, \ + #expr1 " == " #expr2, \ + "Expected %#llx (%lld), got %#llx (%lld)", \ + (unsigned long long)_val1, \ + (unsigned long long)_val1, \ + (unsigned long long)_val2, \ + (unsigned long long)_val2); \ + return CMD_RET_FAILURE; \ + } \ +} + /* Assert that two string expressions are equal */ #define ut_asserteq_str(expr1, expr2) { \ const char *_val1 = (expr1), *_val2 = (expr2); \ diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 75ae08081c..50bff4fdfb 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -867,6 +867,7 @@ static int dm_test_read_int(struct unit_test_state *uts) u32 val32; s32 sval; uint val; + u64 val64;
ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev)); ut_asserteq_str("a-test", dev->name); @@ -891,6 +892,15 @@ static int dm_test_read_int(struct unit_test_state *uts) ut_assertok(dev_read_u32u(dev, "uint-value", &val)); ut_asserteq(-1234, val);
+ ut_assertok(dev_read_u64(dev, "int64-value", &val64)); + ut_asserteq_64(0x1111222233334444, val64); + + ut_asserteq_64(-EINVAL, dev_read_u64(dev, "missing", &val64)); + ut_asserteq_64(6, dev_read_u64_default(dev, "missing", 6)); + + ut_asserteq_64(0x1111222233334444, + dev_read_u64_default(dev, "int64-value", 6)); + return 0; } DM_TEST(dm_test_read_int, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
Add test case to cover dev_read_u64 and dev_read_u64_default functions.
Signed-off-by: Dario Binacchi dariobin@libero.it
arch/sandbox/dts/test.dts | 1 + include/test/ut.h | 16 ++++++++++++++++ test/dm/test-fdt.c | 10 ++++++++++ 3 files changed, 27 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
Add test case to cover dev_read_u64 and dev_read_u64_default functions.
Signed-off-by: Dario Binacchi dariobin@libero.it
arch/sandbox/dts/test.dts | 1 + include/test/ut.h | 16 ++++++++++++++++ test/dm/test-fdt.c | 10 ++++++++++ 3 files changed, 27 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
The patch adds helper functions to allow reading a single indexed u32 value from a device-tree property containing multiple u32 values, that is an array of integers.
Signed-off-by: Dario Binacchi dariobin@libero.it ---
arch/sandbox/dts/test.dts | 1 + drivers/core/of_access.c | 22 +++++++++++++++++++++ drivers/core/ofnode.c | 40 +++++++++++++++++++++++++++++++++++++++ drivers/core/read.c | 13 +++++++++++++ include/dm/of_access.h | 19 +++++++++++++++++++ include/dm/ofnode.h | 25 ++++++++++++++++++++++++ include/dm/read.h | 40 +++++++++++++++++++++++++++++++++++++++ test/dm/test-fdt.c | 29 ++++++++++++++++++++++++++++ 8 files changed, 189 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 6664adb385..270eb3d87d 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -94,6 +94,7 @@ int-value = <1234>; uint-value = <(-1234)>; int64-value = /bits/ 64 <0x1111222233334444>; + int-array = <5678 9123 4567>; interrupts-extended = <&irq 3 0>; };
diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c index acd745c121..55e4a38fc5 100644 --- a/drivers/core/of_access.c +++ b/drivers/core/of_access.c @@ -485,6 +485,28 @@ int of_read_u32_array(const struct device_node *np, const char *propname, return 0; }
+int of_read_u32_index(const struct device_node *np, const char *propname, + int index, u32 *outp) +{ + const __be32 *val; + + debug("%s: %s: ", __func__, propname); + if (!np) + return -EINVAL; + + val = of_find_property_value_of_size(np, propname, + sizeof(*outp) * (index + 1)); + if (IS_ERR(val)) { + debug("(not found)\n"); + return PTR_ERR(val); + } + + *outp = be32_to_cpup(val + index); + debug("%#x (%d)\n", *outp, *outp); + + return 0; +} + int of_read_u64(const struct device_node *np, const char *propname, u64 *outp) { const __be64 *val; diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 96a5dd20bd..5bc3b02996 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -48,6 +48,46 @@ u32 ofnode_read_u32_default(ofnode node, const char *propname, u32 def) return def; }
+int ofnode_read_u32_index(ofnode node, const char *propname, int index, + u32 *outp) +{ + const fdt32_t *cell; + int len; + + assert(ofnode_valid(node)); + debug("%s: %s: ", __func__, propname); + + if (ofnode_is_np(node)) + return of_read_u32_index(ofnode_to_np(node), propname, index, + outp); + + cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), propname, + &len); + if (!cell) { + debug("(not found)\n"); + return -EINVAL; + } + + if (len < (sizeof(int) * (index + 1))) { + debug("(not large enough)\n"); + return -EOVERFLOW; + } + + *outp = fdt32_to_cpu(cell[index]); + debug("%#x (%d)\n", *outp, *outp); + + return 0; +} + +u32 ofnode_read_u32_index_default(ofnode node, const char *propname, int index, + u32 def) +{ + assert(ofnode_valid(node)); + ofnode_read_u32_index(node, propname, index, &def); + + return def; +} + int ofnode_read_s32_default(ofnode node, const char *propname, s32 def) { assert(ofnode_valid(node)); diff --git a/drivers/core/read.c b/drivers/core/read.c index 1f999b1b31..ce78f09d28 100644 --- a/drivers/core/read.c +++ b/drivers/core/read.c @@ -22,6 +22,19 @@ int dev_read_u32_default(const struct udevice *dev, const char *propname, return ofnode_read_u32_default(dev_ofnode(dev), propname, def); }
+int dev_read_u32_index(struct udevice *dev, const char *propname, int index, + u32 *outp) +{ + return ofnode_read_u32_index(dev_ofnode(dev), propname, index, outp); +} + +u32 dev_read_u32_index_default(struct udevice *dev, const char *propname, + int index, u32 def) +{ + return ofnode_read_u32_index_default(dev_ofnode(dev), propname, index, + def); +} + int dev_read_s32(const struct udevice *dev, const char *propname, s32 *outp) { return ofnode_read_u32(dev_ofnode(dev), propname, (u32 *)outp); diff --git a/include/dm/of_access.h b/include/dm/of_access.h index 13fedb7cf5..92876b3ecb 100644 --- a/include/dm/of_access.h +++ b/include/dm/of_access.h @@ -234,6 +234,25 @@ struct device_node *of_find_node_by_phandle(phandle handle); */ int of_read_u32(const struct device_node *np, const char *propname, u32 *outp);
+/** + * of_read_u32_index() - Find and read a 32-bit value from a multi-value + * property + * + * Search for a property in a device node and read a 32-bit value from + * it. + * + * @np: device node from which the property value is to be read. + * @propname: name of the property to be searched. + * @index: index of the u32 in the list of values + * @outp: pointer to return value, modified only if return value is 0. + * + * @return 0 on success, -EINVAL if the property does not exist, + * -ENODATA if property does not have a value, and -EOVERFLOW if the + * property data isn't large enough. + */ +int of_read_u32_index(const struct device_node *np, const char *propname, + int index, u32 *outp); + /** * of_read_u64() - Find and read a 64-bit integer from a property * diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index b5a50e8849..ce5e366c06 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -202,6 +202,18 @@ static inline ofnode ofnode_null(void) */ int ofnode_read_u32(ofnode node, const char *propname, u32 *outp);
+/** + * ofnode_read_u32_index() - Read a 32-bit integer from a multi-value property + * + * @ref: valid node reference to read property from + * @propname: name of the property to read from + * @index: index of the integer to return + * @outp: place to put value (if found) + * @return 0 if OK, -ve on error + */ +int ofnode_read_u32_index(ofnode node, const char *propname, int index, + u32 *outp); + /** * ofnode_read_s32() - Read a 32-bit integer from a property * @@ -226,6 +238,19 @@ static inline int ofnode_read_s32(ofnode node, const char *propname, */ u32 ofnode_read_u32_default(ofnode ref, const char *propname, u32 def);
+/** + * ofnode_read_u32_index_default() - Read a 32-bit integer from a multi-value + * property + * + * @ref: valid node reference to read property from + * @propname: name of the property to read from + * @index: index of the integer to return + * @def: default value to return if the property has no value + * @return property value, or @def if not found + */ +u32 ofnode_read_u32_index_default(ofnode ref, const char *propname, int index, + u32 def); + /** * ofnode_read_s32_default() - Read a 32-bit integer from a property * diff --git a/include/dm/read.h b/include/dm/read.h index da8c7f25e7..77d3bc8db5 100644 --- a/include/dm/read.h +++ b/include/dm/read.h @@ -66,6 +66,32 @@ int dev_read_u32(const struct udevice *dev, const char *propname, u32 *outp); int dev_read_u32_default(const struct udevice *dev, const char *propname, int def);
+/** + * dev_read_u32_index() - read an indexed 32-bit integer from a device's DT + * property + * + * @dev: device to read DT property from + * @propname: name of the property to read from + * @index: index of the integer to return + * @outp: place to put value (if found) + * @return 0 if OK, -ve on error + */ +int dev_read_u32_index(struct udevice *dev, const char *propname, int index, + u32 *outp); + +/** + * dev_read_u32_index_default() - read an indexed 32-bit integer from a device's + * DT property + * + * @dev: device to read DT property from + * @propname: name of the property to read from + * @index: index of the integer to return + * @def: default value to return if the property has no value + * @return property value, or @def if not found + */ +u32 dev_read_u32_index_default(struct udevice *dev, const char *propname, + int index, u32 def); + /** * dev_read_s32() - read a signed 32-bit integer from a device's DT property * @@ -621,6 +647,20 @@ static inline int dev_read_u32_default(const struct udevice *dev, return ofnode_read_u32_default(dev_ofnode(dev), propname, def); }
+static inline int dev_read_u32_index(struct udevice *dev, + const char *propname, int index, u32 *outp) +{ + return ofnode_read_u32_index(dev_ofnode(dev), propname, index, outp); +} + +static inline u32 dev_read_u32_index_default(struct udevice *dev, + const char *propname, int index, + u32 def) +{ + return ofnode_read_u32_index_default(dev_ofnode(dev), propname, index, + def); +} + static inline int dev_read_s32(const struct udevice *dev, const char *propname, s32 *outp) { diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 50bff4fdfb..5fa1238d94 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -905,6 +905,35 @@ static int dm_test_read_int(struct unit_test_state *uts) } DM_TEST(dm_test_read_int, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+static int dm_test_read_int_index(struct unit_test_state *uts) +{ + struct udevice *dev; + u32 val32; + + ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev)); + ut_asserteq_str("a-test", dev->name); + + ut_asserteq(-EINVAL, dev_read_u32_index(dev, "missing", 0, &val32)); + ut_asserteq(19, dev_read_u32_index_default(dev, "missing", 0, 19)); + + ut_assertok(dev_read_u32_index(dev, "int-array", 0, &val32)); + ut_asserteq(5678, val32); + ut_assertok(dev_read_u32_index(dev, "int-array", 1, &val32)); + ut_asserteq(9123, val32); + ut_assertok(dev_read_u32_index(dev, "int-array", 2, &val32)); + ut_asserteq(4567, val32); + ut_asserteq(-EOVERFLOW, dev_read_u32_index(dev, "int-array", 3, + &val32)); + + ut_asserteq(5678, dev_read_u32_index_default(dev, "int-array", 0, 2)); + ut_asserteq(9123, dev_read_u32_index_default(dev, "int-array", 1, 2)); + ut_asserteq(4567, dev_read_u32_index_default(dev, "int-array", 2, 2)); + ut_asserteq(2, dev_read_u32_index_default(dev, "int-array", 3, 2)); + + return 0; +} +DM_TEST(dm_test_read_int_index, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + /* Test iteration through devices by drvdata */ static int dm_test_uclass_drvdata(struct unit_test_state *uts) {
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
The patch adds helper functions to allow reading a single indexed u32 value from a device-tree property containing multiple u32 values, that is an array of integers.
Signed-off-by: Dario Binacchi dariobin@libero.it
arch/sandbox/dts/test.dts | 1 + drivers/core/of_access.c | 22 +++++++++++++++++++++ drivers/core/ofnode.c | 40 +++++++++++++++++++++++++++++++++++++++ drivers/core/read.c | 13 +++++++++++++ include/dm/of_access.h | 19 +++++++++++++++++++ include/dm/ofnode.h | 25 ++++++++++++++++++++++++ include/dm/read.h | 40 +++++++++++++++++++++++++++++++++++++++ test/dm/test-fdt.c | 29 ++++++++++++++++++++++++++++ 8 files changed, 189 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Very nice
Do you think it would make sense to add indexed access also for the s32, u32u and u64 types or at least some of those ?
Thanks and regards Dario Binacchi
Il 31 marzo 2020 alle 1.57 Simon Glass sjg@chromium.org ha scritto:
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
The patch adds helper functions to allow reading a single indexed u32 value from a device-tree property containing multiple u32 values, that is an array of integers.
Signed-off-by: Dario Binacchi dariobin@libero.it
arch/sandbox/dts/test.dts | 1 + drivers/core/of_access.c | 22 +++++++++++++++++++++ drivers/core/ofnode.c | 40 +++++++++++++++++++++++++++++++++++++++ drivers/core/read.c | 13 +++++++++++++ include/dm/of_access.h | 19 +++++++++++++++++++ include/dm/ofnode.h | 25 ++++++++++++++++++++++++ include/dm/read.h | 40 +++++++++++++++++++++++++++++++++++++++ test/dm/test-fdt.c | 29 ++++++++++++++++++++++++++++ 8 files changed, 189 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Very nice
Hi Dario,
On Wed, 1 Apr 2020 at 13:43, dariobin@libero.it wrote:
Do you think it would make sense to add indexed access also for the s32, u32u and u64 types or at least some of those ?
I really don't know. I suspect it would be used at some point.
Regards, Simon
Thanks and regards Dario Binacchi
Il 31 marzo 2020 alle 1.57 Simon Glass sjg@chromium.org ha scritto:
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
The patch adds helper functions to allow reading a single indexed u32 value from a device-tree property containing multiple u32 values, that is an array of integers.
Signed-off-by: Dario Binacchi dariobin@libero.it
arch/sandbox/dts/test.dts | 1 + drivers/core/of_access.c | 22 +++++++++++++++++++++ drivers/core/ofnode.c | 40 +++++++++++++++++++++++++++++++++++++++ drivers/core/read.c | 13 +++++++++++++ include/dm/of_access.h | 19 +++++++++++++++++++ include/dm/ofnode.h | 25 ++++++++++++++++++++++++ include/dm/read.h | 40 +++++++++++++++++++++++++++++++++++++++ test/dm/test-fdt.c | 29 ++++++++++++++++++++++++++++ 8 files changed, 189 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Very nice
Now reading a 32 bit value from a device-tree property can be expressed as reading the first element of an array with a single value.
Signed-off-by: Dario Binacchi dariobin@libero.it
---
drivers/core/of_access.c | 16 +--------------- drivers/core/ofnode.c | 23 ++--------------------- 2 files changed, 3 insertions(+), 36 deletions(-)
diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c index 55e4a38fc5..d116d106d9 100644 --- a/drivers/core/of_access.c +++ b/drivers/core/of_access.c @@ -449,21 +449,7 @@ static void *of_find_property_value_of_size(const struct device_node *np,
int of_read_u32(const struct device_node *np, const char *propname, u32 *outp) { - const __be32 *val; - - debug("%s: %s: ", __func__, propname); - if (!np) - return -EINVAL; - val = of_find_property_value_of_size(np, propname, sizeof(*outp)); - if (IS_ERR(val)) { - debug("(not found)\n"); - return PTR_ERR(val); - } - - *outp = be32_to_cpup(val); - debug("%#x (%d)\n", *outp, *outp); - - return 0; + return of_read_u32_index(np, propname, 0, outp); }
int of_read_u32_array(const struct device_node *np, const char *propname, diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 5bc3b02996..b0be7cbe19 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -18,32 +18,13 @@
int ofnode_read_u32(ofnode node, const char *propname, u32 *outp) { - assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, propname); - - if (ofnode_is_np(node)) { - return of_read_u32(ofnode_to_np(node), propname, outp); - } else { - const fdt32_t *cell; - int len; - - cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), - propname, &len); - if (!cell || len < sizeof(int)) { - debug("(not found)\n"); - return -EINVAL; - } - *outp = fdt32_to_cpu(cell[0]); - } - debug("%#x (%d)\n", *outp, *outp); - - return 0; + return ofnode_read_u32_index(node, propname, 0, outp); }
u32 ofnode_read_u32_default(ofnode node, const char *propname, u32 def) { assert(ofnode_valid(node)); - ofnode_read_u32(node, propname, &def); + ofnode_read_u32_index(node, propname, 0, &def);
return def; }
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
Now reading a 32 bit value from a device-tree property can be expressed as reading the first element of an array with a single value.
Signed-off-by: Dario Binacchi dariobin@libero.it
drivers/core/of_access.c | 16 +--------------- drivers/core/ofnode.c | 23 ++--------------------- 2 files changed, 3 insertions(+), 36 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Can you please check the code-size delta in SPL on a suitable board?
Il 31 marzo 2020 alle 1.57 Simon Glass sjg@chromium.org ha scritto:
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
Now reading a 32 bit value from a device-tree property can be expressed as reading the first element of an array with a single value.
Signed-off-by: Dario Binacchi dariobin@libero.it
drivers/core/of_access.c | 16 +--------------- drivers/core/ofnode.c | 23 ++--------------------- 2 files changed, 3 insertions(+), 36 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Can you please check the code-size delta in SPL on a suitable board?
I have a black beaglebone available (am335x_evm_defconfig).
u-boot-spl.map generated without applying the refactoring patch .... .text.ofnode_read_u32 0x0000000000000000 0x2e drivers/built-in.o .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x12 drivers/built-in.o
u-boot-spl.map genarated with the refactoring patch applied .... .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32 0x0000000000000000 0x8 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x14 drivers/built-in.o
I hope I have correctly answered what you asked me. Otherwise I am available to perform the right checks on your indications.
Thanks and regards, Dario Binacchi
Hi Dario,
On Wed, 1 Apr 2020 at 13:34, dariobin@libero.it wrote:
Il 31 marzo 2020 alle 1.57 Simon Glass sjg@chromium.org ha scritto:
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
Now reading a 32 bit value from a device-tree property can be expressed as reading the first element of an array with a single value.
Signed-off-by: Dario Binacchi dariobin@libero.it
drivers/core/of_access.c | 16 +--------------- drivers/core/ofnode.c | 23 ++--------------------- 2 files changed, 3 insertions(+), 36 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Can you please check the code-size delta in SPL on a suitable board?
I have a black beaglebone available (am335x_evm_defconfig).
u-boot-spl.map generated without applying the refactoring patch .... .text.ofnode_read_u32 0x0000000000000000 0x2e drivers/built-in.o .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x12 drivers/built-in.o
u-boot-spl.map genarated with the refactoring patch applied .... .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32 0x0000000000000000 0x8 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x14 drivers/built-in.o
Possibly, but a better test is to build your branch with the patch in:
buildman -b <branch> <board>
Then check the size:
buildman -b <branch> -sS
or function detail:
buildman -b <branch> -sSB
That will give us a true picture for SPL. It will show incremental size increase with your patch.
See buildman docs for more info.
Regards, Simon
Il 2 aprile 2020 alle 20.54 Simon Glass sjg@chromium.org ha scritto:
Hi Dario,
On Wed, 1 Apr 2020 at 13:34, dariobin@libero.it wrote:
Il 31 marzo 2020 alle 1.57 Simon Glass sjg@chromium.org ha scritto:
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
Now reading a 32 bit value from a device-tree property can be expressed as reading the first element of an array with a single value.
Signed-off-by: Dario Binacchi dariobin@libero.it
drivers/core/of_access.c | 16 +--------------- drivers/core/ofnode.c | 23 ++--------------------- 2 files changed, 3 insertions(+), 36 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Can you please check the code-size delta in SPL on a suitable board?
I have a black beaglebone available (am335x_evm_defconfig).
u-boot-spl.map generated without applying the refactoring patch .... .text.ofnode_read_u32 0x0000000000000000 0x2e drivers/built-in.o .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x12 drivers/built-in.o
u-boot-spl.map genarated with the refactoring patch applied .... .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32 0x0000000000000000 0x8 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x14 drivers/built-in.o
Possibly, but a better test is to build your branch with the patch in:
buildman -b <branch> <board>
Then check the size:
buildman -b <branch> -sS
or function detail:
buildman -b <branch> -sSB
That will give us a true picture for SPL. It will show incremental size increase with your patch.
Hi Simon, this is the buildman response: ... 03: dm: core: support reading a single indexed u32 value 04: dm: core: refactor functions reading an u32 from dt arm: (for 4/708 boards) all +11.5 bss -10.0 text +21.5 am335x_hs_evm : all +24 text +24 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38 am335x_hs_evm_uart: all +24 text +24 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38 am335x_evm : bss -24 text +24 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38 am335x_boneblack_vboot: all -2 bss -16 text +14 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38
Regards, Dario
See buildman docs for more info.
Regards, Simon
Hi Dario,
On Sat, 4 Apr 2020 at 06:49, dariobin@libero.it wrote:
Il 2 aprile 2020 alle 20.54 Simon Glass sjg@chromium.org ha scritto:
Hi Dario,
On Wed, 1 Apr 2020 at 13:34, dariobin@libero.it wrote:
Il 31 marzo 2020 alle 1.57 Simon Glass sjg@chromium.org ha scritto:
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
Now reading a 32 bit value from a device-tree property can be expressed as reading the first element of an array with a single value.
Signed-off-by: Dario Binacchi dariobin@libero.it
drivers/core/of_access.c | 16 +--------------- drivers/core/ofnode.c | 23 ++--------------------- 2 files changed, 3 insertions(+), 36 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Can you please check the code-size delta in SPL on a suitable board?
I have a black beaglebone available (am335x_evm_defconfig).
u-boot-spl.map generated without applying the refactoring patch .... .text.ofnode_read_u32 0x0000000000000000 0x2e drivers/built-in.o .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x12 drivers/built-in.o
u-boot-spl.map genarated with the refactoring patch applied .... .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32 0x0000000000000000 0x8 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x14 drivers/built-in.o
Possibly, but a better test is to build your branch with the patch in:
buildman -b <branch> <board>
Then check the size:
buildman -b <branch> -sS
or function detail:
buildman -b <branch> -sSB
That will give us a true picture for SPL. It will show incremental size increase with your patch.
Hi Simon, this is the buildman response: ... 03: dm: core: support reading a single indexed u32 value 04: dm: core: refactor functions reading an u32 from dt arm: (for 4/708 boards) all +11.5 bss -10.0 text +21.5 am335x_hs_evm : all +24 text +24 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38 am335x_hs_evm_uart: all +24 text +24 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38 am335x_evm : bss -24 text +24 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38 am335x_boneblack_vboot: all -2 bss -16 text +14 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38
This does not actually look like SPL though, only U-Boot. I hope that means that SPL doesn't increase in size.
Anyway for U-Boot proper it looks like it adds about 24 bytes of code. I think it is worth it.
Regards, Simon
Hi Dario,
On Sat, 4 Apr 2020 at 06:49, dariobin@libero.it wrote:
Il 2 aprile 2020 alle 20.54 Simon Glass sjg@chromium.org ha scritto:
Hi Dario,
On Wed, 1 Apr 2020 at 13:34, dariobin@libero.it wrote:
Il 31 marzo 2020 alle 1.57 Simon Glass sjg@chromium.org ha scritto:
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi dariobin@libero.it wrote:
Now reading a 32 bit value from a device-tree property can be expressed as reading the first element of an array with a single value.
Signed-off-by: Dario Binacchi dariobin@libero.it
drivers/core/of_access.c | 16 +--------------- drivers/core/ofnode.c | 23 ++--------------------- 2 files changed, 3 insertions(+), 36 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Can you please check the code-size delta in SPL on a suitable board?
I have a black beaglebone available (am335x_evm_defconfig).
u-boot-spl.map generated without applying the refactoring patch .... .text.ofnode_read_u32 0x0000000000000000 0x2e drivers/built-in.o .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x12 drivers/built-in.o
u-boot-spl.map genarated with the refactoring patch applied .... .text.ofnode_read_u32_index 0x0000000000000000 0x38 drivers/built-in.o .text.ofnode_read_u32 0x0000000000000000 0x8 drivers/built-in.o .text.ofnode_read_u32_default 0x0000000000000000 0x14 drivers/built-in.o
Possibly, but a better test is to build your branch with the patch in:
buildman -b <branch> <board>
Then check the size:
buildman -b <branch> -sS
or function detail:
buildman -b <branch> -sSB
That will give us a true picture for SPL. It will show incremental size increase with your patch.
Hi Simon, this is the buildman response: ... 03: dm: core: support reading a single indexed u32 value 04: dm: core: refactor functions reading an u32 from dt arm: (for 4/708 boards) all +11.5 bss -10.0 text +21.5 am335x_hs_evm : all +24 text +24 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38 am335x_hs_evm_uart: all +24 text +24 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38 am335x_evm : bss -24 text +24 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38 am335x_boneblack_vboot: all -2 bss -16 text +14 u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26) function old new delta ofnode_read_u32_index - 62 +62 ofnode_read_u32_default 18 20 +2 ofnode_read_u32 46 8 -38
This does not actually look like SPL though, only U-Boot. I hope that means that SPL doesn't increase in size.
Anyway for U-Boot proper it looks like it adds about 24 bytes of code. I think it is worth it.
Regards, Simon
Applied to u-boot-dm/next, thanks!
participants (4)
-
Dario Binacchi -
dariobin＠libero.it
-
Simon Glass -
sjg＠google.com