arm64: dts: adi: fix all dtc W=1 warnings and DT convention violations - #3484
Conversation
mhennerich
added
sc846
| }; | ||
|
|
||
| sport0b: channel@1 { | ||
| reg = <1>; |
There was a problem hiding this comment.
Fine my me. But I guess I follow up change should be getting rid of adi,id
There was a problem hiding this comment.
Agreed. Kept adi,id here so this series stays DTS-only, but the plan is a small follow-up that switches adi-dma.c to read reg (with an adi,id fallback during the transition) and then deletes the adi,id properties from the channel nodes.
| clock-frequency = <24576000>; | ||
| clock-output-names = "mclk"; | ||
| }; | ||
| mclk: clock-mclk { |
There was a problem hiding this comment.
should the above have a generic node name? I guess it is not a clock-controller but maybe clock-ref or spelling it out clock-reference?
There was a problem hiding this comment.
It's indeed not a clock-controller. The common in-tree pattern for fixed clocks is a frequency-suffixed name, so I renamed it to clock-24576000 (same as qcom/rockchip boards do); clock-ref doesn't have in-tree precedent.
| compatible = "adi,adsp-pinctrl"; | ||
| pads_syscon: pinctrl0: pinctrl@31004600 { | ||
| compatible = "adi,adsp-pinctrl", "adi,sc5xx-pads-syscon", | ||
| "syscon"; |
There was a problem hiding this comment.
Not really opposing the change but never really saw anything like the above? Do we have any intree example using the same thing (I could search myself but being lazy). FWIW, now the pads_syscon will have all the same properties of pinctrl@31004600
"while the emac glue and the sru-ctrl driver access the block
through a syscon regmap."
The above also let me thinking if we could not use the pinctrl consumer API for the above rather than syscon. Just thinking out load (did not looked at any code).
Is this causing any DT warning? If not, maybe we should live with the duplication (maybe adding a comment so it's not forgotten) and then think how this should be really handled! Looking at how sru-ctrl-adsp.c uses syscon, really feels like a hack. IIRC, @sipraga did some worked related to this (maybe his pinctrl rewrite) so he might have some comments about this
There was a problem hiding this comment.
Double labels on one node are legal and there are in-tree examples (clk: rstc: reset-controller@f0801000 in bcm4908, pcie0: pcie: pcie@33800000 in imx8mp), so that part works — but I dropped this commit anyway, for a stronger reason than the one you raised: syscon.yaml in this tree documents adi,sc5xx-pads-syscon (added by @sipraga in 92951dd, "dt-bindings: mfd: syscon: Document PADS syscon for ADI SC5xx SoCs") and requires exactly ["adi,sc5xx-pads-syscon", "syscon"] as the compatible list, so the merged three-entry compatible fails dtbs_check. Trading one W=1 dtc warning (unique_unit_address_if_enabled, the only warning the duplication causes) for a schema
violation is a bad trade — and since the standalone pads-syscon binding already exists as part of Alvin's pads/pinctrl work, keeping the two-node structure is clearly the right direction anyway.
I restored the two nodes and added a short comment on both stating the overlap at 0x31004600 is intentional (same PADS0 block: pinctrl programs it directly, emac/sru-ctrl reach it via syscon regmap) so it isn't "rediscovered" later. Longer term I agree the sru-ctrl syscon usage looks like a hack — happy to revisit once the pinctrl rework lands; the pinctrl consumer API idea sounds like the right direction for that.
There was a problem hiding this comment.
I parked my PR for a while because I figured it would make more sense to merge it after sc846 support lands. I will follow up on it though - should resolve the peculiarities this commit was trying to address too.
486720c to
93f3856
Compare
| clock-frequency = <24576000>; | ||
| clock-output-names = "mclk"; | ||
| }; | ||
| mclk: clock-24576000 { |
There was a problem hiding this comment.
Aahah, actually good point by the LLM :)
6fbda9d to
8f1da79
Compare
93f3856 to
03e2aec
Compare
The adi,dma-controller / adi,mdma-controller channel subnodes carried a unit address (taken from the decimal channel id) but no reg property, producing a dtc unit_address_vs_reg warning for every channel, and the decimal unit addresses did not follow the lowercase-hex convention either (channel@10 for channel 10, i.e. 0xa). Give the controller nodes #address-cells = <1> / #size-cells = <0>, add a reg property carrying the channel id to every channel and rename the nodes to the hex unit address matching reg. The adi,id property is kept as-is: the adi-dma driver binds channels by adi,id, and reg mirrors it. Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
The lp0/lp1 nodes have no reg property, use compatible strings that are
not valid vendor,device compatibles ("linkport0"/"linkport1"), have no
binding, and no driver in the tree matches them. They only produce dtc
warnings (unit name without reg, simple-bus child without reg). Remove
them; proper nodes can be added together with a driver and binding if
link port support ever materialises.
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Nodes without a reg property do not belong under the scb simple-bus (dtc simple_bus_reg / unit_address_vs_reg warnings): the gptimer-counter aggregator, the emac1 fixed-clock, the usb-nop-xceiv phy, the rpmsg virtual devices and the sound cards are all virtual devices, so move them to the root node where of_platform_default_populate picks them up just the same. Drop the meaningless @0 unit addresses from the moved nodes, the sram-mmap nodes and the tru routing channels while at it, and drop the bogus simple-bus compatible from the ezlite clocks container by making mclk a plain root fixed-clock. No functional change: all consumers reference these nodes by label. Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Bring the remaining node names in line with the devicetree spec and the dtc checks: - unit addresses must be lowercase hex without leading zeros: fix cnt/pkte/rng/sru-ctrl/rtc/mmc/uart3 and the vdev reserved-memory entries (register values lowercased to match) - the i2s nodes used fake unit addresses (i2s0@0, i2s@4): use the first reg entry (i2s@31002000 / i2s@31002400) - the fixed-clock oscillators had unit addresses without reg: rename to clock-sys-clkin0/1 - cpu@0 carried a two-cell reg <0x0 0x0> under #address-cells = <1>, #size-cells = <0>: use the single-cell <0x0> - reserved-memory: both resource-table nodes were named rsc_tbl0 and both vdev1 nodes were named vdev0*: fix the copy-paste duplicates and the underscores in node names - fixed-partitions children must be named partition@<offset>: rename the qspi_N/ospi partition nodes, using the real offsets (the sc846-som rootfs partition claimed @2000000 but starts at 0x1210000, the ezkit ospi partitions were numbered @1/@2/@3) All consumers use labels; no functional change. Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
The pinctrl, sru-ctrl and cnt nodes and the spi-nor flash nodes declare the #address-cells/#size-cells pair although none of their children carry a reg property (the pinctrl/sru children are pinmux groups, the flash partition table lives one level down inside the fixed-partitions container, which has its own cell sizes). dtc flags these with avoid_unnecessary_addr_size; remove them. Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
The devicetree spec asks for generic node names describing the class of device, and several upstream bindings enforce name patterns on children. Rename: - uart@ -> serial@, twi@ -> i2c@, gport@ -> gpio@, cnt@ -> counter@, hadc@ -> adc@, thermal@ -> thermal-sensor@, pkte@ -> crypto@, tru@ -> mailbox@ (it is the #mbox-cells provider), clocks@ -> clock-controller@ - I2C devices: the adau codecs -> audio-codec@, the adp5587/adp5588/ mcp23018 expanders -> gpio@ - mdio0 -> mdio (the snps,dwmac binding requires ^mdio(@.*)?$) - mmc-mux-controller -> mux-controller (gpio-mux $nodename pattern) - snps,tso is a flag in the snps,dwmac binding, not an integer All in-tree drivers bind by compatible or phandle, and aliases follow their labels; no functional change. Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
The pads syscon and the pinctrl node both describe the PADS0 block at
0x31004600 (dtc flags the duplicate unit address at W=1): the pinctrl
driver programs the pads registers directly, while the emac glue and
the sru-ctrl driver access them through the syscon regmap. Merging the
two nodes is not an option today -- the syscon binding fixes the
compatible list to ("adi,sc5xx-pads-syscon", "syscon") -- and the
sru-ctrl syscon usage is expected to be revisited with the pinctrl
rework, so document the overlap instead of restructuring the nodes.
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
03e2aec to
cf2c635
Compare
4 participants
Cleans up all remaining dtc W=1 warnings and devicetree-convention
violations across arch/arm64/boot/dts/adi: 322 warnings -> 6 on all six
buildable boards (sc598-som-ezkit/-sd/ezlite/htol, sc846-som,
sc846-som-ezkit).
Stacked on #3480 (base staging/sc84x-platform-fixes); retarget to
adsp-6.18.31-y once that merges. Independent of #3481 — the same
commits cherry-pick cleanly on top of it.
Commits
lowercase-hex unit addresses and #address-cells/#size-cells on the
controllers (was ~200 unit_address_vs_reg warnings). adi,id is kept;
the driver is unchanged.
invalid compatibles ("linkport0"), no reg.
root node (sound cards, usb phy, fixed clocks, gptimer-counter,
rpmsg) — of_platform_default_populate picks them up identically.
the i2s nodes, cpu@0's stray two-cell reg, reserved-memory
copy-paste names, fixed-partitions children renamed
partition@ — two had unit addresses that did not match
their actual offset.
sru-ctrl, cnt, flash nodes).
twi->i2c, gport->gpio, tru->mailbox, hadc->adc, pkte->crypto,
clocks->clock-controller, codecs->audio-codec@, expanders->gpio@,
mdio0->mdio, snps,tso as a flag, vendor-prefixed spi-nor compatible.
0x31004600 (the one remaining W=1 warning, once per board): the
syscon binding pins the compatible list so the nodes cannot be
merged today, and the sru-ctrl syscon usage is expected to be
revisited with the pinctrl rework.
Validation
documented pads/pinctrl duplicate unit address), down from 322
warnings in total.
without this series, on top of the iio: adc: ADEMA124/ADEMA127 driver with hardware-triggered SPI Offload streaming for SC846 #3481 driver stack): dmesg
warn/error scan identical, clk_summary line-for-line identical, every
renamed platform device re-binds to the same driver, Ethernet (the
adi,pads-syscon consumer) unaffected, ADEMA127 SPI-offload streaming
captures with the expected interrupt-per-watermark cadence.
Known follow-ups (out of scope here)
work: adi,trigger-routing-unit.yaml expects channel@N children with
no reg,
gpio-hog node names lack the -hog suffix, the scb bus name, and a
number of undocumented legacy adi,* compatibles.