Re: [PATCH] i2c-axxia: properly handle master timeout

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 2973 bytes --]

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@nokia.com>
> Reviewed-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>

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.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]