Re: [PATCH 1/6] dt-bindings: PCI: toshiba,tc9563: Restrict Tx Amplitude, DFE and N_FTS to USP, DSP1 and DSP2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Missing schema constraints on Ethernet MAC node silently allows all undocumented properties.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but there is an out-of-bounds array write in `tc9563_pwrctrl_probe` when parsing multiple Ethernet child nodes.
- [High] This problem wasn't introduced by this patch, but the driver assigns PCI switch port configurations based on fragile child node iteration order rather than `reg` addresses.
--

--- Patch [1]: [PATCH 1/6] dt-bindings: PCI: toshiba,tc9563: Restrict Tx Amplitude, DFE and N_FTS to USP, DSP1 and DSP2 ---
commit 54f91cfce5dde398e9e08b16babbdffbd75f5ca1
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

dt-bindings: PCI: toshiba,tc9563: Restrict Tx Amplitude, DFE and N_FTS to USP, DSP1 and DSP2

This patch restricts Tx Amplitude, DFE, and N_FTS settings to USP, DSP1,
and DSP2 ports in the toshiba,tc9563 PCIe switch binding. It updates the
schema to explicitly disallow these properties on the DSP3 port and the
integrated Ethernet MAC endpoint, and adjusts the example.

> diff --git a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> index b3ad05d90201c..f084830c6d0cb 100644
> --- a/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
> +++ b/Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
[ ... ]
> @@ -64,6 +64,21 @@ patternProperties:
>        - $ref: /schemas/pci/pci-pci-bridge.yaml#
>      unevaluatedProperties: false
>  
> +  "^pcie@[3],0$":
> +    description:
> +      child node describing the internal downstream port connected to the
> +      integrated Ethernet MAC endpoint of the tc9563 switch.
> +    type: object
> +    $ref: /schemas/pci/pci-pci-bridge.yaml#
> +    properties:
> +      n-fts: false
> +    patternProperties:
> +      "^ethernet@0,[0-1]$":
> +        type: object
> +        properties:
> +          n-fts: false

[Severity: Medium]
Does this ethernet node block need an additionalProperties: false constraint?

Without it, the schema defaults to allowing any undeclared properties. 
Could this allow toshiba,tx-amplitude-microvolt to still be placed in the
ethernet node without failing validation?

> +    unevaluatedProperties: false
> +

While reviewing the binding changes, I noticed a couple of pre-existing issues
in the related pci-pwrctrl-tc9563 driver code that might be worth looking into
since they relate to how these device tree nodes are parsed.

[Severity: High]
This isn't a bug introduced by this patch, but there is a potential
out-of-bounds array write in tc9563_pwrctrl_probe() when parsing multiple
Ethernet child nodes:

drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c:tc9563_pwrctrl_probe() {
    ...
    for_each_child_of_node_scoped(node, child) {
        ...
        port++;
        ret = tc9563_pwrctrl_parse_device_dt(tc9563, child1, port);
}

If the device tree contains more than one Ethernet child node under the
pcie@3,0 DSP3 port (which this YAML binding allows via the
^ethernet@0,[0-1]$ pattern), the inner loop can increment the port variable
to 5.

Could this corrupt memory? There are no bounds checks on port before it is
passed to tc9563_pwrctrl_parse_device_dt(), which uses it to index the
cfg[TC9563_MAX] array (size 5). Writing to cfg[5] would overwrite subsequent
struct fields.


[Severity: High]
This is a pre-existing issue, but the driver assigns PCI switch port
configurations based on child node iteration order rather than reg addresses.

drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c:tc9563_pwrctrl_probe() {
    ...
    for_each_child_of_node_scoped(node, child) {
        port++;
}

If a board's Device Tree legitimately omits an unused downstream port (e.g.,
leaving out pcie@1,0), the iteration shifts.

Will this mistakenly map the next port (like pcie@2,0) to TC9563_DSP1? It
seems this would desynchronize the driver's port layout from the physical
hardware, routing commands to the wrong switch ports and missing the DSP3
ethernet handling entirely.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725-tc9563-fix-v1-0-ec4286e31331@oss.qualcomm.com?part=1