linux-kernel - Re: [PATCH v2 03/10] dt-bindings: PCI: Add ASPEED PCIe RC support

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: <20250716-watchful-enigmatic-condor-0fc6b3@krzk-bin>
Date: Wed, 16 Jul 2025 10:27:16 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Jacky Chou <jacky_chou@...eedtech.com>
Cc: bhelgaas@...gle.com, lpieralisi@...nel.org, kwilczynski@...nel.org, 
	mani@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	joel@....id.au, andrew@...econstruct.com.au, linux-aspeed@...ts.ozlabs.org, 
	linux-pci@...r.kernel.org, devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, openbmc@...ts.ozlabs.org, linux-gpio@...r.kernel.org, 
	linus.walleij@...aro.org, p.zabel@...gutronix.de, BMC-SW@...eedtech.com
Subject: Re: [PATCH v2 03/10] dt-bindings: PCI: Add ASPEED PCIe RC support

On Tue, Jul 15, 2025 at 11:43:13AM +0800, Jacky Chou wrote:
> This binding describes the required and optional properties for

No, describe the hardware, not "this binding".

> configuring the PCIe RC node, including support for syscon phandles,
> MSI, clocks, resets, and interrupt mapping. The schema enforces strict
> property validation and provides a comprehensive example for reference.

Don't say what schema does or does not. It's completely redundant.
Describe the hardware.

Your entire commit is redundantn and not helpful at all.

> 

...

> +
> +  aspeed,ahbc:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to the ASPEED AHB Controller (AHBC) syscon node.
> +      This reference is used by the PCIe controller to access
> +      system-level configuration registers related to the AHB bus.
> +      To enable AHB access for the PCIe controller.
> +
> +  aspeed,pciecfg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to the ASPEED PCIe configuration syscon node.
> +      This reference allows the PCIe controller to access
> +      SoC-specific PCIe configuration registers. There are the others
> +      functions such PCIe RC and PCIe EP will use this common register
> +      to configure the SoC interfaces.
> +
> +  aspeed,pciephy:

No, phys are not syscons. I already told you that in v1.

> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to the ASPEED PCIe PHY syscon node.
> +      This property provides access to the PCIe PHY control
> +      registers required for link initialization and management.
> +      The other functions such PCIe RC and PCIe EP will use this
> +      common register to configure the PHY interfaces and get some
> +      information from the PHY.
> +
> +  interrupt-controller:
> +    description: Interrupt controller node for handling legacy PCI interrupts.
> +    type: object
> +    properties:
> +      '#address-cells':
> +        const: 0
> +      '#interrupt-cells':
> +        const: 1
> +      interrupt-controller: true
> +
> +    required:
> +      - '#address-cells'
> +      - '#interrupt-cells'
> +      - interrupt-controller
> +
> +    additionalProperties: false
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus-common.yaml#

No other binding references this. Don't write completely different code
than all other SoCs. This entire binding is written such way.

> +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> +  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: aspeed,ast2600-pcie
> +    then:
> +      required:
> +        - aspeed,ahbc
> +    else:
> +      properties:
> +        aspeed,ahbc: false
> +
> +required:
> +  - reg
> +  - interrupts
> +  - bus-range
> +  - ranges
> +  - resets
> +  - reset-names
> +  - msi-parent
> +  - msi-controller
> +  - aspeed,pciecfg
> +  - interrupt-map-mask
> +  - interrupt-map
> +  - interrupt-controller
> +
> +unevaluatedProperties: false
> +
> +patternProperties:
> +  "^pcie@[0-9a-f,]+$":

Why do you need it? Also, order things according to example schema.

Best regards,
Krzysztof