On 09/09/2014 09:31 AM, Roger Quadros wrote:
> Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti().
> They seem to have been swapped in the usage instances.
> 
> TI's RAMINIT DONE mechanism is buggy and may not always be
> set after the START bit is set. So add a timeout mechanism to
> c_can_hw_raminit_wait_ti().
> 
> Signed-off-by: Roger Quadros <[email protected]>
> ---
>  drivers/net/can/c_can/c_can_platform.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/c_can/c_can_platform.c 
> b/drivers/net/can/c_can/c_can_platform.c
> index 109cb44..b144e71 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -75,10 +75,18 @@ static void c_can_plat_write_reg_aligned_to_32bit(const 
> struct c_can_priv *priv,
>  static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
>                                 u32 val)
>  {
> +     int timeout = 0;
>       /* We look only at the bits of our instance. */
>       val &= mask;
> -     while ((readl(priv->raminit_ctrlreg) & mask) != val)
> +     while ((readl(priv->raminit_ctrlreg) & mask) != val) {
>               udelay(1);
> +             timeout++;
> +
> +             if (timeout == 1000) {

How did we come up with this number?

> +                     dev_err(&priv->dev->dev, "%s: time out\n", __func__);
> +                     break;
lets say we did timeout..
see below:
> +             }
> +     }
>  }
>  
>  static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
> @@ -97,14 +105,14 @@ static void c_can_hw_raminit_ti(const struct c_can_priv 
> *priv, bool enable)
>       ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
>       writel(ctrl, priv->raminit_ctrlreg);
>       ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance);
> -     c_can_hw_raminit_wait_ti(priv, ctrl, mask);
> +     c_can_hw_raminit_wait_ti(priv, mask, ctrl);
>  
>       if (enable) {
>               /* Set start bit and wait for the done bit. */
>               ctrl |= CAN_RAMINIT_START_MASK(priv->instance);
>               writel(ctrl, priv->raminit_ctrlreg);
>               ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
> -             c_can_hw_raminit_wait_ti(priv, ctrl, mask);
> +             c_can_hw_raminit_wait_ti(priv, mask, ctrl);

is it possible for us to continue? does it make sense for us to change
that void to a int and handle error cascading?

>       }
>       spin_unlock(&raminit_lock);
>  }
> 


-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to