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 <rogerq@xxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
