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

[Val Packett]

On 3/16/26 2:13 PM, Jonathan Marek wrote:
> Reviewed-by: Jonathan Marek <jonathan@xxxxxxxx>
>
> 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@xxxxxxxxxxxx>


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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/

> 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@xxxxxxxxxxxxxxxx>
> > ---
> > 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