Re: [PATCH 3/3] dm: core: refactor functions reading an u32 from dt
Hi Dario, On Sat, 4 Apr 2020 at 06:49, wrote: > > > Il 2 aprile 2020 alle 20.54 Simon Glass ha scritto: > > > > > > Hi Dario, > > > > On Wed, 1 Apr 2020 at 13:34, wrote: > > > > > > > > > > Il 31 marzo 2020 alle 1.57 Simon Glass ha scritto: > > > > > > > > > > > > On Sun, 29 Mar 2020 at 10:05, Dario Binacchi 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 > > > > > > > > > > --- > > > > > > > > > > drivers/core/of_access.c | 16 +--- > > > > > drivers/core/ofnode.c| 23 ++- > > > > > 2 files changed, 3 insertions(+), 36 deletions(-) > > > > > > > > Reviewed-by: Simon Glass > > > > > > > > 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 > > > 0x 0x2e drivers/built-in.o > > > .text.ofnode_read_u32_index > > > 0x 0x38 drivers/built-in.o > > > .text.ofnode_read_u32_default > > > 0x 0x12 drivers/built-in.o > > > > > > u-boot-spl.map genarated with the refactoring patch applied > > > > > > .text.ofnode_read_u32_index > > > 0x 0x38 drivers/built-in.o > > > .text.ofnode_read_u32 > > > 0x0x8 drivers/built-in.o > > > .text.ofnode_read_u32_default > > > 0x 0x14 drivers/built-in.o > > > > Possibly, but a better test is to build your branch with the patch in: > > > > buildman -b > > > > Then check the size: > > > > buildman -b -sS > > > > or function detail: > > > > buildman -b -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
Re: [PATCH 3/3] dm: core: refactor functions reading an u32 from dt
> Il 2 aprile 2020 alle 20.54 Simon Glass ha scritto: > > > Hi Dario, > > On Wed, 1 Apr 2020 at 13:34, wrote: > > > > > > > Il 31 marzo 2020 alle 1.57 Simon Glass ha scritto: > > > > > > > > > On Sun, 29 Mar 2020 at 10:05, Dario Binacchi 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 > > > > > > > > --- > > > > > > > > drivers/core/of_access.c | 16 +--- > > > > drivers/core/ofnode.c| 23 ++- > > > > 2 files changed, 3 insertions(+), 36 deletions(-) > > > > > > Reviewed-by: Simon Glass > > > > > > 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 > > 0x 0x2e drivers/built-in.o > > .text.ofnode_read_u32_index > > 0x 0x38 drivers/built-in.o > > .text.ofnode_read_u32_default > > 0x 0x12 drivers/built-in.o > > > > u-boot-spl.map genarated with the refactoring patch applied > > > > .text.ofnode_read_u32_index > > 0x 0x38 drivers/built-in.o > > .text.ofnode_read_u32 > > 0x0x8 drivers/built-in.o > > .text.ofnode_read_u32_default > > 0x 0x14 drivers/built-in.o > > Possibly, but a better test is to build your branch with the patch in: > > buildman -b > > Then check the size: > > buildman -b -sS > > or function detail: > > buildman -b -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
Re: [PATCH 3/3] dm: core: refactor functions reading an u32 from dt
Hi Dario, On Wed, 1 Apr 2020 at 13:34, wrote: > > > > Il 31 marzo 2020 alle 1.57 Simon Glass ha scritto: > > > > > > On Sun, 29 Mar 2020 at 10:05, Dario Binacchi 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 > > > > > > --- > > > > > > drivers/core/of_access.c | 16 +--- > > > drivers/core/ofnode.c| 23 ++- > > > 2 files changed, 3 insertions(+), 36 deletions(-) > > > > Reviewed-by: Simon Glass > > > > 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 > 0x 0x2e drivers/built-in.o > .text.ofnode_read_u32_index > 0x 0x38 drivers/built-in.o > .text.ofnode_read_u32_default > 0x 0x12 drivers/built-in.o > > u-boot-spl.map genarated with the refactoring patch applied > > .text.ofnode_read_u32_index > 0x 0x38 drivers/built-in.o > .text.ofnode_read_u32 > 0x0x8 drivers/built-in.o > .text.ofnode_read_u32_default > 0x 0x14 drivers/built-in.o Possibly, but a better test is to build your branch with the patch in: buildman -b Then check the size: buildman -b -sS or function detail: buildman -b -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
Re: [PATCH 3/3] dm: core: refactor functions reading an u32 from dt
> Il 31 marzo 2020 alle 1.57 Simon Glass ha scritto: > > > On Sun, 29 Mar 2020 at 10:05, Dario Binacchi 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 > > > > --- > > > > drivers/core/of_access.c | 16 +--- > > drivers/core/ofnode.c| 23 ++- > > 2 files changed, 3 insertions(+), 36 deletions(-) > > Reviewed-by: Simon Glass > > 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 0x 0x2e drivers/built-in.o .text.ofnode_read_u32_index 0x 0x38 drivers/built-in.o .text.ofnode_read_u32_default 0x 0x12 drivers/built-in.o u-boot-spl.map genarated with the refactoring patch applied .text.ofnode_read_u32_index 0x 0x38 drivers/built-in.o .text.ofnode_read_u32 0x0x8 drivers/built-in.o .text.ofnode_read_u32_default 0x 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
Re: [PATCH 3/3] dm: core: refactor functions reading an u32 from dt
On Sun, 29 Mar 2020 at 10:05, Dario Binacchi 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 > > --- > > drivers/core/of_access.c | 16 +--- > drivers/core/ofnode.c| 23 ++- > 2 files changed, 3 insertions(+), 36 deletions(-) Reviewed-by: Simon Glass Can you please check the code-size delta in SPL on a suitable board?
[PATCH 3/3] dm: core: refactor functions reading an u32 from dt
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
---
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;
}
--
2.17.1
