Re: [PATCH v1] spi: geni-qcom: Fix CPHA and CPOL mode change detection

[Val Packett <val@packett.cool>]

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