> > >
> > >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
