From: sashiko-bot@kernel.org
To: "Praveen Talari" <praveen.talari@oss.qualcomm.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 4/4] spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms
Date: Sat, 30 May 2026 19:12:25 +0000 [thread overview]
Message-ID: <20260530191225.80E461F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260530-enable-spi-on-sa8255p-v2-4-17574601bd63@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] GPI DMA mode (`setup_gsi_xfer()`) bypasses the `set_rate` abstraction, breaking SPI transfers on the SA8255P platform.
--
commit 51c1e984c5666b6d16483f6836d8eccfa71a9fa2
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms
The patch enables SPI for the SA8255P platform using SCMI power and
performance protocols. It introduces a dev_data struct with function
pointers to abstract initialization and rate-setting operations.
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 7696412c6e879..98e34e58fae21 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);
[Severity: High]
Should setup_gsi_xfer() also be updated to use this new set_rate abstraction?
Looking at setup_gsi_xfer(), it appears to still hardcode a call to
get_spi_clk_cfg():
drivers/spi/spi-geni-qcom.c:setup_gsi_xfer() {
...
peripheral.pack_en = true;
peripheral.word_len = xfer->bits_per_word - MIN_WORD_LEN;
ret = get_spi_clk_cfg(mas->cur_speed_hz, mas,
&peripheral.clk_src, &peripheral.clk_div);
if (ret) {
dev_err(mas->dev, "Err in get_spi_clk_cfg() :%d\n", ret);
return ret;
}
...
}
Since SCMI-based platforms like SA8255P rely on geni_se_domain_attach()
which leaves se->clk uninitialized (NULL), get_spi_clk_cfg() will fail
when calling clk_round_rate() and geni_se_clk_freq_match().
Does this unconditional bypass of the set_rate abstraction break all
SPI transfers using GPI DMA on the SA8255P platform?
> if (ret)
> return ret;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260530-enable-spi-on-sa8255p-v2-0-17574601bd63@oss.qualcomm.com?part=4
prev parent reply other threads:[~2026-05-30 19:12 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-30 18:28 [PATCH v2 0/4] Enable SPI on SA8255p Qualcomm platforms Praveen Talari
2026-05-30 18:28 ` [PATCH v2 1/4] spi: dt-bindings: describe SA8255p Praveen Talari
2026-05-30 18:38 ` sashiko-bot
2026-05-30 18:28 ` [PATCH v2 2/4] spi: qcom-geni: Use geni_se_resources_init() for resource initialization Praveen Talari
2026-05-30 18:52 ` sashiko-bot
2026-05-30 18:28 ` [PATCH v2 3/4] spi: qcom-geni: Use resources helper APIs in runtime PM functions Praveen Talari
2026-05-30 19:02 ` sashiko-bot
2026-05-30 18:28 ` [PATCH v2 4/4] spi: qcom-geni: Enable SPI on SA8255p Qualcomm platforms Praveen Talari
2026-05-30 19:12 ` sashiko-bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260530191225.80E461F00893@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=praveen.talari@oss.qualcomm.com \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.