Re: [PATCH v3 4/4] spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incomplete abstraction of SPI clock management breaks both GPI DMA and FIFO/SE_DMA modes for SCMI-based platforms.
- [High] Unchecked return value of `device_get_match_data()` leads to a NULL pointer dereference in probe.
--

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

spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms

The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
This patch adds support for these platforms by introducing an abstraction
for setting the frequency using the SCMI performance protocol.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 4205349376955..1b92fb1bd8bfb 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c

[ ... ]

> @@ -820,7 +829,7 @@ static int setup_se_xfer(struct spi_transfer *xfer,
>  	}
>  
>  	/* Speed and bits per word can be overridden per transfer */
> -	ret = geni_spi_set_clock_and_bw(mas, xfer->speed_hz);
> +	ret = mas->dev_data->set_rate(&mas->se, xfer->speed_hz);
>  	if (ret)
>  		return ret;

[Severity: High]
Does this new abstraction break GPI DMA mode for SCMI-based platforms?

In GPI DMA mode, setup_gsi_xfer() completely bypasses this set_rate
abstraction and directly calls get_spi_clk_cfg():

setup_gsi_xfer() {
    ...
    ret = get_spi_clk_cfg(mas->cur_speed_hz, mas,
                          &peripheral.clk_src, &peripheral.clk_div);
    ...
}

Because SCMI platforms do not initialize se->clk, clk_round_rate() inside
geni_se_clk_tbl_get() will return 0 on a NULL clock, unconditionally failing
the transfer.

Also, for FIFO/SE_DMA modes, setup_se_xfer() correctly uses the new
set_rate() callback (mapped to geni_se_set_perf_opp()). However,
geni_se_set_perf_opp() only sets the SCMI performance OPP and completely
omits the required hardware register writes to SE_GENI_CLK_SEL and
GENI_SER_M_CLK_CFG.

Without setting the SER_CLK_EN bit in GENI_SER_M_CLK_CFG, will the internal
serial engine clock remain disabled and prevent the SPI hardware from
functioning?

[ ... ]

> @@ -1064,7 +1073,9 @@ static int spi_geni_probe(struct platform_device *pdev)
>  	mas->se.wrapper = dev_get_drvdata(dev->parent);
>  	mas->se.base = base;
>  
> -	ret = geni_se_resources_init(&mas->se);
> +	mas->dev_data = device_get_match_data(&pdev->dev);
> +
> +	ret = mas->dev_data->resources_init(&mas->se);

[Severity: High]
Could device_get_match_data() return NULL here?

If the driver is instantiated via a method that does not use the OF match
table, such as manual bind via sysfs or legacy platform device instantiation,
device_get_match_data() will return NULL. Dereferencing it immediately to call
resources_init() would lead to a kernel panic.

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