> On Nov 16, 2018, at 3:26 PM, Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Fri, Nov 16, 2018 at 12:46 PM Kumar Gala <kumar.gala@xxxxxxxxxx> wrote:
>>
>> There are various SoCs that have 2 different peripheral blocks at the
>> same register offset. However, we might have one block marked as
>> status = "disabled" and the other status = "ok". In such cases we
>> shouldn't warn about duplicate unit-address.
>
> How does that work exactly? Is it 2 blocks controlled by a fuse or
> some external control or a single block with some internal mode
> switch? This could also be solved by putting each block in an include
> and boards including the block they use.
Not sure, guessing some mode switch based on a config bit. Having the board include which one isn’t really desirable solution.
>
> This would get messy if we needed to do the same thing for other
> checks, but I wouldn't want to globally ignore disabled nodes (as
> that's the only thing that comes to mind for how to make it cleaner).
> I also want to look into running checks just on SoC dtsi files instead
> of the board dts files since lots of boards create lots of duplicates.
> And SoC dtsi files have lots of disabled nodes.
>
> We could make this check off by default if that helps.
Maybe, would have 2 checks, one check_unique_unit_address as its in the code base, and new check ‘check_unique_unit_address_if_enabled’ be an acceptable way forward, maybe with check_unique_unit_address_if_enabled disabled by default?
>> Here's a cut down example that we would warning about before:
>>
>> /dts-v1/;
>>
>> / {
>> #address-cells = <0x01>;
>> #size-cells = <0x01>;
>>
>> soc {
>> #address-cells = <0x01>;
>> #size-cells = <0x01>;
>> compatible = "simple-bus";
>> ranges;
>>
>> i2c0: i2c@40003000 {
>> compatible = "nordic,nrf-i2c";
>> reg = <0x40003000 0x1000>;
>> status = "ok";
>> };
>>
>> spi0: spi@40003000 {
>> compatible = "nordic,nrf-spi";
>> reg = <0x40003000 0x1000>;
>> status = "disabled";
>> };
>> };
>> };
>>
>> Signed-off-by: Kumar Gala <kumar.gala@xxxxxxxxxx>
>> ---
>> checks.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/checks.c b/checks.c
>> index ed84e03..8f787e6 100644
>> --- a/checks.c
>> +++ b/checks.c
>> @@ -1192,15 +1192,30 @@ static void check_unique_unit_address(struct check *c, struct dt_info *dti,
>> for_each_child(node, childa) {
>> struct node *childb;
>> const char *addr_a = get_unitname(childa);
>> + struct property *prop;
>>
>> if (!strlen(addr_a))
>> continue;
>>
>> + prop = get_property(childa, "status");
>> + if (prop) {
>> + char *str = prop->val.val;
>> + if (!strcmp("disabled", str))
>> + continue;
>> + }
>> +
>> for_each_child(node, childb) {
>> const char *addr_b = get_unitname(childb);
>> if (childa == childb)
>> break;
>>
>> + prop = get_property(childb, "status");
>> + if (prop) {
>> + char *str = prop->val.val;
>> + if (!strcmp("disabled", str))
>> + continue;
>> + }
>
> You've got the same code twice. This needs a node_is_disabled() helper.
Will make this change.
>
>> +
>> if (streq(addr_a, addr_b))
>> FAIL(c, dti, childb, "duplicate unit-address (also used in node %s)", childa->fullpath);
>> }
>> --
>> 2.14.5
