| Message ID | 1336584856-30882-1-git-send-email-twarren@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 2012-05-09 at 10:34 -0700, Tom Warren wrote: > Simon Glass's proposal to fix this on Seaboard was NAK'd, so I > removed his NS16550 references and added a small delay before > SPI/UART muxing. Tested on my Seaboard with large SPI reads/writes > and saw no corruption (crc's matched) and no spurious comm chars. > > Signed-off-by: Tom Warren <twarren@nvidia.com> Tested-by: Jimmy Zhang <jimmzhang@nvidia.com> ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
Hi Tom, On Fri, May 11, 2012 at 1:14 PM, jimmzhang <jimmzhang@nvidia.com> wrote: > On Wed, 2012-05-09 at 10:34 -0700, Tom Warren wrote: > > Simon Glass's proposal to fix this on Seaboard was NAK'd, so I > > removed his NS16550 references and added a small delay before > > SPI/UART muxing. Tested on my Seaboard with large SPI reads/writes > > and saw no corruption (crc's matched) and no spurious comm chars.\ > I'm afraid this version does not work fully for me. The problem is I think that the UART gets zero bytes in it from when the SPI was active. This causes the next command to be ignored. So for example: Tegra2 (SeaBoard) # echo fred fred Tegra2 (SeaBoard) # echo edmund edmund Tegra2 (SeaBoard) # saveenv Saving Environment to SPI Flash... Erasing SPI flash...Writing to SPI flash...done Tegra2 (SeaBoard) # echo blackadder ^^^^ this command does nothing! Tegra2 (SeaBoard) # echo blackadder blackadder Tegra2 (SeaBoard) # This is the reason why my original patch cleaned out the UART before reading further characters. If that logic really in impossible to have in U-Boot then the only option is (after a SPI operation) to read UART output using tstc() and getc() until there is nothing more. Ick. That said, this is an improvement, since it allows SPI to work. So I am going to ack it so we can hopefully get it in there. Acked-by: Simon Glass <sjg@chromium.org> > > > > Signed-off-by: Tom Warren <twarren@nvidia.com> > > Tested-by: Jimmy Zhang <jimmzhang@nvidia.com> > > > > > ----------------------------------------------------------------------------------- > This email message is for the sole use of the intended recipient(s) and > may contain > confidential information. Any unauthorized review, use, disclosure or > distribution > is prohibited. If you are not the intended recipient, please contact the > sender by > reply email and destroy all copies of the original message. > > ----------------------------------------------------------------------------------- > Regards, Simon
Simon, On Fri, May 11, 2012 at 5:16 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Tom, > > On Fri, May 11, 2012 at 1:14 PM, jimmzhang <jimmzhang@nvidia.com> wrote: >> >> On Wed, 2012-05-09 at 10:34 -0700, Tom Warren wrote: >> > Simon Glass's proposal to fix this on Seaboard was NAK'd, so I >> > removed his NS16550 references and added a small delay before >> > SPI/UART muxing. Tested on my Seaboard with large SPI reads/writes >> > and saw no corruption (crc's matched) and no spurious comm chars.\ > > > I'm afraid this version does not work fully for me. The problem is I think > that the UART gets zero bytes in it from when the SPI was active. This > causes the next command to be ignored. So for example: > > Tegra2 (SeaBoard) # echo fred > fred > Tegra2 (SeaBoard) # echo edmund > edmund > Tegra2 (SeaBoard) # saveenv > Saving Environment to SPI Flash... > Erasing SPI flash...Writing to SPI flash...done > Tegra2 (SeaBoard) # echo blackadder > > ^^^^ this command does nothing! > > Tegra2 (SeaBoard) # echo blackadder > blackadder > Tegra2 (SeaBoard) # > > This is the reason why my original patch cleaned out the UART before reading > further characters. If that logic really in impossible to have in U-Boot > then the only option is (after a SPI operation) to read UART output using > tstc() and getc() until there is nothing more. Ick. I saw the same thing, and debugged it down to a NUL in the input buffer. Not sure how it gets there - some combo of GPIO toggle, pinmux switch, etc. I have a patch to fix it by dropping the NUL in common/main.c's input processing switch(), but I haven't posted it because I don't think it'll pass muster, being in common code and all. I can do some more debugging later to see if moving/changing the sequence of GPIO/pinmux toggling and the delays around can eliminate the NUL, but it's a low priority. I may also post the main.c NUL drop just so anyone with a Seaboard && SPI build can apply it to their private build if they want and fix the problem. As it stands, though, this patch does get SPI working on Seaboard, which is the only T20 board with this (really) poor UART/SPI design, so I'll add it to the next pull request to Albert. Thanks, Tom > > That said, this is an improvement, since it allows SPI to work. So I am > going to ack it so we can hopefully get it in there. > > Acked-by: Simon Glass <sjg@chromium.org> > > >> >> > >> > Signed-off-by: Tom Warren <twarren@nvidia.com> >> >> Tested-by: Jimmy Zhang <jimmzhang@nvidia.com> >> >> >> >> >> ----------------------------------------------------------------------------------- >> This email message is for the sole use of the intended recipient(s) and >> may contain >> confidential information. Any unauthorized review, use, disclosure or >> distribution >> is prohibited. If you are not the intended recipient, please contact the >> sender by >> reply email and destroy all copies of the original message. >> >> ----------------------------------------------------------------------------------- > > > Regards, > Simon >
Hi Tom, On Mon, May 14, 2012 at 10:26 AM, Tom Warren <twarren.nvidia@gmail.com>wrote: > Simon, > > On Fri, May 11, 2012 at 5:16 PM, Simon Glass <sjg@chromium.org> wrote: > > Hi Tom, > > > > On Fri, May 11, 2012 at 1:14 PM, jimmzhang <jimmzhang@nvidia.com> wrote: > >> > >> On Wed, 2012-05-09 at 10:34 -0700, Tom Warren wrote: > >> > Simon Glass's proposal to fix this on Seaboard was NAK'd, so I > >> > removed his NS16550 references and added a small delay before > >> > SPI/UART muxing. Tested on my Seaboard with large SPI reads/writes > >> > and saw no corruption (crc's matched) and no spurious comm chars.\ > > > > > > I'm afraid this version does not work fully for me. The problem is I > think > > that the UART gets zero bytes in it from when the SPI was active. This > > causes the next command to be ignored. So for example: > > > > Tegra2 (SeaBoard) # echo fred > > fred > > Tegra2 (SeaBoard) # echo edmund > > edmund > > Tegra2 (SeaBoard) # saveenv > > Saving Environment to SPI Flash... > > Erasing SPI flash...Writing to SPI flash...done > > Tegra2 (SeaBoard) # echo blackadder > > > > ^^^^ this command does nothing! > > > > Tegra2 (SeaBoard) # echo blackadder > > blackadder > > Tegra2 (SeaBoard) # > > > > This is the reason why my original patch cleaned out the UART before > reading > > further characters. If that logic really in impossible to have in U-Boot > > then the only option is (after a SPI operation) to read UART output using > > tstc() and getc() until there is nothing more. Ick. > I saw the same thing, and debugged it down to a NUL in the input > buffer. Not sure how it gets there - some combo of GPIO toggle, pinmux > switch, etc. > I have a patch to fix it by dropping the NUL in common/main.c's input > processing switch(), but I haven't posted it because I don't think > it'll pass muster, being in common code and all. > > I can do some more debugging later to see if moving/changing the > sequence of GPIO/pinmux toggling and the delays around can eliminate > the NUL, but it's a low priority. I may also post the main.c NUL drop > just so anyone with a Seaboard && SPI build can apply it to their > private build if they want and fix the problem. > > As it stands, though, this patch does get SPI working on Seaboard, > which is the only T20 board with this (really) poor UART/SPI design, > so I'll add it to the next pull request to Albert. > > Yes, this is probably the best we can do without touching the uart code. Regards, Simon > Thanks, > > Tom > > > > That said, this is an improvement, since it allows SPI to work. So I am > > going to ack it so we can hopefully get it in there. > > > > Acked-by: Simon Glass <sjg@chromium.org> > > > > > >> > >> > > >> > Signed-off-by: Tom Warren <twarren@nvidia.com> > >> > >> Tested-by: Jimmy Zhang <jimmzhang@nvidia.com> > >> > >> > >> > >> > >> > ----------------------------------------------------------------------------------- > >> This email message is for the sole use of the intended recipient(s) and > >> may contain > >> confidential information. Any unauthorized review, use, disclosure or > >> distribution > >> is prohibited. If you are not the intended recipient, please contact > the > >> sender by > >> reply email and destroy all copies of the original message. > >> > >> > ----------------------------------------------------------------------------------- > > > > > > Regards, > > Simon > > >
diff --git a/arch/arm/include/asm/arch-tegra2/uart-spi-switch.h b/arch/arm/include/asm/arch-tegra2/uart-spi-switch.h index e4503b1..82ac180 100644 --- a/arch/arm/include/asm/arch-tegra2/uart-spi-switch.h +++ b/arch/arm/include/asm/arch-tegra2/uart-spi-switch.h @@ -29,7 +29,7 @@ * time! If the board file provides this, the board config will declare it. * Let this be a lesson for others. */ -void pinmux_select_uart(NS16550_t regs); +void pinmux_select_uart(void); /* * Signal that we are about the use the SPI bus. @@ -38,7 +38,7 @@ void pinmux_select_spi(void); #else /* not CONFIG_SPI_UART_SWITCH */ -static inline void pinmux_select_uart(NS16550_t regs) {} +static inline void pinmux_select_uart(void) {} static inline void pinmux_select_spi(void) {} #endif diff --git a/board/nvidia/common/uart-spi-switch.c b/board/nvidia/common/uart-spi-switch.c index 23aa0b9..1ba1afd 100644 --- a/board/nvidia/common/uart-spi-switch.c +++ b/board/nvidia/common/uart-spi-switch.c @@ -21,7 +21,6 @@ */ #include <common.h> -#include <ns16550.h> #include <asm/gpio.h> #include <asm/arch/pinmux.h> #include <asm/arch/uart-spi-switch.h> @@ -40,7 +39,6 @@ enum spi_uart_switch { /* Information about the spi/uart switch */ struct spi_uart { int gpio; /* GPIO to control switch */ - NS16550_t regs; /* Address of UART affected */ u32 port; /* Port number of UART affected */ }; @@ -52,7 +50,6 @@ static void get_config(struct spi_uart *config) { #if defined CONFIG_SPI_CORRUPTS_UART config->gpio = CONFIG_UART_DISABLE_GPIO; - config->regs = (NS16550_t)CONFIG_SPI_CORRUPTS_UART; config->port = CONFIG_SPI_CORRUPTS_UART_NR; #else config->gpio = -1; @@ -101,34 +98,24 @@ static void spi_uart_switch(struct spi_uart *config, if (switch_pos == SWITCH_BOTH || new_pos == switch_pos) return; - /* if the UART was selected, allow it to drain */ - if (switch_pos == SWITCH_UART) - NS16550_drain(config->regs, config->port); + /* pre-delay, allow SPI/UART to settle, FIFO to empty, etc. */ + udelay(CONFIG_SPI_CORRUPTS_UART_DLY); /* We need to dynamically change the pinmux, shared w/UART RXD/CTS */ pinmux_set_func(PINGRP_GMC, new_pos == SWITCH_SPI ? PMUX_FUNC_SFLASH : PMUX_FUNC_UARTD); /* - * On Seaboard, MOSI/MISO are shared w/UART. - * Use GPIO I3 (UART_DISABLE) to tristate UART during SPI activity. - * Enable UART later (cs_deactivate) so we can use it for U-Boot comms. - */ + * On Seaboard, MOSI/MISO are shared w/UART. + * Use GPIO I3 (UART_DISABLE) to tristate UART during SPI activity. + * Enable UART later (cs_deactivate) so we can use it for U-Boot comms. + */ gpio_direction_output(config->gpio, new_pos == SWITCH_SPI); switch_pos = new_pos; - - /* if the SPI was selected, clear any junk bytes in the UART */ - if (switch_pos == SWITCH_UART) { - /* TODO: What if it is part-way through clocking in junk? */ - udelay(100); - NS16550_clear(config->regs, config->port); - } } -void pinmux_select_uart(NS16550_t regs) +void pinmux_select_uart(void) { - /* Also prevents calling spi_uart_switch() before relocation */ - if (regs == local.regs) spi_uart_switch(&local, SWITCH_UART); } diff --git a/drivers/spi/tegra2_spi.c b/drivers/spi/tegra2_spi.c index 56cb229..fe7b405 100644 --- a/drivers/spi/tegra2_spi.c +++ b/drivers/spi/tegra2_spi.c @@ -28,13 +28,18 @@ #include <spi.h> #include <asm/io.h> #include <asm/gpio.h> -#include <ns16550.h> #include <asm/arch/clk_rst.h> #include <asm/arch/clock.h> #include <asm/arch/pinmux.h> #include <asm/arch/uart-spi-switch.h> #include <asm/arch/tegra2_spi.h> +#if defined(CONFIG_SPI_CORRUPTS_UART) + #define corrupt_delay() udelay(CONFIG_SPI_CORRUPTS_UART_DLY); +#else + #define corrupt_delay() +#endif + struct tegra_spi_slave { struct spi_slave slave; struct spi_tegra *regs; @@ -161,14 +166,20 @@ void spi_cs_activate(struct spi_slave *slave) /* CS is negated on Tegra, so drive a 1 to get a 0 */ setbits_le32(&spi->regs->command, SPI_CMD_CS_VAL); + + corrupt_delay(); /* Let UART settle */ } void spi_cs_deactivate(struct spi_slave *slave) { struct tegra_spi_slave *spi = to_tegra_spi(slave); + pinmux_select_uart(); + /* CS is negated on Tegra, so drive a 0 to get a 1 */ clrbits_le32(&spi->regs->command, SPI_CMD_CS_VAL); + + corrupt_delay(); /* Let SPI settle */ } int spi_xfer(struct spi_slave *slave, unsigned int bitlen, diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h index 46d4228..a9fd15e 100644 --- a/include/configs/seaboard.h +++ b/include/configs/seaboard.h @@ -52,6 +52,10 @@ /* On Seaboard: GPIO_PI3 = Port I = 8, bit = 3 */ #define CONFIG_UART_DISABLE_GPIO GPIO_PI3 +#define CONFIG_SPI_UART_SWITCH +#define CONFIG_SPI_CORRUPTS_UART NV_PA_APB_UARTD_BASE +#define CONFIG_SPI_CORRUPTS_UART_NR 3 +#define CONFIG_SPI_CORRUPTS_UART_DLY 2500 #define CONFIG_MACH_TYPE MACH_TYPE_SEABOARD #define CONFIG_SYS_BOARD_ODMDATA 0x300d8011 /* lp1, 1GB */
Simon Glass's proposal to fix this on Seaboard was NAK'd, so I removed his NS16550 references and added a small delay before SPI/UART muxing. Tested on my Seaboard with large SPI reads/writes and saw no corruption (crc's matched) and no spurious comm chars. Signed-off-by: Tom Warren <twarren@nvidia.com> --- arch/arm/include/asm/arch-tegra2/uart-spi-switch.h | 4 +- board/nvidia/common/uart-spi-switch.c | 27 +++++-------------- drivers/spi/tegra2_spi.c | 13 +++++++++- include/configs/seaboard.h | 4 +++ 4 files changed, 25 insertions(+), 23 deletions(-)