SPI: mxc_spi: fix swapping bug and add missing swapping in unaligned rx case

PATCH New Anatolij Gustschin 3 participants 10 messages Jan 16, 2011 – Jan 20, 2011

Subsystems (2)

Arm i.MX and Layerscape SPI

lore

Share & tools

lore

Anatolij Gustschin

author

2011-01-16 18:17 UTC #0 lore raw

PATCH 1/2 SPI: mxc_spi: fix swapping bug and add missing swapping in unaligned rx case

We need to shift only one time in each cycle in the swapping loop for unaligned tx case. Currently two byte shift operations are performed in each loop cycle causing zero gaps in the transmited data, so not all data scheduled for transmition is actually transmited.

The proper swapping in unaligned rx case is missing, so add it as we need to put the received data into the rx buffer in the correct byte order.

Signed-off-by Anatolij Gustschin <[email protected]>

---
 drivers/spi/mxc_spi.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

drivers/spi/mxc_spi.c +4 -4

index d558137..9ed2891 100644
@@ -304,7 +304,7 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 			/* Buffer is not 32-bit aligned */
 			if ((unsigned long)dout & 0x03) {
 				data = 0;
-				for (i = 0; i < 4; i++, data <<= 8) {
+				for (i = 0; i < 4; i++) {
 					data = (data << 8) | (*dout++ & 0xFF);
 				}
 			} else {

@@ -337,11 +337,11 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 	if (bitlen % 32) {
 		data = reg_read(mxcs->base + MXC_CSPIRXDATA);
 		cnt = (bitlen % 32) / 8;
+		data = cpu_to_be32(data) >> ((sizeof(data) - cnt) * 8);
 		debug("SPI Rx unaligned: 0x%x\n", data);
 		if (din) {
-			for (i = 0; i < cnt; i++, data >>= 8) {
-				*din++ = data & 0xFF;
-			}
+			memcpy(din, &data, cnt);
+			din += cnt;
 		}
 		nbytes -= cnt;
 	}

Signature · 1 line

1.7.1

Anatolij Gustschin

author

2011-01-16 18:17 UTC #1 lore raw

PATCH 2/2 SPI: mxc_spi: add SPI clock calculation and setup to the driver

The MXC SPI driver didn't calculate the SPI clock up to now and just used lowest possible divider 512 for DATA RATE in the control register. This results in very low transfer rates.

The patch adds code to calculate and setup the SPI clock frequency for transfers.

Signed-off-by Anatolij Gustschin <[email protected]>

---
 drivers/spi/mxc_spi.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

drivers/spi/mxc_spi.c +21 -1

index 9ed2891..07c62c2 100644
@@ -438,12 +438,25 @@ static int decode_cs(struct mxc_spi_slave *mxcs, unsigned int cs)
 	return cs;
 }
 
+u32 get_cspi_div(u32 div)
+{
+	int i;
+
+	for (i = 0; i < 8; i++) {
+		if (div <= (4 << i))
+			return i;
+	}
+	return i;
+}
+
 struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 			unsigned int max_hz, unsigned int mode)
 {
 	unsigned int ctrl_reg;
 	struct mxc_spi_slave *mxcs;
 	int ret;
+	u32 clk_src;
+	u32 div;
 
 	if (bus >= ARRAY_SIZE(spi_bases))
 		return NULL;

@@ -477,9 +490,16 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 		return NULL;
 	}
 #else
+	clk_src = mx31_get_ipg_clk();
+	div = clk_src / max_hz;
+	div = get_cspi_div(div);
+
+	debug("clk %d Hz, div %d, real clk %d Hz\n",
+		max_hz, div, clk_src / (4 << div));
+
 	ctrl_reg = MXC_CSPICTRL_CHIPSELECT(cs) |
 		MXC_CSPICTRL_BITCOUNT(31) |
-		MXC_CSPICTRL_DATARATE(7) | /* FIXME: calculate data rate */
+		MXC_CSPICTRL_DATARATE(div) |
 		MXC_CSPICTRL_EN |
 		MXC_CSPICTRL_MODE;

Signature · 1 line

1.7.1

Stefano Babic

2011-01-17 09:00 UTC #2 lore raw

PATCH 2/2 SPI: mxc_spi: add SPI clock calculation and setup to the driver

Quoted 5 lines · Anatolij Gustschin

The MXC SPI driver didn't calculate the SPI clock up to now and just used lowest possible divider 512 for DATA RATE in the control register. This results in very low transfer rates.

Hi August,

I sent last friday a patch for mxc_spi.c, too, adding support for mx35, and the two patches now conflict. Is it ok for you if I rebase your patch on top of mine and I post it again ?

Best Regards,
Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

Anatolij Gustschin

author

2011-01-17 09:30 UTC #3 lore raw

PATCH 2/2 SPI: mxc_spi: add SPI clock calculation and setup to the driver

Hello Stefano,

Quoted 7 lines · On Mon, 17 Jan 2011 10:00:51 +0100 Stefano Babic <[email protected]> wrote:

Quoted 5 lines · Anatolij Gustschin
The MXC SPI driver didn't calculate the SPI clock up to now and just used lowest possible divider 512 for DATA RATE in the control register. This results in very low transfer rates.

Quoted 5 lines

Hi August,
I sent last friday a patch for mxc_spi.c, too, adding support for mx35, and the two patches now conflict. Is it ok for you if I rebase your patch on top of mine and I post it again ?

Yes, it is ok.

Thanks,
Anatolij

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de

Anatolij Gustschin

author

2011-01-17 09:36 UTC #4 lore raw

PATCH 2/2 SPI: mxc_spi: add SPI clock calculation and setup to the driver

Quoted 12 lines · On Mon, 17 Jan 2011 10:30:09 +0100 Anatolij Gustschin <[email protected]> wrote:

Hello Stefano,
Quoted 7 lines · On Mon, 17 Jan 2011 10:00:51 +0100 Stefano Babic <[email protected]> wrote:
Quoted 5 lines · Anatolij Gustschin
The MXC SPI driver didn't calculate the SPI clock up to now and just used lowest possible divider 512 for DATA RATE in the control register. This results in very low transfer rates.

Quoted 7 lines

Quoted 5 lines
Hi August,
I sent last friday a patch for mxc_spi.c, too, adding support for mx35, and the two patches now conflict. Is it ok for you if I rebase your patch on top of mine and I post it again ?
Yes, it is ok.

BTW: While at it, could you then also please fix a mistake in the commit description: it should be "... just highest possible divider 512".

Thanks,
Anatolij

Stefano Babic

2011-01-17 09:41 UTC #5 lore raw

PATCH 2/2 SPI: mxc_spi: add SPI clock calculation and setup to the driver

Quoted 3 lines · Anatolij Gustschin

BTW: While at it, could you then also please fix a mistake in the commit description: it should be "... just highest possible divider 512".

Of course, I will do it.

Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

Stefano Babic

2011-01-18 11:22 UTC #6 lore raw

PATCH v2 2/2 SPI: mxc_spi: add SPI clock calculation and setup to the driver

In reply to Anatolij Gustschin

From: Anatolij Gustschin <[email protected]>

The MXC SPI driver didn't calculate the SPI clock up to now and just highest possible divider 512 for DATA RATE in the control register. This results in very low transfer rates.

The patch adds code to calculate and setup the SPI clock frequency for transfers.

Signed-off-by Anatolij Gustschin <[email protected]>

Signed-off-by Stefano Babic <[email protected]>

---
Changes:

The patch is rebased on the current patches already sent for this driver (support for MX35). Tested on a MX35 board.

Modified commit message as suggested by Anatolij

 drivers/spi/mxc_spi.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

drivers/spi/mxc_spi.c +23 -1

index 0e42d41..32be7b0 100644
@@ -71,6 +71,7 @@ static unsigned long spi_bases[] = {
 };
 
 #define spi_cfg	spi_cfg_mx3
+#define mxc_get_clock(x)	mx31_get_ipg_clk()
 
 #elif defined(CONFIG_MX51)
 #include <asm/arch/imx-regs.h>

@@ -201,15 +202,36 @@ void spi_cs_deactivate(struct spi_slave *slave)
 			      !(mxcs->ss_pol));
 }
 
+u32 get_cspi_div(u32 div)
+{
+	int i;
+
+	for (i = 0; i < 8; i++) {
+		if (div <= (4 << i))
+			return i;
+	}
+	return i;
+}
+
 #if defined(CONFIG_MX31) || defined(CONFIG_MX35)
 static s32 spi_cfg_mx3(struct mxc_spi_slave *mxcs, unsigned int cs,
 		unsigned int max_hz, unsigned int mode)
 {
 	unsigned int ctrl_reg;
+	u32 clk_src;
+	u32 div;
+
+	clk_src = mxc_get_clock(MXC_CSPI_CLK);
+
+	div = clk_src / max_hz;
+	div = get_cspi_div(div);
+
+	debug("clk %d Hz, div %d, real clk %d Hz\n",
+		max_hz, div, clk_src / (4 << div));
 
 	ctrl_reg = MXC_CSPICTRL_CHIPSELECT(cs) |
 		MXC_CSPICTRL_BITCOUNT(MXC_CSPICTRL_MAXBITS) |
-		MXC_CSPICTRL_DATARATE(7) | /* FIXME: calculate data rate */
+		MXC_CSPICTRL_DATARATE(div) |
 		MXC_CSPICTRL_EN |
 #ifdef CONFIG_MX35
 		MXC_CSPICTRL_SSCTL |

Signature · 1 line

1.7.1

Sergei Shtylyov

2011-01-18 15:37 UTC #7 lore raw

PATCH v2 2/2 SPI: mxc_spi: add SPI clock calculation and setup to the driver

Hello.

Quoted 4 lines · Stefano Babic

From: Anatolij Gustschin <[email protected]>
The MXC SPI driver didn't calculate the SPI clock up to now and just highest possible divider 512 for DATA

^
I think "used" shouldn't have been ommitted here.

Quoted 8 lines

RATE in the control register. This results in very low transfer rates.
The patch adds code to calculate and setup the SPI clock frequency for transfers.
Signed-off-by Anatolij Gustschin <[email protected]>
Signed-off-by Stefano Babic <[email protected]>

WBR, Sergei

Stefano Babic

2011-01-20 08:11 UTC #8 lore raw

PATCH v2 2/2 SPI: mxc_spi: add SPI clock calculation and setup to the driver

Quoted 11 lines · Sergei Shtylyov

Hello.
Quoted 4 lines · Stefano Babic
From: Anatolij Gustschin <[email protected]>
The MXC SPI driver didn't calculate the SPI clock up to now and just highest possible divider 512 for DATA
              ^
I think "used" shouldn't have been ommitted here.

Right. I fix it.

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

Stefano Babic

2011-01-18 11:20 UTC #9 lore raw

PATCH 1/2 SPI: mxc_spi: fix swapping bug and add missing swapping in unaligned rx case

In reply to Anatolij Gustschin

Quoted 5 lines · Anatolij Gustschin

We need to shift only one time in each cycle in the swapping loop for unaligned tx case. Currently two byte shift operations are performed in each loop cycle causing zero gaps in the transmited data, so not all data scheduled for transmition is actually transmited.

Tested on a mx35pdk.

Tested-by Stefano Babic <[email protected]>

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================