Re: [PATCH v4 2/3] i2c: iproc: Add Broadcom iProc I2C Driver

From: Ray Jui
Date: Sat Jan 17 2015 - 19:30:44 EST




On 1/17/2015 2:40 PM, Russell King - ARM Linux wrote:
> On Sat, Jan 17, 2015 at 01:26:41PM -0800, Ray Jui wrote:
>> time_left = wait_for_completion_timeout(&iproc_i2c->done, time_left);
>>
>> /* disable all interrupts */
>> writel(0, iproc_i2c->base + IE_OFFSET);
>>
>> if (!time_left && !atomic_read(&iproc_i2c->transfer_is_successful)) {
>
> Why are you using atomic_read() here?
>
transfer_is_successful 1) will be reset to 0 in this function (before
kick start the I2C transfer), 2) will be set to 1 in the ISR (to signal
completion of the I2C transfer), and 3) will be checked in this function
here. I thought that means I should declare it volatile, because it can
be modified in both the process context and interrupt context (and I use
atomic because I remember Linux checkpatch warns against using volatile)?
--
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/