Hi Heiner, On Fri, 3 Dec 2021 13:55:30 +0100, Heiner Kallweit wrote: > On 03.12.2021 10:59, Jean Delvare wrote: > > On Thu, 02 Dec 2021 10:53:05 +0100, Heiner Kallweit wrote: > >> I see no need to read back the registers to verify that the bits > >> have actually been cleared. I can't imagine any scenario where > >> the bits would remain set after a write to them. > > > > This happened at least once in the past. See this archived message: > > > > https://www.spinics.net/lists/linux-i2c/msg02651.html > > "My last attempt locked the SMBus, but I was able to > recover by repeatedly writing to the HST_STS register, as may times as > the block length." > > OK, this was 11 yrs ago, so at least I wouldn't be able to recall in > detail what happened back then .. I definitely do not remember, so all the information available now is what can be read in this archived thread. > Question is how you did this "repeatedly writing to the HST_STS > register". Something like the following? Sorry for the confusion, Felix was the guy hitting the bug, and I was helping him understand what was happening and how to fix it. > while (status = in (STATUS)) > out(STATUS, status); Most probably yes. > Or maybe the driver started the loop to process the next byte? > I think it's not likely that when writing a status bit it > remained set. As we now know E32B is ignored in I2C mode, therefore > the chip can read/write only one byte in a row, and w/o setting > SMBHSTCNT_START in between it wouldn't touch the next byte. > Of course I may be wrong with my assumptions .. The important bit was probably SMBHSTSTS_BYTE_DONE. The driver set the block buffer mode, expecting the hardware to support that, but the hardware didn't and thus expected a byte-by-byte block transaction, which requires ack-ing every byte by clearing SMBHSTSTS_BYTE_DONE. > > (...) > > So I'm not too sure what to do with this. On the one hand, the code > > you want to remove could be useful to catch and investigate future > > bugs. The rest of the code does assume that the status bits are > > properly cleared before starting a new transaction. On the other > > hand, it is slowing down the driver a bit when all is fine. Is that > > really worth optimizing? As I got some time to think about it, answering myself: I'm fine removing the checks. If we ever hit the problem (unable to clear the error flags), it means that something went wrong _before_, and we must have logged these problems already. As a matter of fact, that was exactly the situation for Felix, the message you want to remove was the 4th error message logged, so in fact it did not really add any value. -- Jean Delvare SUSE L3 Support
