On Sun, Apr 18, 2021 at 4:49 PM Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > > On Sun, Apr 18, 2021 at 2:07 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > Thanks for review, my answers below. > > > On Wed, 14 Apr 2021 22:54:53 +0300 > > Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > > > We can utilize separate drivers for accelerometer and magnetometer, > > > so here is the glue driver to enable LSM9DS0 IMU support. > > > > > > The idea was suggested by Crestez Dan Leonard in [1]. The proposed change > > > was sent as RFC due to race condition concerns, which are indeed possible. > > > > If you are going to mention races, good to give some flavour in here! > > I meant that the initial idea is racy due to different devices > communicating to the same i2c address. > So, any sequence of transfers are not serialized and you may end up with > > drv1 -> i2c > drv2 -> i2c > drv1 <- i2c # garbage > > > This driver makes me very nervous indeed. > > Why?! This one is race free as far as I can see. Or maybe I interpret > this wrongly and you are talking about initial RFC? > > > I haven't 'found' any places > > where the fact we'll write the same registers from each of the drivers > > causes problems (e.g. int pin setup etc) but perhaps I'm missing something. > > > > Shall we say that makes me rather keener to get eyes (and thought) on this > > patch than normal :) > > How should I amend the commit message to state: > 1. First idea (RFC by the link) *is* racy AFAIU > 2. This one *is not* racy. I re-read this and now understand better what you meant. So, it may be that the initial proposal may work without any amendment, but since I haven't investigated much, I should rather use the phrase "potentially racy". In my variant it's using one regmap for both drivers (not two), which makes the register state consistent. Am I wrong? Do we have some places where we may write to the same register concurrently? -- With Best Regards, Andy Shevchenko
