Re: [PATCH v2 01/23] dt-bindings: display: msm: Add Qualcomm DCP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: qcom,dcp-virtio: invalid uint32 $ref in iommu-ranges items
- [Medium] dt-bindings: qcom,dcp-virtio: mismatched reg and iommu-ranges cell counts
--

commit 148fd24e1536920168a0cc19486ca6a7b8039591
Author: Jayant Shekhar <jayant.shekhar@oss.qualcomm.com>

dt-bindings: display: msm: Add Qualcomm DCP

This commit adds Device Tree binding schemas for the Qualcomm Display
Co-Processor (DCP) VirtIO transport wrapper, the DCP VirtIO display
subsystem, and its child MIPI DSI host controller.

> diff --git a/Documentation/devicetree/bindings/virtio/qcom,dcp-virtio.yaml b/Documentation/devicetree/bindings/virtio/qcom,dcp-virtio.yaml
> new file mode 100644
> index 0000000000000..6020e848ce259
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/virtio/qcom,dcp-virtio.yaml
> @@ -0,0 +1,132 @@

[ ... ]

> +  iommu-ranges:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: |
> +      The firmware-visible IOVA range, specified as a four-cell tuple
> +      containing a 64-bit IOVA base followed by a 64-bit range size.
> +    items:
> +      - description: IOVA base, high cell
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +      - description: IOVA base, low cell
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +      - description: IOVA range size, high cell
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +      - description: IOVA range size, low cell
> +        $ref: /schemas/types.yaml#/definitions/uint32

[Severity: Medium]
Does make dt_binding_check pass on properties.iommu-ranges in
Documentation/devicetree/bindings/virtio/qcom,dcp-virtio.yaml with these
per-item $ref entries?

When iommu-ranges uses /schemas/types.yaml#/definitions/uint32-array at the
property level, dtschema validates each entry under items against
meta-schemas/cell.yaml#/$defs/single-cell, which sets additionalProperties:
false and does not permit $ref, causing dt-doc-validate to fail.

In addition, /schemas/types.yaml#/definitions/uint32 is a property-level 2D
matrix schema rather than a scalar cell schema, so applying it to individual
cells of a uint32-array creates an array-versus-integer type conflict when
dt-validate checks the example's iommu-ranges property. Should the four $ref:
/schemas/types.yaml#/definitions/uint32 lines under items be dropped?

> +
> +  dma-coherent: true
> +
> +  mboxes:
> +    maxItems: 1
> +
> +  mbox-names:
> +    const: dcp-power-tx
> +
> +  dcp-subsystem:
> +    $ref: ../display/msm/qcom,dcp.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts-extended
> +  - iommus
> +  - iommu-ranges
> +  - dma-coherent
> +  - mboxes
> +  - mbox-names
> +  - dcp-subsystem
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/mailbox/qcom-ipcc.h>
> +
> +    virtio@9be8000 {
> +        compatible = "qcom,dcp-virtio";
> +        reg = <0x09be8000 0x1000>;
> +        interrupts-extended = <&intc GIC_ESPI 956 IRQ_TYPE_LEVEL_HIGH>,
> +                              <&ipcc 13 5 IRQ_TYPE_EDGE_RISING>;
> +        iommus = <&apps_smmu 0x15a3 0x0>;
> +        iommu-ranges = <0x0 0x84000000 0x0 0x04000000>;

[Severity: Medium]
In the examples block of
Documentation/devicetree/bindings/virtio/qcom,dcp-virtio.yaml, reg is
specified with 1 address cell and 1 size cell under the default example bus
(#address-cells = <1>; #size-cells = <1>;), whereas iommu-ranges is written
as a 4-cell tuple (2 address cells and 2 size cells).

When the kernel parses iommu-ranges via
of_iommu_get_resv_regions() -> of_iommu_derive_resv_regions() ->
of_translate_dma_region() in drivers/of/address.c:

    struct device_node *parent __free(device_node) = __of_get_dma_parent(dev);
    ...
    na = of_bus_n_addr_cells(parent);
    ns = of_bus_n_size_cells(parent);

    address = of_translate_dma_address(dev, prop);
    if (address == OF_BAD_ADDR)
        return NULL;

    size = of_read_number(prop + na, ns);

both reg and iommu-ranges use the parent bus #address-cells and #size-cells.
With a 1-cell parent bus, wouldn't of_iommu_derive_resv_regions() misparse
<0x0 0x84000000 0x0 0x04000000> as two 32-bit ranges ([0x0, 0x84000000) and
[0x0, 0x04000000)) instead of one 64-bit range ([0x84000000, 0x88000000))?
Should the example wrap virtio@9be8000 in a parent bus node with
#address-cells = <2> and #size-cells = <2> and use a 4-cell reg property?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-jshekhar-dcp-hlos-v2-0-7cf014efdee1@oss.qualcomm.com?part=1