Re: [PATCH] checks: check_unique_unit_address: Update to skip if node is disabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



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.

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.

> 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.

> +
>                         if (streq(addr_a, addr_b))
>                                 FAIL(c, dti, childb, "duplicate unit-address (also used in node %s)", childa->fullpath);
>                 }
> --
> 2.14.5
>



[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux