Re: [PATCH v4 3/5] PCI/pwrctrl: tc9563: Add GPIO auxiliary device support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Devm vs manual cleanup ordering bug: Auxiliary device is unregistered via devres after the hardware is manually powered off and the I2C adapter reference is dropped.
--

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

PCI/pwrctrl: tc9563: Add GPIO auxiliary device support
    
The commit adds GPIO auxiliary device support to the TC9563 PCI power control
driver, allowing a companion GPIO driver to handle per-port reset signals.
It passes the tc9563 regmap to the auxiliary device and uses devres to
manage unregistration.

> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> index 59ad219c26c02..3fb862105fa58 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
[ ... ]
> @@ -393,6 +398,77 @@ static int tc9563_pwrctrl_parse_device_dt(struct device_node *node,
>  	return 0;
>  }
>  
> +static void tc9563_pwrctrl_adev_release(struct device *dev)
[ ... ]
> +static int tc9563_pwrctrl_adev_add(struct device *dev, const char *name,
> +				   struct device_node *of_node,
> +				   void *priv_data)
> +{
> +	struct auxiliary_device *adev;
[ ... ]
> +	ret = auxiliary_device_add(adev);
> +	if (ret) {
> +		auxiliary_device_uninit(adev);
> +		return ret;
> +	}
> +
> +	return devm_add_action_or_reset(dev, tc9563_pwrctrl_adev_remove, adev);

[Severity: High]
Does registering the auxiliary device unregistration to the devm cleanup phase
cause a teardown ordering issue?

Because devm actions execute after .remove() returns, tc9563_pwrctrl_remove()
will execute first:

static void tc9563_pwrctrl_remove(struct platform_device *pdev)
{
	...
	tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
	i2c_put_adapter(tc9563->adapter);
}

Will this cause the hardware to be manually powered off and the I2C adapter
reference dropped before the child device is unregistered?

Could this lead to I2C transactions on a powered-off device if the GPIO
controller is accessed during the unbind window, and a potential use-after-free
if the adapter module is unloaded while the dummy client and regmap are still
active?

> +}
[ ... ]
> @@ -596,6 +672,10 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
>  	tc9563->pwrctrl.power_on = tc9563_pwrctrl_power_on;
>  	tc9563->pwrctrl.power_off = tc9563_pwrctrl_power_off;
>  
> +	ret = tc9563_pwrctrl_add_gpio_adev(tc9563);
> +	if (ret)
> +		goto remove_i2c;
> +
>  	ret = devm_pci_pwrctrl_device_set_ready(dev, &tc9563->pwrctrl);
>  	if (ret)
>  		goto power_off;
>  
>  	return 0;
>  
>  power_off:
>  	tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
>  remove_i2c:
>  	i2c_put_adapter(tc9563->adapter);
>  	return ret;

[Severity: High]
Is the same teardown ordering regression reproduced in this error path?

If devm_pci_pwrctrl_device_set_ready() fails, the code jumps to power_off,
which manually powers off the hardware and drops the I2C adapter reference.
Then probe returns the error, and devres will clean up the auxiliary device
afterward.

Could this create a race window where the child device is fully bound
and accessible to userspace or other subsystems while the underlying hardware is
already powered off?

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