Re: [PATCH v2 2/2] of: Add unit tests for applying overlays

Rob Herring
2017-04-25T16:45:02Z

On Mon, Apr 24, 2017 at 6:05 PM, <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@sony.com>
>
> Existing overlay unit tests examine individual pieces of the overlay
> code. The new tests target the entire process of applying an overlay.

Just a few nits.

> Signed-off-by: Frank Rowand <frank.rowand@sony.com>

[...]

> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 18bbb4517e25..cc76b3b81eab 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -55,6 +55,17 @@ static inline int of_property_notify(int action, struct device_node *np,
> }
> #endif /* CONFIG_OF_DYNAMIC */
>
> +#ifdef CONFIG_OF_UNITTEST
> +extern void __init unittest_unflatten_overlay_base(void);
> +extern void *__unflatten_device_tree(const void *blob,

This can and should be outside the ifdef.

> + struct device_node *dad,
> + struct device_node **mynodes,
> + void *(*dt_alloc)(u64 size, u64 align),
> + bool detached);
> +#else
> +static inline void unittest_unflatten_overlay_base(void) {};
> +#endif
> +
> /**
> * General utilities for working with live trees.
> *

[...]

> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 62db55b97c10..884f6c1f8ae9 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -8,6 +8,7 @@
> #include <linux/err.h>
> #include <linux/errno.h>
> #include <linux/hashtable.h>
> +#include <linux/libfdt.h>
> #include <linux/of.h>
> #include <linux/of_fdt.h>
> #include <linux/of_irq.h>
> @@ -1925,6 +1926,320 @@ static void __init of_unittest_overlay(void)
> static inline void __init of_unittest_overlay(void) { }
> #endif
>
> +#ifdef CONFIG_OF_OVERLAY

This can move down to...

> +
> +/*
> + * __dtb_ot_begin[] and __dtb_ot_end[] are created by cmd_dt_S_dtb
> + * in scripts/Makefile.lib
> + */
> +
> +#define OVERLAY_INFO_EXTERN(name) \
> + extern uint8_t __dtb_##name##_begin[]; \
> + extern uint8_t __dtb_##name##_end[]
> +
> +#define OVERLAY_INFO(name, expected) \
> +{ .dtb_begin = __dtb_##name##_begin, \
> + .dtb_end = __dtb_##name##_end, \
> + .expected_result = expected, \
> +}
> +
> +struct overlay_info {
> + uint8_t *dtb_begin;
> + uint8_t *dtb_end;
> + void *data;
> + struct device_node *np_overlay;
> + int expected_result;
> + int overlay_id;
> +};
> +
> +OVERLAY_INFO_EXTERN(overlay_base);
> +OVERLAY_INFO_EXTERN(overlay);
> +OVERLAY_INFO_EXTERN(overlay_bad_phandle);

...here. Maybe we want to move all this to a separate file instead.

> +
> +/* order of entries is hard-coded into users of overlays[] */
> +struct overlay_info overlays[] = {

static?

> + OVERLAY_INFO(overlay_base, -9999),
> + OVERLAY_INFO(overlay, 0),
> + OVERLAY_INFO(overlay_bad_phandle, -EINVAL),
> + {}
> +};

[...]

> @@ -1962,6 +2277,9 @@ static int __init of_unittest(void)
> /* Double check linkage after removing testcase data */
> of_unittest_check_tree_linkage();
>
> +

Extra blank line.

> + of_unittest_overlay_high_level();
> +
> pr_info("end of unittest - %i passed, %i failed\n",
> unittest_results.passed, unittest_results.failed);
>
> --
> Frank Rowand <frank.rowand@sony.com>
>