Re: [PATCH v2 6/6] arm64: dts: cix: Describe the Sky1 DMA-350 bus integration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt_bindings: audss_cru syscon lacks 'syscon' compatible string, causing regmap lookup failure
- [Medium] dts: cix: invalid generic fallback compatible due to custom irq routing
- [Low] dt_bindings: bus node has unit address without reg or non-empty ranges, triggering dtc warning
--

commit ba64255159af2e234a904fd8fbccd0e1066fbfb5
Author: Jelly Jia <Jelly.Jia@cixtech.com>

arm64: dts: cix: Describe the Sky1 DMA-350 bus integration

This commit describes the integration of the FCH and AUDSS DMA-350 controllers
behind interconnect windows in the Sky1 devicetree. It introduces wrapper bus
nodes with identity ranges; and specific dma-ranges, and configures clocks,
interrupts, and reset lines for the DMA blocks.

> - The same shape is used in mainline: the Agilex5 DMA bus
>   (dma-bus@10db0000, a simple-bus with dma-ranges whose controller
>   carries "altr,agilex5-axi-dma", "snps,axi-dma-1.01a") and the Marvell
>   AC5 buses (bus@80500000 with "ranges;" and "dma-ranges", and
>   behind-32bit-controller@7f000000, whose only purpose is the DMA
>   mapping of the devices below it).

[Severity: Low]
Is this description accurate regarding mainline usage without a reg property?

The Marvell AC5 bus (bus@80500000) explicitly defines a reg property, and the
Agilex5 bus (dma-bus@10db0000) has a non-empty ranges property.

> diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
> index 0a820f45feb96..e3bd6c0e383fb 100644
> --- a/arch/arm64/boot/dts/cix/sky1.dtsi
> +++ b/arch/arm64/boot/dts/cix/sky1.dtsi
> @@ -518,18 +518,29 @@ iomuxc: pinctrl@4170000 {
[ ... ]
> +		bus@4190000 {
> +			compatible = "simple-bus";
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;

[Severity: Low]
Does this node trigger a dtc warning for having a unit address without a reg or
non-empty ranges property?

Compiling the devicetree will generate the warning "node has a unit name, but
no reg or ranges property" for the wrapper buses. The dtc compiler specifically
ignores empty ranges; properties (length 0) when evaluating the
unit_address_vs_reg check.

To fix this, the nodes must either omit the @4190000 suffix or add a dummy reg
property.

> +			dma-ranges = <0x0 0x040a0000 0x0 0x040b0000 0x0 0x00040000>,
> +				     <0x0 0x80000000 0x0 0x80000000 0x8 0x00000000>,
> +				     <0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>;
> +
> +			fch_dmac: dma-controller@4190000 {
[ ... ]
> @@ -576,6 +587,37 @@ mbox_pm2ap: mailbox@65a0080 {
[ ... ]
> +		bus@7010000 {
> +			compatible = "simple-bus";
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;

[Severity: Low]
Does this node also trigger a dtc warning for the same reason?  It uses a unit
address with an empty ranges property and no reg property.

> +			dma-ranges = <0x0 0x20000000 0x0 0x07010000 0x0 0x00100000>,
> +				     <0x0 0x30000000 0x0 0xc0000000 0x0 0x20000000>;
> +
> +			audss_dmac: dma-controller@7010000 {
> +				compatible = "cix,sky1-dma350", "arm,dma-350";

[Severity: Medium]
Is the "arm,dma-350" generic fallback compatible valid here, given the custom
IRQ routing below?

Devicetree rules dictate that a fallback compatible must only be used if the
generic driver can fully drive the hardware without vendor extensions. A generic
driver binding to "arm,dma-350" will ignore cix,irq-router, fail to ungate the
interrupts, and thus the hardware will be non-functional (e.g., interrupt
timeouts).

The custom routing should either be modeled using a standard interrupt-parent /
interrupt-map so the IRQ subsystem handles it transparently, or the generic
fallback compatible must be removed.

> +				reg = <0x0 0x07010000 0x0 0x10000>;
> +				clocks = <&audss_cru CLK_DMAC_AXI>;
> +				resets = <&audss_cru AUDSS_DMAC_SW_RST>;
> +				/*
> +				 * The AUDSS CRU gates each DMAC channel interrupt
> +				 * before it reaches the GIC. AUDSS_DMAC_INFO_AP_IRQ
> +				 * is at CRU offset 0x54.
> +				 */
> +				cix,irq-router = <&audss_cru>;

[Severity: Medium]
Will this syscon lookup fail because the audss_cru node below lacks the
"syscon" compatible string?

The DMA controller references the CRU via cix,irq-router = <&audss_cru>;. The
DMA driver resolves this using syscon_regmap_lookup_by_phandle(), which relies
on syscon_node_to_regmap().

This core function only instantiates a regmap if
of_device_is_compatible(np, "syscon") is true. Occurs unconditionally on device
boot: the audss_dmac DMA controller will fail to probe, receiving -EPROBE_DEFER
indefinitely.

> +				interrupts = <GIC_SPI 230 IRQ_TYPE_LEVEL_HIGH 0>,
[ ... ]
> +		audss_cru: clock-controller@7110000 {
> +			compatible = "cix,sky1-audss-cru";

[Severity: Medium]
Can the "syscon" compatible string be added here?

Because audss_cru only has compatible = "cix,sky1-audss-cru"; and omits the
standard "syscon" fallback compatible, the regmap lookup will fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009051846.1115962-1-Jelly.Jia@cixtech.com?part=6