| Message ID | 20181130064709.6998-3-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | spi: imx: Fix polarity switching for mx51-ecspi | expand |
> -----Original Message----- > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Sent: 2018年11月30日 14:47 > To: Mark Brown <broonie@kernel.org>; Robin Gong <yibin.gong@nxp.com> > Cc: Marek Vasut <marex@denx.de>; dl-linux-imx <linux-imx@nxp.com>; > kernel@pengutronix.de; linux-spi@vger.kernel.org; > linux-arm-kernel@lists.infradead.org > Subject: [PATCH v3 2/5] spi: imx: mx51-ecspi: Move some initialisation to > prepare_message hook. > > The relevant difference between prepare_message and config is that the > former is run before the CS signal is asserted. So the polarity of the CLK line > must be configured in prepare_message as an edge generated by config might > already result in a latch of the MOSI line. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/spi/spi-imx.c | 67 ++++++++++++++++++++++++++----------------- > 1 file changed, 40 insertions(+), 27 deletions(-) > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index > c7db42d6b3bc..d3a1e7104556 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -490,14 +490,9 @@ static void mx51_ecspi_disable(struct spi_imx_data > *spi_imx) static int mx51_ecspi_prepare_message(struct spi_imx_data > *spi_imx, > struct spi_message *msg) > { > - return 0; > -} > - > -static int mx51_ecspi_config(struct spi_device *spi) -{ > - struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master); > + struct spi_device *spi = msg->spi; > u32 ctrl = MX51_ECSPI_CTRL_ENABLE; > - u32 clk = spi_imx->speed_hz, delay, reg; > + u32 testreg; > u32 cfg = readl(spi_imx->base + MX51_ECSPI_CONFIG); > > /* set Master or Slave mode */ > @@ -512,19 +507,21 @@ static int mx51_ecspi_config(struct spi_device *spi) > if (spi->mode & SPI_READY) > ctrl |= MX51_ECSPI_CTRL_DRCTL(spi_imx->spi_drctl); > > - /* set clock speed */ > - ctrl |= mx51_ecspi_clkdiv(spi_imx, spi_imx->speed_hz, &clk); > - spi_imx->spi_bus_clk = clk; > - > /* set chip select to use */ > ctrl |= MX51_ECSPI_CTRL_CS(spi->chip_select); > > - if (spi_imx->slave_mode && is_imx53_ecspi(spi_imx)) > - ctrl |= (spi_imx->slave_burst * 8 - 1) > - << MX51_ECSPI_CTRL_BL_OFFSET; > + /* > + * The ctrl register must be written first, with the EN bit set other > + * registers must not be written to. > + */ > + writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); > + > + testreg = readl(spi_imx->base + MX51_ECSPI_TESTREG); > + if (spi->mode & SPI_LOOP) > + testreg |= MX51_ECSPI_TESTREG_LBC; > else > - ctrl |= (spi_imx->bits_per_word - 1) > - << MX51_ECSPI_CTRL_BL_OFFSET; > + testreg &= ~MX51_ECSPI_TESTREG_LBC; > + writel(testreg, spi_imx->base + MX51_ECSPI_TESTREG); > > /* > * eCSPI burst completion by Chip Select signal in Slave mode @@ -548,25 > +545,42 @@ static int mx51_ecspi_config(struct spi_device *spi) > cfg &= ~MX51_ECSPI_CONFIG_SCLKPOL(spi->chip_select); > cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(spi->chip_select); > } > + > if (spi->mode & SPI_CS_HIGH) > cfg |= MX51_ECSPI_CONFIG_SSBPOL(spi->chip_select); > else > cfg &= ~MX51_ECSPI_CONFIG_SSBPOL(spi->chip_select); > > - if (spi_imx->usedma) > - ctrl |= MX51_ECSPI_CTRL_SMC; > + writel(cfg, spi_imx->base + MX51_ECSPI_CONFIG); > > - /* CTRL register always go first to bring out controller from reset */ > - writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); > + return 0; > +} > > - reg = readl(spi_imx->base + MX51_ECSPI_TESTREG); > - if (spi->mode & SPI_LOOP) > - reg |= MX51_ECSPI_TESTREG_LBC; > +static int mx51_ecspi_config(struct spi_device *spi) { > + struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master); > + u32 ctrl = readl(spi_imx->base + MX51_ECSPI_CTRL); > + u32 clk = spi_imx->speed_hz, delay; > + > + /* Clear BL field and set the right value */ > + ctrl &= ~MX51_ECSPI_CTRL_BL_MASK; > + if (spi_imx->slave_mode && is_imx53_ecspi(spi_imx)) > + ctrl |= (spi_imx->slave_burst * 8 - 1) > + << MX51_ECSPI_CTRL_BL_OFFSET; > else > - reg &= ~MX51_ECSPI_TESTREG_LBC; > - writel(reg, spi_imx->base + MX51_ECSPI_TESTREG); > + ctrl |= (spi_imx->bits_per_word - 1) > + << MX51_ECSPI_CTRL_BL_OFFSET; > > - writel(cfg, spi_imx->base + MX51_ECSPI_CONFIG); > + /* set clock speed */ > + ctrl &= ~(0xf << MX51_ECSPI_CTRL_POSTDIV_OFFSET | > + 0xf << MX51_ECSPI_CTRL_PREDIV_OFFSET); > + ctrl |= mx51_ecspi_clkdiv(spi_imx, spi_imx->speed_hz, &clk); > + spi_imx->spi_bus_clk = clk; > + > + if (spi_imx->usedma) > + ctrl |= MX51_ECSPI_CTRL_SMC; > + > + writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); > > /* > * Wait until the changes in the configuration register CONFIGREG @@ Seems we don't need this commit anymore, your patch fix it by the way? commit 6fd8b8503a0dcf66510314dc054745087ae89f94 Author: Marek Vasut <marex@denx.de> Date: Wed Dec 18 18:31:47 2013 +0100 spi: spi-imx: Fix out-of-order CS/SCLK operation at low speeds > -594,7 +608,6 @@ static void mx51_setup_wml(struct spi_imx_data *spi_imx) > * Configure the DMA register: setup the watermark > * and enable DMA request. > */ > - > writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml - 1) | > MX51_ECSPI_DMA_TX_WML(spi_imx->wml) | > MX51_ECSPI_DMA_RXT_WML(spi_imx->wml) | > -- > 2.19.2
Hello, On Mon, Dec 03, 2018 at 08:00:21AM +0000, Robin Gong wrote: > > /* > > * Wait until the changes in the configuration register CONFIGREG @@ > Seems we don't need this commit anymore, your patch fix it by the way? > > commit 6fd8b8503a0dcf66510314dc054745087ae89f94 > Author: Marek Vasut <marex@denx.de> > Date: Wed Dec 18 18:31:47 2013 +0100 > > spi: spi-imx: Fix out-of-order CS/SCLK operation at low speeds I don't know, that's why I put Marek on Cc and talked to him in irc. I couldn't lure him into testing on his machine if this indeed fixes his problem. Best regards Uwe
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index c7db42d6b3bc..d3a1e7104556 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -490,14 +490,9 @@ static void mx51_ecspi_disable(struct spi_imx_data *spi_imx) static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx, struct spi_message *msg) { - return 0; -} - -static int mx51_ecspi_config(struct spi_device *spi) -{ - struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master); + struct spi_device *spi = msg->spi; u32 ctrl = MX51_ECSPI_CTRL_ENABLE; - u32 clk = spi_imx->speed_hz, delay, reg; + u32 testreg; u32 cfg = readl(spi_imx->base + MX51_ECSPI_CONFIG); /* set Master or Slave mode */ @@ -512,19 +507,21 @@ static int mx51_ecspi_config(struct spi_device *spi) if (spi->mode & SPI_READY) ctrl |= MX51_ECSPI_CTRL_DRCTL(spi_imx->spi_drctl); - /* set clock speed */ - ctrl |= mx51_ecspi_clkdiv(spi_imx, spi_imx->speed_hz, &clk); - spi_imx->spi_bus_clk = clk; - /* set chip select to use */ ctrl |= MX51_ECSPI_CTRL_CS(spi->chip_select); - if (spi_imx->slave_mode && is_imx53_ecspi(spi_imx)) - ctrl |= (spi_imx->slave_burst * 8 - 1) - << MX51_ECSPI_CTRL_BL_OFFSET; + /* + * The ctrl register must be written first, with the EN bit set other + * registers must not be written to. + */ + writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); + + testreg = readl(spi_imx->base + MX51_ECSPI_TESTREG); + if (spi->mode & SPI_LOOP) + testreg |= MX51_ECSPI_TESTREG_LBC; else - ctrl |= (spi_imx->bits_per_word - 1) - << MX51_ECSPI_CTRL_BL_OFFSET; + testreg &= ~MX51_ECSPI_TESTREG_LBC; + writel(testreg, spi_imx->base + MX51_ECSPI_TESTREG); /* * eCSPI burst completion by Chip Select signal in Slave mode @@ -548,25 +545,42 @@ static int mx51_ecspi_config(struct spi_device *spi) cfg &= ~MX51_ECSPI_CONFIG_SCLKPOL(spi->chip_select); cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(spi->chip_select); } + if (spi->mode & SPI_CS_HIGH) cfg |= MX51_ECSPI_CONFIG_SSBPOL(spi->chip_select); else cfg &= ~MX51_ECSPI_CONFIG_SSBPOL(spi->chip_select); - if (spi_imx->usedma) - ctrl |= MX51_ECSPI_CTRL_SMC; + writel(cfg, spi_imx->base + MX51_ECSPI_CONFIG); - /* CTRL register always go first to bring out controller from reset */ - writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); + return 0; +} - reg = readl(spi_imx->base + MX51_ECSPI_TESTREG); - if (spi->mode & SPI_LOOP) - reg |= MX51_ECSPI_TESTREG_LBC; +static int mx51_ecspi_config(struct spi_device *spi) +{ + struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master); + u32 ctrl = readl(spi_imx->base + MX51_ECSPI_CTRL); + u32 clk = spi_imx->speed_hz, delay; + + /* Clear BL field and set the right value */ + ctrl &= ~MX51_ECSPI_CTRL_BL_MASK; + if (spi_imx->slave_mode && is_imx53_ecspi(spi_imx)) + ctrl |= (spi_imx->slave_burst * 8 - 1) + << MX51_ECSPI_CTRL_BL_OFFSET; else - reg &= ~MX51_ECSPI_TESTREG_LBC; - writel(reg, spi_imx->base + MX51_ECSPI_TESTREG); + ctrl |= (spi_imx->bits_per_word - 1) + << MX51_ECSPI_CTRL_BL_OFFSET; - writel(cfg, spi_imx->base + MX51_ECSPI_CONFIG); + /* set clock speed */ + ctrl &= ~(0xf << MX51_ECSPI_CTRL_POSTDIV_OFFSET | + 0xf << MX51_ECSPI_CTRL_PREDIV_OFFSET); + ctrl |= mx51_ecspi_clkdiv(spi_imx, spi_imx->speed_hz, &clk); + spi_imx->spi_bus_clk = clk; + + if (spi_imx->usedma) + ctrl |= MX51_ECSPI_CTRL_SMC; + + writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); /* * Wait until the changes in the configuration register CONFIGREG @@ -594,7 +608,6 @@ static void mx51_setup_wml(struct spi_imx_data *spi_imx) * Configure the DMA register: setup the watermark * and enable DMA request. */ - writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml - 1) | MX51_ECSPI_DMA_TX_WML(spi_imx->wml) | MX51_ECSPI_DMA_RXT_WML(spi_imx->wml) |
The relevant difference between prepare_message and config is that the former is run before the CS signal is asserted. So the polarity of the CLK line must be configured in prepare_message as an edge generated by config might already result in a latch of the MOSI line. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/spi/spi-imx.c | 67 ++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 27 deletions(-)