Re: [PATCH v2 3/4] dt-bindings: PCI: Add UltraRISC DP1000 PCIe controller

Krzysztof Kozlowski
2026-04-07T07:50:44Z

On Tue, Apr 07, 2026 at 10:40:54AM +0800, Jia Wang wrote:
> Add UltraRISC DP1000 SoC PCIe controller devicetree bindings.
>
> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> ---
> .../bindings/pci/ultrarisc,dp1000-pcie.yaml | 103 +++++++++++++++++++++
> 1 file changed, 103 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/ultrarisc,dp1000-pcie.yaml b/Documentation/devicetree/bindings/pci/ultrarisc,dp1000-pcie.yaml
> new file mode 100644
> index 000000000000..d0517130e127
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/ultrarisc,dp1000-pcie.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/ultrarisc,dp1000-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: UltraRISC DP1000 PCIe Host Controller
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> + UltraRISC DP1000 SoC PCIe host controller is based on the DesignWare PCIe IP.
> + This binding describes the UltraRISC specific extensions to the base
> + DesignWare PCIe binding.

Drop sentence. Do not describe in description what the binding
describes. It's circular / repetitive. Just describe that.

> +
> +maintainers:
> + - Xincheng Zhang <zhangxincheng@ultrarisc.com>
> + - Jia Wang <wangjia@ultrarisc.com>
> +
> +allOf:
> + - $ref: /schemas/pci/snps,dw-pcie.yaml#
> +
> +properties:
> + compatible:
> + const: ultrarisc,dp1000-pcie
> +
> + reg:
> + items:
> + - description: Data Bus Interface (DBI) registers.
> + - description: PCIe configuration space region.
> +
> + reg-names:
> + items:
> + - const: dbi
> + - const: config
> +
> + num-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [4, 16]
> + description: Number of lanes to use.
> +
> + max-link-speed:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + const: 4

If const then deducible from the compatible. Drop the property.

> + description: Maximum PCIe link speed supported.
> +
> + interrupts:
> + description: List of interrupt specifiers used by the controller

Drop description. Obvious.

> + items:
> + - description: MSI interrupt
> + - description: Legacy INTA interrupt
> + - description: Legacy INTB interrupt
> + - description: Legacy INTC interrupt
> + - description: Legacy INTD interrupt
> +
> + interrupt-names:
> + items:
> + - const: msi
> + - const: inta
> + - const: intb
> + - const: intc
> + - const: intd
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - interrupt-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pcie_x16: pcie@21000000 {

Drop unused label

> + compatible = "ultrarisc,dp1000-pcie";

reg, names and ranges go here. Please follow DTS coding style.

> + #address-cells = <3>;
> + #size-cells = <2>;
> + #interrupt-cells = <1>;
> + reg = <0x0 0x21000000 0x0 0x01000000>,
> + <0x0 0x4fff0000 0x0 0x00010000>;
> + reg-names = "dbi", "config";
> + device_type = "pci";
> + dma-coherent;
> + bus-range = <0x0 0xff>;
> + num-lanes = <16>;
> + ranges = <0x81000000 0x0 0x4fbf0000 0x0 0x4fbf0000 0x0 0x00400000>,
> + <0x82000000 0x0 0x40000000 0x0 0x40000000 0x0 0x0fbf0000>,
> + <0xc3000000 0x40 0x00000000 0x40 0x00000000 0xd 0x00000000>;
> +
> + max-link-speed = <4>;

Drop, compatible defines this.

Best regards,
Krzysztof