setup_fifo_params computes mode_changed from spi->mode flags but tests
it against SE_SPI_CPHA and SE_SPI_CPOL, which are register offsets,
not SPI mode bits. This causes CPHA and CPOL updates to be skipped
on mode switches, leaving the controller with stale clock phase
and polarity settings.

Fix this by using SPI_CPHA and SPI_CPOL to detect mode changes before
updating the corresponding registers.

Fixes: 781c3e71c94c ("spi: spi-geni-qcom: rework setup_fifo_params")
Signed-off-by: Maramaina Naresh <naresh.maramaina@oss.qualcomm.com>
---
This patch fixes SPI mode change detection in the spi-geni-qcom driver.

setup_fifo_params compared spi->mode against SE_SPI_CPHA/SE_SPI_CPOL,
which are register offsets instead of SPI_CPHA/SPI_CPOL mode bits.
This could skip CPHA/CPOL updates on mode switches and leave stale
clock configuration.

This is a single-patch series.
---
 drivers/spi/spi-geni-qcom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 43ce47f2454c..772b7148ba5f 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -359,9 +359,9 @@ static int setup_fifo_params(struct spi_device *spi_slv,
 		writel((spi_slv->mode & SPI_LOOP) ? LOOPBACK_ENABLE : 0, se->base + SE_SPI_LOOPBACK);
 	if (cs_changed)
 		writel(chipselect, se->base + SE_SPI_DEMUX_SEL);
-	if (mode_changed & SE_SPI_CPHA)
+	if (mode_changed & SPI_CPHA)
 		writel((spi_slv->mode & SPI_CPHA) ? CPHA : 0, se->base + SE_SPI_CPHA);
-	if (mode_changed & SE_SPI_CPOL)
+	if (mode_changed & SPI_CPOL)
 		writel((spi_slv->mode & SPI_CPOL) ? CPOL : 0, se->base + SE_SPI_CPOL);
 	if ((mode_changed & SPI_CS_HIGH) || (cs_changed && (spi_slv->mode & SPI_CS_HIGH)))
 		writel((spi_slv->mode & SPI_CS_HIGH) ? BIT(chipselect) : 0, se->base + SE_SPI_DEMUX_OUTPUT_INV);

---
base-commit: 7109a2155340cc7b21f27e832ece6df03592f2e8
change-id: 20260316-spi-geni-cpha-cpol-fix-89126ed55325

Best regards,
-- 
Maramaina Naresh <naresh.maramaina@oss.qualcomm.com>

On Mon, 16 Mar 2026 18:53:31 +0530, Maramaina Naresh wrote:
> spi: geni-qcom: Fix CPHA and CPOL mode change detection

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.0

Thanks!

[1/1] spi: geni-qcom: Fix CPHA and CPOL mode change detection
      https://git.kernel.org/broonie/spi/c/ba3402f6c85b

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

On Mon, 16 Mar 2026 18:53:31 +0530, Maramaina Naresh wrote:
> setup_fifo_params computes mode_changed from spi->mode flags but tests
> it against SE_SPI_CPHA and SE_SPI_CPOL, which are register offsets,
> not SPI mode bits. This causes CPHA and CPOL updates to be skipped
> on mode switches, leaving the controller with stale clock phase
> and polarity settings.
> 
> Fix this by using SPI_CPHA and SPI_CPOL to detect mode changes before
> updating the corresponding registers.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: geni-qcom: Fix CPHA and CPOL mode change detection
      https://git.kernel.org/broonie/misc/c/ba3402f6c85b

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Reviewed-by: Jonathan Marek <jonathan@marek.ca>

at least it doesn't look like this stupid mistake breaks anything 
upstream (no spi-cpha/spi-cpol in any qcom dts)

On 3/16/26 9:23 AM, Maramaina Naresh wrote:
> setup_fifo_params computes mode_changed from spi->mode flags but tests
> it against SE_SPI_CPHA and SE_SPI_CPOL, which are register offsets,
> not SPI mode bits. This causes CPHA and CPOL updates to be skipped
> on mode switches, leaving the controller with stale clock phase
> and polarity settings.
> 
> Fix this by using SPI_CPHA and SPI_CPOL to detect mode changes before
> updating the corresponding registers.
> 
> Fixes: 781c3e71c94c ("spi: spi-geni-qcom: rework setup_fifo_params")
> Signed-off-by: Maramaina Naresh <naresh.maramaina@oss.qualcomm.com>
> ---
> This patch fixes SPI mode change detection in the spi-geni-qcom driver.
> 
> setup_fifo_params compared spi->mode against SE_SPI_CPHA/SE_SPI_CPOL,
> which are register offsets instead of SPI_CPHA/SPI_CPOL mode bits.
> This could skip CPHA/CPOL updates on mode switches and leave stale
> clock configuration.
> 
> This is a single-patch series.
> ---
>   drivers/spi/spi-geni-qcom.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 43ce47f2454c..772b7148ba5f 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -359,9 +359,9 @@ static int setup_fifo_params(struct spi_device *spi_slv,
>   		writel((spi_slv->mode & SPI_LOOP) ? LOOPBACK_ENABLE : 0, se->base + SE_SPI_LOOPBACK);
>   	if (cs_changed)
>   		writel(chipselect, se->base + SE_SPI_DEMUX_SEL);
> -	if (mode_changed & SE_SPI_CPHA)
> +	if (mode_changed & SPI_CPHA)
>   		writel((spi_slv->mode & SPI_CPHA) ? CPHA : 0, se->base + SE_SPI_CPHA);
> -	if (mode_changed & SE_SPI_CPOL)
> +	if (mode_changed & SPI_CPOL)
>   		writel((spi_slv->mode & SPI_CPOL) ? CPOL : 0, se->base + SE_SPI_CPOL);
>   	if ((mode_changed & SPI_CS_HIGH) || (cs_changed && (spi_slv->mode & SPI_CS_HIGH)))
>   		writel((spi_slv->mode & SPI_CS_HIGH) ? BIT(chipselect) : 0, se->base + SE_SPI_DEMUX_OUTPUT_INV);
> 
> ---
> base-commit: 7109a2155340cc7b21f27e832ece6df03592f2e8
> change-id: 20260316-spi-geni-cpha-cpol-fix-89126ed55325
> 
> Best regards,
>

On 3/16/26 2:13 PM, Jonathan Marek wrote:
> Reviewed-by: Jonathan Marek <jonathan@marek.ca>
>
> at least it doesn't look like this stupid mistake breaks anything 
> upstream (no spi-cpha/spi-cpol in any qcom dts)

There might not have been any users upstream but you never know what 
people are working on :)

Looks like this might've unblocked my progress with one of the phones I 
have WIP for (sm6115-motorola-guamp) which has a Himax touchscreen 
(yuck[1]) that uses SPI mode 3. Only tested quickly but at least one 
reply in the early init sequence makes sense now!

Before:

hx83102j spi0.0: himax_spi_read: xfer_tx_data: f3 08 00
hx83102j spi0.0: himax_spi_read: xfer_rx_data: 08 00 00 00
hx83102j spi0.0: hx83102j_sense_off: Do not need wait FW, Status = 0x08!
[..]
hx83102j spi0.0: himax_spi_read: xfer_tx_data: f3 08 00
hx83102j spi0.0: himax_spi_read: xfer_rx_data: 18 00 00 00

After:

hx83102j spi0.0: himax_spi_read: xfer_tx_data: f3 08 00
hx83102j spi0.0: himax_spi_read: xfer_rx_data: 04 00 00 00
hx83102j spi0.0: hx83102j_sense_off: Do not need wait FW, Status = 0x04!
[..]
hx83102j spi0.0: himax_spi_read: xfer_tx_data: f3 08 00
hx83102j spi0.0: himax_spi_read: xfer_rx_data: 0c 00 00 00
hx83102j spi0.0: hx83102j_sense_off: Safe mode entered

So that's a very late

Tested-by: Val Packett <val@packett.cool>


BTW, spi-cpha/spi-cpol in DTS is not an entirely reliable indicator it 
seems? Loooots of drivers set SPI_MODE_x in code explicitly, my 
understanding is that that overrides dts.


[1]: 
https://lore.kernel.org/all/TY0PR06MB561105A3386E9D76F429110D9E0F2@TY0PR06MB5611.apcprd06.prod.outlook.com/

> On 3/16/26 9:23 AM, Maramaina Naresh wrote:
>> setup_fifo_params computes mode_changed from spi->mode flags but tests
>> it against SE_SPI_CPHA and SE_SPI_CPOL, which are register offsets,
>> not SPI mode bits. This causes CPHA and CPOL updates to be skipped
>> on mode switches, leaving the controller with stale clock phase
>> and polarity settings.
>>
>> Fix this by using SPI_CPHA and SPI_CPOL to detect mode changes before
>> updating the corresponding registers.
>>
>> Fixes: 781c3e71c94c ("spi: spi-geni-qcom: rework setup_fifo_params")
>> Signed-off-by: Maramaina Naresh <naresh.maramaina@oss.qualcomm.com>
>> ---
>> This patch fixes SPI mode change detection in the spi-geni-qcom driver.
>>
>> setup_fifo_params compared spi->mode against SE_SPI_CPHA/SE_SPI_CPOL,
>> which are register offsets instead of SPI_CPHA/SPI_CPOL mode bits.
>> This could skip CPHA/CPOL updates on mode switches and leave stale
>> clock configuration.
>> […]

Thanks a lot for finding this!!

~val

On 3/16/26 2:23 PM, Maramaina Naresh wrote:
> setup_fifo_params computes mode_changed from spi->mode flags but tests
> it against SE_SPI_CPHA and SE_SPI_CPOL, which are register offsets,
> not SPI mode bits. This causes CPHA and CPOL updates to be skipped
> on mode switches, leaving the controller with stale clock phase
> and polarity settings.
> 
> Fix this by using SPI_CPHA and SPI_CPOL to detect mode changes before
> updating the corresponding registers.
> 
> Fixes: 781c3e71c94c ("spi: spi-geni-qcom: rework setup_fifo_params")
> Signed-off-by: Maramaina Naresh <naresh.maramaina@oss.qualcomm.com>
> ---
> This patch fixes SPI mode change detection in the spi-geni-qcom driver.
> 
> setup_fifo_params compared spi->mode against SE_SPI_CPHA/SE_SPI_CPOL,
> which are register offsets instead of SPI_CPHA/SPI_CPOL mode bits.
> This could skip CPHA/CPOL updates on mode switches and leave stale
> clock configuration.
> 
> This is a single-patch series.

Note this ""cover letter"" is unnecessary for such single-patch

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad