Re: [PATCH v4 2/4] spi: qcom-geni: Use geni_se_resources_init() for resource initialization

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The `geni_se_resources_init()` function is missing from the kernel tree.

Pre-existing issues:
- [High] Asynchronous `pm_runtime_put()` is used on error paths that immediately call `pm_runtime_disable()`, risking the hardware being left in an invalid active state.
--

commit e8e291bf8f15d73b4da4bf256d08b86fec92a093
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
spi: qcom-geni: Use geni_se_resources_init() for resource initialization

This commit replaces individual resource initializations such as clocks, ICC
paths, and OPP tables with a common geni_se_resources_init() function. This
aims to reduce code duplication and ensure consistent resource management
across GENI SE drivers.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 26e723cfea61e..09feb6577efcb 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c

[ ... ]

> @@ -1079,17 +1074,10 @@ static int spi_geni_probe(struct platform_device *pdev)
>  	mas->se.dev = dev;
>  	mas->se.wrapper = dev_get_drvdata(dev->parent);
>  	mas->se.base = base;
> -	mas->se.clk = clk;
>  
> -	ret = devm_pm_opp_set_clkname(&pdev->dev, "se");
> +	ret = geni_se_resources_init(&mas->se);

[Severity: Low]
Is this function defined anywhere in the kernel tree?

It doesn't appear to be declared or defined in this commit or the existing
codebase, which results in an implicit declaration error and linker failure.

>  	if (ret)
>  		return ret;

[ ... ]

> @@ -1115,24 +1103,12 @@ static int spi_geni_probe(struct platform_device *pdev)
>  	if (spi->target)
>  		spi->target_abort = spi_geni_target_abort;
>  
> -	ret = geni_icc_get(&mas->se, NULL);
> -	if (ret)
> -		return ret;
> -
>  	pm_runtime_use_autosuspend(&pdev->dev);
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, 250);
>  	ret = devm_pm_runtime_enable(dev);

[Severity: High]
This is a pre-existing issue, but I noticed a potential resource leak related
to how runtime PM is disabled.

If probe fails later on, the devres framework will immediately call
pm_runtime_disable() to undo devm_pm_runtime_enable().

Does this cause the asynchronous pm_runtime_put() called inside
spi_geni_init() to have its queued idle/suspend work cancelled? This could
leave the device permanently powered on after a probe failure.

>  	if (ret)
>  		return ret;

[ ... ]

>  	ret = spi_geni_init(mas);
>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-enable-spi-on-sa8255p-v4-0-f5b5067e7e1e@oss.qualcomm.com?part=2