> On 29 Aug 2017, at 22:15, Simon Glass <s...@chromium.org> wrote:
>
> When using 32-bit addresses dtoc works correctly. For 64-bit addresses it
> does not since it ignores the #address-cells and #size-cells properties.
>
> Update the tool to use fdt64_t as the element type for reg properties when
> either the address or size is larger than one cell. Use the correct value
> so that C code can obtain the information from the device tree easily.
>
> Alos create a new type, fdt_val_t, which is defined to either fdt32_t or
> fdt64_t depending on the word size of the machine. This type corresponds
> to fdt_addr_t and fdt_size_t. Unfortunately we cannot just use those types
> since they are defined to phys_addr_t and phys_size_t which use
> 'unsigned long' in the 32-bit case, rather than 'unsigned int'.
>
> Add tests for the four combinations of address and size values (32/32,
> 64/64, 32/64, 64/32). Also update existing uses for rk3399 and rk3368
> which now need to use the new fdt_val_t type.
>
> Signed-off-by: Simon Glass <s...@chromium.org>
>
> Suggested-by: Heiko Stuebner <he...@sntech.de>
> Reported-by: Kever Yang <kever.y...@rock-chips.com>
Reviewed-by: Philipp Tomsich <philipp.toms...@theobroma-systems.com>
See below.
> ---
>
> Changes in v2:
> - Support 'reg' properties with a single cell (e.g. #size-cells = 0)
> - Introduce an fdt_val_t type which is either 32- or 64-bits long
> - Update rk3368 and rk3399 uses
> - Drop review tags since there are significant changes in this patch
>
> drivers/clk/rockchip/clk_rk3368.c | 2 +-
> drivers/clk/rockchip/clk_rk3399.c | 4 +-
> drivers/core/regmap.c | 2 +-
> include/fdtdec.h | 2 +
> include/regmap.h | 2 +-
> include/syscon.h | 6 +-
> tools/dtoc/dtb_platdata.py | 61 +++++++++++
> tools/dtoc/dtoc_test_addr32.dts | 27 +++++
> tools/dtoc/dtoc_test_addr32_64.dts | 33 ++++++
> tools/dtoc/dtoc_test_addr64.dts | 33 ++++++
> tools/dtoc/dtoc_test_addr64_32.dts | 33 ++++++
> tools/dtoc/fdt_util.py | 2 +
> tools/dtoc/test_dtoc.py | 212 +++++++++++++++++++++++++++++++++++++
> 13 files changed, 413 insertions(+), 6 deletions(-)
> create mode 100644 tools/dtoc/dtoc_test_addr32.dts
> create mode 100644 tools/dtoc/dtoc_test_addr32_64.dts
> create mode 100644 tools/dtoc/dtoc_test_addr64.dts
> create mode 100644 tools/dtoc/dtoc_test_addr64_32.dts
>
> diff --git a/drivers/clk/rockchip/clk_rk3368.c
> b/drivers/clk/rockchip/clk_rk3368.c
> index 2be1f572d7..0160d50c03 100644
> --- a/drivers/clk/rockchip/clk_rk3368.c
> +++ b/drivers/clk/rockchip/clk_rk3368.c
> @@ -471,7 +471,7 @@ static int rk3368_clk_probe(struct udevice *dev)
> #if CONFIG_IS_ENABLED(OF_PLATDATA)
> struct rk3368_clk_plat *plat = dev_get_platdata(dev);
>
> - priv->cru = map_sysmem(plat->dtd.reg[1], plat->dtd.reg[3]);
> + priv->cru = map_sysmem(plat->dtd.reg[0], plat->dtd.reg[1]);
> #endif
> #if IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_TPL_BUILD)
> rkclk_init(priv->cru);
> diff --git a/drivers/clk/rockchip/clk_rk3399.c
> b/drivers/clk/rockchip/clk_rk3399.c
> index 3edafea140..0de3db620d 100644
> --- a/drivers/clk/rockchip/clk_rk3399.c
> +++ b/drivers/clk/rockchip/clk_rk3399.c
> @@ -963,7 +963,7 @@ static int rk3399_clk_probe(struct udevice *dev)
> #if CONFIG_IS_ENABLED(OF_PLATDATA)
> struct rk3399_clk_plat *plat = dev_get_platdata(dev);
>
> - priv->cru = map_sysmem(plat->dtd.reg[1], plat->dtd.reg[3]);
> + priv->cru = map_sysmem(plat->dtd.reg[0], plat->dtd.reg[0]);
The second argument needs to be reg[1].
> #endif
> rkclk_init(priv->cru);
> #endif
> @@ -1145,7 +1145,7 @@ static int rk3399_pmuclk_probe(struct udevice *dev)
> #if CONFIG_IS_ENABLED(OF_PLATDATA)
> struct rk3399_pmuclk_plat *plat = dev_get_platdata(dev);
>
> - priv->pmucru = map_sysmem(plat->dtd.reg[1], plat->dtd.reg[3]);
> + priv->pmucru = map_sysmem(plat->dtd.reg[0], plat->dtd.reg[1]);
> #endif
>
> #ifndef CONFIG_SPL_BUILD
> diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
> index d4e16a27ef..0f1d30820c 100644
> --- a/drivers/core/regmap.c
> +++ b/drivers/core/regmap.c
> @@ -40,7 +40,7 @@ static struct regmap *regmap_alloc_count(int count)
> }
>
> #if CONFIG_IS_ENABLED(OF_PLATDATA)
> -int regmap_init_mem_platdata(struct udevice *dev, u32 *reg, int count,
> +int regmap_init_mem_platdata(struct udevice *dev, fdt_val_t *reg, int count,
> struct regmap **mapp)
> {
> struct regmap_range *range;
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 4a0947c626..1ba02be8e1 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -27,10 +27,12 @@ typedef phys_size_t fdt_size_t;
> #define FDT_ADDR_T_NONE (-1ULL)
> #define fdt_addr_to_cpu(reg) be64_to_cpu(reg)
> #define fdt_size_to_cpu(reg) be64_to_cpu(reg)
> +typedef fdt64_t fdt_val_t;
> #else
> #define FDT_ADDR_T_NONE (-1U)
> #define fdt_addr_to_cpu(reg) be32_to_cpu(reg)
> #define fdt_size_to_cpu(reg) be32_to_cpu(reg)
> +typedef fdt32_t fdt_val_t;
> #endif
>
> /* Information obtained about memory from the FDT */
> diff --git a/include/regmap.h b/include/regmap.h
> index 1eed94e47a..493a5d8eff 100644
> --- a/include/regmap.h
> +++ b/include/regmap.h
> @@ -69,7 +69,7 @@ int regmap_init_mem(struct udevice *dev, struct regmap
> **mapp);
> * @count: Number of pairs (e.g. 1 if the regmap has a single entry)
> * @mapp: Returns allocated map
> */
> -int regmap_init_mem_platdata(struct udevice *dev, u32 *reg, int count,
> +int regmap_init_mem_platdata(struct udevice *dev, fdt_val_t *reg, int count,
> struct regmap **mapp);
>
> /**
> diff --git a/include/syscon.h b/include/syscon.h
> index 34842aa470..5d52b1cc3c 100644
> --- a/include/syscon.h
> +++ b/include/syscon.h
> @@ -8,6 +8,8 @@
> #ifndef __SYSCON_H
> #define __SYSCON_H
>
> +#include <fdtdec.h>
> +
> /**
> * struct syscon_uc_info - Information stored by the syscon UCLASS_UCLASS
> *
> @@ -28,9 +30,11 @@ struct syscon_ops {
> * We don't support 64-bit machines. If they are so resource-contrained that
> * they need to use OF_PLATDATA, something is horribly wrong with the
> * education of our hardware engineers.
> + *
> + * Update: 64-bit is now supported and we have an education crisis.
> */
> struct syscon_base_platdata {
> - u32 reg[2];
> + fdt_val_t reg[2];
> };
> #endif
>
> diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
> index 3243bccfe8..0c719310b1 100644
> --- a/tools/dtoc/dtb_platdata.py
> +++ b/tools/dtoc/dtb_platdata.py
> @@ -242,6 +242,66 @@ class DtbPlatdata(object):
> self._valid_nodes = []
> return self.scan_node(self._fdt.GetRoot())
>
> + @staticmethod
> + def get_num_cells(node):
> + """Get the number of cells in addresses and sizes for this node
> +
> + Args:
> + node: Node to check
> +
> + Returns:
> + Tuple:
> + Number of address cells for this node
> + Number of size cells for this node
> + """
> + parent = node.parent
> + na, ns = 2, 2
> + if parent:
> + na_prop = parent.props.get('#address-cells')
> + ns_prop = parent.props.get('#size-cells')
> + if na_prop:
> + na = fdt_util.fdt32_to_cpu(na_prop.value)
> + if ns_prop:
> + ns = fdt_util.fdt32_to_cpu(ns_prop.value)
> + return na, ns
> +
> + def scan_reg_sizes(self):
> + """Scan for 64-bit 'reg' properties and update the values
> +
> + This finds 'reg' properties with 64-bit data and converts the value
> to
> + an array of 64-values. This allows it to be output in a way that the
> + C code can read.
> + """
> + for node in self._valid_nodes:
> + reg = node.props.get('reg')
> + if not reg:
> + continue
> + na, ns = self.get_num_cells(node)
> + total = na + ns
> +
> + if reg.type != fdt.TYPE_INT:
> + raise ValueError("Node '%s' reg property is not an int")
> + if len(reg.value) % total:
> + raise ValueError("Node '%s' reg property has %d cells "
> + 'which is not a multiple of na + ns = %d + %d)' %
> + (node.name, len(reg.value), na, ns))
> + reg.na = na
> + reg.ns = ns
> + if na != 1 or ns != 1:
> + reg.type = fdt.TYPE_INT64
> + i = 0
> + new_value = []
> + val = reg.value
> + if not isinstance(val, list):
> + val = [val]
> + while i < len(val):
> + addr = fdt_util.fdt_cells_to_cpu(val[i:], reg.na)
> + i += na
> + size = fdt_util.fdt_cells_to_cpu(val[i:], reg.ns)
> + i += ns
> + new_value += [addr, size]
> + reg.value = new_value
> +
> def scan_structs(self):
> """Scan the device tree building up the C structures we will use.
>
> @@ -450,6 +510,7 @@ def run_steps(args, dtb_file, include_disabled, output):
> plat = DtbPlatdata(dtb_file, include_disabled)
> plat.scan_dtb()
> plat.scan_tree()
> + plat.scan_reg_sizes()
> plat.setup_output(output)
> structs = plat.scan_structs()
> plat.scan_phandles()
> diff --git a/tools/dtoc/dtoc_test_addr32.dts b/tools/dtoc/dtoc_test_addr32.dts
> new file mode 100644
> index 0000000000..bcfdcae10b
> --- /dev/null
> +++ b/tools/dtoc/dtoc_test_addr32.dts
> @@ -0,0 +1,27 @@
> +/*
> + * Test device tree file for dtoc
> + *
> + * Copyright 2017 Google, Inc
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> + /dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + test1 {
> + u-boot,dm-pre-reloc;
> + compatible = "test1";
> + reg = <0x1234 0x5678>;
> + };
> +
> + test2 {
> + u-boot,dm-pre-reloc;
> + compatible = "test2";
> + reg = <0x12345678 0x98765432 2 3>;
> + };
> +
> +};
> diff --git a/tools/dtoc/dtoc_test_addr32_64.dts
> b/tools/dtoc/dtoc_test_addr32_64.dts
> new file mode 100644
> index 0000000000..1c96243310
> --- /dev/null
> +++ b/tools/dtoc/dtoc_test_addr32_64.dts
> @@ -0,0 +1,33 @@
> +/*
> + * Test device tree file for dtoc
> + *
> + * Copyright 2017 Google, Inc
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> + /dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <2>;
> +
> + test1 {
> + u-boot,dm-pre-reloc;
> + compatible = "test1";
> + reg = <0x1234 0x5678 0x0>;
> + };
> +
> + test2 {
> + u-boot,dm-pre-reloc;
> + compatible = "test2";
> + reg = <0x12345678 0x98765432 0x10987654>;
> + };
> +
> + test3 {
> + u-boot,dm-pre-reloc;
> + compatible = "test3";
> + reg = <0x12345678 0x98765432 0x10987654 2 0 3>;
> + };
> +
> +};
> diff --git a/tools/dtoc/dtoc_test_addr64.dts b/tools/dtoc/dtoc_test_addr64.dts
> new file mode 100644
> index 0000000000..4c0ad0ec36
> --- /dev/null
> +++ b/tools/dtoc/dtoc_test_addr64.dts
> @@ -0,0 +1,33 @@
> +/*
> + * Test device tree file for dtoc
> + *
> + * Copyright 2017 Google, Inc
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> + /dts-v1/;
> +
> +/ {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + test1 {
> + u-boot,dm-pre-reloc;
> + compatible = "test1";
> + reg = /bits/ 64 <0x1234 0x5678>;
> + };
> +
> + test2 {
> + u-boot,dm-pre-reloc;
> + compatible = "test2";
> + reg = /bits/ 64 <0x1234567890123456 0x9876543210987654>;
> + };
> +
> + test3 {
> + u-boot,dm-pre-reloc;
> + compatible = "test3";
> + reg = /bits/ 64 <0x1234567890123456 0x9876543210987654 2 3>;
> + };
> +
> +};
> diff --git a/tools/dtoc/dtoc_test_addr64_32.dts
> b/tools/dtoc/dtoc_test_addr64_32.dts
> new file mode 100644
> index 0000000000..c36f6b726e
> --- /dev/null
> +++ b/tools/dtoc/dtoc_test_addr64_32.dts
> @@ -0,0 +1,33 @@
> +/*
> + * Test device tree file for dtoc
> + *
> + * Copyright 2017 Google, Inc
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> + /dts-v1/;
> +
> +/ {
> + #address-cells = <2>;
> + #size-cells = <1>;
> +
> + test1 {
> + u-boot,dm-pre-reloc;
> + compatible = "test1";
> + reg = <0x1234 0x0 0x5678>;
> + };
> +
> + test2 {
> + u-boot,dm-pre-reloc;
> + compatible = "test2";
> + reg = <0x12345678 0x90123456 0x98765432>;
> + };
> +
> + test3 {
> + u-boot,dm-pre-reloc;
> + compatible = "test3";
> + reg = <0x12345678 0x90123456 0x98765432 0 2 3>;
> + };
> +
> +};
> diff --git a/tools/dtoc/fdt_util.py b/tools/dtoc/fdt_util.py
> index bec6ee947a..338d47a5e1 100644
> --- a/tools/dtoc/fdt_util.py
> +++ b/tools/dtoc/fdt_util.py
> @@ -38,6 +38,8 @@ def fdt_cells_to_cpu(val, cells):
> Return:
> A native-endian long value
> """
> + if not cells:
> + return 0
> out = long(fdt32_to_cpu(val[0]))
> if cells == 2:
> out = out << 32 | fdt32_to_cpu(val[1])
> diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
> index 5040f23325..09cfddaaaa 100644
> --- a/tools/dtoc/test_dtoc.py
> +++ b/tools/dtoc/test_dtoc.py
> @@ -270,4 +270,216 @@ U_BOOT_DEVICE(spl_test) = {
> \t.platdata_size\t= sizeof(dtv_spl_test),
> };
>
> +''', data)
> +
> + def test_addresses64(self):
> + """Test output from a node with a 'reg' property with na=2, ns=2"""
> + dtb_file = get_dtb_file('dtoc_test_addr64.dts')
> + output = tools.GetOutputFilename('output')
> + dtb_platdata.run_steps(['struct'], dtb_file, False, output)
> + with open(output) as infile:
> + data = infile.read()
> + self.assertEqual('''#include <stdbool.h>
> +#include <libfdt.h>
> +struct dtd_test1 {
> +\tfdt64_t\t\treg[2];
> +};
> +struct dtd_test2 {
> +\tfdt64_t\t\treg[2];
> +};
> +struct dtd_test3 {
> +\tfdt64_t\t\treg[4];
> +};
> +''', data)
> +
> + dtb_platdata.run_steps(['platdata'], dtb_file, False, output)
> + with open(output) as infile:
> + data = infile.read()
> + self.assertEqual('''#include <common.h>
> +#include <dm.h>
> +#include <dt-structs.h>
> +
> +static struct dtd_test1 dtv_test1 = {
> +\t.reg\t\t\t= {0x1234, 0x5678},
> +};
> +U_BOOT_DEVICE(test1) = {
> +\t.name\t\t= "test1",
> +\t.platdata\t= &dtv_test1,
> +\t.platdata_size\t= sizeof(dtv_test1),
> +};
> +
> +static struct dtd_test2 dtv_test2 = {
> +\t.reg\t\t\t= {0x1234567890123456, 0x9876543210987654},
> +};
> +U_BOOT_DEVICE(test2) = {
> +\t.name\t\t= "test2",
> +\t.platdata\t= &dtv_test2,
> +\t.platdata_size\t= sizeof(dtv_test2),
> +};
> +
> +static struct dtd_test3 dtv_test3 = {
> +\t.reg\t\t\t= {0x1234567890123456, 0x9876543210987654, 0x2, 0x3},
> +};
> +U_BOOT_DEVICE(test3) = {
> +\t.name\t\t= "test3",
> +\t.platdata\t= &dtv_test3,
> +\t.platdata_size\t= sizeof(dtv_test3),
> +};
> +
> +''', data)
> +
> + def test_addresses32(self):
> + """Test output from a node with a 'reg' property with na=1, ns=1"""
> + dtb_file = get_dtb_file('dtoc_test_addr32.dts')
> + output = tools.GetOutputFilename('output')
> + dtb_platdata.run_steps(['struct'], dtb_file, False, output)
> + with open(output) as infile:
> + data = infile.read()
> + self.assertEqual('''#include <stdbool.h>
> +#include <libfdt.h>
> +struct dtd_test1 {
> +\tfdt32_t\t\treg[2];
> +};
> +struct dtd_test2 {
> +\tfdt32_t\t\treg[4];
> +};
> +''', data)
> +
> + dtb_platdata.run_steps(['platdata'], dtb_file, False, output)
> + with open(output) as infile:
> + data = infile.read()
> + self.assertEqual('''#include <common.h>
> +#include <dm.h>
> +#include <dt-structs.h>
> +
> +static struct dtd_test1 dtv_test1 = {
> +\t.reg\t\t\t= {0x1234, 0x5678},
> +};
> +U_BOOT_DEVICE(test1) = {
> +\t.name\t\t= "test1",
> +\t.platdata\t= &dtv_test1,
> +\t.platdata_size\t= sizeof(dtv_test1),
> +};
> +
> +static struct dtd_test2 dtv_test2 = {
> +\t.reg\t\t\t= {0x12345678, 0x98765432, 0x2, 0x3},
> +};
> +U_BOOT_DEVICE(test2) = {
> +\t.name\t\t= "test2",
> +\t.platdata\t= &dtv_test2,
> +\t.platdata_size\t= sizeof(dtv_test2),
> +};
> +
> +''', data)
> +
> + def test_addresses64_32(self):
> + """Test output from a node with a 'reg' property with na=2, ns=1"""
> + dtb_file = get_dtb_file('dtoc_test_addr64_32.dts')
> + output = tools.GetOutputFilename('output')
> + dtb_platdata.run_steps(['struct'], dtb_file, False, output)
> + with open(output) as infile:
> + data = infile.read()
> + self.assertEqual('''#include <stdbool.h>
> +#include <libfdt.h>
> +struct dtd_test1 {
> +\tfdt64_t\t\treg[2];
> +};
> +struct dtd_test2 {
> +\tfdt64_t\t\treg[2];
> +};
> +struct dtd_test3 {
> +\tfdt64_t\t\treg[4];
> +};
> +''', data)
> +
> + dtb_platdata.run_steps(['platdata'], dtb_file, False, output)
> + with open(output) as infile:
> + data = infile.read()
> + self.assertEqual('''#include <common.h>
> +#include <dm.h>
> +#include <dt-structs.h>
> +
> +static struct dtd_test1 dtv_test1 = {
> +\t.reg\t\t\t= {0x123400000000, 0x5678},
> +};
> +U_BOOT_DEVICE(test1) = {
> +\t.name\t\t= "test1",
> +\t.platdata\t= &dtv_test1,
> +\t.platdata_size\t= sizeof(dtv_test1),
> +};
> +
> +static struct dtd_test2 dtv_test2 = {
> +\t.reg\t\t\t= {0x1234567890123456, 0x98765432},
> +};
> +U_BOOT_DEVICE(test2) = {
> +\t.name\t\t= "test2",
> +\t.platdata\t= &dtv_test2,
> +\t.platdata_size\t= sizeof(dtv_test2),
> +};
> +
> +static struct dtd_test3 dtv_test3 = {
> +\t.reg\t\t\t= {0x1234567890123456, 0x98765432, 0x2, 0x3},
> +};
> +U_BOOT_DEVICE(test3) = {
> +\t.name\t\t= "test3",
> +\t.platdata\t= &dtv_test3,
> +\t.platdata_size\t= sizeof(dtv_test3),
> +};
> +
> +''', data)
> +
> + def test_addresses32_64(self):
> + """Test output from a node with a 'reg' property with na=1, ns=2"""
> + dtb_file = get_dtb_file('dtoc_test_addr32_64.dts')
> + output = tools.GetOutputFilename('output')
> + dtb_platdata.run_steps(['struct'], dtb_file, False, output)
> + with open(output) as infile:
> + data = infile.read()
> + self.assertEqual('''#include <stdbool.h>
> +#include <libfdt.h>
> +struct dtd_test1 {
> +\tfdt64_t\t\treg[2];
> +};
> +struct dtd_test2 {
> +\tfdt64_t\t\treg[2];
> +};
> +struct dtd_test3 {
> +\tfdt64_t\t\treg[4];
> +};
> +''', data)
> +
> + dtb_platdata.run_steps(['platdata'], dtb_file, False, output)
> + with open(output) as infile:
> + data = infile.read()
> + self.assertEqual('''#include <common.h>
> +#include <dm.h>
> +#include <dt-structs.h>
> +
> +static struct dtd_test1 dtv_test1 = {
> +\t.reg\t\t\t= {0x1234, 0x567800000000},
> +};
> +U_BOOT_DEVICE(test1) = {
> +\t.name\t\t= "test1",
> +\t.platdata\t= &dtv_test1,
> +\t.platdata_size\t= sizeof(dtv_test1),
> +};
> +
> +static struct dtd_test2 dtv_test2 = {
> +\t.reg\t\t\t= {0x12345678, 0x9876543210987654},
> +};
> +U_BOOT_DEVICE(test2) = {
> +\t.name\t\t= "test2",
> +\t.platdata\t= &dtv_test2,
> +\t.platdata_size\t= sizeof(dtv_test2),
> +};
> +
> +static struct dtd_test3 dtv_test3 = {
> +\t.reg\t\t\t= {0x12345678, 0x9876543210987654, 0x2, 0x3},
> +};
> +U_BOOT_DEVICE(test3) = {
> +\t.name\t\t= "test3",
> +\t.platdata\t= &dtv_test3,
> +\t.platdata_size\t= sizeof(dtv_test3),
> +};
> +
> ''', data)
> --
> 2.14.1.342.g6490525c54-goog
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
