Re: [PATCH v2 1/4] dt-bindings: PCI: renesas,r9a08g045-pcie: Add RZ/V2H(P) support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch mandates `linux,pci-domain` to identify the hardware controller instance and determine system controller register offsets, which is a Device Tree architectural violation.
--

commit ed13e82a06909d08d89ba9cf97e57413bb3dc6c8
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

dt-bindings: PCI: renesas,r9a08g045-pcie: Add RZ/V2H(P) support
    
This commit adds support for the PCIe controller found on the Renesas
RZ/V2H(P) SoC. It makes the linux,pci-domain and num-lanes properties
mandatory for identifying the controller instance and mapping system
controller registers.

>     Unlike earlier SoCs supported by this driver which only feature a single
>     PCIe controller, the RZ/V2H(P) SoC implements two controllers. Both
>     controllers rely on the system controller (`sysc`) for configuration,
>     but the required registers reside at different offsets for each instance.
>     To correctly identify the controller instance and map the corresponding
>     system controller registers, make the "linux,pci-domain" and "num-lanes"
>     properties mandatory for this SoC and restrict their values according to
>     the hardware capabilities.

Is it appropriate to use linux,pci-domain to identify the hardware
controller instance?

Device Tree is designed to describe hardware rather than software policy
or configuration. The linux,pci-domain property is an OS-specific hint
used to assign logical PCI domain numbers.

Using it to identify hardware instances strictly couples the hardware
register mapping to an OS numbering scheme.

> diff --git a/Documentation/devicetree/bindings/pci/renesas,r9a08g045-pcie.yaml b/Documentation/devicetree/bindings/pci/renesas,r9a08g045-pcie.yaml
> index 90086909e9210..149a2973b16eb 100644
> --- a/Documentation/devicetree/bindings/pci/renesas,r9a08g045-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/renesas,r9a08g045-pcie.yaml
[ ... ]
> @@ -235,6 +238,20 @@ allOf:
>            maxItems: 1
>          reset-names:
>            maxItems: 1
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a09g057-pcie
> +    then:
> +      properties:
> +        linux,pci-domain:
> +          enum: [0, 1]

Does limiting this to [0, 1] cause a regression for valid software
configurations?

For example, if a board with multiple PCIe controllers assigns domains 2
and 3 to these instances to avoid conflicts, the binding will fail
validation and the driver will fail to probe.

Could the hardware instances instead be identified via standard hardware
descriptive mechanisms, such as their physical base address (reg),
standard aliases, or by adding a phandle specifier argument to
renesas,sysc?

> +        num-lanes:
> +          enum: [2, 4]
> +      required:
> +        - linux,pci-domain
> +        - num-lanes
>  
>  unevaluatedProperties: false

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518155324.168948-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=1