diff mbox series

[v2] spi: spi-imx: fix mixing of native and gpio chipselects for imx51/imx53/imx6 variants

Message ID 20230602115731.708883-1-linux@rasmusvillemoes.dk (mailing list archive)
State New, archived
Headers show
Series [v2] spi: spi-imx: fix mixing of native and gpio chipselects for imx51/imx53/imx6 variants | expand

Commit Message

Rasmus Villemoes June 2, 2023, 11:57 a.m. UTC 
Commit 87c614175bbf (spi: spi-imx: fix MX51_ECSPI_* macros when cs >
3) ensured that the argument passed to the macros was masked with &3,
so that we no longer write outside the intended fields in the various
control registers. When all chip selects are gpios, this works just
fine.

However, when a mix of native and gpio chip selects are in use, that
masking is too naive. Say, for example, that SS0 is muxed as native
chip select, and there is also a chip at 4 (obviously with a gpio
cs). In that case, when accessing the latter chip, both the SS0 pin
and the gpio pin will be asserted low.

The fix for this is to use the ->unused_native_cs value as channel
number for any spi device which uses a gpio as chip select.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---

Previous submission: https://lore.kernel.org/lkml/20230425134527.483607-1-linux@rasmusvillemoes.dk/

The first two patches have already been applied in -next. This adapts
3/3 to -next as of next-20230602, taking both 6a983ff5102f and
9e264f3f85a5 into account.

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

Comments

Mark Brown June 2, 2023, 3:16 p.m. UTC | #1 
On Fri, 02 Jun 2023 13:57:30 +0200, Rasmus Villemoes wrote:
> Commit 87c614175bbf (spi: spi-imx: fix MX51_ECSPI_* macros when cs >
> 3) ensured that the argument passed to the macros was masked with &3,
> so that we no longer write outside the intended fields in the various
> control registers. When all chip selects are gpios, this works just
> fine.
> 
> However, when a mix of native and gpio chip selects are in use, that
> masking is too naive. Say, for example, that SS0 is muxed as native
> chip select, and there is also a chip at 4 (obviously with a gpio
> cs). In that case, when accessing the latter chip, both the SS0 pin
> and the gpio pin will be asserted low.
> 
> [...]

Applied to

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

Thanks!

[1/1] spi: spi-imx: fix mixing of native and gpio chipselects for imx51/imx53/imx6 variants
      commit: a34e0353a681bbdd0402825e25410c3236109f31

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 mbox series

Patch

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 6f4d3cb81fdf..528ae46c087f 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -517,6 +517,13 @@  static void mx51_ecspi_disable(struct spi_imx_data *spi_imx)
 	writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL);
 }
 
+static int mx51_ecspi_channel(const struct spi_device *spi)
+{
+	if (!spi_get_csgpiod(spi, 0))
+		return spi_get_chipselect(spi, 0);
+	return spi->controller->unused_native_cs;
+}
+
 static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx,
 				      struct spi_message *msg)
 {
@@ -527,6 +534,7 @@  static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx,
 	u32 testreg, delay;
 	u32 cfg = readl(spi_imx->base + MX51_ECSPI_CONFIG);
 	u32 current_cfg = cfg;
+	int channel = mx51_ecspi_channel(spi);
 
 	/* set Master or Slave mode */
 	if (spi_imx->slave_mode)
@@ -541,7 +549,7 @@  static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx,
 		ctrl |= MX51_ECSPI_CTRL_DRCTL(spi_imx->spi_drctl);
 
 	/* set chip select to use */
-	ctrl |= MX51_ECSPI_CTRL_CS(spi_get_chipselect(spi, 0));
+	ctrl |= MX51_ECSPI_CTRL_CS(channel);
 
 	/*
 	 * The ctrl register must be written first, with the EN bit set other
@@ -562,27 +570,27 @@  static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx,
 	 * BURST_LENGTH + 1 bits are received
 	 */
 	if (spi_imx->slave_mode && is_imx53_ecspi(spi_imx))
-		cfg &= ~MX51_ECSPI_CONFIG_SBBCTRL(spi_get_chipselect(spi, 0));
+		cfg &= ~MX51_ECSPI_CONFIG_SBBCTRL(channel);
 	else
-		cfg |= MX51_ECSPI_CONFIG_SBBCTRL(spi_get_chipselect(spi, 0));
+		cfg |= MX51_ECSPI_CONFIG_SBBCTRL(channel);
 
 	if (spi->mode & SPI_CPOL) {
-		cfg |= MX51_ECSPI_CONFIG_SCLKPOL(spi_get_chipselect(spi, 0));
-		cfg |= MX51_ECSPI_CONFIG_SCLKCTL(spi_get_chipselect(spi, 0));
+		cfg |= MX51_ECSPI_CONFIG_SCLKPOL(channel);
+		cfg |= MX51_ECSPI_CONFIG_SCLKCTL(channel);
 	} else {
-		cfg &= ~MX51_ECSPI_CONFIG_SCLKPOL(spi_get_chipselect(spi, 0));
-		cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(spi_get_chipselect(spi, 0));
+		cfg &= ~MX51_ECSPI_CONFIG_SCLKPOL(channel);
+		cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(channel);
 	}
 
 	if (spi->mode & SPI_MOSI_IDLE_LOW)
-		cfg |= MX51_ECSPI_CONFIG_DATACTL(spi_get_chipselect(spi, 0));
+		cfg |= MX51_ECSPI_CONFIG_DATACTL(channel);
 	else
-		cfg &= ~MX51_ECSPI_CONFIG_DATACTL(spi_get_chipselect(spi, 0));
+		cfg &= ~MX51_ECSPI_CONFIG_DATACTL(channel);
 
 	if (spi->mode & SPI_CS_HIGH)
-		cfg |= MX51_ECSPI_CONFIG_SSBPOL(spi_get_chipselect(spi, 0));
+		cfg |= MX51_ECSPI_CONFIG_SSBPOL(channel);
 	else
-		cfg &= ~MX51_ECSPI_CONFIG_SSBPOL(spi_get_chipselect(spi, 0));
+		cfg &= ~MX51_ECSPI_CONFIG_SSBPOL(channel);
 
 	if (cfg == current_cfg)
 		return 0;
@@ -627,14 +635,15 @@  static void mx51_configure_cpha(struct spi_imx_data *spi_imx,
 	bool cpha = (spi->mode & SPI_CPHA);
 	bool flip_cpha = (spi->mode & SPI_RX_CPHA_FLIP) && spi_imx->rx_only;
 	u32 cfg = readl(spi_imx->base + MX51_ECSPI_CONFIG);
+	int channel = mx51_ecspi_channel(spi);
 
 	/* Flip cpha logical value iff flip_cpha */
 	cpha ^= flip_cpha;
 
 	if (cpha)
-		cfg |= MX51_ECSPI_CONFIG_SCLKPHA(spi_get_chipselect(spi, 0));
+		cfg |= MX51_ECSPI_CONFIG_SCLKPHA(channel);
 	else
-		cfg &= ~MX51_ECSPI_CONFIG_SCLKPHA(spi_get_chipselect(spi, 0));
+		cfg &= ~MX51_ECSPI_CONFIG_SCLKPHA(channel);
 
 	writel(cfg, spi_imx->base + MX51_ECSPI_CONFIG);
 }