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@xxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/ > > v2: https://lore.kernel.org/linux-i2c/MN2PR01MB535821C8058C7814B2F8EEDF9F599@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/ > > v1: https://lore.kernel.org/linux-i2c/MN2PR01MB535838492432C910F2381F929F6F9@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/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@xxxxxxxxx> - Hector