diff mbox series

[v1] spi: imx: mx51-ecspi: fix clk polarity and phase configuration for CS > 4

Message ID 20220523073143.778664-1-o.rempel@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [v1] spi: imx: mx51-ecspi: fix clk polarity and phase configuration for CS > 4 | expand

Commit Message

Oleksij Rempel May 23, 2022, 7:31 a.m. UTC 
Fix support for boards with more then 4 chip select lines. Other wise if
CS > 4 is used, we will write trash to the clk configuration register.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/spi/spi-imx.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

Comments

Mark Brown May 23, 2022, 1:27 p.m. UTC | #1 
On Mon, May 23, 2022 at 09:31:43AM +0200, Oleksij Rempel wrote:

> -	/* set chip select to use */
> -	ctrl |= MX51_ECSPI_CTRL_CS(spi->chip_select);
> +	if (spi->cs_gpiod) {
> +		chip_select = 0;

What if someone mixed GPIO and regular chip selects and 0 is one of the
in use chip selects?  Ideally we should check for an unused chip select
here, though the current change is still an improvement since we'll at
least only write in the chip select field.
Oleksij Rempel May 23, 2022, 3:40 p.m. UTC | #2 
On Mon, May 23, 2022 at 02:27:15PM +0100, Mark Brown wrote:
> On Mon, May 23, 2022 at 09:31:43AM +0200, Oleksij Rempel wrote:
> 
> > -	/* set chip select to use */
> > -	ctrl |= MX51_ECSPI_CTRL_CS(spi->chip_select);
> > +	if (spi->cs_gpiod) {
> > +		chip_select = 0;
> 
> What if someone mixed GPIO and regular chip selects and 0 is one of the
> in use chip selects?  Ideally we should check for an unused chip select
> here, though the current change is still an improvement since we'll at
> least only write in the chip select field.

In case some HW variant has real issue with it, we will need to reduce
amount of supported HW CS and use blacklisted one for GPIOs.

Regards,
Oleksij
Mark Brown May 26, 2022, 3:51 p.m. UTC | #3 
On Mon, May 23, 2022 at 09:31:43AM +0200, Oleksij Rempel wrote:
> Fix support for boards with more then 4 chip select lines. Other wise if
> CS > 4 is used, we will write trash to the clk configuration register.

This doesn't apply against current code, please check and resend.
diff mbox series

Patch

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index b2dd0a4d2446..2d29b893e9e0 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -265,6 +265,7 @@  static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi,
 #define MX51_ECSPI_CONFIG_SBBCTRL(cs)	(1 << ((cs) +  8))
 #define MX51_ECSPI_CONFIG_SSBPOL(cs)	(1 << ((cs) + 12))
 #define MX51_ECSPI_CONFIG_SCLKCTL(cs)	(1 << ((cs) + 20))
+#define MX51_ECSPI_MAX_HW_CS		4
 
 #define MX51_ECSPI_INT		0x10
 #define MX51_ECSPI_INT_TEEN		(1 <<  0)
@@ -515,6 +516,7 @@  static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx,
 	u32 min_speed_hz = ~0U;
 	u32 testreg, delay;
 	u32 cfg = readl(spi_imx->base + MX51_ECSPI_CONFIG);
+	int chip_select;
 
 	/* set Master or Slave mode */
 	if (spi_imx->slave_mode)
@@ -528,8 +530,19 @@  static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx,
 	if (spi->mode & SPI_READY)
 		ctrl |= MX51_ECSPI_CTRL_DRCTL(spi_imx->spi_drctl);
 
-	/* set chip select to use */
-	ctrl |= MX51_ECSPI_CTRL_CS(spi->chip_select);
+	if (spi->cs_gpiod) {
+		chip_select = 0;
+	} else {
+		if (spi->chip_select >= MX51_ECSPI_MAX_HW_CS) {
+			dev_err(spi_imx->dev, "Native chip_select is out of supported range: %i\n",
+				spi->chip_select);
+			return -EINVAL;
+		}
+
+		/* set HW chip select to use */
+		ctrl |= MX51_ECSPI_CTRL_CS(spi->chip_select);
+		chip_select = spi->chip_select;
+	}
 
 	/*
 	 * The ctrl register must be written first, with the EN bit set other
@@ -550,27 +563,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->chip_select);
+		cfg &= ~MX51_ECSPI_CONFIG_SBBCTRL(chip_select);
 	else
-		cfg |= MX51_ECSPI_CONFIG_SBBCTRL(spi->chip_select);
+		cfg |= MX51_ECSPI_CONFIG_SBBCTRL(chip_select);
 
 	if (spi->mode & SPI_CPHA)
-		cfg |= MX51_ECSPI_CONFIG_SCLKPHA(spi->chip_select);
+		cfg |= MX51_ECSPI_CONFIG_SCLKPHA(chip_select);
 	else
-		cfg &= ~MX51_ECSPI_CONFIG_SCLKPHA(spi->chip_select);
+		cfg &= ~MX51_ECSPI_CONFIG_SCLKPHA(chip_select);
 
 	if (spi->mode & SPI_CPOL) {
-		cfg |= MX51_ECSPI_CONFIG_SCLKPOL(spi->chip_select);
-		cfg |= MX51_ECSPI_CONFIG_SCLKCTL(spi->chip_select);
+		cfg |= MX51_ECSPI_CONFIG_SCLKPOL(chip_select);
+		cfg |= MX51_ECSPI_CONFIG_SCLKCTL(chip_select);
 	} else {
-		cfg &= ~MX51_ECSPI_CONFIG_SCLKPOL(spi->chip_select);
-		cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(spi->chip_select);
+		cfg &= ~MX51_ECSPI_CONFIG_SCLKPOL(chip_select);
+		cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(chip_select);
 	}
 
 	if (spi->mode & SPI_CS_HIGH)
-		cfg |= MX51_ECSPI_CONFIG_SSBPOL(spi->chip_select);
+		cfg |= MX51_ECSPI_CONFIG_SSBPOL(chip_select);
 	else
-		cfg &= ~MX51_ECSPI_CONFIG_SSBPOL(spi->chip_select);
+		cfg &= ~MX51_ECSPI_CONFIG_SSBPOL(chip_select);
 
 	writel(cfg, spi_imx->base + MX51_ECSPI_CONFIG);