| Message ID | 1f2b3af4-2b7a-4ac8-ab95-c80120ebf44c@arm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0bb394067a792e7119abc9e0b7158ef19381f456 |
| Headers | show |
| Series | spi: rockchip: Fix PM runtime count on no-op cs | expand |
On Fri, 06 Dec 2024 19:50:55 +0000, Christian Loehle wrote: > The early bail out that caused an out-of-bounds write was removed with > commit 5c018e378f91 ("spi: spi-rockchip: Fix out of bounds array > access") > Unfortunately that caused the PM runtime count to be unbalanced and > underflowed on the first call. To fix that reintroduce a no-op check > by reading the register directly. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: rockchip: Fix PM runtime count on no-op cs commit: 0bb394067a792e7119abc9e0b7158ef19381f456 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
diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c index 864e58167417..1bc012fce7cb 100644 --- a/drivers/spi/spi-rockchip.c +++ b/drivers/spi/spi-rockchip.c @@ -241,6 +241,20 @@ static void rockchip_spi_set_cs(struct spi_device *spi, bool enable) struct spi_controller *ctlr = spi->controller; struct rockchip_spi *rs = spi_controller_get_devdata(ctlr); bool cs_asserted = spi->mode & SPI_CS_HIGH ? enable : !enable; + bool cs_actual; + + /* + * SPI subsystem tries to avoid no-op calls that would break the PM + * refcount below. It can't however for the first time it is used. + * To detect this case we read it here and bail out early for no-ops. + */ + if (spi_get_csgpiod(spi, 0)) + cs_actual = !!(readl_relaxed(rs->regs + ROCKCHIP_SPI_SER) & 1); + else + cs_actual = !!(readl_relaxed(rs->regs + ROCKCHIP_SPI_SER) & + BIT(spi_get_chipselect(spi, 0))); + if (unlikely(cs_actual == cs_asserted)) + return; if (cs_asserted) { /* Keep things powered as long as CS is asserted */
The early bail out that caused an out-of-bounds write was removed with commit 5c018e378f91 ("spi: spi-rockchip: Fix out of bounds array access") Unfortunately that caused the PM runtime count to be unbalanced and underflowed on the first call. To fix that reintroduce a no-op check by reading the register directly. Cc: stable@vger.kernel.org Fixes: 5c018e378f91 ("spi: spi-rockchip: Fix out of bounds array access") Signed-off-by: Christian Loehle <christian.loehle@arm.com> --- Not particularly happy with it, it's just the most straightforward fix for the issue at hand. I couldn't quite convince myself that no-op rockchip_spi_set_cs() are 100% ruled out just from spi.c after the first call either, but maybe someone more familiar with the code can. I have only seen the issue for the first call / during boot FWIW. Any more elegant fix suggestions welcome! The other host controller drivers don't need the read before CS change, so this shouldn't really either. drivers/spi/spi-rockchip.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)