spi: mpc8xxx: DM conversion

PATCH Superseded Mario Six 2 participants 24 messages Apr 10, 2018 – Apr 19, 2018 SPI

lore

Share & tools

lore

Mario Six

author

2018-04-10 11:01 UTC #0 lore raw

cover spi: mpc8xxx: DM conversion

This patch series adds support for DM to the MPC8XXX SPI driver, cleans up the driver code, fixes a few minor problems.

Some TODOs are left over for later, such as proper SPI speed setting, and support for SPI mode setting. These would be enhancements to the original functionality, and can come later.

The legacy functionality is retained, so older boards will still work after the patches are applied.

Mario Six (19):
spi: mpc8xxx: Use short type names
spi: mpc8xxx: Fix comments
spi: mpc8xxx: Rename camel-case variables
spi: mpc8xxx: Fix space after cast
spi: mpc8xxx: Fix function names in strings
spi: mpc8xxx: Replace defines with enums
spi: mpc8xxx: Use IO accessors
spi: mpc8xxx: Simplify if
spi: mpc8xxx: Get rid of is_read
spi: mpc8xxx: Simplify logic a bit
spi: mpc8xxx: Reduce scope of loop variables
spi: mpc8xxx: Make code more readable
spi: mpc8xxx: Rename variable
spi: mpc8xxx: Document LEN setting better
spi: mpc8xxx: Re-order transfer setup
spi: mpc8xxx: Fix if check
spi: mpc8xxx: Use get_timer
spi: mpc8xxx: Prepare DM conversion
spi: mpc8xxx: Add DM support

 drivers/spi/mpc8xxx_spi.c | 386 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 287 insertions(+), 99 deletions(-)

--
2.11.0

Mario Six

author

2018-04-10 11:01 UTC #1 lore raw

PATCH 1/19 spi: mpc8xxx: Use short type names

The function signatures in the driver are quite long as is. Use short type names (uint etc.) to make them more readable.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

drivers/spi/mpc8xxx_spi.c +5 -7

index 00cbcbf9fc..2e1a6caf03 100644
@@ -21,8 +21,7 @@
 
 #define SPI_TIMEOUT	1000
 
-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
-		unsigned int max_hz, unsigned int mode)
+struct spi_slave *spi_setup_slave(uint bus, uint cs, uint max_hz, uint mode)
 {
 	struct spi_slave *slave;

@@ -69,17 +68,16 @@ int spi_claim_bus(struct spi_slave *slave)
 
 void spi_release_bus(struct spi_slave *slave)
 {
-
 }
 
-int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
-		void *din, unsigned long flags)
+int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
+	     ulong flags)
 {
 	volatile spi8xxx_t *spi = &((immap_t *) (CONFIG_SYS_IMMR))->spi;
-	unsigned int tmpdout, tmpdin, event;
+	uint tmpdout, tmpdin, event;
 	int numBlks = DIV_ROUND_UP(bitlen, 32);
 	int tm, isRead = 0;
-	unsigned char charSize = 32;
+	uchar charSize = 32;
 
 	debug("spi_xfer: slave %u:%u dout %08X din %08X bitlen %u\n",
 	      slave->bus, slave->cs, *(uint *) dout, *(uint *) din, bitlen);

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #2 lore raw

PATCH 2/19 spi: mpc8xxx: Fix comments

In reply to Mario Six

There are some comments on the same line as the code they document. Put comments above the code lines they document, so the line length is not unnecessarily increased.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

drivers/spi/mpc8xxx_spi.c +14 -8

index 2e1a6caf03..2f6afaf7ac 100644
@@ -54,11 +54,14 @@ void spi_init(void)
 	 * some registers
 	 */
 	spi->mode = SPI_MODE_REV | SPI_MODE_MS | SPI_MODE_EN;
-	spi->mode = (spi->mode & 0xfff0ffff) | BIT(16); /* Use SYSCLK / 8
-							     (16.67MHz typ.) */
-	spi->event = 0xffffffff;	/* Clear all SPI events */
-	spi->mask = 0x00000000;	/* Mask  all SPI interrupts */
-	spi->com = 0;		/* LST bit doesn't do anything, so disregard */
+	/* Use SYSCLK / 8 (16.67MHz typ.) */
+	spi->mode = (spi->mode & 0xfff0ffff) | BIT(16);
+	/* Clear all SPI events */
+	spi->event = 0xffffffff;
+	/* Mask  all SPI interrupts */
+	spi->mask = 0x00000000;
+	/* LST bit doesn't do anything, so disregard */
+	spi->com = 0;
 }
 
 int spi_claim_bus(struct spi_slave *slave)

@@ -85,9 +88,10 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 	if (flags & SPI_XFER_BEGIN)
 		spi_cs_activate(slave);
 
-	spi->event = 0xffffffff;	/* Clear all SPI events */
+	/* Clear all SPI events */
+	spi->event = 0xffffffff;
 
-	/* handle data in 32-bit chunks */
+	/* Handle data in 32-bit chunks */
 	while (numBlks--) {
 		tmpdout = 0;
 		charSize = (bitlen >= 32 ? 32 : bitlen);

@@ -121,7 +125,9 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 
 		spi->mode |= SPI_MODE_EN;
 
-		spi->tx = tmpdout;	/* Write the data out */
+		/* Write the data out */
+		spi->tx = tmpdout;
+
 		debug("*** spi_xfer: ... %08x written\n", tmpdout);
 
 		/*

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #3 lore raw

PATCH 3/19 spi: mpc8xxx: Rename camel-case variables

In reply to Mario Six

There are three variables that have camel-case names, which is not the preferred naming style.

Give those variables more compliant names instead.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

drivers/spi/mpc8xxx_spi.c +11 ~~-11~~

index 2f6afaf7ac..3ac9f2f8e8 100644
@@ -78,9 +78,9 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 {
 	volatile spi8xxx_t *spi = &((immap_t *) (CONFIG_SYS_IMMR))->spi;
 	uint tmpdout, tmpdin, event;
-	int numBlks = DIV_ROUND_UP(bitlen, 32);
-	int tm, isRead = 0;
-	uchar charSize = 32;
+	int num_blks = DIV_ROUND_UP(bitlen, 32);
+	int tm, is_read = 0;
+	uchar char_size = 32;
 
 	debug("spi_xfer: slave %u:%u dout %08X din %08X bitlen %u\n",
 	      slave->bus, slave->cs, *(uint *) dout, *(uint *) din, bitlen);

@@ -92,12 +92,12 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 	spi->event = 0xffffffff;
 
 	/* Handle data in 32-bit chunks */
-	while (numBlks--) {
+	while (num_blks--) {
 		tmpdout = 0;
-		charSize = (bitlen >= 32 ? 32 : bitlen);
+		char_size = (bitlen >= 32 ? 32 : bitlen);
 
 		/* Shift data so it's msb-justified */
-		tmpdout = *(u32 *) dout >> (32 - charSize);
+		tmpdout = *(u32 *) dout >> (32 - char_size);
 
 		/* The LEN field of the SPMODE register is set as follows:
 		 *

@@ -135,15 +135,15 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		 * or time out (1 second = 1000 ms)
 		 * The NE event must be read and cleared first
 		 */
-		for (tm = 0, isRead = 0; tm < SPI_TIMEOUT; ++tm) {
+		for (tm = 0, is_read = 0; tm < SPI_TIMEOUT; ++tm) {
 			event = spi->event;
 			if (event & SPI_EV_NE) {
 				tmpdin = spi->rx;
 				spi->event |= SPI_EV_NE;
-				isRead = 1;
+				is_read = 1;
 
-				*(u32 *) din = (tmpdin << (32 - charSize));
-				if (charSize == 32) {
+				*(u32 *) din = (tmpdin << (32 - char_size));
+				if (char_size == 32) {
 					/* Advance output buffer by 32 bits */
 					din += 4;
 				}

@@ -154,7 +154,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 			 * in the future put an arbitrary delay after writing
 			 * the device.  Arbitrary delays suck, though...
 			 */
-			if (isRead && (event & SPI_EV_NF))
+			if (is_read && (event & SPI_EV_NF))
 				break;
 		}
 		if (tm >= SPI_TIMEOUT)

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #4 lore raw

PATCH 4/19 spi: mpc8xxx: Fix space after cast

In reply to Mario Six

Fix all "superfluous space after case" style errors.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/spi/mpc8xxx_spi.c +3 -3

index 3ac9f2f8e8..079476c3ee 100644
@@ -83,7 +83,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 	uchar char_size = 32;
 
 	debug("spi_xfer: slave %u:%u dout %08X din %08X bitlen %u\n",
-	      slave->bus, slave->cs, *(uint *) dout, *(uint *) din, bitlen);
+	      slave->bus, slave->cs, *(uint *)dout, *(uint *)din, bitlen);
 
 	if (flags & SPI_XFER_BEGIN)
 		spi_cs_activate(slave);

@@ -97,7 +97,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		char_size = (bitlen >= 32 ? 32 : bitlen);
 
 		/* Shift data so it's msb-justified */
-		tmpdout = *(u32 *) dout >> (32 - char_size);
+		tmpdout = *(u32 *)dout >> (32 - char_size);
 
 		/* The LEN field of the SPMODE register is set as follows:
 		 *

@@ -142,7 +142,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 				spi->event |= SPI_EV_NE;
 				is_read = 1;
 
-				*(u32 *) din = (tmpdin << (32 - char_size));
+				*(u32 *)din = (tmpdin << (32 - char_size));
 				if (char_size == 32) {
 					/* Advance output buffer by 32 bits */
 					din += 4;

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #5 lore raw

PATCH 5/19 spi: mpc8xxx: Fix function names in strings

In reply to Mario Six

Replace the function name with a "%s" format string and the __func__ variable in debug statements (as proposed by checkpatch).

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

drivers/spi/mpc8xxx_spi.c +5 -4

index 079476c3ee..0b81db959e 100644
@@ -82,7 +82,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 	int tm, is_read = 0;
 	uchar char_size = 32;
 
-	debug("spi_xfer: slave %u:%u dout %08X din %08X bitlen %u\n",
+	debug("%s: slave %u:%u dout %08X din %08X bitlen %u\n", __func__,
 	      slave->bus, slave->cs, *(uint *)dout, *(uint *)din, bitlen);
 
 	if (flags & SPI_XFER_BEGIN)

@@ -128,7 +128,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		/* Write the data out */
 		spi->tx = tmpdout;
 
-		debug("*** spi_xfer: ... %08x written\n", tmpdout);
+		debug("*** %s: ... %08x written\n", __func__, tmpdout);
 
 		/*
 		 * Wait for SPI transmit to get out

@@ -158,9 +158,10 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 				break;
 		}
 		if (tm >= SPI_TIMEOUT)
-			puts("*** spi_xfer: Time out during SPI transfer");
+			debug("*** %s: Time out during SPI transfer\n",
+			      __func__);
 
-		debug("*** spi_xfer: transfer ended. Value=%08x\n", tmpdin);
+		debug("*** %s: transfer ended. Value=%08x\n", __func__, tmpdin);
 	}
 
 	if (flags & SPI_XFER_END)

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #6 lore raw

PATCH 6/19 spi: mpc8xxx: Replace defines with enums

In reply to Mario Six

Replace pre-processor defines with proper enums, and use the BIT macro where applicable.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

drivers/spi/mpc8xxx_spi.c +19 -7

index 0b81db959e..bd2857ce1a 100644
@@ -11,13 +11,25 @@
 #include <spi.h>
 #include <asm/mpc8xxx_spi.h>
 
-#define SPI_EV_NE	(0x80000000 >> 22)	/* Receiver Not Empty */
-#define SPI_EV_NF	(0x80000000 >> 23)	/* Transmitter Not Full */
-
-#define SPI_MODE_LOOP	(0x80000000 >> 1)	/* Loopback mode */
-#define SPI_MODE_REV	(0x80000000 >> 5)	/* Reverse mode - MSB first */
-#define SPI_MODE_MS	(0x80000000 >> 6)	/* Always master */
-#define SPI_MODE_EN	(0x80000000 >> 7)	/* Enable interface */
+enum {
+	SPI_EV_NE = BIT(31 - 22),	/* Receiver Not Empty */
+	SPI_EV_NF = BIT(31 - 23),	/* Transmitter Not Full */
+};
+
+enum {
+	SPI_MODE_LOOP  = BIT(31 - 1),	/* Loopback mode */
+	SPI_MODE_CI    = BIT(31 - 2),	/* Clock invert */
+	SPI_MODE_CP    = BIT(31 - 3),	/* Clock phase */
+	SPI_MODE_DIV16 = BIT(31 - 4),	/* Divide clock source by 16 */
+	SPI_MODE_REV   = BIT(31 - 5),	/* Reverse mode - MSB first */
+	SPI_MODE_MS    = BIT(31 - 6),	/* Always master */
+	SPI_MODE_EN    = BIT(31 - 7),	/* Enable interface */
+
+	SPI_MODE_LEN_MASK = 0xf00000,
+	SPI_MODE_PM_MASK = 0xf0000,
+
+	SPI_COM_LST = BIT(31 - 9),
+};
 
 #define SPI_TIMEOUT	1000

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #7 lore raw

PATCH 7/19 spi: mpc8xxx: Use IO accessors

In reply to Mario Six

Accesses to the register map are currently done by directly reading and writing the structure.

Switch to the appropriate IO accessors instead.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

drivers/spi/mpc8xxx_spi.c +19 ~~-19~~

index bd2857ce1a..b5546aa502 100644
@@ -59,21 +59,21 @@ void spi_free_slave(struct spi_slave *slave)
 
 void spi_init(void)
 {
-	volatile spi8xxx_t *spi = &((immap_t *) (CONFIG_SYS_IMMR))->spi;
+	spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi;
 
 	/*
 	 * SPI pins on the MPC83xx are not muxed, so all we do is initialize
 	 * some registers
 	 */
-	spi->mode = SPI_MODE_REV | SPI_MODE_MS | SPI_MODE_EN;
+	out_be32(&spi->mode, SPI_MODE_REV | SPI_MODE_MS | SPI_MODE_EN);
 	/* Use SYSCLK / 8 (16.67MHz typ.) */
-	spi->mode = (spi->mode & 0xfff0ffff) | BIT(16);
+	clrsetbits_be32(&spi->mode, 0x000f0000, BIT(16));
 	/* Clear all SPI events */
-	spi->event = 0xffffffff;
+	setbits_be32(&spi->event, 0xffffffff);
 	/* Mask  all SPI interrupts */
-	spi->mask = 0x00000000;
+	clrbits_be32(&spi->mask, 0xffffffff);
 	/* LST bit doesn't do anything, so disregard */
-	spi->com = 0;
+	out_be32(&spi->com, 0);
 }
 
 int spi_claim_bus(struct spi_slave *slave)

@@ -88,7 +88,7 @@ void spi_release_bus(struct spi_slave *slave)
 int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 	     ulong flags)
 {
-	volatile spi8xxx_t *spi = &((immap_t *) (CONFIG_SYS_IMMR))->spi;
+	spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi;
 	uint tmpdout, tmpdin, event;
 	int num_blks = DIV_ROUND_UP(bitlen, 32);
 	int tm, is_read = 0;

@@ -101,7 +101,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		spi_cs_activate(slave);
 
 	/* Clear all SPI events */
-	spi->event = 0xffffffff;
+	setbits_be32(&spi->event, 0xffffffff);
 
 	/* Handle data in 32-bit chunks */
 	while (num_blks--) {

@@ -119,26 +119,26 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		 * len > 16               0
 		 */
 
-		spi->mode &= ~SPI_MODE_EN;
+		clrbits_be32(&spi->mode, SPI_MODE_EN);
 
 		if (bitlen <= 16) {
 			if (bitlen <= 4)
-				spi->mode = (spi->mode & 0xff0fffff) |
-					    (3 << 20);
+				clrsetbits_be32(&spi->mode, 0x00f00000,
+						(3 << 20));
 			else
-				spi->mode = (spi->mode & 0xff0fffff) |
-					    ((bitlen - 1) << 20);
+				clrsetbits_be32(&spi->mode, 0x00f00000,
+						((bitlen - 1) << 20));
 		} else {
-			spi->mode = (spi->mode & 0xff0fffff);
+			clrbits_be32(&spi->mode, 0x00f00000);
 			/* Set up the next iteration if sending > 32 bits */
 			bitlen -= 32;
 			dout += 4;
 		}
 
-		spi->mode |= SPI_MODE_EN;
+		setbits_be32(&spi->mode, SPI_MODE_EN);
 
 		/* Write the data out */
-		spi->tx = tmpdout;
+		out_be32(&spi->tx, tmpdout);
 
 		debug("*** %s: ... %08x written\n", __func__, tmpdout);

@@ -148,10 +148,10 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		 * The NE event must be read and cleared first
 		 */
 		for (tm = 0, is_read = 0; tm < SPI_TIMEOUT; ++tm) {
-			event = spi->event;
+			event = in_be32(&spi->event);
 			if (event & SPI_EV_NE) {
-				tmpdin = spi->rx;
-				spi->event |= SPI_EV_NE;
+				tmpdin = in_be32(&spi->rx);
+				setbits_be32(&spi->event, SPI_EV_NE);
 				is_read = 1;
 
 				*(u32 *)din = (tmpdin << (32 - char_size));

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #8 lore raw

PATCH 8/19 spi: mpc8xxx: Simplify if

In reply to Mario Six

Instead of having a nested if block, just have two branches within the overarching if block to eliminate one nesting level.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

drivers/spi/mpc8xxx_spi.c +5 -7

index b5546aa502..c4a9ef53ef 100644
@@ -121,13 +121,11 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 
 		clrbits_be32(&spi->mode, SPI_MODE_EN);
 
-		if (bitlen <= 16) {
-			if (bitlen <= 4)
-				clrsetbits_be32(&spi->mode, 0x00f00000,
-						(3 << 20));
-			else
-				clrsetbits_be32(&spi->mode, 0x00f00000,
-						((bitlen - 1) << 20));
+		if (bitlen <= 4) {
+			clrsetbits_be32(&spi->mode, 0x00f00000, (3 << 20));
+		} else if (bitlen <= 16) {
+			clrsetbits_be32(&spi->mode, 0x00f00000,
+					((bitlen - 1) << 20));
 		} else {
 			clrbits_be32(&spi->mode, 0x00f00000);
 			/* Set up the next iteration if sending > 32 bits */

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #9 lore raw

PATCH 9/19 spi: mpc8xxx: Get rid of is_read

In reply to Mario Six

Get rid of the is_read variable, and just keep the state of the "not empty" and "not full" events in two boolean variables within the loop body.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/spi/mpc8xxx_spi.c +7 -5

index c4a9ef53ef..a69987ef54 100644
@@ -91,7 +91,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 	spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi;
 	uint tmpdout, tmpdin, event;
 	int num_blks = DIV_ROUND_UP(bitlen, 32);
-	int tm, is_read = 0;
+	int tm;
 	uchar char_size = 32;
 
 	debug("%s: slave %u:%u dout %08X din %08X bitlen %u\n", __func__,

@@ -145,12 +145,14 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		 * or time out (1 second = 1000 ms)
 		 * The NE event must be read and cleared first
 		 */
-		for (tm = 0, is_read = 0; tm < SPI_TIMEOUT; ++tm) {
+		for (tm = 0; tm < SPI_TIMEOUT; ++tm) {
 			event = in_be32(&spi->event);
-			if (event & SPI_EV_NE) {
+			bool have_ne = event & SPI_EV_NE;
+			bool have_nf = event & SPI_EV_NF;
+
+			if (have_ne) {
 				tmpdin = in_be32(&spi->rx);
 				setbits_be32(&spi->event, SPI_EV_NE);
-				is_read = 1;
 
 				*(u32 *)din = (tmpdin << (32 - char_size));
 				if (char_size == 32) {

@@ -164,7 +166,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 			 * in the future put an arbitrary delay after writing
 			 * the device.  Arbitrary delays suck, though...
 			 */
-			if (is_read && (event & SPI_EV_NF))
+			if (have_ne && have_nf)
 				break;
 		}
 		if (tm >= SPI_TIMEOUT)

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #10 lore raw

PATCH 10/19 spi: mpc8xxx: Simplify logic a bit

In reply to Mario Six

We do nothing in the loop if the "not empty" event was not detected. To simplify the logic, check if this is the case, and skip the execution of the loop early to reduce the nesting level and flag checking.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

drivers/spi/mpc8xxx_spi.c +13 ~~-10~~

index a69987ef54..30249cce57 100644
@@ -150,25 +150,28 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 			bool have_ne = event & SPI_EV_NE;
 			bool have_nf = event & SPI_EV_NF;
 
-			if (have_ne) {
-				tmpdin = in_be32(&spi->rx);
-				setbits_be32(&spi->event, SPI_EV_NE);
-
-				*(u32 *)din = (tmpdin << (32 - char_size));
-				if (char_size == 32) {
-					/* Advance output buffer by 32 bits */
-					din += 4;
-				}
+			if (!have_ne)
+				continue;
+
+			tmpdin = in_be32(&spi->rx);
+			setbits_be32(&spi->event, SPI_EV_NE);
+
+			*(u32 *)din = (tmpdin << (32 - char_size));
+			if (char_size == 32) {
+				/* Advance output buffer by 32 bits */
+				din += 4;
 			}
+
 			/*
 			 * Only bail when we've had both NE and NF events.
 			 * This will cause timeouts on RO devices, so maybe
 			 * in the future put an arbitrary delay after writing
 			 * the device.  Arbitrary delays suck, though...
 			 */
-			if (have_ne && have_nf)
+			if (have_nf)
 				break;
 		}
+
 		if (tm >= SPI_TIMEOUT)
 			debug("*** %s: Time out during SPI transfer\n",
 			      __func__);

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #11 lore raw

PATCH 11/19 spi: mpc8xxx: Reduce scope of loop variables

In reply to Mario Six

The transmission loop starts with setting some variables, which are only used inside the loop. Reduce the scope to the loop to make the declaration and initialization of these variables coincide.

In the case of char_size this also always initializes the variable immediately with the final value actually used in the loop (instead of the placeholder value 32).

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

drivers/spi/mpc8xxx_spi.c +5 -6

index 30249cce57..5bf84aaec6 100644
@@ -89,10 +89,8 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 	     ulong flags)
 {
 	spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi;
-	uint tmpdout, tmpdin, event;
+	u32 tmpdin;
 	int num_blks = DIV_ROUND_UP(bitlen, 32);
-	int tm;
-	uchar char_size = 32;
 
 	debug("%s: slave %u:%u dout %08X din %08X bitlen %u\n", __func__,
 	      slave->bus, slave->cs, *(uint *)dout, *(uint *)din, bitlen);

@@ -105,8 +103,9 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 
 	/* Handle data in 32-bit chunks */
 	while (num_blks--) {
-		tmpdout = 0;
-		char_size = (bitlen >= 32 ? 32 : bitlen);
+		int tm;
+		u32 tmpdout = 0;
+		uchar char_size = (bitlen >= 32 ? 32 : bitlen);
 
 		/* Shift data so it's msb-justified */
 		tmpdout = *(u32 *)dout >> (32 - char_size);

@@ -146,7 +145,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		 * The NE event must be read and cleared first
 		 */
 		for (tm = 0; tm < SPI_TIMEOUT; ++tm) {
-			event = in_be32(&spi->event);
+			u32 event = in_be32(&spi->event);
 			bool have_ne = event & SPI_EV_NE;
 			bool have_nf = event & SPI_EV_NF;

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #12 lore raw

PATCH 12/19 spi: mpc8xxx: Make code more readable

In reply to Mario Six

Introduce the to_prescale_mod and set_char_len inline functions to make the code more readable.

Note that the added "if (bitlen > 16)" check does not change the semantics of the current code, and hence only preserves the current error (this will be fixed in a later patch in the series).

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

drivers/spi/mpc8xxx_spi.c +19 -8

index 5bf84aaec6..af3762737f 100644
@@ -31,6 +31,16 @@ enum {
 	SPI_COM_LST = BIT(31 - 9),
 };
 
+static inline u32 to_prescale_mod(u32 val)
+{
+	return (min(val, (u32)15) << 16);
+}
+
+static void set_char_len(spi8xxx_t *spi, u32 val)
+{
+	clrsetbits_be32(&spi->mode, SPI_MODE_LEN_MASK, (val << 20));
+}
+
 #define SPI_TIMEOUT	1000
 
 struct spi_slave *spi_setup_slave(uint bus, uint cs, uint max_hz, uint mode)

@@ -67,7 +77,7 @@ void spi_init(void)
 	 */
 	out_be32(&spi->mode, SPI_MODE_REV | SPI_MODE_MS | SPI_MODE_EN);
 	/* Use SYSCLK / 8 (16.67MHz typ.) */
-	clrsetbits_be32(&spi->mode, 0x000f0000, BIT(16));
+	clrsetbits_be32(&spi->mode, SPI_MODE_PM_MASK, to_prescale_mod(1));
 	/* Clear all SPI events */
 	setbits_be32(&spi->event, 0xffffffff);
 	/* Mask  all SPI interrupts */

@@ -120,13 +130,14 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 
 		clrbits_be32(&spi->mode, SPI_MODE_EN);
 
-		if (bitlen <= 4) {
-			clrsetbits_be32(&spi->mode, 0x00f00000, (3 << 20));
-		} else if (bitlen <= 16) {
-			clrsetbits_be32(&spi->mode, 0x00f00000,
-					((bitlen - 1) << 20));
-		} else {
-			clrbits_be32(&spi->mode, 0x00f00000);
+		if (bitlen <= 4)
+			set_char_len(spi, 3);
+		else if (bitlen <= 16)
+			set_char_len(spi, bitlen - 1);
+		else
+			set_char_len(spi, 0);
+
+		if (bitlen > 16) {
 			/* Set up the next iteration if sending > 32 bits */
 			bitlen -= 32;
 			dout += 4;

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #13 lore raw

PATCH 13/19 spi: mpc8xxx: Rename variable

In reply to Mario Six

The variable "char_size" holds the number of bits to be transferred in the current loop iteration. A better name would be "xfer_bitlen", which we rename this variable to.

Signed-off-by Mario Six <[email protected]>

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

drivers/spi/mpc8xxx_spi.c +4 -4

index af3762737f..ce2a77c1aa 100644
@@ -115,10 +115,10 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 	while (num_blks--) {
 		int tm;
 		u32 tmpdout = 0;
-		uchar char_size = (bitlen >= 32 ? 32 : bitlen);
+		uchar xfer_bitlen = (bitlen >= 32 ? 32 : bitlen);
 
 		/* Shift data so it's msb-justified */
-		tmpdout = *(u32 *)dout >> (32 - char_size);
+		tmpdout = *(u32 *)dout >> (32 - xfer_bitlen);
 
 		/* The LEN field of the SPMODE register is set as follows:
 		 *

@@ -166,8 +166,8 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 			tmpdin = in_be32(&spi->rx);
 			setbits_be32(&spi->event, SPI_EV_NE);
 
-			*(u32 *)din = (tmpdin << (32 - char_size));
-			if (char_size == 32) {
+			*(u32 *)din = (tmpdin << (32 - xfer_bitlen));
+			if (xfer_bitlen == 32) {
 				/* Advance output buffer by 32 bits */
 				din += 4;
 			}

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #14 lore raw

PATCH 14/19 spi: mpc8xxx: Document LEN setting better

In reply to Mario Six

Instead of having a table right before the code implementing the length setting for documentation, have inline comments for the if branches actually implementing the length setting described table's entries (which is readable thanks to the set_char_len function).

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

drivers/spi/mpc8xxx_spi.c +5 ~~-11~~

index ce2a77c1aa..d22206f4b7 100644
@@ -120,21 +120,15 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		/* Shift data so it's msb-justified */
 		tmpdout = *(u32 *)dout >> (32 - xfer_bitlen);
 
-		/* The LEN field of the SPMODE register is set as follows:
-		 *
-		 * Bit length             setting
-		 * len <= 4               3
-		 * 4 < len <= 16          len - 1
-		 * len > 16               0
-		 */
-
 		clrbits_be32(&spi->mode, SPI_MODE_EN);
 
-		if (bitlen <= 4)
+		/* Set up length for this transfer */
+
+		if (bitlen <= 4) /* 4 bits or less */
 			set_char_len(spi, 3);
-		else if (bitlen <= 16)
+		else if (bitlen <= 16) /*@most 16 bits */
 			set_char_len(spi, bitlen - 1);
-		else
+		else /* more than 16 bits -> full 32 bit transfer */
 			set_char_len(spi, 0);
 
 		if (bitlen > 16) {

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #15 lore raw

PATCH 15/19 spi: mpc8xxx: Re-order transfer setup

In reply to Mario Six

Minize the time the adapter is disabled (via SPI_MODE_EN clearing/setting) to just the character length setting, and only set up the temporary data writing variable right before we need it, so there is a more clear distinction between setting up the SPI adapter, and setting up the data to be written.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/spi/mpc8xxx_spi.c +5 -5

index d22206f4b7..fe493f6d40 100644
@@ -117,9 +117,6 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		u32 tmpdout = 0;
 		uchar xfer_bitlen = (bitlen >= 32 ? 32 : bitlen);
 
-		/* Shift data so it's msb-justified */
-		tmpdout = *(u32 *)dout >> (32 - xfer_bitlen);
-
 		clrbits_be32(&spi->mode, SPI_MODE_EN);
 
 		/* Set up length for this transfer */

@@ -131,14 +128,17 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		else /* more than 16 bits -> full 32 bit transfer */
 			set_char_len(spi, 0);
 
+		setbits_be32(&spi->mode, SPI_MODE_EN);
+
+		/* Shift data so it's msb-justified */
+		tmpdout = *(u32 *)dout >> (32 - xfer_bitlen);
+
 		if (bitlen > 16) {
 			/* Set up the next iteration if sending > 32 bits */
 			bitlen -= 32;
 			dout += 4;
 		}
 
-		setbits_be32(&spi->mode, SPI_MODE_EN);
-
 		/* Write the data out */
 		out_be32(&spi->tx, tmpdout);

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #16 lore raw

PATCH 16/19 spi: mpc8xxx: Fix if check

In reply to Mario Six

Decreasing the bit length and increasing the write data pointer should be done when there are more than 32 bit of data, not 16 bit.

This did not produce incorrect behavior, because the only time where the two checks produce different outcomes is the case of 16 < bitlen < 32, and in this case the subsequent transmission is the last one regardless, hence the additional bit length decrease and write data pointer increase has no effect anyway.

Still, the correct check is the check for "bitlen > 32", so correct this behavior.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/spi/mpc8xxx_spi.c +1 -1

index fe493f6d40..b3ed00fcb7 100644
@@ -133,7 +133,7 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		/* Shift data so it's msb-justified */
 		tmpdout = *(u32 *)dout >> (32 - xfer_bitlen);
 
-		if (bitlen > 16) {
+		if (bitlen > 32) {
 			/* Set up the next iteration if sending > 32 bits */
 			bitlen -= 32;
 			dout += 4;

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #17 lore raw

PATCH 17/19 spi: mpc8xxx: Use get_timer

In reply to Mario Six

The comment before the transmission loop in conjunction with the definition of SPI_TIMEOUT as 1000 implies that the loop is supposed to have a timeout value of 1000 ms. But since there is no mdelay(1) or similar in the loop body, the loop just runs 1000 times, without regard for the time elapsed.

To correct this, use the standard get_timer functionality to properly time out the loop after 1000 ms.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

drivers/spi/mpc8xxx_spi.c +7 -4

index b3ed00fcb7..4aa5db8821 100644
@@ -113,9 +113,9 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 
 	/* Handle data in 32-bit chunks */
 	while (num_blks--) {
-		int tm;
 		u32 tmpdout = 0;
 		uchar xfer_bitlen = (bitlen >= 32 ? 32 : bitlen);
+		ulong start;
 
 		clrbits_be32(&spi->mode, SPI_MODE_EN);

@@ -149,7 +149,8 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		 * or time out (1 second = 1000 ms)
 		 * The NE event must be read and cleared first
 		 */
-		for (tm = 0; tm < SPI_TIMEOUT; ++tm) {
+		start = get_timer(0);
+		do {
 			u32 event = in_be32(&spi->event);
 			bool have_ne = event & SPI_EV_NE;
 			bool have_nf = event & SPI_EV_NF;

@@ -174,9 +175,11 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 			 */
 			if (have_nf)
 				break;
-		}
 
-		if (tm >= SPI_TIMEOUT)
+			mdelay(1);
+		} while (get_timer(start) < SPI_TIMEOUT);
+
+		if (get_timer(start) >= SPI_TIMEOUT)
 			debug("*** %s: Time out during SPI transfer\n",
 			      __func__);

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #18 lore raw

PATCH 18/19 spi: mpc8xxx: Prepare DM conversion

In reply to Mario Six

To prepare DM conversion, move all driver functionality into separate functions, and call them from the driver functions; these functions will also be called from the DM-driver functions, so that we can keep the legacy functions for the boards that still need them.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 120 +++++++++++++++++++++++++++++-----------------
 1 file changed, 75 insertions(+), 45 deletions(-)

drivers/spi/mpc8xxx_spi.c +75 ~~-45~~

index 4aa5db8821..5724a00585 100644
@@ -43,41 +43,27 @@ static void set_char_len(spi8xxx_t *spi, u32 val)
 
 #define SPI_TIMEOUT	1000
 
-struct spi_slave *spi_setup_slave(uint bus, uint cs, uint max_hz, uint mode)
+static void __spi_init_head(spi8xxx_t *spi)
 {
-	struct spi_slave *slave;
-
-	if (!spi_cs_is_valid(bus, cs))
-		return NULL;
-
-	slave = spi_alloc_slave_base(bus, cs);
-	if (!slave)
-		return NULL;
-
 	/*
-	 * TODO: Some of the code in spi_init() should probably move
-	 * here, or into spi_claim_bus() below.
+	 * SPI pins on the MPC83xx are not muxed, so all we do is initialize
+	 * some registers
 	 */
-
-	return slave;
-}
-
-void spi_free_slave(struct spi_slave *slave)
-{
-	free(slave);
+	out_be32(&spi->mode, SPI_MODE_REV | SPI_MODE_MS | SPI_MODE_EN);
 }
 
-void spi_init(void)
+static int __spi_set_speed(spi8xxx_t *spi, uint speed)
 {
-	spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi;
+	/* TODO: This only ever sets one fixed speed */
 
-	/*
-	 * SPI pins on the MPC83xx are not muxed, so all we do is initialize
-	 * some registers
-	 */
-	out_be32(&spi->mode, SPI_MODE_REV | SPI_MODE_MS | SPI_MODE_EN);
 	/* Use SYSCLK / 8 (16.67MHz typ.) */
 	clrsetbits_be32(&spi->mode, SPI_MODE_PM_MASK, to_prescale_mod(1));
+
+	return 0;
+}
+
+static void __spi_init_tail(spi8xxx_t *spi)
+{
 	/* Clear all SPI events */
 	setbits_be32(&spi->event, 0xffffffff);
 	/* Mask  all SPI interrupts */

@@ -86,28 +72,12 @@ void spi_init(void)
 	out_be32(&spi->com, 0);
 }
 
-int spi_claim_bus(struct spi_slave *slave)
+static int __spi_xfer(spi8xxx_t *spi, uint bitlen, const u8 *dout, u8 *din,
+		      ulong flags)
 {
-	return 0;
-}
-
-void spi_release_bus(struct spi_slave *slave)
-{
-}
-
-int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
-	     ulong flags)
-{
-	spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi;
 	u32 tmpdin;
 	int num_blks = DIV_ROUND_UP(bitlen, 32);
 
-	debug("%s: slave %u:%u dout %08X din %08X bitlen %u\n", __func__,
-	      slave->bus, slave->cs, *(uint *)dout, *(uint *)din, bitlen);
-
-	if (flags & SPI_XFER_BEGIN)
-		spi_cs_activate(slave);
-
 	/* Clear all SPI events */
 	setbits_be32(&spi->event, 0xffffffff);

@@ -186,8 +156,68 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 		debug("*** %s: transfer ended. Value=%08x\n", __func__, tmpdin);
 	}
 
+	return 0;
+}
+
+struct spi_slave *spi_setup_slave(uint bus, uint cs, uint max_hz, uint mode)
+{
+	struct spi_slave *slave;
+
+	if (!spi_cs_is_valid(bus, cs))
+		return NULL;
+
+	slave = spi_alloc_slave_base(bus, cs);
+	if (!slave)
+		return NULL;
+
+	/*
+	 * TODO: Some of the code in spi_init() should probably move
+	 * here, or into spi_claim_bus() below.
+	 */
+
+	return slave;
+}
+
+void spi_free_slave(struct spi_slave *slave)
+{
+	free(slave);
+}
+
+void spi_init(void)
+{
+	spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi;
+
+	out_be32(&spi->mode, SPI_MODE_REV | SPI_MODE_MS | SPI_MODE_EN);
+	__spi_init_head(spi);
+	__spi_set_speed(spi, 16666667);
+	__spi_init_tail(spi);
+}
+
+int spi_claim_bus(struct spi_slave *slave)
+{
+	return 0;
+}
+
+void spi_release_bus(struct spi_slave *slave)
+{
+}
+
+int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
+	     ulong flags)
+{
+	spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi;
+	int ret;
+
+	debug("%s: slave %u:%u dout %08X din %08X bitlen %u\n", __func__,
+	      slave->bus, slave->cs, *(uint *)dout, *(uint *)din, bitlen);
+
+	if (flags & SPI_XFER_BEGIN)
+		spi_cs_activate(slave);
+
+	ret = __spi_xfer(spi, bitlen, dout, din, flags);
+
 	if (flags & SPI_XFER_END)
 		spi_cs_deactivate(slave);
 
-	return 0;
+	return ret;
 }

Signature · 1 line

2.11.0

Mario Six

author

2018-04-10 11:01 UTC #19 lore raw

PATCH 19/19 spi: mpc8xxx: Add DM support

In reply to Mario Six

Support DM for the MPC8XXX SPI driver.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 135 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 2 deletions(-)

drivers/spi/mpc8xxx_spi.c +133 -2

index 5724a00585..6e51826665 100644
@@ -10,6 +10,11 @@
 #include <malloc.h>
 #include <spi.h>
 #include <asm/mpc8xxx_spi.h>
+#ifdef CONFIG_DM_SPI
+#include <errno.h>
+#include <dm.h>
+#include <asm-generic/gpio.h>
+#endif
 
 enum {
 	SPI_EV_NE = BIT(31 - 22),	/* Receiver Not Empty */

@@ -31,6 +36,14 @@ enum {
 	SPI_COM_LST = BIT(31 - 9),
 };
 
+#ifdef CONFIG_DM_SPI
+struct mpc8xxx_priv {
+	spi8xxx_t *spi;
+	struct gpio_desc gpios[16];
+	int max_cs;
+};
+#endif
+
 static inline u32 to_prescale_mod(u32 val)
 {
 	return (min(val, (u32)15) << 16);

@@ -54,7 +67,7 @@ static void __spi_init_head(spi8xxx_t *spi)
 
 static int __spi_set_speed(spi8xxx_t *spi, uint speed)
 {
-	/* TODO: This only ever sets one fixed speed */
+	/* TODO([email protected]): This only ever sets one fixed speed */
 
 	/* Use SYSCLK / 8 (16.67MHz typ.) */
 	clrsetbits_be32(&spi->mode, SPI_MODE_PM_MASK, to_prescale_mod(1));

@@ -75,7 +88,7 @@ static void __spi_init_tail(spi8xxx_t *spi)
 static int __spi_xfer(spi8xxx_t *spi, uint bitlen, const u8 *dout, u8 *din,
 		      ulong flags)
 {
-	u32 tmpdin;
+	u32 tmpdin = 0;
 	int num_blks = DIV_ROUND_UP(bitlen, 32);
 
 	/* Clear all SPI events */

@@ -159,6 +172,110 @@ static int __spi_xfer(spi8xxx_t *spi, uint bitlen, const u8 *dout, u8 *din,
 	return 0;
 }
 
+#ifdef CONFIG_DM_SPI
+
+static int mpc8xxx_spi_ofdata_to_platdata(struct udevice *dev)
+{
+	struct mpc8xxx_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	priv->spi = (spi8xxx_t *)dev_read_addr(dev);
+
+	/* TODO(mario.six at gdsys.cc): Read clock and save the value */
+
+	ret = gpio_request_list_by_name(dev, "gpios", priv->gpios,
+					ARRAY_SIZE(priv->gpios), GPIOD_IS_OUT | GPIOD_ACTIVE_LOW);
+	if (ret < 0)
+		return -EINVAL;
+
+	priv->max_cs = ret;
+
+	return 0;
+}
+
+static int mpc8xxx_spi_probe(struct udevice *dev)
+{
+	struct mpc8xxx_priv *priv = dev_get_priv(dev);
+
+	__spi_init_head(priv->spi);
+	__spi_set_speed(priv->spi, 16666667);
+	__spi_init_tail(priv->spi);
+
+	return 0;
+}
+
+static void mpc8xxx_spi_cs_activate(struct udevice *dev)
+{
+	struct mpc8xxx_priv *priv = dev_get_priv(dev->parent);
+	struct dm_spi_slave_platdata *platdata = dev_get_parent_platdata(dev);
+
+	dm_gpio_set_dir_flags(&priv->gpios[platdata->cs], GPIOD_IS_OUT);
+	dm_gpio_set_value(&priv->gpios[platdata->cs], 0);
+}
+
+static void mpc8xxx_spi_cs_deactivate(struct udevice *dev)
+{
+	struct mpc8xxx_priv *priv = dev_get_priv(dev->parent);
+	struct dm_spi_slave_platdata *platdata = dev_get_parent_platdata(dev);
+
+	dm_gpio_set_dir_flags(&priv->gpios[platdata->cs], GPIOD_IS_OUT);
+	dm_gpio_set_value(&priv->gpios[platdata->cs], 1);
+}
+
+static int mpc8xxx_spi_xfer(struct udevice *dev, uint bitlen,
+			    const void *dout, void *din, ulong flags)
+{
+	int ret;
+	struct udevice *bus = dev->parent;
+	struct mpc8xxx_priv *priv = dev_get_priv(bus);
+	struct dm_spi_slave_platdata *platdata = dev_get_parent_platdata(dev);
+
+	debug("%s: slave %s:%u dout %08X din %08X bitlen %u\n", __func__,
+	      bus->name, platdata->cs, *(uint *)dout, *(uint *)din, bitlen);
+
+	if (flags & SPI_XFER_BEGIN)
+		mpc8xxx_spi_cs_activate(dev);
+
+	ret = __spi_xfer(priv->spi, bitlen, dout, din, flags);
+
+	if (flags & SPI_XFER_END)
+		mpc8xxx_spi_cs_deactivate(dev);
+
+	return ret;
+}
+
+static int mpc8xxx_spi_set_speed(struct udevice *dev, uint speed)
+{
+	struct mpc8xxx_priv *priv = dev_get_priv(dev);
+
+	return __spi_set_speed(priv->spi, speed);
+}
+
+static int mpc8xxx_spi_set_mode(struct udevice *dev, uint mode)
+{
+	/* TODO(mario.six at gdsys.cc): Using SPI_CPHA (for clock phase) and
+	 * SPI_CPOL (for clock polarity) should work
+	 */
+	return 0;
+}
+
+static const struct dm_spi_ops mpc8xxx_spi_ops = {
+	.xfer		= mpc8xxx_spi_xfer,
+	.set_speed	= mpc8xxx_spi_set_speed,
+	.set_mode	= mpc8xxx_spi_set_mode,
+	/*
+	 * cs_info is not needed, since we require all chip selects to be
+	 * in the device tree explicitly
+	 */
+};
+
+static const struct udevice_id mpc8xxx_spi_ids[] = {
+	{ .compatible = "fsl,spi" },
+	{ }
+};
+
+#else
+
 struct spi_slave *spi_setup_slave(uint bus, uint cs, uint max_hz, uint mode)
 {
 	struct spi_slave *slave;

@@ -221,3 +338,17 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din,
 
 	return ret;
 }
+
+#endif
+
+#ifdef CONFIG_DM_SPI
+U_BOOT_DRIVER(mpc8xxx_spi) = {
+	.name	= "mpc8xxx_spi",
+	.id	= UCLASS_SPI,
+	.of_match = mpc8xxx_spi_ids,
+	.ops	= &mpc8xxx_spi_ops,
+	.ofdata_to_platdata = mpc8xxx_spi_ofdata_to_platdata,
+	.probe	= mpc8xxx_spi_probe,
+	.priv_auto_alloc_size = sizeof(struct mpc8xxx_priv),
+};
+#endif

Signature · 1 line

2.11.0

Jagan Teki

2018-04-19 11:32 UTC #20 lore raw

PATCH 19/19 spi: mpc8xxx: Add DM support

Quoted 24 lines · Mario Six

Support DM for the MPC8XXX SPI driver.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 135 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 2 deletions(-)

drivers/spi/mpc8xxx_spi.c +5 -0

index 5724a00585..6e51826665 100644
@@ -10,6 +10,11 @@
 #include <malloc.h>
 #include <spi.h>
 #include <asm/mpc8xxx_spi.h>
+#ifdef CONFIG_DM_SPI
+#include <errno.h>
+#include <dm.h>
+#include <asm-generic/gpio.h>
+#endif
 
 enum {
        SPI_EV_NE = BIT(31 - 22),       /* Receiver Not Empty */

drivers/spi/mpc8xxx_spi.c enum { · line 39

Quoted 4 lines

drivers/spi/mpc8xxx_spi.c +1 -0

@@ -31,6 +36,14 @@ enum {
        SPI_COM_LST = BIT(31 - 9),
 };
 
+#ifdef CONFIG_DM_SPI

How about updating this into fully dm-driven with dt or platdata?

Mario Six

author

2018-04-19 11:45 UTC #21 lore raw

PATCH 19/19 spi: mpc8xxx: Add DM support

Hi Jagan,

Quoted 28 lines · Jagan Teki

Quoted 27 lines · Mario Six

Support DM for the MPC8XXX SPI driver.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 135 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 2 deletions(-)

drivers/spi/mpc8xxx_spi.c +5 -0

index 5724a00585..6e51826665 100644
@@ -10,6 +10,11 @@
 #include <malloc.h>
 #include <spi.h>
 #include <asm/mpc8xxx_spi.h>
+#ifdef CONFIG_DM_SPI
+#include <errno.h>
+#include <dm.h>
+#include <asm-generic/gpio.h>
+#endif
 
 enum {
        SPI_EV_NE = BIT(31 - 22),       /* Receiver Not Empty */

@@ -31,6 +36,14 @@ enum {
        SPI_COM_LST = BIT(31 - 9),
 };

Quoted 4 lines

drivers/spi/mpc8xxx_spi.c enum { · line 39
Quoted 1 line
drivers/spi/mpc8xxx_spi.c +1 -0
+#ifdef CONFIG_DM_SPI
How about updating this into fully dm-driven with dt or platdata?

The series does that, but also keeps the legacy interface to not break boards in the tree. Once the legacy SPI interface is removed, the compatibility layer can be removed, and what's left is a pure DM-driver.

Best regards,
Mario

Jagan Teki

2018-04-19 11:48 UTC #22 lore raw

PATCH 19/19 spi: mpc8xxx: Add DM support

Quoted 31 lines · Mario Six

Hi Jagan,

Quoted 28 lines · Jagan Teki

Quoted 27 lines · Mario Six

Support DM for the MPC8XXX SPI driver.

Signed-off-by Mario Six <[email protected]>

---
 drivers/spi/mpc8xxx_spi.c | 135 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 2 deletions(-)

drivers/spi/mpc8xxx_spi.c +5 -0

index 5724a00585..6e51826665 100644
@@ -10,6 +10,11 @@
 #include <malloc.h>
 #include <spi.h>
 #include <asm/mpc8xxx_spi.h>
+#ifdef CONFIG_DM_SPI
+#include <errno.h>
+#include <dm.h>
+#include <asm-generic/gpio.h>
+#endif
 
 enum {
        SPI_EV_NE = BIT(31 - 22),       /* Receiver Not Empty */

@@ -31,6 +36,14 @@ enum {
        SPI_COM_LST = BIT(31 - 9),
 };

Quoted 8 lines

Quoted 4 lines
drivers/spi/mpc8xxx_spi.c enum { · line 39
Quoted 1 line
drivers/spi/mpc8xxx_spi.c +1 -0
+#ifdef CONFIG_DM_SPI
How about updating this into fully dm-driven with dt or platdata?
The series does that, but also keeps the legacy interface to not break boards in the tree. Once the legacy SPI interface is removed, the compatibility layer can be removed, and what's left is a pure DM-driver.

True, but we are planning to move all by 2018.09 release, so better take care all at once. I did few drivers fully moved and asked board maintainers for move their boards try to do the same.

Jagan.

Mario Six

author

2018-04-19 11:51 UTC #23 lore raw

PATCH 19/19 spi: mpc8xxx: Add DM support

Quoted 37 lines · Jagan Teki

Quoted 36 lines · Mario Six
Hi Jagan,
Quoted 32 lines · Jagan Teki
Quoted 24 lines · Mario Six
Support DM for the MPC8XXX SPI driver.
Signed-off-by Mario Six <[email protected]>
---
 drivers/spi/mpc8xxx_spi.c | 135 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 2 deletions(-)
drivers/spi/mpc8xxx_spi.c +5 -0
index 5724a00585..6e51826665 100644
@@ -10,6 +10,11 @@
 #include <malloc.h>
 #include <spi.h>
 #include <asm/mpc8xxx_spi.h>
+#ifdef CONFIG_DM_SPI
+#include <errno.h>
+#include <dm.h>
+#include <asm-generic/gpio.h>
+#endif
 
 enum {
        SPI_EV_NE = BIT(31 - 22),       /* Receiver Not Empty */
drivers/spi/mpc8xxx_spi.c enum { · line 39
Quoted 4 lines
drivers/spi/mpc8xxx_spi.c +1 -0
@@ -31,6 +36,14 @@ enum {
        SPI_COM_LST = BIT(31 - 9),
 };
 
+#ifdef CONFIG_DM_SPI
How about updating this into fully dm-driven with dt or platdata?

Quoted 8 lines

Quoted 3 lines
The series does that, but also keeps the legacy interface to not break boards in the tree. Once the legacy SPI interface is removed, the compatibility layer can be removed, and what's left is a pure DM-driver.
True, but we are planning to move all by 2018.09 release, so better take care all at once. I did few drivers fully moved and asked board maintainers for move their boards try to do the same.

OK, I'll remove the compatibility layer in v2. Thanks for reviewing!

Quoted 2 lines

Jagan.

Best regards,
Mario