On Sat, Aug 19, 2017 at 2:33 AM, David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Aug 18, 2017 at 11:02:01AM -0500, Rob Herring wrote:
>> On Thu, Aug 17, 2017 at 11:35 PM, David Gibson
>> <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>> > On Mon, Aug 14, 2017 at 04:48:06PM -0500, Rob Herring wrote:
>> >
>> > Yay! Someone actually implementing checks.
>>
>> Not like it is my first. I'll celebrate when someone else does.
>>
>>
>> >> Many common bindings follow the same pattern of client properties
>> >> containing a phandle and N arg cells where N is defined in the provider
>> >> with a '#<specifier>-cells' property. Add a checks for properties
>> >> following this pattern.
>> >
>> > I think this description would be easier to follow if you led with an example.
>> >
>> >>
>> >> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
>> >
>> > Looks pretty good, though I have some suggestions.
>> >
>> >> ---
>> >> checks.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >> 1 file changed, 117 insertions(+)
>> >>
>> >> diff --git a/checks.c b/checks.c
>> >> index afabf64337d5..c0450e118043 100644
>> >> --- a/checks.c
>> >> +++ b/checks.c
>> >> @@ -956,6 +956,120 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
>> >> WARNING(obsolete_chosen_interrupt_controller,
>> >> check_obsolete_chosen_interrupt_controller, NULL);
>> >>
>> >> +struct provider {
>> >> + const char *prop_name;
>> >> + const char *cell_name;
>> >> + bool optional;
>> >> +};
>> >> +
>> >> +static void check_property_phandle_args(struct check *c,
>> >> + struct dt_info *dti,
>> >> + struct node *node,
>> >> + struct property *prop,
>> >> + const struct provider *provider)
>> >> +{
>> >> + struct marker *m = prop->val.markers;
>> >> +
>> >> + if (!m) {
>> >> + FAIL(c, dti, "Missing phandles in %s:%s",
>> >> + node->fullpath, prop->name);
>> >> + return;
>> >> + }
>> >> + for_each_marker_of_type(m, REF_PHANDLE) {
>> >
>> > So going through the markers I think is not the best approach.
>> > That'll work if the source contains a reference here, which it usually
>> > will, but there are some circumstances where it could contain a "raw"
>> > phandle value (the most obvious being when you're decompiling an
>> > existing dtb).
>> >
>> > But I don't think you really need to. Instead you should be able to
>> > read the actual value, look it up with get_node_by_phandle(). You can
>> > make this check dependent on fixup_phandle_references to make sure
>> > it's executed after the references are resolved.
>>
>> That's how I implemented it initially...
>
> Ok.. why did you change?
Just because the code is a bit more compact using markers. But
decompiling a dtb is a good reason I didn't think of.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
