[U-Boot] [PATCH v2 1/5] spi, spi_mxc: do not hang in spi_xchg_single

[Jagan Teki]

Can you send the next level patch by removing this delay and verified
on your target.
I will push the same.

On Mon, Jul 14, 2014 at 12:56 PM, Heiko Schocher <hs at denx.de> wrote:
> Hello Jagan,
>
> Am 12.07.2014 14:54, schrieb Jagan Teki:
>
>> On Sat, Jul 12, 2014 at 9:40 AM, Heiko Schocher<hs at 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 at denx.de>
>>> Cc: Dirk Behme<dirk.behme at gmail.com>
>>> Cc: Jagannadha Sutradharudu Teki<jagannadh.teki at 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
>
> [...]
>>>
>>> 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);
>>
>>
>> Why you still used explicit delay here, get_timer will do the job
>> finite delay right?
>
>
> Yes, thats right. Good catch, thanks.

thanks!
-- 
Jagan.