On Fri, Nov 16, 2018 at 12:46:26PM -0600, Kumar Gala 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.
>
I guess this raises the question of how wide the meaning of status =
"disabled" is allowed to be. Here it's assumed to mean "disconnected
in hardware, you can just ignore it", however I think in some other
circumstances it can mean "initially inactive, but can be enabled by
correctly poking somthing (e.g. feature register, power control unit,
etc.). In that latter case having duplicate reg values would still be
an error.
On balance I'm inclined to apply this, with a nit fixed:
> 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))
Use 'streq' here please. It exists specifically to mitigate the pause
of "uh.. !strcmp, wait, does that mean they're equal or not equal"
that I for one always get on seeing the construct above.
> + 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))
and here.
> + continue;
> + }
> +
> if (streq(addr_a, addr_b))
> FAIL(c, dti, childb, "duplicate unit-address (also used in node %s)", childa->fullpath);
> }
--
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:
signature.asc
Description: PGP signature
