On Sat, Jan 29, 2022 at 08:43:30PM -0600, Samuel Holland wrote: > On 1/29/22 8:05 PM, Ondřej Jirman wrote: > > > > Please use a single read transfer to get both command result and data. > > There will be less risk that some userspace app will issue another command > > in between command status being read as 0 and data byte being read. > > > > Otherwise if you use this in some read/modify/write operation, you > > may write unexpected value to PMIC. I2C register layout is designed > > to make this as optimal as possible in a single I2C transaction, so > > you only need 3 bytes to start command and 2 bytes to read the result > > and data, both in a single xfer. There's very high likelihood the command > > will complete in those 300 - 500 us anyway, because the timing is > > predictable. If this delay is set right, it's almost guaranteed, > > only two xfers will be necessary to run the command and get the result+ > > status. > > I did this originally, but it causes a different race condition: since the data > is read first, the command can complete between when the data is read and when > the result is read. If this happens, the command will be seen as complete, but > the data will be garbage. > > This caused occasional read errors for the charger's power supply properties, > because I2C reads sometimes returned nonsensical values for those bytes. Oh, well. :) I guess the firmware would need to wait for any ongoing I2C tranfer to finish before setting the command status to 0, for this to work. Another lesson learned. :( > > And if possible, it would be best if the bus was somehow made busy for > > other users, until the whole comand/result sequence completes, to eliminate > > the possibility of another command being issued by other bus users > > around [1]. > > Yes, I can add a call to i2c_lock_bus() here. Perfect. thank you, o. > Regards, > Samuel