diff mbox

[U-Boot,v2,02/04] spi, spi_mxc: do not hang in spi_xchg_single

Message ID 1401687404-3806-1-git-send-email-hs@denx.de
State Accepted
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Heiko Schocher June 2, 2014, 5:36 a.m. UTC 
if status register do never set MXC_CSPICTRL_TC, spi_xchg_single
endless loops. Add a timeout here to prevent endless hang.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Dirk Behme <dirk.behme@gmail.com>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>

---
- changes for v2:
  - use timer api to poll till TC bit is set as Jagan Teki suggested
    and make this timeout configurable through CONFIG_SYS_SPI_MXC_WAIT
---
 README                |  4 ++++
 drivers/spi/mxc_spi.c | 18 ++++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

Comments

Jagan Teki June 8, 2014, 5:33 p.m. UTC | #1 
On Mon, Jun 2, 2014 at 11:06 AM, Heiko Schocher <hs@denx.de> wrote:
> if status register do never set MXC_CSPICTRL_TC, spi_xchg_single
> endless loops. Add a timeout here to prevent endless hang.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Dirk Behme <dirk.behme@gmail.com>
> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>
> ---
> - changes for v2:
>   - use timer api to poll till TC bit is set as Jagan Teki suggested
>     and make this timeout configurable through CONFIG_SYS_SPI_MXC_WAIT
> ---
>  README                |  4 ++++
>  drivers/spi/mxc_spi.c | 18 ++++++++++++++++--
>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/README b/README
> index a280435..ff8928b 100644
> --- a/README
> +++ b/README
> @@ -2564,6 +2564,10 @@ CBFS (Coreboot Filesystem) support
>                 Enables the driver for the SPI controllers on i.MX and MXC
>                 SoCs. Currently i.MX31/35/51 are supported.
>
> +               CONFIG_SYS_SPI_MXC_WAIT
> +               Timeout for waiting until spi transfer completed.
> +               default: (CONFIG_SYS_HZ/100)     /* 10 ms */
> +
>  - FPGA Support: CONFIG_FPGA
>
>                 Enables FPGA subsystem.
> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
> index f3f029d..4732850 100644
> --- a/drivers/spi/mxc_spi.c
> +++ b/drivers/spi/mxc_spi.c
> @@ -30,6 +30,10 @@ static unsigned long spi_bases[] = {
>  #define reg_read readl
>  #define reg_write(a, v) writel(v, a)
>
> +#if !defined(CONFIG_SYS_SPI_MXC_WAIT)
> +#define CONFIG_SYS_SPI_MXC_WAIT                (CONFIG_SYS_HZ/100)     /* 10 ms */
> +#endif
> +
>  struct mxc_spi_slave {
>         struct spi_slave slave;
>         unsigned long   base;
> @@ -212,6 +216,8 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
>         int nbytes = DIV_ROUND_UP(bitlen, 8);
>         u32 data, cnt, i;
>         struct cspi_regs *regs = (struct cspi_regs *)mxcs->base;
> +       u32 ts;
> +       int status;
>
>         debug("%s: bitlen %d dout 0x%x din 0x%x\n",
>                 __func__, bitlen, (u32)dout, (u32)din);
> @@ -272,9 +278,17 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
>         reg_write(&regs->ctrl, mxcs->ctrl_reg |
>                 MXC_CSPICTRL_EN | MXC_CSPICTRL_XCH);
>
> +       ts = get_timer(0);
> +       status = reg_read(&regs->stat);
>         /* Wait until the TC (Transfer completed) bit is set */
> -       while ((reg_read(&regs->stat) & MXC_CSPICTRL_TC) == 0)
> -               ;
> +       while ((status & MXC_CSPICTRL_TC) == 0) {
> +               if (get_timer(ts) > CONFIG_SYS_SPI_MXC_WAIT) {
> +                       printf("spi_xchg_single: Timeout!\n");
> +                       return -1;
> +               }
> +               udelay(10);
> +               status = reg_read(&regs->stat);
> +       }
>
>         /* Transfer completed, clear any pending request */
>         reg_write(&regs->stat, MXC_CSPICTRL_TC | MXC_CSPICTRL_RXOVF);
> --
> 1.8.3.1
>

Reviewed-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
Jagan Teki June 8, 2014, 5:36 p.m. UTC | #2 
On Sun, Jun 8, 2014 at 11:03 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Mon, Jun 2, 2014 at 11:06 AM, Heiko Schocher <hs@denx.de> wrote:
>> if status register do never set MXC_CSPICTRL_TC, spi_xchg_single
>> endless loops. Add a timeout here to prevent endless hang.
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> Cc: Dirk Behme <dirk.behme@gmail.com>
>> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>>
>> ---
>> - changes for v2:
>>   - use timer api to poll till TC bit is set as Jagan Teki suggested
>>     and make this timeout configurable through CONFIG_SYS_SPI_MXC_WAIT
>> ---
>>  README                |  4 ++++
>>  drivers/spi/mxc_spi.c | 18 ++++++++++++++++--
>>  2 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/README b/README
>> index a280435..ff8928b 100644
>> --- a/README
>> +++ b/README
>> @@ -2564,6 +2564,10 @@ CBFS (Coreboot Filesystem) support
>>                 Enables the driver for the SPI controllers on i.MX and MXC
>>                 SoCs. Currently i.MX31/35/51 are supported.
>>
>> +               CONFIG_SYS_SPI_MXC_WAIT
>> +               Timeout for waiting until spi transfer completed.
>> +               default: (CONFIG_SYS_HZ/100)     /* 10 ms */
>> +
>>  - FPGA Support: CONFIG_FPGA
>>
>>                 Enables FPGA subsystem.
>> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
>> index f3f029d..4732850 100644
>> --- a/drivers/spi/mxc_spi.c
>> +++ b/drivers/spi/mxc_spi.c
>> @@ -30,6 +30,10 @@ static unsigned long spi_bases[] = {
>>  #define reg_read readl
>>  #define reg_write(a, v) writel(v, a)
>>
>> +#if !defined(CONFIG_SYS_SPI_MXC_WAIT)
>> +#define CONFIG_SYS_SPI_MXC_WAIT                (CONFIG_SYS_HZ/100)     /* 10 ms */
>> +#endif
>> +
>>  struct mxc_spi_slave {
>>         struct spi_slave slave;
>>         unsigned long   base;
>> @@ -212,6 +216,8 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
>>         int nbytes = DIV_ROUND_UP(bitlen, 8);
>>         u32 data, cnt, i;
>>         struct cspi_regs *regs = (struct cspi_regs *)mxcs->base;
>> +       u32 ts;
>> +       int status;
>>
>>         debug("%s: bitlen %d dout 0x%x din 0x%x\n",
>>                 __func__, bitlen, (u32)dout, (u32)din);
>> @@ -272,9 +278,17 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
>>         reg_write(&regs->ctrl, mxcs->ctrl_reg |
>>                 MXC_CSPICTRL_EN | MXC_CSPICTRL_XCH);
>>
>> +       ts = get_timer(0);
>> +       status = reg_read(&regs->stat);
>>         /* Wait until the TC (Transfer completed) bit is set */
>> -       while ((reg_read(&regs->stat) & MXC_CSPICTRL_TC) == 0)
>> -               ;
>> +       while ((status & MXC_CSPICTRL_TC) == 0) {
>> +               if (get_timer(ts) > CONFIG_SYS_SPI_MXC_WAIT) {
>> +                       printf("spi_xchg_single: Timeout!\n");
>> +                       return -1;
>> +               }
>> +               udelay(10);
>> +               status = reg_read(&regs->stat);
>> +       }
>>
>>         /* Transfer completed, clear any pending request */
>>         reg_write(&regs->stat, MXC_CSPICTRL_TC | MXC_CSPICTRL_RXOVF);
>> --
>> 1.8.3.1
>>
>
> Reviewed-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>

Tom,

I guess this will goes along with the series, if not I will pick.
Let me know for any.

thanks!
diff mbox

Patch

diff --git a/README b/README
index a280435..ff8928b 100644
--- a/README
+++ b/README
@@ -2564,6 +2564,10 @@  CBFS (Coreboot Filesystem) support
 		Enables the driver for the SPI controllers on i.MX and MXC
 		SoCs. Currently i.MX31/35/51 are supported.
 
+		CONFIG_SYS_SPI_MXC_WAIT
+		Timeout for waiting until spi transfer completed.
+		default: (CONFIG_SYS_HZ/100)     /* 10 ms */
+
 - FPGA Support: CONFIG_FPGA
 
 		Enables FPGA subsystem.
diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
index f3f029d..4732850 100644
--- a/drivers/spi/mxc_spi.c
+++ b/drivers/spi/mxc_spi.c
@@ -30,6 +30,10 @@  static unsigned long spi_bases[] = {
 #define reg_read readl
 #define reg_write(a, v) writel(v, a)
 
+#if !defined(CONFIG_SYS_SPI_MXC_WAIT)
+#define CONFIG_SYS_SPI_MXC_WAIT		(CONFIG_SYS_HZ/100)	/* 10 ms */
+#endif
+
 struct mxc_spi_slave {
 	struct spi_slave slave;
 	unsigned long	base;
@@ -212,6 +216,8 @@  int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 	int nbytes = DIV_ROUND_UP(bitlen, 8);
 	u32 data, cnt, i;
 	struct cspi_regs *regs = (struct cspi_regs *)mxcs->base;
+	u32 ts;
+	int status;
 
 	debug("%s: bitlen %d dout 0x%x din 0x%x\n",
 		__func__, bitlen, (u32)dout, (u32)din);
@@ -272,9 +278,17 @@  int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 	reg_write(&regs->ctrl, mxcs->ctrl_reg |
 		MXC_CSPICTRL_EN | MXC_CSPICTRL_XCH);
 
+	ts = get_timer(0);
+	status = reg_read(&regs->stat);
 	/* Wait until the TC (Transfer completed) bit is set */
-	while ((reg_read(&regs->stat) & MXC_CSPICTRL_TC) == 0)
-		;
+	while ((status & MXC_CSPICTRL_TC) == 0) {
+		if (get_timer(ts) > CONFIG_SYS_SPI_MXC_WAIT) {
+			printf("spi_xchg_single: Timeout!\n");
+			return -1;
+		}
+		udelay(10);
+		status = reg_read(&regs->stat);
+	}
 
 	/* Transfer completed, clear any pending request */
 	reg_write(&regs->stat, MXC_CSPICTRL_TC | MXC_CSPICTRL_RXOVF);