lkml.org 
[lkml]   [2022]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v4] i2c/pasemi: PASemi I2C controller IRQ enablement
From
On 05/11/2022 20.56, Arminder Singh wrote:
> This patch adds IRQ support to the PASemi I2C controller driver to
> increase the performace of I2C transactions on platforms with PASemi I2C
> controllers. While primarily intended for Apple silicon platforms, this
> patch should also help in enabling IRQ support for older PASemi hardware
> as well should the need arise.
>
> This version of the patch has been tested on an M1 Ultra Mac Studio,
> as well as an M1 MacBook Pro, and userspace launches successfully
> while using the IRQ path for I2C transactions.
>
> Signed-off-by: Arminder Singh <arminders208@outlook.com>
> ---
> This version of the patch fixes some reliability issues brought up by
> Hector and Sven in the v3 patch email thread. First, this patch
> increases the timeout value in pasemi_smb_waitready to 100ms from 10ms,
> as the original 10ms timeout in the driver was incorrect according to the
> controller's datasheet as Hector pointed out in the v3 patch email thread.
> This incorrect timeout had caused some issues with the tps6598x controller
> on Apple silicon platforms.
>
> This version of the patch also adds a reg_write to REG_IMASK in the IRQ
> handler, because as Sven pointed out in the previous thread, the I2C
> transaction interrupt is level sensitive so not masking the interrupt in
> REG_IMASK will cause the interrupt to trigger again when it leaves the IRQ
> handler until it reaches the call to reg_write after the completion expires.
>
> Patch changelog:
>
> v3 to v4 changes:
> - Increased the timeout value for I2C transactions to 100ms, as the original
> 10ms timeout in the driver was incorrect according to the I2C chip's
> datasheet. Mitigates an issue with the tps6598x controller on Apple
> silicon platforms.
> - Added a reg_write to REG_IMASK inside the IRQ handler, which prevents
> the IRQ from triggering again after leaving the IRQ handler, as the
> IRQ is level-sensitive.
>
> v2 to v3 changes:
> - Fixed some whitespace and alignment issues found in v2 of the patch
>
> v1 to v2 changes:
> - moved completion setup from pasemi_platform_i2c_probe to
> pasemi_i2c_common_probe to allow PASemi and Apple platforms to share
> common completion setup code in case PASemi hardware gets IRQ support
> added
> - initialized the status variable in pasemi_smb_waitready when going down
> the non-IRQ path
> - removed an unnecessary cast of dev_id in the IRQ handler
> - fixed alignment of struct member names in i2c-pasemi-core.h
> (addresses Christophe's feedback in the original submission)
> - IRQs are now disabled after the wait_for_completion_timeout call
> instead of inside the IRQ handler
> (prevents the IRQ from going off after the completion times out)
> - changed the request_irq call to a devm_request_irq call to obviate
> the need for a remove function and a free_irq call
> (thanks to Sven for pointing this out in the original submission)
> - added a reinit_completion call to pasemi_reset
> as a failsafe to prevent missed interrupts from causing the completion
> to never complete (thanks to Arnd Bergmann for pointing this out)
> - removed the bitmask variable in favor of just using the value
> directly (it wasn't used anywhere else)
>
> v3: https://lore.kernel.org/linux-i2c/MN2PR01MB5358ED8FC32C0CFAEBD4A0E19F5F9@MN2PR01MB5358.prod.exchangelabs.com/T/
>
> v2: https://lore.kernel.org/linux-i2c/MN2PR01MB535821C8058C7814B2F8EEDF9F599@MN2PR01MB5358.prod.exchangelabs.com/T/
>
> v1: https://lore.kernel.org/linux-i2c/MN2PR01MB535838492432C910F2381F929F6F9@MN2PR01MB5358.prod.exchangelabs.com/T/
>
> drivers/i2c/busses/i2c-pasemi-core.c | 32 ++++++++++++++++++++----
> drivers/i2c/busses/i2c-pasemi-core.h | 5 ++++
> drivers/i2c/busses/i2c-pasemi-platform.c | 6 +++++
> 3 files changed, 38 insertions(+), 5 deletions(-)
>

Reviewed-by: Hector Martin <marcan@marcan.st>

- Hector

\
 
 \ /
  Last update: 2022-11-07 06:14    [W:0.041 / U:0.372 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site