devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
To: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 3/5] checks: Warn on node name unit-addresses with '0x' or leading 0s
Date: Tue, 31 Jan 2017 11:17:11 +1100	[thread overview]
Message-ID: <20170131001711.GC14879@umbus.fritz.box> (raw)
In-Reply-To: <20170124174534.3865-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

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

On Tue, Jan 24, 2017 at 11:45:32AM -0600, Rob Herring wrote:
> Node name unit-addresses should never begin with 0x or leading 0s
> regardless of whether they have a bus specific address (i.e. one with
> commas) or not. Add warnings to check for these cases.
> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

I'd prefer to see this check split out on its own rather than folded
into unit_address_vs_reg.  Strictly speaking you could have a bus
binding which does specify unit addresses starting with leading 0 or
0x, although it's pretty unlikely.  Splitting this out will make it
easier to override this when we start having cases where we can really
check the unit address is correct versus the bus binding.

> ---
>  checks.c                       | 10 ++++++++++
>  tests/run_tests.sh             |  2 ++
>  tests/unit-addr-leading-0s.dts | 10 ++++++++++
>  tests/unit-addr-leading-0x.dts | 10 ++++++++++
>  4 files changed, 32 insertions(+)
>  create mode 100644 tests/unit-addr-leading-0s.dts
>  create mode 100644 tests/unit-addr-leading-0x.dts
> 
> diff --git a/checks.c b/checks.c
> index 0c78d69316bc..8e310d69ca3d 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -288,6 +288,16 @@ static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti,
>  		if (!unitname[0])
>  			FAIL(c, "Node %s has a reg or ranges property, but no unit name",
>  			    node->fullpath);
> +
> +		if (!strncmp(unitname, "0x", 2)) {
> +			FAIL(c, "Node %s unit name should not have leading \"0x\"",
> +			    node->fullpath);
> +			/* skip over 0x for next test */
> +			unitname += 2;
> +		}
> +		if (unitname[0] == '0' && isxdigit(unitname[1]))
> +			FAIL(c, "Node %s unit name should not have leading 0s",
> +			    node->fullpath);
>  	} else {
>  		if (unitname[0])
>  			FAIL(c, "Node %s has a unit name, but no reg property",
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 157dbaea7600..2c06666a9e23 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -539,6 +539,8 @@ dtc_tests () {
>      check_tests obsolete-chosen-interrupt-controller.dts obsolete_chosen_interrupt_controller
>      check_tests reg-without-unit-addr.dts unit_address_vs_reg
>      check_tests unit-addr-without-reg.dts unit_address_vs_reg
> +    check_tests unit-addr-leading-0x.dts unit_address_vs_reg
> +    check_tests unit-addr-leading-0s.dts unit_address_vs_reg
>      run_sh_test dtc-checkfails.sh node_name_chars -- -I dtb -O dtb bad_node_char.dtb
>      run_sh_test dtc-checkfails.sh node_name_format -- -I dtb -O dtb bad_node_format.dtb
>      run_sh_test dtc-checkfails.sh prop_name_chars -- -I dtb -O dtb bad_prop_char.dtb
> diff --git a/tests/unit-addr-leading-0s.dts b/tests/unit-addr-leading-0s.dts
> new file mode 100644
> index 000000000000..7c8e2cebbc84
> --- /dev/null
> +++ b/tests/unit-addr-leading-0s.dts
> @@ -0,0 +1,10 @@
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	node@001 {
> +		reg = <1 0>;
> +	};
> +};
> diff --git a/tests/unit-addr-leading-0x.dts b/tests/unit-addr-leading-0x.dts
> new file mode 100644
> index 000000000000..7ed7254e8dc2
> --- /dev/null
> +++ b/tests/unit-addr-leading-0x.dts
> @@ -0,0 +1,10 @@
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	node@0x1 {
> +		reg = <1 0>;
> +	};
> +};

-- 
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: 819 bytes --]

  parent reply	other threads:[~2017-01-31  0:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-24 17:45 [PATCH 0/5] dtc unit-address and character set checks Rob Herring
     [not found] ` <20170124174534.3865-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-01-24 17:45   ` [PATCH 1/5] checks: Add Warning for stricter property name character checking Rob Herring
     [not found]     ` <20170124174534.3865-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-01-31  0:32       ` David Gibson
2017-01-24 17:45   ` [PATCH 2/5] checks: Add Warning for stricter node " Rob Herring
     [not found]     ` <20170124174534.3865-3-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-01-31  3:14       ` David Gibson
     [not found]         ` <20170131031434.GK14879-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2017-01-31 13:46           ` Rob Herring
2017-01-24 17:45   ` [PATCH 3/5] checks: Warn on node name unit-addresses with '0x' or leading 0s Rob Herring
     [not found]     ` <20170124174534.3865-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-01-31  0:17       ` David Gibson [this message]
2017-01-24 17:45   ` [PATCH 4/5] checks: Add infrastructure for setting bus type of nodes Rob Herring
     [not found]     ` <20170124174534.3865-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-01-31  0:26       ` David Gibson
     [not found]         ` <20170131002634.GD14879-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2017-02-01 21:54           ` Rob Herring
2017-02-06  2:14             ` David Gibson
2017-01-24 17:45   ` [PATCH 5/5] checks: Add bus checks for PCI buses Rob Herring
     [not found]     ` <20170124174534.3865-6-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-01-25 22:37       ` Stephen Boyd
2017-01-27 22:54         ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170131001711.GC14879@umbus.fritz.box \
    --to=david-xt8fgy+axnrb3ne2bgzf6laj5h9x9tb+@public.gmane.org \
    --cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).