> > > >
> > > >the other thing i was thinking how will multithreading .
> > > >Should we have a lock here.
> > > >
> > > >> - err = xiic_bus_busy(i2c);
> > > >> - while (err && tries--) {
> > > >> - msleep(1);
> > > >> + if (i2c->multimaster) {
> > > >> + /* for instance if previous transfer was terminated due to TX
> > > >> + * error it might be that the bus is on it's way to become
> > > >> + * available give it at most 3 ms to wake
> > > >> + */
> > > >> err = xiic_bus_busy(i2c);
> > > >> + while (err && tries--) {
> > > >> + msleep(1);
> > > >> + err = xiic_bus_busy(i2c);
> > > >> + }
> > > >> }
> > > >>
> > > >> return err;
> > >
> > > Which resource specifically are you worried about needing locking here?
> > >
> > Earlier multiple threads on the same processor will wait for bus busy.
> >
> > Now my concern was
> >
> > thread1 -> makes a transaction
> >
> > thread2 -> this will not wait for bus busy and access.
> Since i2c->tx_msg is set before anything is sent to FPGA and only returned to NULL after transaction has finished,
> I think thread2 would already exit with -EBUSY before xiic_bus_busy(i2c) is called because off:
> if (i2c->tx_msg)
> return -EBUSY;
> in same function.
>
> This is why my understanding is that xiic_bus_busy(i2c) only practically guards against other masters operating on bus.
> In my understanding xiic_bus_busy(i2c) reads the register on FPGA, which can't change state before thread1 is already so far into transmitting its data that FPGA has received something to send and > has reserved the bus. This would leave an interval of time between checking xiic_bus_busy and its register value changing during which thread2 could also check xiic_bus_busy and proceed to transmit at the same time with thread1. (Until hitting a transaction lock later, but only after it has already overwritten the pointer to transmit buffer i2c->tx_msg, and possibly messed up the transmissions for thread1).
>
> Now it seems to me that even with i2c->tx_msg being checked, thread2 could get past it before thread1 has set it to not NULL, since thread performs no locking between checking it and setting it, like I mentioned in previous reply. This issue has apparently already existed for some time though and is probably quite unlikely, since it has been there for some time.
> -Jaakko
Just to update you about the previously mentioned possible multithreading issue, I took some time to try and force the race condition to happen by adding some delay between checking and setting "i2c->tx_msg".
I was planning to fix it in separate patch since the possible issue would have existed already.
I discovered that I am unable to reproduce the threading issue, since the bus is already protected by a mutex in "i2c_transfer()" implemented in "i2c-core-base.c".
I suppose this may have been obvious for someone who has worked longer than me with the kernel i2c -drivers, but it seems that the driver implementations don't have to worry about access from multiple threads, since they are already protected in a more general level.
-Jaakko
