On Sun, 22 Nov 2020 at 09:11, Dario Binacchi <dario...@libero.it> wrote:
>
> The __of_translate_address routine translates an address from the
> device tree into a CPU physical address. A note in the description of
> the routine explains that the crossing of any level with
> #size-cells = <0> is to be considered an error not by specification but
> since inherited from IBM. This does not happen for Texas Instruments, or
> at least for the beaglebone device tree. Without this patch, in fact,
> the translation into physical addresses of the registers contained in the
> am33xx-clocks.dtsi nodes would not be possible. They all have a parent
> with #size-cells = <0>.
>
> The CONFIG_OF_TRANSLATE_ZERO_SIZE_CELLS symbol makes translation
> possible even in the case of crossing levels with #size-cells = <0>.
>
> The patch acts conservatively on address translation, except for
> removing a check within the of_translate_one function in the
> drivers/core/of_addr.c file:
>
> +
> ranges = of_get_property(parent, rprop, &rlen);
> - if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
> - debug("no ranges; cannot translate\n");
> - return 1;
> - }
> if (ranges == NULL || rlen == 0) {
> offset = of_read_number(addr, na);
> memset(addr, 0, pna * 4);
> debug("empty ranges; 1:1 translation\n");
>
> There are two reasons:
> 1 The function of_empty_ranges_quirk always returns false, invalidating
> the following if statement in case of null ranges. Therefore one of
> the two checks is useless.
>
> 2 The implementation of the of_translate_one function found in the
> common/fdt_support.c file has removed this check while keeping the one
> about the 1:1 translation.
>
> The patch adds a test and modifies a check for the correctness of an
> address in the case of enabling translation also for zero size cells.
> The added test checks translations of addresses generated by nodes of
> a device tree similar to those you can find in the files am33xx.dtsi
> and am33xx-clocks.dtsi for which the patch was created.
>
> The patch was also tested on a beaglebone black board. The addresses
> generated for the registers of the loaded drivers are those specified
> by the AM335x reference manual.
>
> Signed-off-by: Dario Binacchi <dario...@libero.it>
> Tested-by: Dario Binacchi <dario...@libero.it>
> Reviewed-by: Simon Glass <s...@chromium.org>
>
> ---
>
> (no changes since v4)
>
> Changes in v4:
> - Add Sphinx documentation for dm_flags.
> - Convert GD_DM_FLG_* to enum.
> - Include device_compat.h header in test/dm/test-fdt.c for dev_xxx macros.
>
> Changes in v3:
> - Comment dm_flags field in the global_data structure.
>
> Changes in v2:
> - Fix a missing line in the commit message.
> - Add dm_flags to global_data structure and GD_DM_FLG_SIZE_CELLS_0 macro
> to test without recompiling.
> - Update the OF_CHECK_COUNTS macro in order to have just one
> #define by bringing the GD_DM_FLG_SIZE_CELLS_0 into the expression.
> - Lower-case the 0xC019 hex number.
>
> arch/sandbox/dts/test.dts | 21 ++++++++++
> common/fdt_support.c | 6 ++-
> drivers/core/Kconfig | 12 ++++++
> drivers/core/fdtaddr.c | 2 +-
> drivers/core/of_addr.c | 14 ++-----
> drivers/core/ofnode.c | 7 +++-
> drivers/core/root.c | 3 ++
> include/asm-generic/global_data.h | 18 ++++++++
> test/dm/test-fdt.c | 69 ++++++++++++++++++++++++++++++-
> 9 files changed, 136 insertions(+), 16 deletions(-)
Still looks good to me.
