Re: [PATCH v3 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver ignores the return value of pm_runtime_get_sync() before accessing hardware registers in spi_geni_init().
--

commit 297907766125fb69231313b230385906933609be
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

spi: qcom-geni: Use geni_se_resources_init() for resource initialization

Replace resources initialization such as clocks, ICC path and OPP with the
common geni_se_resources_init() function to avoid code duplication. This
handles all these resources internally, ensuring consistent resource
management across GENI SE drivers.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index a04cdc1e5ad4d..333216df922e8 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c

[ ... ]

> @@ -1104,24 +1092,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);
>  	if (ret)
>  		return ret;
>  
> -	/* Set the bus quota to a reasonable value for register access */
> -	mas->se.icc_paths[GENI_TO_CORE].avg_bw = Bps_to_icc(CORE_2X_50_MHZ);
> -	mas->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
> -
> -	ret = geni_icc_set_bw(&mas->se);
> -	if (ret)
> -		return ret;
> -
>  	ret = spi_geni_init(mas);

[Severity: High]
This is a pre-existing issue, but does spi_geni_init() fail to check the
return value of pm_runtime_get_sync() before accessing hardware registers?

drivers/spi/spi-geni-qcom.c:spi_geni_init() {
    ...
    pm_runtime_get_sync(mas->dev);

    proto = geni_se_read_proto(se);
    ...
}

The pm_runtime_get_sync() function can return a negative error code if the
device fails to resume. By not checking the return value, spi_geni_init()
proceeds to read from the hardware via geni_se_read_proto().

On QCOM platforms, reading from unclocked or powered-off peripheral
registers causes a synchronous external abort (bus error), leading to a
kernel panic.

>  	if (ret)
>  		return ret;
>  
> -	/*
> -	 * TX is required per GSI spec, see setup_gsi_xfer().
> -	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-enable-spi-on-sa8255p-v3-0-43984eac4c67@oss.qualcomm.com?part=2