[U-Boot] [PATCH v2 00/18] spi: mpc8xxx: DM conversion
This is v2 of a patch series that 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 removed in this version, so old boards in the tree might end up with broken SPI functionality.
Mario Six (18): 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: Convert to DM
drivers/spi/mpc8xxx_spi.c | 279 +++++++++++++++++++++++++++++++--------------- 1 file changed, 188 insertions(+), 91 deletions(-)
-- 2.16.1
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 mario.six@gdsys.cc --- drivers/spi/mpc8xxx_spi.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) ---
Changes v1 -> v2: None
--- diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 00cbcbf9fc..2e1a6caf03 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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); -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 2e1a6caf03..2f6afaf7ac 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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);
/* -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 2f6afaf7ac..3ac9f2f8e8 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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) -- 2.16.1
Fix all "superfluous space after case" style errors.
Signed-off-by: Mario Six mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 3ac9f2f8e8..079476c3ee 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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; -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 079476c3ee..0b81db959e 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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) -- 2.16.1
Replace pre-processor defines with proper enums, and use the BIT macro where applicable.
Signed-off-by: Mario Six mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 0b81db959e..bd2857ce1a 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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
-- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index bd2857ce1a..b5546aa502 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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)); -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index b5546aa502..c4a9ef53ef 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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 */ -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index c4a9ef53ef..a69987ef54 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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) -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index a69987ef54..30249cce57 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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__); -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 30249cce57..5bf84aaec6 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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;
-- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 5bf84aaec6..af3762737f 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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; -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index af3762737f..ce2a77c1aa 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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; } -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index ce2a77c1aa..d22206f4b7 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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) /* at 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) { -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index d22206f4b7..fe493f6d40 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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);
-- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index fe493f6d40..b3ed00fcb7 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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; -- 2.16.1
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 mario.six@gdsys.cc ---
Changes v1 -> v2: None
--- drivers/spi/mpc8xxx_spi.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index b3ed00fcb7..4aa5db8821 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -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__);
-- 2.16.1
Support DM in the MPC8xxx SPI driver, and remove the legacy SPI interface.
Signed-off-by: Mario Six mario.six@gdsys.cc --- drivers/spi/mpc8xxx_spi.c | 144 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 107 insertions(+), 37 deletions(-) ---
Changes v1 -> v2: * Removed legacy layer * Sorted includes * Now return -ETIMEDOUT on timeout during transfer
---
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 4aa5db8821..9c9dd67ec2 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -6,10 +6,12 @@ */
#include <common.h> - +#include <dm.h> +#include <errno.h> #include <malloc.h> #include <spi.h> #include <asm/mpc8xxx_spi.h> +#include <asm-generic/gpio.h>
enum { SPI_EV_NE = BIT(31 - 22), /* Receiver Not Empty */ @@ -31,6 +33,12 @@ enum { SPI_COM_LST = BIT(31 - 9), };
+struct mpc8xxx_priv { + spi8xxx_t *spi; + struct gpio_desc gpios[16]; + int max_cs; +}; + static inline u32 to_prescale_mod(u32 val) { return (min(val, (u32)15) << 16); @@ -43,70 +51,90 @@ 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 int __spi_set_speed(spi8xxx_t *spi, uint speed) { - struct spi_slave *slave; + /* TODO(mario.six@gdsys.cc): This only ever sets one fixed speed */
- 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. - */ + /* Use SYSCLK / 8 (16.67MHz typ.) */ + clrsetbits_be32(&spi->mode, SPI_MODE_PM_MASK, to_prescale_mod(1));
- return slave; + return 0; }
-void spi_free_slave(struct spi_slave *slave) +static int mpc8xxx_spi_ofdata_to_platdata(struct udevice *dev) { - free(slave); + struct mpc8xxx_priv *priv = dev_get_priv(dev); + int ret; + + priv->spi = (spi8xxx_t *)dev_read_addr(dev); + + /* TODO(mario.six@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; }
-void spi_init(void) +static int mpc8xxx_spi_probe(struct udevice *dev) { - spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi; + struct mpc8xxx_priv *priv = dev_get_priv(dev);
/* * 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)); + out_be32(&priv->spi->mode, SPI_MODE_REV | SPI_MODE_MS | SPI_MODE_EN); + + __spi_set_speed(priv->spi, 16666667); + /* Clear all SPI events */ - setbits_be32(&spi->event, 0xffffffff); + setbits_be32(&priv->spi->event, 0xffffffff); /* Mask all SPI interrupts */ - clrbits_be32(&spi->mask, 0xffffffff); + clrbits_be32(&priv->spi->mask, 0xffffffff); /* LST bit doesn't do anything, so disregard */ - out_be32(&spi->com, 0); + out_be32(&priv->spi->com, 0); + + return 0; }
-int spi_claim_bus(struct spi_slave *slave) +static void mpc8xxx_spi_cs_activate(struct udevice *dev) { - return 0; + 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); }
-void spi_release_bus(struct spi_slave *slave) +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); }
-int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din, - ulong flags) +static int mpc8xxx_spi_xfer(struct udevice *dev, uint bitlen, + const void *dout, void *din, ulong flags) { - spi8xxx_t *spi = &((immap_t *)(CONFIG_SYS_IMMR))->spi; - u32 tmpdin; + struct udevice *bus = dev->parent; + struct mpc8xxx_priv *priv = dev_get_priv(bus); + spi8xxx_t *spi = priv->spi; + struct dm_spi_slave_platdata *platdata = dev_get_parent_platdata(dev); + u32 tmpdin = 0; 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); + 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) - spi_cs_activate(slave); + mpc8xxx_spi_cs_activate(dev);
/* Clear all SPI events */ setbits_be32(&spi->event, 0xffffffff); @@ -179,15 +207,57 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, void *din, mdelay(1); } while (get_timer(start) < SPI_TIMEOUT);
- if (get_timer(start) >= SPI_TIMEOUT) + if (get_timer(start) >= SPI_TIMEOUT) { debug("*** %s: Time out during SPI transfer\n", __func__); + return -ETIMEDOUT; + }
debug("*** %s: transfer ended. Value=%08x\n", __func__, tmpdin); }
if (flags & SPI_XFER_END) - spi_cs_deactivate(slave); + mpc8xxx_spi_cs_deactivate(dev); + + return 0; +} + +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@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" }, + { } +}; + +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), +}; -- 2.16.1
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
Support DM in the MPC8xxx SPI driver, and remove the legacy SPI interface.
Signed-off-by: Mario Six mario.six@gdsys.cc
drivers/spi/mpc8xxx_spi.c | 144 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 107 insertions(+), 37 deletions(-)
Changes v1 -> v2:
- Removed legacy layer
- Sorted includes
- Now return -ETIMEDOUT on timeout during transfer
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 4aa5db8821..9c9dd67ec2 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -6,10 +6,12 @@ */
#include <common.h>
+#include <dm.h> +#include <errno.h> #include <malloc.h> #include <spi.h> #include <asm/mpc8xxx_spi.h> +#include <asm-generic/gpio.h>
[snip]
+};
+static const struct udevice_id mpc8xxx_spi_ids[] = {
{ .compatible = "fsl,spi" },
Look like new binding right? don't we have suitable binding for this driver? or what is the equivalent driver for this in Linux?
Jagan.
Hi Jagan,
On Thu, Apr 26, 2018 at 7:34 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
Support DM in the MPC8xxx SPI driver, and remove the legacy SPI interface.
Signed-off-by: Mario Six mario.six@gdsys.cc
drivers/spi/mpc8xxx_spi.c | 144 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 107 insertions(+), 37 deletions(-)
Changes v1 -> v2:
- Removed legacy layer
- Sorted includes
- Now return -ETIMEDOUT on timeout during transfer
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 4aa5db8821..9c9dd67ec2 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -6,10 +6,12 @@ */
#include <common.h>
+#include <dm.h> +#include <errno.h> #include <malloc.h> #include <spi.h> #include <asm/mpc8xxx_spi.h> +#include <asm-generic/gpio.h>
[snip]
+};
+static const struct udevice_id mpc8xxx_spi_ids[] = {
{ .compatible = "fsl,spi" },Look like new binding right? don't we have suitable binding for this driver? or what is the equivalent driver for this in Linux?
I took that compatible string from the Linux kernel, actually. See for example
https://raw.githubusercontent.com/torvalds/linux/master/arch/powerpc/boot/dt...
There are more specific ones, like "fsl,mpc8610-spi", but "fsl,spi" is the most generic.
Jagan.
Best regards,
Mario
On Thu, Apr 26, 2018 at 11:13 AM, Mario Six mario.six@gdsys.cc wrote:
Hi Jagan,
On Thu, Apr 26, 2018 at 7:34 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
Support DM in the MPC8xxx SPI driver, and remove the legacy SPI interface.
Signed-off-by: Mario Six mario.six@gdsys.cc
drivers/spi/mpc8xxx_spi.c | 144 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 107 insertions(+), 37 deletions(-)
Changes v1 -> v2:
- Removed legacy layer
- Sorted includes
- Now return -ETIMEDOUT on timeout during transfer
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 4aa5db8821..9c9dd67ec2 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -6,10 +6,12 @@ */
#include <common.h>
+#include <dm.h> +#include <errno.h> #include <malloc.h> #include <spi.h> #include <asm/mpc8xxx_spi.h> +#include <asm-generic/gpio.h>
[snip]
+};
+static const struct udevice_id mpc8xxx_spi_ids[] = {
{ .compatible = "fsl,spi" },Look like new binding right? don't we have suitable binding for this driver? or what is the equivalent driver for this in Linux?
I took that compatible string from the Linux kernel, actually. See for example
https://raw.githubusercontent.com/torvalds/linux/master/arch/powerpc/boot/dt...
There are more specific ones, like "fsl,mpc8610-spi", but "fsl,spi" is the most generic.
So this driver is same as drivers/spi/spi-fsl-spi.c from Linux, is it?
Hi Jagan,
On Thu, Apr 26, 2018 at 7:49 AM, Jagan Teki jagan@amarulasolutions.com wrote:
On Thu, Apr 26, 2018 at 11:13 AM, Mario Six mario.six@gdsys.cc wrote:
Hi Jagan,
On Thu, Apr 26, 2018 at 7:34 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
Support DM in the MPC8xxx SPI driver, and remove the legacy SPI interface.
Signed-off-by: Mario Six mario.six@gdsys.cc
drivers/spi/mpc8xxx_spi.c | 144 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 107 insertions(+), 37 deletions(-)
Changes v1 -> v2:
- Removed legacy layer
- Sorted includes
- Now return -ETIMEDOUT on timeout during transfer
diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c index 4aa5db8821..9c9dd67ec2 100644 --- a/drivers/spi/mpc8xxx_spi.c +++ b/drivers/spi/mpc8xxx_spi.c @@ -6,10 +6,12 @@ */
#include <common.h>
+#include <dm.h> +#include <errno.h> #include <malloc.h> #include <spi.h> #include <asm/mpc8xxx_spi.h> +#include <asm-generic/gpio.h>
[snip]
+};
+static const struct udevice_id mpc8xxx_spi_ids[] = {
{ .compatible = "fsl,spi" },Look like new binding right? don't we have suitable binding for this driver? or what is the equivalent driver for this in Linux?
I took that compatible string from the Linux kernel, actually. See for example
https://raw.githubusercontent.com/torvalds/linux/master/arch/powerpc/boot/dt...
There are more specific ones, like "fsl,mpc8610-spi", but "fsl,spi" is the most generic.
So this driver is same as drivers/spi/spi-fsl-spi.c from Linux, is it?
Yes, with reduced functionality, of course, but compatible to the same devices.
Best regards, Mario
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
This is v2 of a patch series that 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 removed in this version, so old boards in the tree might end up with broken SPI functionality.
Mario Six (18): 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: Convert to DM
Boards with - configs/MPC8349EMDS_defconfig - configs/ids8313_defconfig
are using this driver, so Kim, Heiko please convert enable DM_SPI for the same.
Use below tree for respective changes and update on top of this. http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next
Jagan.
Hi Jagan,
On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
This is v2 of a patch series that 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 removed in this version, so old boards in the tree might end up with broken SPI functionality.
Mario Six (18): 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: Convert to DM
Boards with
- configs/MPC8349EMDS_defconfig
- configs/ids8313_defconfig
are using this driver, so Kim, Heiko please convert enable DM_SPI for the same.
Use below tree for respective changes and update on top of this. http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next
I have a few series in the making that will enable DM on the MPC83xx platform (I'm doing a respin on the first right now). If there is still interests in the boards, I could push it to the MPC83xx repository (but mind that the work required per board is quite extensive).
Also, MPC8349EMDS is de facto abandoned, and I don't have access to the hardware, so I can't really maintain it.
Jagan.
Best regards, Mario
On Thu, Apr 26, 2018 at 11:24 AM, Mario Six mario.six@gdsys.cc wrote:
Hi Jagan,
On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
This is v2 of a patch series that 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 removed in this version, so old boards in the tree might end up with broken SPI functionality.
Mario Six (18): 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: Convert to DM
Boards with
- configs/MPC8349EMDS_defconfig
- configs/ids8313_defconfig
are using this driver, so Kim, Heiko please convert enable DM_SPI for the same.
Use below tree for respective changes and update on top of this. http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next
I have a few series in the making that will enable DM on the MPC83xx platform (I'm doing a respin on the first right now). If there is still interests in the boards, I could push it to the MPC83xx repository (but mind that the work required per board is quite extensive).
Also, MPC8349EMDS is de facto abandoned, and I don't have access to the hardware, so I can't really maintain it.
It's up to you, look like this board maintained by Kim is not available with freescale e-mail (or may be changed) if none can't maintain, it better to drop the board.
Jagan.
On Thu, 2018-04-26 at 11:35 +0530, Jagan Teki wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Thu, Apr 26, 2018 at 11:24 AM, Mario Six mario.six@gdsys.cc wrote:
Hi Jagan,
On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
This is v2 of a patch series that 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 removed in this version, so old boards in the tree might end up with broken SPI functionality.
Mario Six (18): 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: Convert to DM
Boards with
- configs/MPC8349EMDS_defconfig
- configs/ids8313_defconfig
are using this driver, so Kim, Heiko please convert enable DM_SPI for the same.
Use below tree for respective changes and update on top of this. http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next
I have a few series in the making that will enable DM on the MPC83xx platform (I'm doing a respin on the first right now). If there is still interests in the boards, I could push it to the MPC83xx repository (but mind that the work required per board is quite extensive).
Also, MPC8349EMDS is de facto abandoned, and I don't have access to the hardware, so I can't really maintain it.
It's up to you, look like this board maintained by Kim is not available with freescale e-mail (or may be changed) if none can't maintain, it better to drop the board.
we use custom 832x boards so please don't remove 83xx from u-boot.
Jocke
Hi Joakim,
On Thu, Apr 26, 2018 at 10:23 AM, Joakim Tjernlund Joakim.Tjernlund@infinera.com wrote:
On Thu, 2018-04-26 at 11:35 +0530, Jagan Teki wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Thu, Apr 26, 2018 at 11:24 AM, Mario Six mario.six@gdsys.cc wrote:
Hi Jagan,
On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
This is v2 of a patch series that 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 removed in this version, so old boards in the tree might end up with broken SPI functionality.
Mario Six (18): 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: Convert to DM
Boards with
- configs/MPC8349EMDS_defconfig
- configs/ids8313_defconfig
are using this driver, so Kim, Heiko please convert enable DM_SPI for the same.
Use below tree for respective changes and update on top of this. http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next
I have a few series in the making that will enable DM on the MPC83xx platform (I'm doing a respin on the first right now). If there is still interests in the boards, I could push it to the MPC83xx repository (but mind that the work required per board is quite extensive).
Also, MPC8349EMDS is de facto abandoned, and I don't have access to the hardware, so I can't really maintain it.
It's up to you, look like this board maintained by Kim is not available with freescale e-mail (or may be changed) if none can't maintain, it better to drop the board.
we use custom 832x boards so please don't remove 83xx from u-boot.
I'm not planning to do that; on the contrary: I'm trying to update the platform to fully support DM (I hope to get a fully converted board in after the next release).
The problem is that we only use MPC8308 SoCs, so I can only vouche for the correctness of that specific SoC. Everything else is a bit up in the air, since I'm changing code blindly pretty much.
Best regards, Mario
Hello Mario,
Le 26/04/2018 à 10:36, Mario Six a écrit :
Hi Joakim,
On Thu, Apr 26, 2018 at 10:23 AM, Joakim Tjernlund Joakim.Tjernlund@infinera.com wrote:
On Thu, 2018-04-26 at 11:35 +0530, Jagan Teki wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Thu, Apr 26, 2018 at 11:24 AM, Mario Six mario.six@gdsys.cc wrote:
Hi Jagan,
On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote:
This is v2 of a patch series that 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 removed in this version, so old boards in the tree might end up with broken SPI functionality.
Mario Six (18): 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: Convert to DM
Boards with
- configs/MPC8349EMDS_defconfig
- configs/ids8313_defconfig
are using this driver, so Kim, Heiko please convert enable DM_SPI for the same.
Use below tree for respective changes and update on top of this. http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next
I have a few series in the making that will enable DM on the MPC83xx platform (I'm doing a respin on the first right now). If there is still interests in the boards, I could push it to the MPC83xx repository (but mind that the work required per board is quite extensive).
Also, MPC8349EMDS is de facto abandoned, and I don't have access to the hardware, so I can't really maintain it.
It's up to you, look like this board maintained by Kim is not available with freescale e-mail (or may be changed) if none can't maintain, it better to drop the board.
we use custom 832x boards so please don't remove 83xx from u-boot.
I'm not planning to do that; on the contrary: I'm trying to update the platform to fully support DM (I hope to get a fully converted board in after the next release).
The problem is that we only use MPC8308 SoCs, so I can only vouche for the correctness of that specific SoC. Everything else is a bit up in the air, since I'm changing code blindly pretty much.
I have a MPC8321 board so I may test it on it if it helps.
In the meantime, I was thinking about using your converted driver and see if I can adapt it to support MPC8xx as well, instead of converting the mpc8xx_spi driver to DM, however I've not been able to find your patches in the master tree allthough they are flagged as accepted in patchwork.
Are they on another branch somewhere ?
Thanks Christophe
Best regards, Mario _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hi Christophe,
On Fri, Aug 10, 2018 at 9:35 AM Christophe LEROY christophe.leroy@c-s.fr wrote:
Hello Mario,
Le 26/04/2018 à 10:36, Mario Six a écrit :
Hi Joakim,
On Thu, Apr 26, 2018 at 10:23 AM, Joakim Tjernlund Joakim.Tjernlund@infinera.com wrote:
On Thu, 2018-04-26 at 11:35 +0530, Jagan Teki wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Thu, Apr 26, 2018 at 11:24 AM, Mario Six mario.six@gdsys.cc wrote:
Hi Jagan,
On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote: > This is v2 of a patch series that 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 removed in this version, so old boards in > the tree might end up with broken SPI functionality. > > Mario Six (18): > 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: Convert to DM
Boards with
- configs/MPC8349EMDS_defconfig
- configs/ids8313_defconfig
are using this driver, so Kim, Heiko please convert enable DM_SPI for the same.
Use below tree for respective changes and update on top of this. http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next
I have a few series in the making that will enable DM on the MPC83xx platform (I'm doing a respin on the first right now). If there is still interests in the boards, I could push it to the MPC83xx repository (but mind that the work required per board is quite extensive).
Also, MPC8349EMDS is de facto abandoned, and I don't have access to the hardware, so I can't really maintain it.
It's up to you, look like this board maintained by Kim is not available with freescale e-mail (or may be changed) if none can't maintain, it better to drop the board.
we use custom 832x boards so please don't remove 83xx from u-boot.
I'm not planning to do that; on the contrary: I'm trying to update the platform to fully support DM (I hope to get a fully converted board in after the next release).
The problem is that we only use MPC8308 SoCs, so I can only vouche for the correctness of that specific SoC. Everything else is a bit up in the air, since I'm changing code blindly pretty much.
I have a MPC8321 board so I may test it on it if it helps.
That would be very much appreciated! Thanks.
In the meantime, I was thinking about using your converted driver and see if I can adapt it to support MPC8xx as well, instead of converting the mpc8xx_spi driver to DM, however I've not been able to find your patches in the master tree allthough they are flagged as accepted in patchwork.
Are they on another branch somewhere ?
Hmm, indeed. I thought they would be in the SPI custodian repository, but apparently they are not?
@Jagan: Were those patches forgotten somehow?
Thanks Christophe
Best regards, Mario
On Fri, Aug 10, 2018 at 1:27 PM, Mario Six mario.six@gdsys.cc wrote:
Hi Christophe,
On Fri, Aug 10, 2018 at 9:35 AM Christophe LEROY christophe.leroy@c-s.fr wrote:
Hello Mario,
Le 26/04/2018 à 10:36, Mario Six a écrit :
Hi Joakim,
On Thu, Apr 26, 2018 at 10:23 AM, Joakim Tjernlund Joakim.Tjernlund@infinera.com wrote:
On Thu, 2018-04-26 at 11:35 +0530, Jagan Teki wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Thu, Apr 26, 2018 at 11:24 AM, Mario Six mario.six@gdsys.cc wrote:
Hi Jagan,
On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki jagannadh.teki@gmail.com wrote: > On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote: >> This is v2 of a patch series that 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 removed in this version, so old boards in >> the tree might end up with broken SPI functionality. >> >> Mario Six (18): >> 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: Convert to DM > > Boards with > - configs/MPC8349EMDS_defconfig > - configs/ids8313_defconfig > > are using this driver, so Kim, Heiko please convert enable DM_SPI for the same. > > Use below tree for respective changes and update on top of this. > http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next >
I have a few series in the making that will enable DM on the MPC83xx platform (I'm doing a respin on the first right now). If there is still interests in the boards, I could push it to the MPC83xx repository (but mind that the work required per board is quite extensive).
Also, MPC8349EMDS is de facto abandoned, and I don't have access to the hardware, so I can't really maintain it.
It's up to you, look like this board maintained by Kim is not available with freescale e-mail (or may be changed) if none can't maintain, it better to drop the board.
we use custom 832x boards so please don't remove 83xx from u-boot.
I'm not planning to do that; on the contrary: I'm trying to update the platform to fully support DM (I hope to get a fully converted board in after the next release).
The problem is that we only use MPC8308 SoCs, so I can only vouche for the correctness of that specific SoC. Everything else is a bit up in the air, since I'm changing code blindly pretty much.
I have a MPC8321 board so I may test it on it if it helps.
That would be very much appreciated! Thanks.
In the meantime, I was thinking about using your converted driver and see if I can adapt it to support MPC8xx as well, instead of converting the mpc8xx_spi driver to DM, however I've not been able to find your patches in the master tree allthough they are flagged as accepted in patchwork.
Are they on another branch somewhere ?
Hmm, indeed. I thought they would be in the SPI custodian repository, but apparently they are not?
@Jagan: Were those patches forgotten somehow?
http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/spi-dm-migrate
If all the boards which are using this driver enabled DM_SPI, then I will pick the same.
Hi Mario,
Le 10/08/2018 à 09:57, Mario Six a écrit :
Hi Christophe,
On Fri, Aug 10, 2018 at 9:35 AM Christophe LEROY christophe.leroy@c-s.fr wrote:
Hello Mario,
Le 26/04/2018 à 10:36, Mario Six a écrit :
Hi Joakim,
On Thu, Apr 26, 2018 at 10:23 AM, Joakim Tjernlund Joakim.Tjernlund@infinera.com wrote:
On Thu, 2018-04-26 at 11:35 +0530, Jagan Teki wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Thu, Apr 26, 2018 at 11:24 AM, Mario Six mario.six@gdsys.cc wrote:
Hi Jagan,
On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki jagannadh.teki@gmail.com wrote: > On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc wrote: >> This is v2 of a patch series that 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 removed in this version, so old boards in >> the tree might end up with broken SPI functionality. >> >> Mario Six (18): >> 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: Convert to DM > > Boards with > - configs/MPC8349EMDS_defconfig > - configs/ids8313_defconfig > > are using this driver, so Kim, Heiko please convert enable DM_SPI for the same. > > Use below tree for respective changes and update on top of this. > http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/next >
I have a few series in the making that will enable DM on the MPC83xx platform (I'm doing a respin on the first right now). If there is still interests in the boards, I could push it to the MPC83xx repository (but mind that the work required per board is quite extensive).
Also, MPC8349EMDS is de facto abandoned, and I don't have access to the hardware, so I can't really maintain it.
It's up to you, look like this board maintained by Kim is not available with freescale e-mail (or may be changed) if none can't maintain, it better to drop the board.
we use custom 832x boards so please don't remove 83xx from u-boot.
I'm not planning to do that; on the contrary: I'm trying to update the platform to fully support DM (I hope to get a fully converted board in after the next release).
The problem is that we only use MPC8308 SoCs, so I can only vouche for the correctness of that specific SoC. Everything else is a bit up in the air, since I'm changing code blindly pretty much.
I have a MPC8321 board so I may test it on it if it helps.
That would be very much appreciated! Thanks.
Indeed, your driver implements SPI in CPU Mode AFAIU
In the MPC8321 Reference Manual this state that 'SPI in CPU Mode applies to MPC8360E and MPC8568E only'.
And as the driver doesn't implement QUICC Engine Mode, I won't use it for the 8xx yet. I'll port the mpc8xx driver to DM and see later if we can implement QE Mode in mpc8xxx and merge it with the mpc8xx once we have generic GPIOs on the 8xx.
Christophe
In the meantime, I was thinking about using your converted driver and see if I can adapt it to support MPC8xx as well, instead of converting the mpc8xx_spi driver to DM, however I've not been able to find your patches in the master tree allthough they are flagged as accepted in patchwork.
Are they on another branch somewhere ?
Hmm, indeed. I thought they would be in the SPI custodian repository, but apparently they are not?
@Jagan: Were those patches forgotten somehow?
Thanks Christophe
Best regards, Mario
Hi Mario/Kim/Heiko,
On Tue, Aug 21, 2018 at 8:42 PM, Christophe LEROY christophe.leroy@c-s.fr wrote:
Hi Mario,
Le 10/08/2018 à 09:57, Mario Six a écrit :
Hi Christophe,
On Fri, Aug 10, 2018 at 9:35 AM Christophe LEROY christophe.leroy@c-s.fr wrote:
Hello Mario,
Le 26/04/2018 à 10:36, Mario Six a écrit :
Hi Joakim,
On Thu, Apr 26, 2018 at 10:23 AM, Joakim Tjernlund Joakim.Tjernlund@infinera.com wrote:
On Thu, 2018-04-26 at 11:35 +0530, Jagan Teki wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
On Thu, Apr 26, 2018 at 11:24 AM, Mario Six mario.six@gdsys.cc wrote: > > Hi Jagan, > > On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki > jagannadh.teki@gmail.com wrote: >> >> On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc >> wrote: >>> >>> This is v2 of a patch series that 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 removed in this version, so old boards >>> in >>> the tree might end up with broken SPI functionality. >>> >>> Mario Six (18): >>> 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: Convert to DM >> >> >> Boards with >> - configs/MPC8349EMDS_defconfig >> - configs/ids8313_defconfig
Can you convert this boards to use DM_SPI, we have build issue[2]
[2] https://travis-ci.org/openedev/u-boot-amarula/jobs/412956049
Hi Jagan, Heiko,
On Tue, Sep 4, 2018 at 8:10 AM Jagan Teki jagan@amarulasolutions.com wrote:
Hi Mario/Kim/Heiko,
On Tue, Aug 21, 2018 at 8:42 PM, Christophe LEROY christophe.leroy@c-s.fr wrote:
Hi Mario,
Le 10/08/2018 à 09:57, Mario Six a écrit :
Hi Christophe,
On Fri, Aug 10, 2018 at 9:35 AM Christophe LEROY christophe.leroy@c-s.fr wrote:
Hello Mario,
Le 26/04/2018 à 10:36, Mario Six a écrit :
Hi Joakim,
On Thu, Apr 26, 2018 at 10:23 AM, Joakim Tjernlund Joakim.Tjernlund@infinera.com wrote:
On Thu, 2018-04-26 at 11:35 +0530, Jagan Teki wrote: > > On Thu, Apr 26, 2018 at 11:24 AM, Mario Six mario.six@gdsys.cc > wrote: >> >> Hi Jagan, >> >> On Thu, Apr 26, 2018 at 7:30 AM, Jagan Teki >> jagannadh.teki@gmail.com wrote: >>> >>> On Thu, Apr 19, 2018 at 6:06 PM, Mario Six mario.six@gdsys.cc >>> wrote: >>>> >>>> This is v2 of a patch series that 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 removed in this version, so old boards >>>> in >>>> the tree might end up with broken SPI functionality. >>>> >>>> Mario Six (18): >>>> 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: Convert to DM >>> >>> >>> Boards with >>> - configs/MPC8349EMDS_defconfig >>> - configs/ids8313_defconfig
Can you convert this boards to use DM_SPI, we have build issue[2]
Heiko, if you agree, I'd say we disable the SPI support for this board for now (and possibly permanently), since really supporting the SPI multiplexer would entail writing a DM driver for it, which would entail creating a SPI mux uclass, and similar inconveniences (and I think putting that much work in a board that old is at least dubious). As far as I can tell, this is the only board with the multiplexer mechanism.
I have two more larger conversion series in my queue, which will hopefully make it easier to convert the MPC83xx boards to DM (including SPI), but since I'm busy working on non-U-Boot-related things right now, I won't be able to finalize the first this week; hopefully next week.
[2] https://travis-ci.org/openedev/u-boot-amarula/jobs/412956049
Best regards, Mario
participants (5)
-
Christophe LEROY -
Jagan Teki -
Jagan Teki -
Joakim Tjernlund -
Mario Six