Message-ID: <20181127122649.GL1107@kunai>
Date:   Tue, 27 Nov 2018 13:26:49 +0100
From:   Wolfram Sang <wsa@...-dreams.de>
To:     "Adamski, Krzysztof (Nokia - PL/Wroclaw)" 
        <krzysztof.adamski@...ia.com>
Cc:     "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Guenter Roeck <linux@...ck-us.net>,
        "Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@...ia.com>,
        Tobias Jordan <Tobias.Jordan@...ktrobit.com>,
        Peter Rosin <peda@...ntia.se>,
        Anders Berg <anders.berg@...el.com>
Subject: Re: [PATCH] i2c-axxia: properly handle master timeout

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


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists