Re: [PATCH v4 5/5] arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but in `pci_pwrctrl_power_on_device()`, if powering on a child node fails (e.g., returning `-EPROBE_DEFER`), previously successfully powered-on sibling children within the same loop iteration are not rolled back, leading to left-on power states.
--

commit f88f26b5508f76a873435c0070f5f6adab4564f7
Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

arm64: dts: qcom: qcs6490-rb3gen2: Enable TC9563 embedded GPIO controller

This commit adds the TC9563 embedded GPIO controller to the QCS6490-RB3Gen2
device tree. It describes the switch node as a GPIO controller and uses it
to drive the PERST# reset lines for two external downstream PCIe ports
(pcie@1,0 and pcie@2,0).

> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index a13315bf0fb07..4da7342ca90d6 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts

[ ... ]

> @@ -891,6 +891,9 @@ pcie@0,0 {
>  		pinctrl-0 = <&tc9563_resx_n>;
>  		pinctrl-names = "default";
>  
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +
>  		pcie1_switch0_dsp1: pcie@1,0 {
>  			reg = <0x20800 0x0 0x0 0x0 0x0>;
>  			#address-cells = <3>;
> @@ -899,6 +902,7 @@ pcie1_switch0_dsp1: pcie@1,0 {
>  			device_type = "pci";
>  			ranges;
>  			bus-range = <0x3 0xff>;
> +			reset-gpios = <&tc9563 2 GPIO_ACTIVE_LOW>;
>  		};
>  
>  		pcie@2,0 {
> @@ -909,6 +913,7 @@ pcie@2,0 {
>  			device_type = "pci";
>  			ranges;
>  			bus-range = <0x4 0xff>;
> +			reset-gpios = <&tc9563 3 GPIO_ACTIVE_LOW>;

[Severity: High]
This isn't a bug introduced by this patch, but adding multiple downstream PCIe
ports under a single power-controlled switch node here exposes a pre-existing
resource leak in the PCI power control core.

In pci_pwrctrl_power_on_device(), if powering on a child node fails (for
example, returning -EPROBE_DEFER for pcie@2,0), previously successfully
powered-on sibling children within the same loop iteration (like pcie@1,0)
are not rolled back:

drivers/pci/pwrctrl/core.c:pci_pwrctrl_power_on_device() {
    ...
    for_each_available_child_of_node_scoped(np, child) {
        ret = pci_pwrctrl_power_on_device(child);
        if (ret)
            return ret;
    }
    ...
}

Because the function returns immediately on failure, the regulator and clock
enable counts for the earlier siblings are permanently leaked. Since probe
deferral is a standard boot path, this leads to unbounded resource enable
increments.

Should pci_pwrctrl_power_on_device() be updated to power off successfully
processed children during the error path before returning?

>  
>  			/* Renesas μPD720201 PCIe USB3.0 Host Controller */
>  			usb-controller@0,0 {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-pci-tc9563-aux-v4-0-c7fc7778292b@oss.qualcomm.com?part=5