Re: [dtc RFC PATCH] Enforce node name unit-address presence/absence

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

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

On Wed, Sep 18, 2013 at 01:41:16PM -0700, Olof Johansson wrote:
> On Wed, Sep 18, 2013 at 1:23 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> > From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> >
> > ePAPR 1.1 section 2.2.1.1 "Node Name Requirements" specifies that any
> > node that has a reg property must include a unit address in its name
> > with value matching the first entry in its reg property. Conversely, if
> > a node does not have a reg property, the node name must not include a
> > unit address.
> >
> > Implement a check for this. The code doesn't validate the format of the
> > unit address; ePAPR implies this may vary from binding to binding, so
> > I'm not sure that it's possible to validate the value itself.
> >
> > Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> > This depends on my previous patch "Ensure all tests have matching reg
> > and unit address".
> >
> > Note that this patch should not yet be applied; it will cause many real-
> > world *.dts files to fail to compile. Those need to be fixed first.
> > However, if/when that happens, this patch may be useful.
> > ---
> >  checks.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/checks.c b/checks.c
> > index ee96a25..c80a055 100644
> > --- a/checks.c
> > +++ b/checks.c
> > @@ -287,9 +287,25 @@ NODE_ERROR(node_name_chars, PROPNODECHARS "@");
> >  static void check_node_name_format(struct check *c, struct node *dt,
> >                                    struct node *node)
> >  {
> > -       if (strchr(get_unitname(node), '@'))
> > +       const char *unitname;
> > +       struct property *prop;
> > +
> > +       unitname = get_unitname(node);
> > +
> > +       if (strchr(unitname, '@'))
> >                 FAIL(c, "Node %s has multiple '@' characters in name",
> >                      node->fullpath);
> > +
> > +       prop = get_property(node, "reg");
> > +       if (prop) {
> > +               if (!unitname[0])
> > +                       FAIL(c, "Node %s has a reg property, but no unit name",
> > +                           node->fullpath);
> > +       } else {
> > +               if (unitname[0])
> > +                       FAIL(c, "Node %s has a unit name, but no reg property",
> > +                           node->fullpath);
> 
> These checks are very useful, even though they might sort of cross
> over the domain to what a dtc linter would do instead of the compiler.

So, I think inside the compiler is the best place for a linter anyway
- that way people will actually run it.

The entire checks infrastructure was built specifically to allow
linting inside the compiler - I just never had time to implement many
checks beyond the basics.

> Anyway, I think it'd be better to produce warnings than errors for
> this. That way we could also merge it now while the trees are fixed
> up.

Yes, I agree.

> Also, maybe warn for @0x<foo>, which is another unpreferred syntax, it
> should just be @<foo> (with foo being in hex).

Well.. that comes to validating the contents of the unit address.  And
as Ben points out that needs per bus type information to know how
they're conventionally formatted.

-- 
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: Type: application/pgp-signature, Size: 836 bytes --]