Skip to contentSkip to conversation

spi/imx: select master mode for all channels

PATCH New Uwe Kleine-König 2 participants 3 messages Feb 8, 2011 – Feb 16, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 spi/imx: select master mode for all channels

From: Sascha Hauer <[email protected]>

The hardware seems to have a race condition when the inactive channels are in slave mode. We support master mode only, so we can just switch all channels to master mode.

[ukleinek: add more verbose comment about the race]

---
Hello,

maybe one of our friends at Freescale can comment?

Best regards
Uwe

 drivers/spi/spi_imx.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
drivers/spi/spi_imx.c +9 -3
index 1cf9d5f..69d6dba 100644
@@ -174,7 +174,7 @@ static unsigned int spi_imx_clkdiv_2(unsigned int fin,
 #define SPI_IMX2_3_CTRL		0x08
 #define SPI_IMX2_3_CTRL_ENABLE		(1 <<  0)
 #define SPI_IMX2_3_CTRL_XCH		(1 <<  2)
-#define SPI_IMX2_3_CTRL_MODE(cs)	(1 << ((cs) +  4))
+#define SPI_IMX2_3_CTRL_MODE_MASK	(0xf << 4)
 #define SPI_IMX2_3_CTRL_POSTDIV_OFFSET	8
 #define SPI_IMX2_3_CTRL_PREDIV_OFFSET	12
 #define SPI_IMX2_3_CTRL_CS(cs)		((cs) << 18)
@@ -253,8 +253,14 @@ static int __maybe_unused spi_imx2_3_config(struct spi_imx_data *spi_imx,
 {
 	u32 ctrl = SPI_IMX2_3_CTRL_ENABLE, cfg = 0;
 
-	/* set master mode */
-	ctrl |= SPI_IMX2_3_CTRL_MODE(config->cs);
+	/*
+	 * The hardware seems to have a race condition when changing modes. The
+	 * current assumption is that the selection of the channel arrives
+	 * earlier in the hardware than the mode bits when they are written at
+	 * the same time.
+	 * So set master mode for all channels as we do not support slave mode.
+	 */
+	ctrl |= SPI_IMX2_3_CTRL_MODE_MASK;
 
 	/* set clock speed */
 	ctrl |= spi_imx2_3_clkdiv(spi_imx->spi_clk, config->speed_hz);
Signature · 1 line
1.7.2.3
#1 lore raw

PATCH 1/1 spi/imx: select master mode for all channels

Quoted 6 lines · Uwe Kleine-K?nig

From: Sascha Hauer <[email protected]>

The hardware seems to have a race condition when the inactive channels are in slave mode. We support master mode only, so we can just switch all channels to master mode.

Quoted 7 lines
[ukleinek: add more verbose comment about the race]

---
Hello,

maybe one of our friends at Freescale can comment?

Any feedback on this one? Or should I just merge it?

g.

Quoted 51 lines

Best regards
Uwe

 drivers/spi/spi_imx.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
drivers/spi/spi_imx.c +9 -3
index 1cf9d5f..69d6dba 100644
@@ -174,7 +174,7 @@ static unsigned int spi_imx_clkdiv_2(unsigned int fin,
 #define SPI_IMX2_3_CTRL		0x08
 #define SPI_IMX2_3_CTRL_ENABLE		(1 <<  0)
 #define SPI_IMX2_3_CTRL_XCH		(1 <<  2)
-#define SPI_IMX2_3_CTRL_MODE(cs)	(1 << ((cs) +  4))
+#define SPI_IMX2_3_CTRL_MODE_MASK	(0xf << 4)
 #define SPI_IMX2_3_CTRL_POSTDIV_OFFSET	8
 #define SPI_IMX2_3_CTRL_PREDIV_OFFSET	12
 #define SPI_IMX2_3_CTRL_CS(cs)		((cs) << 18)
@@ -253,8 +253,14 @@ static int __maybe_unused spi_imx2_3_config(struct spi_imx_data *spi_imx,
 {
 	u32 ctrl = SPI_IMX2_3_CTRL_ENABLE, cfg = 0;
 
-	/* set master mode */
-	ctrl |= SPI_IMX2_3_CTRL_MODE(config->cs);
+	/*
+	 * The hardware seems to have a race condition when changing modes. The
+	 * current assumption is that the selection of the channel arrives
+	 * earlier in the hardware than the mode bits when they are written at
+	 * the same time.
+	 * So set master mode for all channels as we do not support slave mode.
+	 */
+	ctrl |= SPI_IMX2_3_CTRL_MODE_MASK;
 
 	/* set clock speed */
 	ctrl |= spi_imx2_3_clkdiv(spi_imx->spi_clk, config->speed_hz);
Signature · 13 lines
1.7.2.3


------------------------------------------------------------------------------
The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE:
Pinpoint memory and threading errors before they happen.
Find and fix more than 250 security defects in the development cycle.
Locate bottlenecks in serial and parallel code that limit performance.
http://p.sf.net/sfu/intel-dev2devfeb
_______________________________________________
spi-devel-general mailing list
spi-devel-general at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
#2 lore raw

PATCH 1/1 spi/imx: select master mode for all channels

2011/2/15 Grant Likely <[email protected]>:

Quoted 13 lines · Uwe Kleine-K?nig
Quoted 12 lines

From: Sascha Hauer <[email protected]>

The hardware seems to have a race condition when the inactive channels are in slave mode. We support master mode only, so we can just switch all channels to master mode.

[ukleinek: add more verbose comment about the race]

---
Hello,

Quoted 3 lines
Quoted 1 line

maybe one of our friends at Freescale can comment?

Any feedback on this one? ?Or should I just merge it?

I've gone ahead and merged it. I can pull it back out if anyone complains though.

Thanks,

g.

Quoted 55 lines

g.

Quoted 51 lines

Best regards
Uwe

?drivers/spi/spi_imx.c | ? 12 +++++++++---
?1 files changed, 9 insertions(+), 3 deletions(-)

drivers/spi/spi_imx.c +0 -0
index 1cf9d5f..69d6dba 100644
@@ -174,7 +174,7 @@ static unsigned int spi_imx_clkdiv_2(unsigned int fin,

?#define SPI_IMX2_3_CTRL ? ? ? ? ? ? ?0x08 ?#define SPI_IMX2_3_CTRL_ENABLE ? ? ? ? ? ? ? (1 << ?0) ?#define SPI_IMX2_3_CTRL_XCH ? ? ? ? ?(1 << ?2)

drivers/spi/spi_imx.c +1 -1
-#define SPI_IMX2_3_CTRL_MODE(cs) ? ? (1 << ((cs) + ?4))
+#define SPI_IMX2_3_CTRL_MODE_MASK ? ?(0xf << 4)

?#define SPI_IMX2_3_CTRL_POSTDIV_OFFSET ? ? ? 8 ?#define SPI_IMX2_3_CTRL_PREDIV_OFFSET ? ? ? ?12 ?#define SPI_IMX2_3_CTRL_CS(cs) ? ? ? ? ? ? ? ((cs) << 18)

drivers/spi/spi_imx.c +8 -2
@@ -253,8 +253,14 @@ static int __maybe_unused spi_imx2_3_config(struct spi_imx_data *spi_imx,
 ?{
 ? ? ? u32 ctrl = SPI_IMX2_3_CTRL_ENABLE, cfg = 0;
 
- ? ? /* set master mode */
- ? ? ctrl |= SPI_IMX2_3_CTRL_MODE(config->cs);
+ ? ? /*
+ ? ? ?* The hardware seems to have a race condition when changing modes. The
+ ? ? ?* current assumption is that the selection of the channel arrives
+ ? ? ?* earlier in the hardware than the mode bits when they are written at
+ ? ? ?* the same time.
+ ? ? ?* So set master mode for all channels as we do not support slave mode.
+ ? ? ?*/
+ ? ? ctrl |= SPI_IMX2_3_CTRL_MODE_MASK;
 

? ? ? /* set clock speed */
? ? ? ctrl |= spi_imx2_3_clkdiv(spi_imx->spi_clk, config->speed_hz); --
1.7.2.3

------------------------------------------------------------------------------
The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE:
Pinpoint memory and threading errors before they happen.
Find and fix more than 250 security defects in the development cycle. Locate bottlenecks in serial and parallel code that limit performance.
http://p.sf.net/sfu/intel-dev2devfeb
_______________________________________________
spi-devel-general mailing list
spi-devel-general at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.