[0/3] Intel SPI unbind fixes

Message

Lukas Wunner May 25, 2020, 12:25 p.m. UTC

Fix unbind ordering issues in the Designware and PXA2xx SPI controllers
built into Intel SoCs.

To test, you'll also need the following commit:
https://git.kernel.org/broonie/spi/c/84855678add8

This is compile-tested only, so please review and test thoroughly.

Pay particular attention to the IRQ handlers dw_spi_irq() and ssp_int():
Both drivers request the interrupt with IRQF_SHARED, so the IRQ handler
may run even though the SPI controller itself hasn't signalled an
interrupt.  The IRQ is requested before registering the controller and
freed after unregistering the controller.  Thus, the IRQ handler should
immediately bail out if the controller is not yet or no longer registered,
e.g. by checking in the controller's registers whether an interrupt is
actually pending.  The drivers may need to disable interrupts in the
controllers' registers upon unbinding and they need to make sure that
the registers are accessible until the IRQ is freed.

I don't have a datasheet for these SPI controllers but I hope someone
who is more familiar with them will be able to check for correctness
of the IRQ handling.

Lukas Wunner (3):
  spi: dw: Fix controller unregister order
  spi: pxa2xx: Fix controller unregister order
  spi: pxa2xx: Fix runtime PM ref imbalance on probe error

 drivers/spi/spi-dw.c     | 4 +++-
 drivers/spi/spi-pxa2xx.c | 5 +++--
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

[e-mail] May 25, 2020, 1:23 p.m. UTC | #1

On Mon, May 25, 2020 at 02:25:00PM +0200, Lukas Wunner wrote:
> Fix unbind ordering issues in the Designware and PXA2xx SPI controllers
> built into Intel SoCs.
> 
> To test, you'll also need the following commit:
> https://git.kernel.org/broonie/spi/c/84855678add8
> 
> This is compile-tested only, so please review and test thoroughly.
> 
> Pay particular attention to the IRQ handlers dw_spi_irq() and ssp_int():
> Both drivers request the interrupt with IRQF_SHARED, so the IRQ handler
> may run even though the SPI controller itself hasn't signalled an
> interrupt.  The IRQ is requested before registering the controller and
> freed after unregistering the controller.  Thus, the IRQ handler should
> immediately bail out if the controller is not yet or no longer registered,
> e.g. by checking in the controller's registers whether an interrupt is
> actually pending.  The drivers may need to disable interrupts in the
> controllers' registers upon unbinding and they need to make sure that
> the registers are accessible until the IRQ is freed.
> 
> I don't have a datasheet for these SPI controllers but I hope someone
> who is more familiar with them will be able to check for correctness
> of the IRQ handling.

Thank you for this!
I reviewed all three, but I have no means to reliably test it. From the
description and logic point of view this sounds correct. So, let's wait for
Tsuchiya Yuto to test and confirm that it's right way to go.

> 
> Lukas Wunner (3):
>   spi: dw: Fix controller unregister order
>   spi: pxa2xx: Fix controller unregister order
>   spi: pxa2xx: Fix runtime PM ref imbalance on probe error
> 
>  drivers/spi/spi-dw.c     | 4 +++-
>  drivers/spi/spi-pxa2xx.c | 5 +++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> -- 
> 2.25.0
>

Mark Brown May 26, 2020, 4:46 p.m. UTC | #2

On Mon, 25 May 2020 14:25:00 +0200, Lukas Wunner wrote:
> Fix unbind ordering issues in the Designware and PXA2xx SPI controllers
> built into Intel SoCs.
> 
> To test, you'll also need the following commit:
> https://git.kernel.org/broonie/spi/c/84855678add8
> 
> This is compile-tested only, so please review and test thoroughly.
> 
> [...]

Applied to

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

Thanks!

[1/3] spi: dw: Fix controller unregister order
      commit: ca8b19d61e3fce5d2d7790cde27a0b57bcb3f341
[2/3] spi: pxa2xx: Fix controller unregister order
      commit: 32e5b57232c0411e7dea96625c415510430ac079
[3/3] spi: pxa2xx: Fix runtime PM ref imbalance on probe error
      commit: 65e318e17358a3fd4fcb5a69d89b14016dee2f06

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