mbox series

[0/3] spi: spi-imx: fix use of more than four chip selects

Message ID 20230425134527.483607-1-linux@rasmusvillemoes.dk
Headers show
Series spi: spi-imx: fix use of more than four chip selects | expand

Message

Rasmus Villemoes April 25, 2023, 1:45 p.m. UTC
The current spi-imx driver completely fails when used with more than
four (gpio) chip-selects, since the chip select number is used
unconditionally as shift amount when updating the control and
configuration registers, so the code ends up modifying random bits
outside the intended fields.

This fixes it by making use of the unused_native_cs variable filled in
by the spi core, and use that as the "channel number" for all gpiod
chip selects.

In the presumably common case where all chip selects are gpios, this
means we end up using channel 0 exclusively, so the optimization where
the config register is left alone if it is unchanged (see
184434fcd617) might become less effective, if the workload consists of
different slaves with differing spi modes being accessed one after the
other. It would be nice if one could make use of the unused native
chip selects in a round-robin manner, but for that the core would have
to tell us not just unused_native_cs, but the whole ~native_cs_mask
from spi_get_gpio_descs(). Maybe a simpler fix, if there is anything
to fix, is to make the new mx51_ecspi_channel() do

	if (!spi->cs_gpiod || spi->controller->num_chipselect <= 4)


Rasmus Villemoes (3):
  spi: spi-imx: use "controller" variable consistently in
    spi_imx_probe()
  spi: spi-imx: set max_native_cs for imx51/imx53/imx6 variants
  spi: spi-imx: fix use of more than four chipselects

 drivers/spi/spi-imx.c | 56 +++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 21 deletions(-)

Comments

Mark Brown May 16, 2023, 3:22 p.m. UTC | #1
On Tue, May 16, 2023 at 01:43:23PM +0200, Rasmus Villemoes wrote:

> So, what's the conclusion here? Will these three patches be applied, or
> will we just live with the status as of next-20230516, namely that

As pointed out by Amit this will need a resend against current code.
Mark Brown May 23, 2023, 9:22 p.m. UTC | #2
On Tue, 25 Apr 2023 15:45:24 +0200, Rasmus Villemoes wrote:
> The current spi-imx driver completely fails when used with more than
> four (gpio) chip-selects, since the chip select number is used
> unconditionally as shift amount when updating the control and
> configuration registers, so the code ends up modifying random bits
> outside the intended fields.
> 
> This fixes it by making use of the unused_native_cs variable filled in
> by the spi core, and use that as the "channel number" for all gpiod
> chip selects.
> 
> [...]

Applied to

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

Thanks!

[1/3] spi: spi-imx: use "controller" variable consistently in spi_imx_probe()
      commit: d9032b304541e1f560349e461611f25d67f44a49
[2/3] spi: spi-imx: set max_native_cs for imx51/imx53/imx6 variants
      commit: 8ce1bb9a5935385e9ef65bda1e8ca923c7fbb887
[3/3] spi: spi-imx: fix use of more than four chipselects
      (no commit info)

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