| Message ID | 20210703022300.296114-1-marex@denx.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 135cbd378eab336da15de9c84bbb22bf743b38a5 |
| Headers | show |
| Series | spi: imx: mx51-ecspi: Reinstate low-speed CONFIGREG delay | expand |
On Sat, 3 Jul 2021 04:23:00 +0200, Marek Vasut wrote: > Since 00b80ac935539 ("spi: imx: mx51-ecspi: Move some initialisation to > prepare_message hook."), the MX51_ECSPI_CONFIG write no longer happens > in prepare_transfer hook, but rather in prepare_message hook, however > the MX51_ECSPI_CONFIG delay is still left in prepare_transfer hook and > thus has no effect. This leads to low bus frequency operation problems > described in 6fd8b8503a0dc ("spi: spi-imx: Fix out-of-order CS/SCLK > operation at low speeds") again. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: imx: mx51-ecspi: Reinstate low-speed CONFIGREG delay commit: 135cbd378eab336da15de9c84bbb22bf743b38a5 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
On 7/7/21 7:52 PM, Mark Brown wrote: > On Sat, 3 Jul 2021 04:23:00 +0200, Marek Vasut wrote: >> Since 00b80ac935539 ("spi: imx: mx51-ecspi: Move some initialisation to >> prepare_message hook."), the MX51_ECSPI_CONFIG write no longer happens >> in prepare_transfer hook, but rather in prepare_message hook, however >> the MX51_ECSPI_CONFIG delay is still left in prepare_transfer hook and >> thus has no effect. This leads to low bus frequency operation problems >> described in 6fd8b8503a0dc ("spi: spi-imx: Fix out-of-order CS/SCLK >> operation at low speeds") again. >> >> [...] > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next > > Thanks! > > [1/1] spi: imx: mx51-ecspi: Reinstate low-speed CONFIGREG delay > commit: 135cbd378eab336da15de9c84bbb22bf743b38a5 > > 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. I messed that one up, here is the incremental fix: https://patchwork.kernel.org/project/spi-devel-general/patch/20210718211143.143557-1-marex@denx.de/
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 39dc02e366f4b..4aee3db6d6df0 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -506,7 +506,7 @@ static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx, { struct spi_device *spi = msg->spi; u32 ctrl = MX51_ECSPI_CTRL_ENABLE; - u32 testreg; + u32 testreg, delay; u32 cfg = readl(spi_imx->base + MX51_ECSPI_CONFIG); /* set Master or Slave mode */ @@ -567,6 +567,23 @@ static int mx51_ecspi_prepare_message(struct spi_imx_data *spi_imx, writel(cfg, spi_imx->base + MX51_ECSPI_CONFIG); + /* + * Wait until the changes in the configuration register CONFIGREG + * propagate into the hardware. It takes exactly one tick of the + * SCLK clock, but we will wait two SCLK clock just to be sure. The + * effect of the delay it takes for the hardware to apply changes + * is noticable if the SCLK clock run very slow. In such a case, if + * the polarity of SCLK should be inverted, the GPIO ChipSelect might + * be asserted before the SCLK polarity changes, which would disrupt + * the SPI communication as the device on the other end would consider + * the change of SCLK polarity as a clock tick already. + */ + delay = (2 * 1000000) / spi_imx->spi_bus_clk; + if (likely(delay < 10)) /* SCLK is faster than 100 kHz */ + udelay(delay); + else /* SCLK is _very_ slow */ + usleep_range(delay, delay + 10); + return 0; } @@ -574,7 +591,7 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx, struct spi_device *spi) { u32 ctrl = readl(spi_imx->base + MX51_ECSPI_CTRL); - u32 clk, delay; + u32 clk; /* Clear BL field and set the right value */ ctrl &= ~MX51_ECSPI_CTRL_BL_MASK; @@ -596,23 +613,6 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx, writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); - /* - * Wait until the changes in the configuration register CONFIGREG - * propagate into the hardware. It takes exactly one tick of the - * SCLK clock, but we will wait two SCLK clock just to be sure. The - * effect of the delay it takes for the hardware to apply changes - * is noticable if the SCLK clock run very slow. In such a case, if - * the polarity of SCLK should be inverted, the GPIO ChipSelect might - * be asserted before the SCLK polarity changes, which would disrupt - * the SPI communication as the device on the other end would consider - * the change of SCLK polarity as a clock tick already. - */ - delay = (2 * 1000000) / clk; - if (likely(delay < 10)) /* SCLK is faster than 100 kHz */ - udelay(delay); - else /* SCLK is _very_ slow */ - usleep_range(delay, delay + 10); - return 0; }
Since 00b80ac935539 ("spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook."), the MX51_ECSPI_CONFIG write no longer happens in prepare_transfer hook, but rather in prepare_message hook, however the MX51_ECSPI_CONFIG delay is still left in prepare_transfer hook and thus has no effect. This leads to low bus frequency operation problems described in 6fd8b8503a0dc ("spi: spi-imx: Fix out-of-order CS/SCLK operation at low speeds") again. Move the MX51_ECSPI_CONFIG write delay into the prepare_message hook as well, thus reinstating the low bus frequency fix. Fixes: 00b80ac935539 ("spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook.") Signed-off-by: Marek Vasut <marex@denx.de> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Cc: Mark Brown <broonie@kernel.org> --- drivers/spi/spi-imx.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)