Re: [PATCH 1/2] dt-bindings: misc: introduce pci1179,0220.yaml

[email protected]
Newsgroups org.kernel.vger.linux-pci,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The example DT nodes map multiple PCI BARs to the exact same parent physical address, and define unused BARs that contradict the hardware diagram.
- [Low] Typographical error in the compatible string comment.
--

--- Patch [1]: [PATCH 1/2] dt-bindings: misc: introduce pci1179,0220.yaml ---
commit ee5f64080011b047dd81e1bbdc568f784c09b77f
Author: Alex Elder <[email protected]>

dt-bindings: misc: introduce pci1179,0220.yaml

Define the binding for the Toshiba TC9564 PCI endpoint function device.
The third downstream PCIe switch port within this chip has an embedded
PCIe controller, and that implements two of these PCIe functions.

> diff --git a/Documentation/devicetree/bindings/misc/pci1179,0220.yaml b/Documentation/devicetree/bindings/misc/pci1179,0220.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/pci1179,0220.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: pci1179,0220 # Toshiba TC96564 (a.k.a. Qualcomm QPS615)

[Severity: Low]
This isn't a bug, but there's a typo in this comment. It says TC96564 instead
of TC9564.

[ ... ]
> +        pci@0,0 {
> +            compatible = "pci1179,0220";
> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            /* Ranges will be updated dynamically */
> +            ranges = <0x0 0x0 0x0  0x83000000 0x0 0x0  0x0 0x4000>,
> +                     <0x2 0x0 0x0  0x83000000 0x0 0x0  0x0 0x80000>,
> +                     <0x4 0x0 0x0  0x83000000 0x0 0x0  0x0 0x200000>;

[Severity: Low]
Are these example ranges accurate for the hardware? The mappings configure
BAR 0, BAR 2, and BAR 4 to use the identical parent physical address 
(0x83000000), which would result in overlapping memory regions.

Additionally, the hardware diagram in the binding description indicates
BAR 2 is not present, but it is configured in this example.

[ ... ]
> +        pci@0,1 {
> +            compatible = "pci1179,0220";
> +            reg = <0x100 0x0 0x0 0x0 0x0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            /* Ranges will be updated dynamically */
> +            ranges = <0x0 0x0 0x0  0x83000100 0x0 0x0  0x0 0x4000>,
> +                     <0x2 0x0 0x0  0x83000100 0x0 0x0  0x0 0x80000>,
> +                     <0x4 0x0 0x0  0x83000100 0x0 0x0  0x0 0x200000>;

[Severity: Low]
Do these ranges match the hardware design for Function 1? The example maps
BAR 0 and BAR 2 here, but the hardware diagram indicates Function 1 only
uses BAR 4.

These mappings also map to the same parent physical address (0x83000100),
causing overlapping memory regions similar to the previous node.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.