Hi,
I admit that I don't understand the I²C subsystem very well, but doesn't
this introduce a potential race condition?
> ...
> @@ -240,7 +241,7 @@ static int brcmstb_i2c_wait_for_completion(struct
brcmstb_i2c_dev *dev)
> ...
- if (dev->irq >= 0) {
+ if (dev->irq >= 0 && !dev->atomic) {
> ...
> @@ -287,7 +288,7 @@ static int brcmstb_send_i2c_cmd(struct
brcmstb_i2c_dev *dev,
> ...
- if (dev->irq >= 0)
+ if (dev->irq >= 0 && !dev->atomic)
> ...
> +static int brcmstb_i2c_xfer_atomic(struct i2c_adapter *adapter,
> + struct i2c_msg msgs[], int num)
> ...
+ dev->atomic = true;
+ ret = brcmstb_i2c_xfer(adapter, msgs, num);
+ dev->atomic = false;
...
What happens when one of the if() branches is taken in one thread while
another thread is just executing the assignment of the atomic flag? My
expectation would be that the first tread still sees the old flag value
and happily executes the branch, while brcmstb_i2c_xfer_atomic() sets
the flag just after and initiates a transfer.
I'd expect that access to the flag must be atomic as well, so maybe
something like
https://www.kernel.org/doc/html/latest/core-api/wrappers/atomic_t.html
is needed, or some other synchronization mechanism.
Or is it guaranteed that brcmstb_i2c_wait_for_completion() and
brcmstb_send_i2c_cmd() can only be called from the same thread as
brcmstb_i2c_xfer_atomic() ?
Regards,
Gregor
