[PATCH] checks: fix handling of unresolved phandles for dts plugins

[PATCH] checks: fix handling of unresolved phandles for dts plugins

[Rob Herring]

In dts plugins, it is valid to have unresolved phandle values. The
check_property_phandle_args and check_interrupts_property checks failed to
account for this resulting in spurious warnings or asserts, respectively.
Fix this by bailing from the checks if we're checking a dts plugin as
there is no way to further validate the properties.

Fixes: ee3d26f6960b ("checks: add interrupts property check")
Fixes: b3bbac02d5e3 ("checks: add phandle with arg property checks")
Reported-by: Alan Tull <atull-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 checks.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/checks.c b/checks.c
index 08a3a29edae3..e66138449886 100644
--- a/checks.c
+++ b/checks.c
@@ -988,6 +988,10 @@ static void check_property_phandle_args(struct check *c,
 		 * entries when each index position has a specific definition.
 		 */
 		if (phandle == 0 || phandle == -1) {
+			/* Give up if this is an overlay with external references */
+			if (dti->dtsflags & DTSF_PLUGIN)
+				break;
+
 			cellsize = 0;
 			continue;
 		}
@@ -1176,6 +1180,11 @@ static void check_interrupts_property(struct check *c,
 		prop = get_property(parent, "interrupt-parent");
 		if (prop) {
 			phandle = propval_cell(prop);
+			/* Give up if this is an overlay with external references */
+			if ((phandle == 0 || phandle == -1) &&
+			    (dti->dtsflags & DTSF_PLUGIN))
+					return;
+
 			irq_node = get_node_by_phandle(root, phandle);
 			if (!irq_node) {
 				FAIL(c, dti, "Bad interrupt-parent phandle for %s",
-- 
2.11.0

Re: [PATCH] checks: fix handling of unresolved phandles for dts plugins

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2070 bytes --]

On Wed, Oct 18, 2017 at 04:29:14PM -0500, Rob Herring wrote:
> In dts plugins, it is valid to have unresolved phandle values. The
> check_property_phandle_args and check_interrupts_property checks failed to
> account for this resulting in spurious warnings or asserts, respectively.
> Fix this by bailing from the checks if we're checking a dts plugin as
> there is no way to further validate the properties.
> 
> Fixes: ee3d26f6960b ("checks: add interrupts property check")
> Fixes: b3bbac02d5e3 ("checks: add phandle with arg property checks")
> Reported-by: Alan Tull <atull-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Applied, thanks.  A testcase that prevents a regression here might be
a nice follow on.

> ---
>  checks.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/checks.c b/checks.c
> index 08a3a29edae3..e66138449886 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -988,6 +988,10 @@ static void check_property_phandle_args(struct check *c,
>  		 * entries when each index position has a specific definition.
>  		 */
>  		if (phandle == 0 || phandle == -1) {
> +			/* Give up if this is an overlay with external references */
> +			if (dti->dtsflags & DTSF_PLUGIN)
> +				break;
> +
>  			cellsize = 0;
>  			continue;
>  		}
> @@ -1176,6 +1180,11 @@ static void check_interrupts_property(struct check *c,
>  		prop = get_property(parent, "interrupt-parent");
>  		if (prop) {
>  			phandle = propval_cell(prop);
> +			/* Give up if this is an overlay with external references */
> +			if ((phandle == 0 || phandle == -1) &&
> +			    (dti->dtsflags & DTSF_PLUGIN))
> +					return;
> +
>  			irq_node = get_node_by_phandle(root, phandle);
>  			if (!irq_node) {
>  				FAIL(c, dti, "Bad interrupt-parent phandle for %s",

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] checks: fix handling of unresolved phandles for dts plugins

[Alan Tull]

On Wed, Oct 18, 2017 at 4:29 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> In dts plugins, it is valid to have unresolved phandle values. The
> check_property_phandle_args and check_interrupts_property checks failed to
> account for this resulting in spurious warnings or asserts, respectively.
> Fix this by bailing from the checks if we're checking a dts plugin as
> there is no way to further validate the properties.
>
> Fixes: ee3d26f6960b ("checks: add interrupts property check")
> Fixes: b3bbac02d5e3 ("checks: add phandle with arg property checks")
> Reported-by: Alan Tull <atull-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  checks.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Hi Rob,

Thanks for the fix!

Alan

>
> diff --git a/checks.c b/checks.c
> index 08a3a29edae3..e66138449886 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -988,6 +988,10 @@ static void check_property_phandle_args(struct check *c,
>                  * entries when each index position has a specific definition.
>                  */
>                 if (phandle == 0 || phandle == -1) {
> +                       /* Give up if this is an overlay with external references */
> +                       if (dti->dtsflags & DTSF_PLUGIN)
> +                               break;
> +
>                         cellsize = 0;
>                         continue;
>                 }
> @@ -1176,6 +1180,11 @@ static void check_interrupts_property(struct check *c,
>                 prop = get_property(parent, "interrupt-parent");
>                 if (prop) {
>                         phandle = propval_cell(prop);
> +                       /* Give up if this is an overlay with external references */
> +                       if ((phandle == 0 || phandle == -1) &&
> +                           (dti->dtsflags & DTSF_PLUGIN))
> +                                       return;
> +
>                         irq_node = get_node_by_phandle(root, phandle);
>                         if (!irq_node) {
>                                 FAIL(c, dti, "Bad interrupt-parent phandle for %s",
> --
> 2.11.0
>