Re: [PATCH 1/2] spi/pl022: timeout on polled transfer v2

[Vitaly Wool <vitalywool-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Hi Linus,

On Thu, May 19, 2011 at 6:05 PM, Linus Walleij
<linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org> wrote:
> From: Magnus Templing <magnus.templing-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
>
> This adds the missing handling of polling timeouts and deletes
> our last todo.
>
> Signed-off-by: Magnus Templing <magnus.templing-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
> Reviewed-by: Srinidhi Kasagar <srinidhi.kasagar-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
> [Fixups from review by Wolfram Sang]
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/spi/amba-pl022.c |   23 +++++++++++++++--------
>  1 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
> index 08de58e..18667de 100644
> --- a/drivers/spi/amba-pl022.c
> +++ b/drivers/spi/amba-pl022.c
> @@ -24,11 +24,6 @@
>  * GNU General Public License for more details.
>  */
>
> -/*
> - * TODO:
> - * - add timeout on polled transfers
> - */
> -
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/device.h>
> @@ -287,6 +282,8 @@
>
>  #define CLEAR_ALL_INTERRUPTS  0x3
>
> +#define SPI_POLLING_TIMEOUT 1000
> +
>
>  /*
>  * The type of reading going on on this chip
> @@ -1378,6 +1375,7 @@ static void do_polling_transfer(struct pl022 *pl022)
>        struct spi_transfer *transfer = NULL;
>        struct spi_transfer *previous = NULL;
>        struct chip_data *chip;
> +       unsigned long time, timeout;
>
>        chip = pl022->cur_chip;
>        message = pl022->cur_msg;
> @@ -1415,9 +1413,18 @@ static void do_polling_transfer(struct pl022 *pl022)
>                       SSP_CR1(pl022->virtbase));
>
>                dev_dbg(&pl022->adev->dev, "polling transfer ongoing ...\n");
> -               /* FIXME: insert a timeout so we don't hang here indefinitely */
> -               while (pl022->tx < pl022->tx_end || pl022->rx < pl022->rx_end)
> +
> +               timeout = jiffies + msecs_to_jiffies(SPI_POLLING_TIMEOUT);
> +               while (pl022->tx < pl022->tx_end || pl022->rx < pl022->rx_end) {
> +                       time = jiffies;
>                        readwriter(pl022);
> +                       if (time_after(time, timeout)) {
> +                               dev_warn(&pl022->adev->dev,
> +                               "%s: timeout!\n", __func__);
> +                               message->state = STATE_ERROR;
> +                               goto out;
> +                       }
> +               }

just out of curiosity: is it a busy wait? Looks like it is...

Thanks,
   Vitaly

------------------------------------------------------------------------------
What Every C/C++ and Fortran developer Should Know!
Read this article and learn how Intel has extended the reach of its 
next-generation tools to help Windows* and Linux* C/C++ and Fortran 
developers boost performance applications - including clusters. 
http://p.sf.net/sfu/intel-dev2devmay