Re: [PATCH v6 4/9] PCI: rzg3s-host: Move host configuration code together

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cleanup: mixing goto and __free() in rzg3s_pcie_probe
--

commit 58c1ad45841aacf58a73f4caada04798bba4e7e2
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

PCI: rzg3s-host: Move host configuration code together

This commit rearranges the host configuration functions together to prepare for
hotplug support. It also adjusts the error handling paths and labels in the
probe and suspend/resume routines.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 077cfb0834b37..3ecada2384023 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c

[ ... ]

> @@ -1902,12 +1893,16 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto rpm_disable;
>  
> +	ret = rzg3s_pcie_power_resets_deassert(host);
> +	if (ret)
> +		goto rpm_put;
> +

[Severity: Medium]
Does this error handling violate the cleanup subsystem requirements?

The rzg3s_pcie_probe() function declares sysc_np using __free(device_node):

    struct device_node *sysc_np __free(device_node) =
        of_parse_phandle(np, "renesas,sysc", 0);

However, the function also heavily uses manual goto rollback labels, and
this patch modifies those paths. According to include/linux/cleanup.h,
mixing goto-based cleanup labels and scope-based __free() in the same
function is not permitted due to the risk of confusing ownership semantics.

Should this function either be converted entirely to scope-based cleanups
or avoid __free() entirely?

>  	raw_spin_lock_init(&host->hw_lock);
>  
>  	ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_init_irqdomain,
>  				    rzg3s_pcie_teardown_irqdomain);
>  	if (ret)
> -		goto rpm_put;
> +		goto power_resets_assert;
>  
>  	bridge->sysdata = host;
>  	bridge->ops = &rzg3s_pcie_root_ops;
> @@ -1922,12 +1917,13 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>  	clk_disable_unprepare(host->port.refclk);
>  	rzg3s_pcie_teardown_irqdomain(host);
>  	host->data->config_deinit(host);
> +power_resets_assert:
> +	reset_control_bulk_assert(host->data->num_power_resets,
> +				  host->power_resets);
>  rpm_put:
>  	pm_runtime_put_sync(dev);
>  rpm_disable:
>  	pm_runtime_disable(dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007053645.1391545-1-claudiu.beznea@kernel.org?part=4