[U-Boot] [PATCH 1/2 RESEND] fdt_support: fdt_translate_address() blob const correctness
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(-)
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);
From: Stephen Warren swarren@nvidia.com
Some code may want to read reg values from DT, but from nodes that aren't associated with DM devices, so using dev_get_addr_index() isn't appropriate. In this case, fdtdec_get_addr_size_*() are the functions to use. However, "translation" (via the chain of ranges properties in parent nodes) may still be desirable. Add a function parameter to request that, and implement it. Update all call sites to default to the original behaviour.
Signed-off-by: Stephen Warren swarren@nvidia.com Reviewed-by: Simon Glass sjg@chromium.org --- drivers/core/device.c | 2 +- drivers/gpio/mpc85xx_gpio.c | 2 +- drivers/i2c/fsl_i2c.c | 2 +- drivers/mmc/msm_sdhci.c | 3 ++- drivers/net/cpsw.c | 3 ++- drivers/spmi/spmi-msm.c | 5 +++-- include/fdtdec.h | 14 +++++++++++--- lib/fdtdec.c | 20 +++++++++++++------- 8 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 5bb1d7793dd6..b737f1c78907 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -671,7 +671,7 @@ fdt_addr_t dev_get_addr_index(struct udevice *dev, int index) addr = fdtdec_get_addr_size_auto_parent(gd->fdt_blob, dev->parent->of_offset, dev->of_offset, "reg", - index, NULL); + index, NULL, false); if (CONFIG_IS_ENABLED(SIMPLE_BUS) && addr != FDT_ADDR_T_NONE) { if (device_get_uclass_id(dev->parent) == UCLASS_SIMPLE_BUS) diff --git a/drivers/gpio/mpc85xx_gpio.c b/drivers/gpio/mpc85xx_gpio.c index 3754a8215c36..168c696c4dc0 100644 --- a/drivers/gpio/mpc85xx_gpio.c +++ b/drivers/gpio/mpc85xx_gpio.c @@ -170,7 +170,7 @@ static int mpc85xx_gpio_ofdata_to_platdata(struct udevice *dev) { fdt_size_t size;
addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, dev->of_offset, - "reg", 0, &size); + "reg", 0, &size, false);
plat->addr = addr; plat->size = size; diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 407c4a7b10b8..c3f826d68c75 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -587,7 +587,7 @@ static int fsl_i2c_ofdata_to_platdata(struct udevice *bus) fdt_size_t size;
addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, bus->of_offset, - "reg", 0, &size); + "reg", 0, &size, false);
dev->base = map_sysmem(CONFIG_SYS_IMMR + addr, size);
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index 70a8d96eeef3..4b95e1837f63 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -180,7 +180,8 @@ static int msm_ofdata_to_platdata(struct udevice *dev) priv->base = (void *)fdtdec_get_addr_size_auto_parent(gd->fdt_blob, parent->of_offset, dev->of_offset, - "reg", 1, NULL); + "reg", 1, NULL, + false); if (priv->base == (void *)FDT_ADDR_T_NONE || host->ioaddr == (void *)FDT_ADDR_T_NONE) return -EINVAL; diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c index 2ce4ec69f1df..2a458955ed05 100644 --- a/drivers/net/cpsw.c +++ b/drivers/net/cpsw.c @@ -1145,7 +1145,8 @@ static const struct eth_ops cpsw_eth_ops = {
static inline fdt_addr_t cpsw_get_addr_by_node(const void *fdt, int node) { - return fdtdec_get_addr_size_auto_noparent(fdt, node, "reg", 0, NULL); + return fdtdec_get_addr_size_auto_noparent(fdt, node, "reg", 0, NULL, + false); }
static int cpsw_eth_ofdata_to_platdata(struct udevice *dev) diff --git a/drivers/spmi/spmi-msm.c b/drivers/spmi/spmi-msm.c index 0cef505e37d0..48bc15759645 100644 --- a/drivers/spmi/spmi-msm.c +++ b/drivers/spmi/spmi-msm.c @@ -153,11 +153,12 @@ static int msm_spmi_probe(struct udevice *dev) priv->spmi_core = fdtdec_get_addr_size_auto_parent(gd->fdt_blob, parent->of_offset, dev->of_offset, - "reg", 1, NULL); + "reg", 1, NULL, + false); priv->spmi_obs = fdtdec_get_addr_size_auto_parent(gd->fdt_blob, parent->of_offset, dev->of_offset, "reg", - 2, NULL); + 2, NULL, false); if (priv->arb_chnl == FDT_ADDR_T_NONE || priv->spmi_core == FDT_ADDR_T_NONE || priv->spmi_obs == FDT_ADDR_T_NONE) diff --git a/include/fdtdec.h b/include/fdtdec.h index 70ea0bfc0bae..aeb6bab1c4c6 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -297,11 +297,13 @@ int fdtdec_next_compatible_subnode(const void *blob, int node, * @param na the number of cells used to represent an address * @param ns the number of cells used to represent a size * @param sizep a pointer to store the size into. Use NULL if not required + * @param translate Indicates whether to translate the returned value + * using the parent node's ranges property. * @return address, if found, or FDT_ADDR_T_NONE if not */ fdt_addr_t fdtdec_get_addr_size_fixed(const void *blob, int node, const char *prop_name, int index, int na, int ns, - fdt_size_t *sizep); + fdt_size_t *sizep, bool translate);
/* * Look up an address property in a node and return the parsed address, and @@ -317,10 +319,13 @@ fdt_addr_t fdtdec_get_addr_size_fixed(const void *blob, int node, * @param prop_name name of property to find * @param index which address to retrieve from a list of addresses. Often 0. * @param sizep a pointer to store the size into. Use NULL if not required + * @param translate Indicates whether to translate the returned value + * using the parent node's ranges property. * @return address, if found, or FDT_ADDR_T_NONE if not */ fdt_addr_t fdtdec_get_addr_size_auto_parent(const void *blob, int parent, - int node, const char *prop_name, int index, fdt_size_t *sizep); + int node, const char *prop_name, int index, fdt_size_t *sizep, + bool translate);
/* * Look up an address property in a node and return the parsed address, and @@ -340,10 +345,13 @@ fdt_addr_t fdtdec_get_addr_size_auto_parent(const void *blob, int parent, * @param prop_name name of property to find * @param index which address to retrieve from a list of addresses. Often 0. * @param sizep a pointer to store the size into. Use NULL if not required + * @param translate Indicates whether to translate the returned value + * using the parent node's ranges property. * @return address, if found, or FDT_ADDR_T_NONE if not */ fdt_addr_t fdtdec_get_addr_size_auto_noparent(const void *blob, int node, - const char *prop_name, int index, fdt_size_t *sizep); + const char *prop_name, int index, fdt_size_t *sizep, + bool translate);
/* * Look up an address property in a node and return the parsed address. diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 462a24f96a97..7d99bdb8ca47 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -9,6 +9,7 @@ #include <errno.h> #include <serial.h> #include <libfdt.h> +#include <fdt_support.h> #include <fdtdec.h> #include <asm/sections.h> #include <linux/ctype.h> @@ -77,7 +78,7 @@ const char *fdtdec_get_compatible(enum fdt_compat_id id)
fdt_addr_t fdtdec_get_addr_size_fixed(const void *blob, int node, const char *prop_name, int index, int na, int ns, - fdt_size_t *sizep) + fdt_size_t *sizep, bool translate) { const fdt32_t *prop, *prop_end; const fdt32_t *prop_addr, *prop_size, *prop_after_size; @@ -112,7 +113,10 @@ fdt_addr_t fdtdec_get_addr_size_fixed(const void *blob, int node, return FDT_ADDR_T_NONE; }
- addr = fdtdec_get_number(prop_addr, na); + if (translate) + addr = fdt_translate_address(blob, node, prop_addr); + else + addr = fdtdec_get_number(prop_addr, na);
if (sizep) { *sizep = fdtdec_get_number(prop_size, ns); @@ -126,7 +130,8 @@ fdt_addr_t fdtdec_get_addr_size_fixed(const void *blob, int node, }
fdt_addr_t fdtdec_get_addr_size_auto_parent(const void *blob, int parent, - int node, const char *prop_name, int index, fdt_size_t *sizep) + int node, const char *prop_name, int index, fdt_size_t *sizep, + bool translate) { int na, ns;
@@ -147,11 +152,12 @@ fdt_addr_t fdtdec_get_addr_size_auto_parent(const void *blob, int parent, debug("na=%d, ns=%d, ", na, ns);
return fdtdec_get_addr_size_fixed(blob, node, prop_name, index, na, - ns, sizep); + ns, sizep, translate); }
fdt_addr_t fdtdec_get_addr_size_auto_noparent(const void *blob, int node, - const char *prop_name, int index, fdt_size_t *sizep) + const char *prop_name, int index, fdt_size_t *sizep, + bool translate) { int parent;
@@ -164,7 +170,7 @@ fdt_addr_t fdtdec_get_addr_size_auto_noparent(const void *blob, int node, }
return fdtdec_get_addr_size_auto_parent(blob, parent, node, prop_name, - index, sizep); + index, sizep, translate); }
fdt_addr_t fdtdec_get_addr_size(const void *blob, int node, @@ -174,7 +180,7 @@ fdt_addr_t fdtdec_get_addr_size(const void *blob, int node,
return fdtdec_get_addr_size_fixed(blob, node, prop_name, 0, sizeof(fdt_addr_t) / sizeof(fdt32_t), - ns, sizep); + ns, sizep, false); }
fdt_addr_t fdtdec_get_addr(const void *blob, int node,
On 5 August 2016 at 09:47, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
Some code may want to read reg values from DT, but from nodes that aren't associated with DM devices, so using dev_get_addr_index() isn't appropriate. In this case, fdtdec_get_addr_size_*() are the functions to use. However, "translation" (via the chain of ranges properties in parent nodes) may still be desirable. Add a function parameter to request that, and implement it. Update all call sites to default to the original behaviour.
Signed-off-by: Stephen Warren swarren@nvidia.com Reviewed-by: Simon Glass sjg@chromium.org
drivers/core/device.c | 2 +- drivers/gpio/mpc85xx_gpio.c | 2 +- drivers/i2c/fsl_i2c.c | 2 +- drivers/mmc/msm_sdhci.c | 3 ++- drivers/net/cpsw.c | 3 ++- drivers/spmi/spmi-msm.c | 5 +++-- include/fdtdec.h | 14 +++++++++++--- lib/fdtdec.c | 20 +++++++++++++------- 8 files changed, 34 insertions(+), 17 deletions(-)
Applied to u-boot-dm, thanks!
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
participants (2)
-
Simon Glass -
Stephen Warren