Re: [patch 02/12] can: c_can: Fix hardware raminit function

[Oliver Hartkopp]

On 18.03.2014 23:15, Thomas Gleixner wrote:
> On Tue, 18 Mar 2014, Marc Kleine-Budde wrote:
>> On 03/18/2014 06:19 PM, Thomas Gleixner wrote:
>>> +static void c_can_hw_raminit_wait(const struct c_can_priv *priv, u32 mask,
>>> +				  u32 val)
>>> +{
>>> +	/* We look only at the bits of our instance. */
>>> +	val &= mask;
>>> +	while ((readl(priv->raminit_ctrlreg) & mask) != val)
>>> +		udelay(1);
>>
>> Do we have to add a timeout here, or is it "safe" to have a potential
>> endless loop here? As you have probably tortured the hardware and driver
>> a lot (or have been tortured by them), I assume you would have added a
>> timeout check if you had seen a lockup here.
> 
> I haven't seen any failure on that. We could add a timeout for
> paranoia reasons. I'm quite sure that the raminit works as advertised
> when we do it the right way. The only way to wreckage it so far is by
> not waiting for it to complete.

As long as it is 100% guaranteed that we

1. really work on a valid C_CAN core
2. this CAN controller can not be unplugged

not adding a timeout would be ok.

I remember a system hang with a SJA1000 based PCMCIA card when unplugged
under heavy load:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a7762b10c12a70c5dbf2253142764b728ac88c3a

Regards,
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/