scripts: dts: gen_defines: Add error check for duplicate labels

[galak]

As the value of label properties get used as device names we shouldn't
have the same label value used more than once. Treat this as an
error condition.

Signed-off-by: Kumar Gala kumar.gala@linaro.org

[galak]

I played around with a similar check for node name (w/unit-address) and there are several cases in which we have duplicates. We have a few cases that we'd have to have some different heuristics. So curious what people feel about adding some checking for this case.

waveshare_open103z:WARNING: Duplicate node name (pwm) between /soc/timers@40000800/pwm and /soc/timers@40000c00/pwm
qemu_riscv32:WARNING: Duplicate node name (interrupt-controller) between /cpus/cpu@2/interrupt-controller and /cpus/cpu@3/interrupt-controller
lpcxpresso55s69_cpu0:WARNING: Duplicate node name (partitions) between /soc/peripheral@50000000/flash-controller@34000/flash@0/partitions and /soc/sram@14000000/partitions
mimxrt1060_evk:WARNING: Duplicate node name (port) between /panel/port and /soc/display-controller@402b8000/port
mimxrt1060_evk:WARNING: Duplicate node name (endpoint) between /panel/port/endpoint and /soc/display-controller@402b8000/port/endpoint

[mbolivar-nordic]

This seems OK, but I'm not sure how it will interact with flash APIs that use labels for partitions. We might want to have multiple partitions subnodes in the devicetree for different flash devices, and they might have the same labels there.

Maybe a better check would be post-processing the actual struct device list and erroring out on duplicates that aren't empty strings?

[pabigot]

Maybe a better check would be post-processing the actual struct device list and erroring out on duplicates that aren't empty strings?

That'd be another use case to give #32129 some love. At the same time we could infer a proper initialization order, or at least diagnose cases where a build-time dependency wasn't satisfied by the init order.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[de-nordic]

This seems OK, but I'm not sure how it will interact with flash APIs that use labels for partitions. We might want to have multiple partitions subnodes in the devicetree for different flash devices, and they might have the same labels there.

I do not think that flash partition nodes, that area of the "okay" status, should be allowed to have the same label property.
The entire idea of label for flash partition is that you do not have to know where it is, you just use name to obtain access to one.
If we would allow to label multiple partitions the same way then the access method would have to change in a form that would allow to distinguish which one we should be accessed, so something like this:

 FLASH_AREA_ID(image_0)

would have to change to

FLASH_AREA_ID(<some additional id>, image_0)

And then you have to know where the partition is and right now you just label partition in DTS and using that label you get everything you need to get access to that partition, but the label needs to be unique.

[de-nordic]

From the perspective of flash map API, where the labels are used to uniquely identify flash area and the device (and bus) it resides on, the change is desired.
Looks OK.

[mbolivar-nordic]

I do not think that flash partition nodes, that area of the "okay" status, should be allowed to have the same label property.

But that's not what this PR is doing. It's operating on all nodes regardless of status. Putting a -1 on until we sort that out.

It sounds like you're requesting a change to work on status "okay" nodes only. Is that right?

[de-nordic]

It sounds like you're requesting a change to work on status "okay" nodes only. Is that right?

OK, I have unfortunately sidetracked the discussion with the "okay" : yes the PR does not change that, but finding duplicates is helpful as it will warn you that for example you have defined two, lets say "storage" partitions and one is in internal flash and other one is on some external device.

The fixed partitions case is specific because this is the only one to generate definitions like this:
#define DT_COMPAT_fixed_partitions_LABEL_<something>
which help you match label to node; this is the only compatible that does so.
If you allow to have two labels of the same name then you will get the same defines pointing to different nodes, and you may get that situation for example when applying overlays.

Btw. the printout, if it is possible should point out which DTS file contributed to the duplicated label.

Now, as I thought about this issue for a longer time I find a case where duplicate labels may be useful: as long as these labels are set for nodes of different types of peripherals; the case is when you dedicate your peripheral, of different purpose, to work on the same area, for example: lets say we have a device that is in control of battery packs at some backup power station, such device will have batteries gathered in groups that can be independently connected/disconnected/charged/tested, etc. and every group will have thermometer, AD, breaker etc. In such case you may give the same label to devices that serve the same group (label "group-a") and in code reference them like this:

BLAH_THERMO(group_a)
BLAH_PW_AXE(group_a)

Of course this stops working when you have multiple peripherals of the same kind.

So in case of fixed partitions, as a special case, every duplicated label is undesired, in other cases it is more complicated.

[mbolivar-nordic]

Thanks for clearing up my misunderstanding @de-nordic; I think I get what you mean now.

@galak -- I think this is OK, but I think it should just warn by default. We have what we need to make this an error in CI similarly to how we handle --err-on-deprecated-properties.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.