From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1DFF8C433F5 for ; Sun, 2 Oct 2022 15:31:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=xWiPXvLG17oB6ouyj0SKwhPBG6xw+den9auvogsM0Go=; b=pBKYt6PhHsEAG+ aeVoeoRjOokudn+Tk22fpaE6SNW6mihMfvOuAb5Woq2RVUDRmOC79Q7gCtEUG71svNL8+/T6gc/2a fLQJgvHKuG71zNBrVRjrjhkWhUeNmIEsz8FB1w6wjNq8qxQT8jonB2faKCkPPUME1JnP+A1uhyicW MPX7K9bAIv+dYyC2aGc/vADuKKLUH7GVAyyQnM5UAtCqSmGJ/4URAkXo6mSRQfXHWDI3EV4v/1vS1 su04YHSaKlw3DP+ubcotXVpS4DSlkgF5sjuVpfrN4R+t4t2kLFIeyqedIiCYIcA6j2vzh3IZsBLNr ltLp0Ik47Lk/G/gfShBg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1of0uW-001crI-Si; Sun, 02 Oct 2022 15:29:44 +0000 Received: from mo4-p01-ob.smtp.rzone.de ([85.215.255.50]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1of0uS-001cpi-Uw for linux-arm-kernel@lists.infradead.org; Sun, 02 Oct 2022 15:29:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1664724518; s=strato-dkim-0002; d=xenosoft.de; h=In-Reply-To:From:References:Cc:To:Subject:Date:Message-ID:Cc:Date: From:Subject:Sender; bh=JWaP7RXZDEtgcqRu/s9q3WTfPXKc6f9i74iz5nGjJPc=; b=CGFGhcp8+RAa84aBp3Cyg9erg1Sk0U8WY+7shWOeJQo3z6FfOeMZe9xwZQVy3XXKbz L+ghPmGdGHIjflXx0xJCjVTtV7BiMEC6wq92dd4V81FO56/xfSeZRYVmi1q7T3nFtOHZ 6Mj2Tz5RvWXmhZrZ4WjyyPJIFoDXlzt/ajYmx42xUkUkb2cjpZi2/v5Wa887hT7IaTsF 3LLxlCgJfwmRpIcQ7LrgL2pp0LQBIjC1rie0xlnu4sihKrDo+DxXWD6ftl/QS2e0fjeZ q/3uKk9CFl8BtLq69ccv3rHGIYG3eL+rtzRrug5NfWdKwI3BB36rbpnolXHWLkEWme+T gAAw== Authentication-Results: strato.com; dkim=none X-RZG-AUTH: ":L2QefEenb+UdBJSdRCXu93KJ1bmSGnhMdmOod1DhGM4l4Hio94KKxRySfLxnHfJ+Dkjp5DdBfio0GngadwiB6Ms880YXt4vX4K4IwSOGijZEpA==" X-RZG-CLASS-ID: mo00 Received: from [IPV6:2a02:8109:8980:4474:d15d:b942:40c7:5459] by smtp.strato.de (RZmta 48.1.3 AUTH) with ESMTPSA id 4958bfy92FSZDBv (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Sun, 2 Oct 2022 17:28:35 +0200 (CEST) Message-ID: <1ad42694-a002-2108-37b5-ffcd8f08d442@xenosoft.de> Date: Sun, 2 Oct 2022 17:28:35 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.3.0 Subject: Re: [PATCH v2] i2c/pasemi: PASemi I2C controller IRQ enablement To: Arminder Singh , linux-kernel@vger.kernel.org Cc: linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, asahi@lists.linux.dev, Alyssa Rosenzweig , Sven Peter , Hector Martin , Paul Mackerras , Benjamin Herrenschmidt , Michael Ellerman , Darren Stevens , "R.T.Dickinson" , Christian Zigotzky References: From: Christian Zigotzky In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221002_082941_290323_ED7D97B7 X-CRM114-Status: GOOD ( 43.26 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 02 October 2022 at 00:25 am, Arminder Singh wrote: > Hello, > > This is v2 of the PASemi I2C controller IRQ enablement patch. > > 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 the patch is 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. > > Tested-by: Arminder Singh > Signed-off-by: Arminder Singh > --- > Changes from v1: > - 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) > > v1 linked here: https://lore.kernel.org/linux-i2c/MN2PR01MB535838492432C910F2381F929F6F9@MN2PR01MB5358.prod.exchangelabs.com/T/#m11b3504c2667517aad7521514c99ca0e07a9381f > > Thanks for all the feedback on the previous submission, I'm sorry > I wasn't able to answer everyone's emails, was just pretty busy, I'll > make sure to be more responsive this time around! Also wasn't sure whether > the v1 changelog belonged before or after the '---' so I put it after > to keep the commit changelog short and concise. > (This is just one patch, didn't think it needed a cover letter) > > drivers/i2c/busses/i2c-pasemi-core.c | 29 ++++++++++++++++++++---- > drivers/i2c/busses/i2c-pasemi-core.h | 7 +++++- > drivers/i2c/busses/i2c-pasemi-platform.c | 6 +++++ > 3 files changed, 37 insertions(+), 5 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-pasemi-core.c b/drivers/i2c/busses/i2c-pasemi-core.c > index 9028ffb58cc0..05af8f3575bc 100644 > --- a/drivers/i2c/busses/i2c-pasemi-core.c > +++ b/drivers/i2c/busses/i2c-pasemi-core.c > @@ -21,6 +21,7 @@ > #define REG_MTXFIFO 0x00 > #define REG_MRXFIFO 0x04 > #define REG_SMSTA 0x14 > +#define REG_IMASK 0x18 > #define REG_CTL 0x1c > #define REG_REV 0x28 > > @@ -66,6 +67,7 @@ static void pasemi_reset(struct pasemi_smbus *smbus) > val |= CTL_EN; > > reg_write(smbus, REG_CTL, val); > + reinit_completion(&smbus->irq_completion); > } > > static void pasemi_smb_clear(struct pasemi_smbus *smbus) > @@ -81,11 +83,18 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus) > int timeout = 10; > unsigned int status; > > - status = reg_read(smbus, REG_SMSTA); > - > - while (!(status & SMSTA_XEN) && timeout--) { > - msleep(1); > + if (smbus->use_irq) { > + reinit_completion(&smbus->irq_completion); > + reg_write(smbus, REG_IMASK, SMSTA_XEN | SMSTA_MTN); > + wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(10)); > + reg_write(smbus, REG_IMASK, 0); > status = reg_read(smbus, REG_SMSTA); > + } else { > + status = reg_read(smbus, REG_SMSTA); > + while (!(status & SMSTA_XEN) && timeout--) { > + msleep(1); > + status = reg_read(smbus, REG_SMSTA); > + } > } > > /* Got NACK? */ > @@ -344,10 +353,14 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus) > > /* set up the sysfs linkage to our parent device */ > smbus->adapter.dev.parent = smbus->dev; > + smbus->use_irq = 0; > + init_completion(&smbus->irq_completion); > > if (smbus->hw_rev != PASEMI_HW_REV_PCI) > smbus->hw_rev = reg_read(smbus, REG_REV); > > + reg_write(smbus, REG_IMASK, 0); > + > pasemi_reset(smbus); > > error = devm_i2c_add_adapter(smbus->dev, &smbus->adapter); > @@ -356,3 +369,11 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus) > > return 0; > } > + > +irqreturn_t pasemi_irq_handler(int irq, void *dev_id) > +{ > + struct pasemi_smbus *smbus = dev_id; > + > + complete(&smbus->irq_completion); > + return IRQ_HANDLED; > +} > diff --git a/drivers/i2c/busses/i2c-pasemi-core.h b/drivers/i2c/busses/i2c-pasemi-core.h > index 4655124a37f3..ba6d6ccf9cdc 100644 > --- a/drivers/i2c/busses/i2c-pasemi-core.h > +++ b/drivers/i2c/busses/i2c-pasemi-core.h > @@ -7,6 +7,7 @@ > #include > #include > #include > +#include > > #define PASEMI_HW_REV_PCI -1 > > @@ -15,7 +16,11 @@ struct pasemi_smbus { > struct i2c_adapter adapter; > void __iomem *ioaddr; > unsigned int clk_div; > - int hw_rev; > + int hw_rev; > + int use_irq; > + struct completion irq_completion; > }; > > int pasemi_i2c_common_probe(struct pasemi_smbus *smbus); > + > +irqreturn_t pasemi_irq_handler(int irq, void *dev_id); > diff --git a/drivers/i2c/busses/i2c-pasemi-platform.c b/drivers/i2c/busses/i2c-pasemi-platform.c > index 88a54aaf7e3c..e35945a91dbe 100644 > --- a/drivers/i2c/busses/i2c-pasemi-platform.c > +++ b/drivers/i2c/busses/i2c-pasemi-platform.c > @@ -49,6 +49,7 @@ static int pasemi_platform_i2c_probe(struct platform_device *pdev) > struct pasemi_smbus *smbus; > u32 frequency; > int error; > + int irq_num; > > data = devm_kzalloc(dev, sizeof(struct pasemi_platform_i2c_data), > GFP_KERNEL); > @@ -82,6 +83,11 @@ static int pasemi_platform_i2c_probe(struct platform_device *pdev) > if (error) > goto out_clk_disable; > > + irq_num = platform_get_irq(pdev, 0); > + error = devm_request_irq(smbus->dev, irq_num, pasemi_irq_handler, 0, "pasemi_apple_i2c", (void *)smbus); > + > + if (!error) > + smbus->use_irq = 1; > platform_set_drvdata(pdev, data); > > return 0; Tested-by: Christian Zigotzky on an A-EON AmigaOne X1000 with a PASemi PWRficient PA6T-1682 processor. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel