On Thu, Mar 31, 2016 at 9:27 PM, David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Mar 31, 2016 at 11:18:25AM -0500, Rob Herring wrote:
>> On Thu, Mar 31, 2016 at 12:29 AM, David Gibson
>> <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>> > On Wed, Mar 23, 2016 at 07:40:20PM -0500, Rob Herring wrote:
>> >> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
>> >
>> > Minor nit: before doing these tests, we should probably add a check
>> > which ensures that any bus bridge node *has* a #address-cells and
>> > #size-cells value.
>>
>> I'll check, but I thought we already had that check because any bridge
>> node has reg or ranges.
>>
>> >
>> >> ---
>> >> checks.c | 87 +++++++++++++++++++++++++++--
>> >> tests/run_tests.sh | 4 ++
>> >> tests/unit-addr-leading-0s.dts | 10 ++++
>> >> tests/unit-addr-leading-0x.dts | 10 ++++
>> >> tests/unit-addr-simple-bus-comma.dts | 18 ++++++
>> >> tests/unit-addr-simple-bus-reg-mismatch.dts | 18 ++++++
>> >> 6 files changed, 142 insertions(+), 5 deletions(-)
>> >> create mode 100644 tests/unit-addr-leading-0s.dts
>> >> create mode 100644 tests/unit-addr-leading-0x.dts
>> >> create mode 100644 tests/unit-addr-simple-bus-comma.dts
>> >> create mode 100644 tests/unit-addr-simple-bus-reg-mismatch.dts
>> >>
>> >> diff --git a/checks.c b/checks.c
>> >> index 48e926e..82a7f38 100644
>> >> --- a/checks.c
>> >> +++ b/checks.c
>> >> @@ -20,6 +20,11 @@
>> >>
>> >> #include "dtc.h"
>> >>
>> >> +#define node_addr_cells(n) \
>> >> + (((n)->addr_cells == -1) ? 2 : (n)->addr_cells)
>> >> +#define node_size_cells(n) \
>> >> + (((n)->size_cells == -1) ? 1 : (n)->size_cells)
>> >> +
>> >> #ifdef TRACE_CHECKS
>> >> #define TRACE(c, ...) \
>> >> do { \
>> >> @@ -578,12 +583,88 @@ static bool is_simple_bridge(struct node *node)
>> >> return false;
>> >> }
>> >>
>> >> +static void default_unit_addr(struct check *c, struct node *dt, struct node *node)
>> >> +{
>> >> + const char *unitname = get_unitname(node);
>> >> +
>> >> + if (strstr(unitname, "0x") == unitname) {
>> >> + FAIL(c, "Node %s unit address 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 address should not have leading 0s",
>> >> + node->fullpath);
>> >
>> > Explicitly checking various aspects of the format seems a bit weird to
>> > me. Why not just generate the expected address from 'reg' and
>> > strcmp()?
>>
>> Because for the default check, I'm only testing these aspects. I found
>> some cases running this thru the kernel tree dts files that the full
>> simple-bus check is too strict. For example, we want to warn on
>> "@0x002,4", but not "@2,4" or "@2blah".
>
> Ok. Thinking about it, I think this might work a bit better separated
> (mostly) from the bus type stuff. Basically treat it as a "common
> unit name problems" test, that will skip itself if a bus type is set
> (which will allow more thorough testing of the unit name).
Ha! That's pretty much back to my original patch, but with the
addition of skipping the test if the bus type is known.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
