linux-kernel - Re: [PATCH v5 10/14] dt-bindings: PCI: Add CIX Sky1 PCIe Root Complex bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250630-graceful-horse-of-science-eecc53@krzk-bin>
Date: Mon, 30 Jun 2025 09:26:03 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: hans.zhang@...tech.com
Cc: bhelgaas@...gle.com, lpieralisi@...nel.org, kw@...ux.com, 
	mani@...nel.org, robh@...nel.org, kwilczynski@...nel.org, krzk+dt@...nel.org, 
	conor+dt@...nel.org, mpillai@...ence.com, fugang.duan@...tech.com, 
	guoyin.chen@...tech.com, peter.chen@...tech.com, cix-kernel-upstream@...tech.com, 
	linux-pci@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 10/14] dt-bindings: PCI: Add CIX Sky1 PCIe Root
 Complex bindings

On Mon, Jun 30, 2025 at 12:15:57PM +0800, hans.zhang@...tech.com wrote:
> From: Hans Zhang <hans.zhang@...tech.com>
> 
> Document the bindings for CIX Sky1 PCIe Controller configured in
> root complex mode with five root port.
> 
> Supports 4 INTx, MSI and MSI-x interrupts from the ARM GICv3 controller.
> 
> Signed-off-by: Hans Zhang <hans.zhang@...tech.com>
> Reviewed-by: Peter Chen <peter.chen@...tech.com>
> Reviewed-by: Manikandan K Pillai <mpillai@...ence.com>
> ---
>  .../bindings/pci/cix,sky1-pcie-host.yaml      | 133 ++++++++++++++++++
>  1 file changed, 133 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/cix,sky1-pcie-host.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/cix,sky1-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cix,sky1-pcie-host.yaml
> new file mode 100644
> index 000000000000..b4395bc06f2f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/cix,sky1-pcie-host.yaml
> @@ -0,0 +1,133 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/cix,sky1-pcie-host.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CIX Sky1 PCIe Root Complex
> +
> +maintainers:
> +  - Hans Zhang <hans.zhang@...tech.com>
> +
> +description:
> +  PCIe root complex controller based on the Cadence PCIe core.
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> +  - $ref: /schemas/pci/cdns-pcie.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: cix,sky1-pcie-host
> +
> +  reg:
> +    items:
> +      - description: PCIe controller registers.
> +      - description: Remote CIX System Unit registers.
> +      - description: ECAM registers.
> +      - description: Region for sending messages registers.
> +
> +  reg-names:
> +    items:
> +      - const: reg
> +      - const: rcsu
> +      - const: cfg

cfg is the second, look at cdns bindings.

> +      - const: msg
> +
> +  "#interrupt-cells":
> +    const: 1
> +
> +  interrupt-map-mask:
> +    items:
> +      - const: 0
> +      - const: 0
> +      - const: 0
> +      - const: 7
> +
> +  interrupt-map:
> +    maxItems: 4
> +
> +  max-link-speed:
> +    maximum: 4

Why are you redefining core properties?

> +
> +  num-lanes:
> +    maximum: 8
> +
> +  ranges:
> +    maxItems: 3
> +
> +  msi-map:
> +    maxItems: 1
> +
> +  vendor-id:
> +    const: 0x1f6c

Why? This is implied by compatible.

> +
> +  device-id:
> +    enum:
> +      - 0x0001

Why? This is implied by compatible.

> +
> +  cdns,no-inbound-bar:

That's not a cdns binding, so wrong prefix.

> +    description: |

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

> +      Indicates the PCIe controller does not require an inbound BAR region.

And anyway this is implied by compatible, drop.

> +    type: boolean
> +
> +  sky1,pcie-ctrl-id:
> +    description: |
> +      Specifies the PCIe controller instance identifier (0-4).

No, you don't get an instance ID. Drop the property and look how other
bindings encoded it (not sure about the purpose and you did not explain
it, so cannot advise).

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 4
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - "#interrupt-cells"
> +  - interrupt-map-mask
> +  - interrupt-map
> +  - max-link-speed
> +  - num-lanes
> +  - bus-range
> +  - device_type
> +  - ranges
> +  - msi-map
> +  - vendor-id
> +  - device-id
> +  - cdns,no-inbound-bar
> +  - sky1,pcie-ctrl-id
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    pcie_x8_rc: pcie@...0000 {

Drop unused label.


Best regards,
Krzysztof