Hi W dniu 27.11.2018 o 13:26, Wolfram Sang pisze: > On Fri, Nov 16, 2018 at 01:24:41PM +0000, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote: >> According to Intel (R) Axxia TM Lionfish Communication Processor >> Peripheral Subsystem Hardware Reference Manual, the AXXIA I2C module >> have a programmable Master Wait Timer, which among others, checks the >> time between commands send in manual mode. When a timeout (25ms) passes, >> TSS bit is set in Master Interrupt Status register and a Stop command is >> issued by the hardware. >> >> The axxia_i2c_xfer(), does not properly handle this situation, however. >> For each message a separate axxia_i2c_xfer_msg() is called and this >> function incorrectly assumes that any interrupt might happen only when >> waiting for completion. This is mostly correct but there is one >> exception - a master timeout can trigger if enough time has passed >> between individual transfers. It will, by definition, happen between >> transfers when the interrupts are disabled by the code. If that happens, >> the hardware issues Stop command. >> >> The interrupt indicating timeout will not be triggered as soon as we >> enable them since the Master Interrupt Status is cleared when master >> mode is entered again (which happens before enabling irqs) meaning this >> error is lost and the transfer is continued even though the Stop was >> issued on the bus. The subsequent operations completes without error but >> a bogus value (0xFF in case of read) is read as the client device is >> confused because aborted transfer. No error is returned from >> master_xfer() making caller believe that a valid value was read. >> >> To fix the problem, the TSS bit (indicating timeout) in Master Interrupt >> Status register is checked before each transfer. If it is set, there was >> a timeout before this transfer and (as described above) the hardware >> already issued Stop command so the transaction should be aborted thus >> -ETIMEOUT is returned from the master_xfer() callback. In order to be >> sure no timeout was issued we can't just read the status just before >> starting new transaction as there will always be a small window of time >> (few CPU cycles at best) where this might still happen. For this reason >> we have to temporally disable the timer before checking for TSS bit. >> Disabling it will, however, clear the TSS bit so in order to preserve >> that information, we have to read it in ISR so we have to ensure that >> the TSS interrupt is not masked between transfers of one transaction. >> There is no need to call bus recovery or controller reinitialization if >> that happens so it's skipped. >> >> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@xxxxxxxxx> >> Reviewed-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx> > Applied to for-current, thanks! > > Since you and/or Alexander are the ones doing functional changes to this > driver, would you be interested in maintaining it? This would ensure you > get notified when someone else has patches for it. Thanks for the offer. Since I think I became quite familiar with his code and the hardware itself and we do have an interest real interest in this driver, I think it is a good idea. I will send a patch to MAINTAINERS soon. Krzysztof
