All of lore.kernel.org
 help / color / mirror / Atom feed
* suggest 00b80ac93553 ("spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook.") for stable backports
@ 2018-12-20 14:43 Uwe Kleine-König
  2018-12-20 14:55 ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2018-12-20 14:43 UTC (permalink / raw)
  To: stable; +Cc: Mark Brown

Hello,

even though the subject sounds harmless it fixes a real bug.

(Yes, I'm aware that commit isn't in Linus' tree yet, but I assume your
tracking of patches targetting stable is better than mine, so I didn't
wait :-)

For backporting you also need its parent commit (i.e. e697271c4e29
("spi: imx: add a device specific prepare_message callback")). I have a
local backport to v4.14 here which isn't entirely trivial. So just tell
me if you need help.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K�nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: suggest 00b80ac93553 ("spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook.") for stable backports
  2018-12-20 14:43 suggest 00b80ac93553 ("spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook.") for stable backports Uwe Kleine-König
@ 2018-12-20 14:55 ` Mark Brown
  2018-12-22  0:06   ` Sasha Levin
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2018-12-20 14:55 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: stable

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

On Thu, Dec 20, 2018 at 03:43:31PM +0100, Uwe Kleine-König wrote:

> even though the subject sounds harmless it fixes a real bug.

> (Yes, I'm aware that commit isn't in Linus' tree yet, but I assume your
> tracking of patches targetting stable is better than mine, so I didn't
> wait :-)

> For backporting you also need its parent commit (i.e. e697271c4e29
> ("spi: imx: add a device specific prepare_message callback")). I have a
> local backport to v4.14 here which isn't entirely trivial. So just tell
> me if you need help.

This makes sense to me as a backport, an oversight on my part.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: suggest 00b80ac93553 ("spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook.") for stable backports
  2018-12-20 14:55 ` Mark Brown
@ 2018-12-22  0:06   ` Sasha Levin
  2018-12-23 21:21     ` [PATCH v4.14.x 1/2] spi: imx: add a device specific prepare_message callback Uwe Kleine-König
  0 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2018-12-22  0:06 UTC (permalink / raw)
  To: Mark Brown; +Cc: Uwe Kleine-König, stable

On Thu, Dec 20, 2018 at 02:55:31PM +0000, Mark Brown wrote:
>On Thu, Dec 20, 2018 at 03:43:31PM +0100, Uwe Kleine-K�nig wrote:
>
>> even though the subject sounds harmless it fixes a real bug.
>
>> (Yes, I'm aware that commit isn't in Linus' tree yet, but I assume your
>> tracking of patches targetting stable is better than mine, so I didn't
>> wait :-)
>
>> For backporting you also need its parent commit (i.e. e697271c4e29
>> ("spi: imx: add a device specific prepare_message callback")). I have a
>> local backport to v4.14 here which isn't entirely trivial. So just tell
>> me if you need help.
>
>This makes sense to me as a backport, an oversight on my part.

I'll happily take your backport here and an ack on it from Mark :)

--
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4.14.x 1/2] spi: imx: add a device specific prepare_message callback
  2018-12-22  0:06   ` Sasha Levin
@ 2018-12-23 21:21     ` Uwe Kleine-König
  2018-12-23 21:21       ` [PATCH v4.14.x 2/2] spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook Uwe Kleine-König
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2018-12-23 21:21 UTC (permalink / raw)
  To: Sasha Levin; +Cc: Mark Brown, stable, kernel

This is just preparatory work which allows to move some initialisation
that currently is done in the per transfer hook .config to an earlier
point in time in the next few patches. There is no change in behaviour
introduced by this patch.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
[ukleinek: backport to v4.14.x]
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/spi/spi-imx.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index d51ca243a028..3fdb0652429b 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -72,6 +72,7 @@ struct spi_imx_data;
 
 struct spi_imx_devtype_data {
 	void (*intctrl)(struct spi_imx_data *, int);
+	int (*prepare_message)(struct spi_imx_data *, struct spi_message *);
 	int (*config)(struct spi_device *);
 	void (*trigger)(struct spi_imx_data *);
 	int (*rx_available)(struct spi_imx_data *);
@@ -439,6 +440,12 @@ static void mx51_ecspi_trigger(struct spi_imx_data *spi_imx)
 	writel(reg, spi_imx->base + MX51_ECSPI_CTRL);
 }
 
+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);
@@ -599,6 +606,12 @@ static void mx31_trigger(struct spi_imx_data *spi_imx)
 	writel(reg, spi_imx->base + MXC_CSPICTRL);
 }
 
+static int mx31_prepare_message(struct spi_imx_data *spi_imx,
+				struct spi_message *msg)
+{
+	return 0;
+}
+
 static int mx31_config(struct spi_device *spi)
 {
 	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
@@ -695,6 +708,12 @@ static void mx21_trigger(struct spi_imx_data *spi_imx)
 	writel(reg, spi_imx->base + MXC_CSPICTRL);
 }
 
+static int mx21_prepare_message(struct spi_imx_data *spi_imx,
+				struct spi_message *msg)
+{
+	return 0;
+}
+
 static int mx21_config(struct spi_device *spi)
 {
 	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
@@ -764,6 +783,12 @@ static void mx1_trigger(struct spi_imx_data *spi_imx)
 	writel(reg, spi_imx->base + MXC_CSPICTRL);
 }
 
+static int mx1_prepare_message(struct spi_imx_data *spi_imx,
+			       struct spi_message *msg)
+{
+	return 0;
+}
+
 static int mx1_config(struct spi_device *spi)
 {
 	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
@@ -798,6 +823,7 @@ static void mx1_reset(struct spi_imx_data *spi_imx)
 
 static struct spi_imx_devtype_data imx1_cspi_devtype_data = {
 	.intctrl = mx1_intctrl,
+	.prepare_message = mx1_prepare_message,
 	.config = mx1_config,
 	.trigger = mx1_trigger,
 	.rx_available = mx1_rx_available,
@@ -810,6 +836,7 @@ static struct spi_imx_devtype_data imx1_cspi_devtype_data = {
 
 static struct spi_imx_devtype_data imx21_cspi_devtype_data = {
 	.intctrl = mx21_intctrl,
+	.prepare_message = mx21_prepare_message,
 	.config = mx21_config,
 	.trigger = mx21_trigger,
 	.rx_available = mx21_rx_available,
@@ -823,6 +850,7 @@ static struct spi_imx_devtype_data imx21_cspi_devtype_data = {
 static struct spi_imx_devtype_data imx27_cspi_devtype_data = {
 	/* i.mx27 cspi shares the functions with i.mx21 one */
 	.intctrl = mx21_intctrl,
+	.prepare_message = mx21_prepare_message,
 	.config = mx21_config,
 	.trigger = mx21_trigger,
 	.rx_available = mx21_rx_available,
@@ -835,6 +863,7 @@ static struct spi_imx_devtype_data imx27_cspi_devtype_data = {
 
 static struct spi_imx_devtype_data imx31_cspi_devtype_data = {
 	.intctrl = mx31_intctrl,
+	.prepare_message = mx31_prepare_message,
 	.config = mx31_config,
 	.trigger = mx31_trigger,
 	.rx_available = mx31_rx_available,
@@ -848,6 +877,7 @@ static struct spi_imx_devtype_data imx31_cspi_devtype_data = {
 static struct spi_imx_devtype_data imx35_cspi_devtype_data = {
 	/* i.mx35 and later cspi shares the functions with i.mx31 one */
 	.intctrl = mx31_intctrl,
+	.prepare_message = mx31_prepare_message,
 	.config = mx31_config,
 	.trigger = mx31_trigger,
 	.rx_available = mx31_rx_available,
@@ -860,6 +890,7 @@ static struct spi_imx_devtype_data imx35_cspi_devtype_data = {
 
 static struct spi_imx_devtype_data imx51_ecspi_devtype_data = {
 	.intctrl = mx51_ecspi_intctrl,
+	.prepare_message = mx51_ecspi_prepare_message,
 	.config = mx51_ecspi_config,
 	.trigger = mx51_ecspi_trigger,
 	.rx_available = mx51_ecspi_rx_available,
@@ -872,6 +903,7 @@ static struct spi_imx_devtype_data imx51_ecspi_devtype_data = {
 
 static struct spi_imx_devtype_data imx53_ecspi_devtype_data = {
 	.intctrl = mx51_ecspi_intctrl,
+	.prepare_message = mx51_ecspi_prepare_message,
 	.config = mx51_ecspi_config,
 	.trigger = mx51_ecspi_trigger,
 	.rx_available = mx51_ecspi_rx_available,
@@ -1310,7 +1342,13 @@ spi_imx_prepare_message(struct spi_master *master, struct spi_message *msg)
 		return ret;
 	}
 
-	return 0;
+	ret = spi_imx->devtype_data->prepare_message(spi_imx, msg);
+	if (ret) {
+		clk_disable(spi_imx->clk_ipg);
+		clk_disable(spi_imx->clk_per);
+	}
+
+	return ret;
 }
 
 static int
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4.14.x 2/2] spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook.
  2018-12-23 21:21     ` [PATCH v4.14.x 1/2] spi: imx: add a device specific prepare_message callback Uwe Kleine-König
@ 2018-12-23 21:21       ` Uwe Kleine-König
  2018-12-27 20:56         ` Sasha Levin
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2018-12-23 21:21 UTC (permalink / raw)
  To: Sasha Levin; +Cc: Mark Brown, stable, kernel

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>
Signed-off-by: Mark Brown <broonie@kernel.org>
[ukleinek: backport to v4.14.x]
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/spi/spi-imx.c | 59 ++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 3fdb0652429b..df18d07d544d 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -443,14 +443,9 @@ static void mx51_ecspi_trigger(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);
 
 	/*
@@ -468,14 +463,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);
 
-	ctrl |= (spi_imx->bits_per_word - 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
+		testreg &= ~MX51_ECSPI_TESTREG_LBC;
+	writel(testreg, spi_imx->base + MX51_ECSPI_TESTREG);
 
 	cfg |= MX51_ECSPI_CONFIG_SBBCTRL(spi->chip_select);
 
@@ -491,26 +493,38 @@ 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);
 
+	writel(cfg, spi_imx->base + MX51_ECSPI_CONFIG);
+
+	return 0;
+}
+
+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;
+	ctrl |= (spi_imx->bits_per_word - 1) << MX51_ECSPI_CTRL_BL_OFFSET;
+
+	/* 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;
 
-	/* CTRL register always go first to bring out controller from reset */
 	writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL);
 
-	reg = readl(spi_imx->base + MX51_ECSPI_TESTREG);
-	if (spi->mode & SPI_LOOP)
-		reg |= MX51_ECSPI_TESTREG_LBC;
-	else
-		reg &= ~MX51_ECSPI_TESTREG_LBC;
-	writel(reg, spi_imx->base + MX51_ECSPI_TESTREG);
-
-	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
@@ -532,7 +546,6 @@ static int mx51_ecspi_config(struct spi_device *spi)
 	 * Configure the DMA register: setup the watermark
 	 * and enable DMA request.
 	 */
-
 	writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml) |
 		MX51_ECSPI_DMA_TX_WML(spi_imx->wml) |
 		MX51_ECSPI_DMA_RXT_WML(spi_imx->wml) |
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4.14.x 2/2] spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook.
  2018-12-23 21:21       ` [PATCH v4.14.x 2/2] spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook Uwe Kleine-König
@ 2018-12-27 20:56         ` Sasha Levin
  0 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2018-12-27 20:56 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: Mark Brown, stable, kernel

On Sun, Dec 23, 2018 at 10:21:22PM +0100, Uwe Kleine-König wrote:
>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>
>Signed-off-by: Mark Brown <broonie@kernel.org>
>[ukleinek: backport to v4.14.x]
>Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Queued both for 4.14, thank you.

--
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-12-27 20:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-20 14:43 suggest 00b80ac93553 ("spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook.") for stable backports Uwe Kleine-König
2018-12-20 14:55 ` Mark Brown
2018-12-22  0:06   ` Sasha Levin
2018-12-23 21:21     ` [PATCH v4.14.x 1/2] spi: imx: add a device specific prepare_message callback Uwe Kleine-König
2018-12-23 21:21       ` [PATCH v4.14.x 2/2] spi: imx: mx51-ecspi: Move some initialisation to prepare_message hook Uwe Kleine-König
2018-12-27 20:56         ` Sasha Levin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.