linux-kernel - Re: [PATCH v2] spi: stm32: enable controller before asserting CS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZkJBqSbJakye6BBc@dell-precision-5540>
Date: Mon, 13 May 2024 12:36:57 -0400
From: Ben Wolsieffer <ben.wolsieffer@...ring.com>
To: Leonard Göhrs <l.goehrs@...gutronix.de>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-stm32@...md-mailman.stormreply.com, linux-spi@...r.kernel.org,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Mark Brown <broonie@...nel.org>,
	Alain Volmat <alain.volmat@...s.st.com>
Subject: Re: [PATCH v2] spi: stm32: enable controller before asserting CS

On Mon, May 13, 2024 at 10:29:49AM +0200, Leonard Göhrs wrote:
> Hi,
> 
> I am in the process of updating an STM32MP157 based device from 6.8 to 6.9
> and have noticed SPI related issues that may be caused by this change.
> 
> I am testing on an LXA TAC Generation 2 (arch/arm/boot/dts/st/stm32mp157c-lxa-tac-gen2.dts)
> and the issues I see are SPI transfer timeouts:
> 
>   [   13.565081] panel-mipi-dbi-spi spi2.0: SPI transfer timed out
>   [   13.565131] spi_master spi2: failed to transfer one message from queue
>   [   13.565134] spi_stm32 44005000.spi: spurious IT (sr=0x00010002, ier=0x00000000)
>   [   13.565145] spi_master spi2: noqueue transfer failed
>   [   13.565183] panel-mipi-dbi-spi spi2.0: error -110 when sending command 0x2a
>   [   13.769113] panel-mipi-dbi-spi spi2.0: SPI transfer timed out
>   [   13.769163] spi_master spi2: failed to transfer one message from queue
>   [   13.769164] spi_stm32 44005000.spi: spurious IT (sr=0x00010002, ier=0x00000000)
>   [   13.769177] spi_master spi2: noqueue transfer failed
>   [   13.769210] panel-mipi-dbi-spi spi2.0: error -110 when sending command 0x2b
>   [   13.977028] panel-mipi-dbi-spi spi2.0: SPI transfer timed out
>   [   13.977082] spi_master spi2: failed to transfer one message from queue
>   [   13.977095] spi_master spi2: noqueue transfer failed
>   [   14.460924] panel-mipi-dbi-spi spi2.0: SPI transfer timed out
> 
> Followed by workqueue lockups and the device becoming unresponsive later
> in the boot, preventing me from logging in and investigating further that way:
> 
>   [   17.026263] spi_master spi2: noqueue transfer failed
> 
>   TAC OS - The LXA TAC operating system 24.04+dev lxatac-00011 ttySTM0
> 
>   lxatac-00011 login: root
>   [   62.434326] BUG: workqueue lockup - pool cpus=0 node=0 flags=0x0 nice=0 stuck for 44s!
>   [   62.441321] BUG: workqueue lockup - pool cpus=0 node=0 flags=0x0 nice=-20 stuck for 44s!
>   …
> 
> Reverting this commit fixes the issue for me. It may be some time before
> I get around to investigating the issue in detail, so I thought I should
> ask if anyone else has already noticed this as well.
> 
> We are currently in the process of adding the device in question to
> KernelCI [1], which may help in catching such problems earlier.
> 
> [1]: https://github.com/kernelci/kernelci-core/pull/2542
> 

Sorry about that; it looks like the STM32H7/MP platforms require the
controller to be enabled later. I agree that it should be reverted and
I'll try to think of another approach.

The STM32H7/MP devices are significantly different from the F4/7
devices, which makes it difficult to change shared code without causing
problems like this. I wonder if it might be better to split the F4/7
support into a separate driver. This would duplicate a bit of code,
namely the initialization in probe, the baud rate divider calculation
and the SPI mode config, but would make testing easier and get rid of
the indirection that handles the different register offsets and field
masks on each platform. The code for actually transcieving data and
handling IRQs is already platform specific.