| Message ID | 20160805154751.7858-1-swarren@wwwdotorg.org |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
On 5 August 2016 at 09:47, Stephen Warren <swarren@wwwdotorg.org> wrote: > From: Stephen Warren <swarren@nvidia.com> > > The next patch will call fdt_translate_address() from somewhere with a > "const void *blob" rather than a "void *blob", so fdt_translate_address() > must accept a const pointer too. Constify the minimum number of function > parameters to achieve this. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > Acked-by: Simon Glass <sjg@chromium.org> > --- > Re-sending since these didn't show up in patchwork for some reason. > > This series will be a dependency for the Tegra BPMP driver. > > common/fdt_support.c | 19 ++++++++++--------- > include/fdt_support.h | 5 +++-- > 2 files changed, 13 insertions(+), 11 deletions(-) Applied to u-boot-dm, thanks!
Hi Stephen, On 5 August 2016 at 19:41, Simon Glass <sjg@chromium.org> wrote: > > On 5 August 2016 at 09:47, Stephen Warren <swarren@wwwdotorg.org> wrote: > > From: Stephen Warren <swarren@nvidia.com> > > > > The next patch will call fdt_translate_address() from somewhere with a > > "const void *blob" rather than a "void *blob", so fdt_translate_address() > > must accept a const pointer too. Constify the minimum number of function > > parameters to achieve this. > > > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > > Acked-by: Simon Glass <sjg@chromium.org> > > --- > > Re-sending since these didn't show up in patchwork for some reason. > > > > This series will be a dependency for the Tegra BPMP driver. > > > > common/fdt_support.c | 19 ++++++++++--------- > > include/fdt_support.h | 5 +++-- > > 2 files changed, 13 insertions(+), 11 deletions(-) > > Applied to u-boot-dm, thanks! Unfortunately these patches cause some build breakages. Can you please take a look? $ buildman -b dm-push -se ... 02: fdt_support: fdt_translate_address() blob const correctness mips: + malta64 malta64el maltael malta + .match = of_bus_isa_match, + ^ w+../common/fdt_support.c:1113:3: warning: initialization from incompatible pointer type w+../common/fdt_support.c:1113:3: warning: (near initialization for 'of_busses[0].match') 03: fdt: allow fdtdec_get_addr_size_*() to translate addresses aarch64: + xilinx_zynqmp_zc1751_xm018_dc4 xilinx_zynqmp_zcu102 xilinx_zynqmp_zc1751_xm015_dc1 xilinx_zynqmp_zc1751_xm019_dc5 xilinx_zynqmp_zc1751_xm016_dc2 xilinx_zynqmp_ep xilinx_zynqmp_zcu102_revB arm: + socfpga_de0_nano_soc uniphier_pro4 uniphier_ld4_sld8 zynq_zc770_xm010 zynq_zc770_xm013 zynq_zc770_xm012 zynq_zc706 evb-rk3288 socfpga_arria5 zynq_zybo rock2 socfpga_socrates uniphier_sld3 zynq_microzed socfpga_sr1500 chromebook_jerry zynq_zed socfpga_sockit zynq_zc702 socfpga_is1 zynq_picozed fennec-rk3288 zynq_zc770_xm011 popmetal-rk3288 socfpga_mcvevk socfpga_cyclone5 socfpga_vining_fpga uniphier_pxs2_ld6b +lib/built-in.o: In function `fdtdec_get_addr_size_fixed': +build/../lib/fdtdec.c:117: undefined reference to `fdt_translate_address' Regards, Simon
On 08/05/2016 09:36 PM, Simon Glass wrote: > Hi Stephen, > > On 5 August 2016 at 19:41, Simon Glass <sjg@chromium.org> wrote: >> >> On 5 August 2016 at 09:47, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> From: Stephen Warren <swarren@nvidia.com> >>> >>> The next patch will call fdt_translate_address() from somewhere with a >>> "const void *blob" rather than a "void *blob", so fdt_translate_address() >>> must accept a const pointer too. Constify the minimum number of function >>> parameters to achieve this. >>> >>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>> Acked-by: Simon Glass <sjg@chromium.org> >>> --- >>> Re-sending since these didn't show up in patchwork for some reason. >>> >>> This series will be a dependency for the Tegra BPMP driver. >>> >>> common/fdt_support.c | 19 ++++++++++--------- >>> include/fdt_support.h | 5 +++-- >>> 2 files changed, 13 insertions(+), 11 deletions(-) >> >> Applied to u-boot-dm, thanks! > > Unfortunately these patches cause some build breakages. Can you please > take a look? The following fixes the issues you mentioned. Do you want to squash it in or should I resend? > diff --git a/common/fdt_support.c b/common/fdt_support.c > index da59f2c8cc07..202058621ae2 100644 > --- a/common/fdt_support.c > +++ b/common/fdt_support.c > @@ -1055,7 +1055,7 @@ static int of_bus_default_translate(fdt32_t *addr, u64 offset, int na) > #ifdef CONFIG_OF_ISA_BUS > > /* ISA bus translator */ > -static int of_bus_isa_match(void *blob, int parentoffset) > +static int of_bus_isa_match(const void *blob, int parentoffset) > { > const char *name; > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > index 7d99bdb8ca47..e638ca5d6a33 100644 > --- a/lib/fdtdec.c > +++ b/lib/fdtdec.c > @@ -113,9 +113,11 @@ fdt_addr_t fdtdec_get_addr_size_fixed(const void *blob, int node, > return FDT_ADDR_T_NONE; > } > > +#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_LIBFDT) > if (translate) > addr = fdt_translate_address(blob, node, prop_addr); > else > +#endif > addr = fdtdec_get_number(prop_addr, na); > > if (sizep) { The ifdef is a bit unfortunate but mirrors that ifdefs in common/Makefile. An alternative might be to define a weak version of fdt_translate_address() in fdtdec.c that does nothing more than call fdtdec_get_number(), but that might be even more confusing.
Hi Stephen, On 6 August 2016 at 00:01, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 08/05/2016 09:36 PM, Simon Glass wrote: >> >> Hi Stephen, >> >> On 5 August 2016 at 19:41, Simon Glass <sjg@chromium.org> wrote: >>> >>> >>> On 5 August 2016 at 09:47, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>> >>>> From: Stephen Warren <swarren@nvidia.com> >>>> >>>> The next patch will call fdt_translate_address() from somewhere with a >>>> "const void *blob" rather than a "void *blob", so >>>> fdt_translate_address() >>>> must accept a const pointer too. Constify the minimum number of function >>>> parameters to achieve this. >>>> >>>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>>> Acked-by: Simon Glass <sjg@chromium.org> >>>> --- >>>> Re-sending since these didn't show up in patchwork for some reason. >>>> >>>> This series will be a dependency for the Tegra BPMP driver. >>>> >>>> common/fdt_support.c | 19 ++++++++++--------- >>>> include/fdt_support.h | 5 +++-- >>>> 2 files changed, 13 insertions(+), 11 deletions(-) >>> >>> >>> Applied to u-boot-dm, thanks! >> >> >> Unfortunately these patches cause some build breakages. Can you please >> take a look? > > > The following fixes the issues you mentioned. Do you want to squash it in or > should I resend? > >> diff --git a/common/fdt_support.c b/common/fdt_support.c >> index da59f2c8cc07..202058621ae2 100644 >> --- a/common/fdt_support.c >> +++ b/common/fdt_support.c >> @@ -1055,7 +1055,7 @@ static int of_bus_default_translate(fdt32_t *addr, >> u64 offset, int na) >> #ifdef CONFIG_OF_ISA_BUS >> >> /* ISA bus translator */ >> -static int of_bus_isa_match(void *blob, int parentoffset) >> +static int of_bus_isa_match(const void *blob, int parentoffset) >> { >> const char *name; >> >> diff --git a/lib/fdtdec.c b/lib/fdtdec.c >> index 7d99bdb8ca47..e638ca5d6a33 100644 >> --- a/lib/fdtdec.c >> +++ b/lib/fdtdec.c >> @@ -113,9 +113,11 @@ fdt_addr_t fdtdec_get_addr_size_fixed(const void >> *blob, int node, >> return FDT_ADDR_T_NONE; >> } >> >> +#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_OF_LIBFDT) >> if (translate) >> addr = fdt_translate_address(blob, node, prop_addr); >> else >> +#endif >> addr = fdtdec_get_number(prop_addr, na); >> >> if (sizep) { > > > The ifdef is a bit unfortunate but mirrors that ifdefs in common/Makefile. > An alternative might be to define a weak version of fdt_translate_address() > in fdtdec.c that does nothing more than call fdtdec_get_number(), but that > might be even more confusing. Yes that works - applied, thanks. - Simon
diff --git a/common/fdt_support.c b/common/fdt_support.c index 5d8eb12f1073..da59f2c8cc07 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -997,8 +997,8 @@ static void of_dump_addr(const char *s, const fdt32_t *addr, int na) { } struct of_bus { const char *name; const char *addresses; - int (*match)(void *blob, int parentoffset); - void (*count_cells)(void *blob, int parentoffset, + int (*match)(const void *blob, int parentoffset); + void (*count_cells)(const void *blob, int parentoffset, int *addrc, int *sizec); u64 (*map)(fdt32_t *addr, const fdt32_t *range, int na, int ns, int pna); @@ -1006,7 +1006,7 @@ struct of_bus { }; /* Default translator (generic bus) */ -void of_bus_default_count_cells(void *blob, int parentoffset, +void of_bus_default_count_cells(const void *blob, int parentoffset, int *addrc, int *sizec) { const fdt32_t *prop; @@ -1066,7 +1066,7 @@ static int of_bus_isa_match(void *blob, int parentoffset) return !strcmp(name, "isa"); } -static void of_bus_isa_count_cells(void *blob, int parentoffset, +static void of_bus_isa_count_cells(const void *blob, int parentoffset, int *addrc, int *sizec) { if (addrc) @@ -1126,7 +1126,7 @@ static struct of_bus of_busses[] = { }, }; -static struct of_bus *of_match_bus(void *blob, int parentoffset) +static struct of_bus *of_match_bus(const void *blob, int parentoffset) { struct of_bus *bus; @@ -1148,7 +1148,7 @@ static struct of_bus *of_match_bus(void *blob, int parentoffset) return NULL; } -static int of_translate_one(void * blob, int parent, struct of_bus *bus, +static int of_translate_one(const void *blob, int parent, struct of_bus *bus, struct of_bus *pbus, fdt32_t *addr, int na, int ns, int pna, const char *rprop) { @@ -1211,8 +1211,8 @@ static int of_translate_one(void * blob, int parent, struct of_bus *bus, * that can be mapped to a cpu physical address). This is not really specified * that way, but this is traditionally the way IBM at least do things */ -static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in_addr, - const char *rprop) +static u64 __of_translate_address(const void *blob, int node_offset, + const fdt32_t *in_addr, const char *rprop) { int parent; struct of_bus *bus, *pbus; @@ -1284,7 +1284,8 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in return result; } -u64 fdt_translate_address(void *blob, int node_offset, const fdt32_t *in_addr) +u64 fdt_translate_address(const void *blob, int node_offset, + const fdt32_t *in_addr) { return __of_translate_address(blob, node_offset, in_addr, "ranges"); } diff --git a/include/fdt_support.h b/include/fdt_support.h index 731809874f48..e9f3497ab642 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -180,7 +180,8 @@ static inline void fdt_fixup_mtdparts(void *fdt, void *node_info, #endif void fdt_del_node_and_alias(void *blob, const char *alias); -u64 fdt_translate_address(void *blob, int node_offset, const __be32 *in_addr); +u64 fdt_translate_address(const void *blob, int node_offset, + const __be32 *in_addr); int fdt_node_offset_by_compat_reg(void *blob, const char *compat, phys_addr_t compat_off); int fdt_alloc_phandle(void *blob); @@ -239,7 +240,7 @@ static inline u64 of_read_number(const fdt32_t *cell, int size) return r; } -void of_bus_default_count_cells(void *blob, int parentoffset, +void of_bus_default_count_cells(const void *blob, int parentoffset, int *addrc, int *sizec); int ft_verify_fdt(void *fdt); int arch_fixup_memory_node(void *blob);