From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752332AbeDQKdm (ORCPT ); Tue, 17 Apr 2018 06:33:42 -0400 Received: from lelnx194.ext.ti.com ([198.47.27.80]:58214 "EHLO lelnx194.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751230AbeDQKdk (ORCPT ); Tue, 17 Apr 2018 06:33:40 -0400 Subject: Re: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support To: Gustavo Pimentel , , , , , , , References: <8b88f8c2b766f36c71659deb0fce635154b2b39f.1523379766.git.gustavo.pimentel@synopsys.com> CC: , , From: Kishon Vijay Abraham I Message-ID: Date: Tue, 17 Apr 2018 16:03:26 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <8b88f8c2b766f36c71659deb0fce635154b2b39f.1523379766.git.gustavo.pimentel@synopsys.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote: > Adds the MSI-X support and updates driver documentation accordingly. > > Changes the driver parameter in order to allow the interruption type > selection. > > Signed-off-by: Gustavo Pimentel > --- > Documentation/misc-devices/pci-endpoint-test.txt | 3 + > drivers/misc/pci_endpoint_test.c | 102 +++++++++++++++++------ > 2 files changed, 79 insertions(+), 26 deletions(-) > > diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt > index 4ebc359..fdfa0f6 100644 > --- a/Documentation/misc-devices/pci-endpoint-test.txt > +++ b/Documentation/misc-devices/pci-endpoint-test.txt > @@ -10,6 +10,7 @@ The PCI driver for the test device performs the following tests > *) verifying addresses programmed in BAR > *) raise legacy IRQ > *) raise MSI IRQ > + *) raise MSI-X IRQ > *) read data > *) write data > *) copy data > @@ -25,6 +26,8 @@ ioctl > PCITEST_LEGACY_IRQ: Tests legacy IRQ > PCITEST_MSI: Tests message signalled interrupts. The MSI number > to be tested should be passed as argument. > + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number > + to be tested should be passed as argument. > PCITEST_WRITE: Perform write tests. The size of the buffer should be passed > as argument. > PCITEST_READ: Perform read tests. The size of the buffer should be passed > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c > index 37db0fc..a7d9354 100644 > --- a/drivers/misc/pci_endpoint_test.c > +++ b/drivers/misc/pci_endpoint_test.c > @@ -42,11 +42,16 @@ > #define PCI_ENDPOINT_TEST_COMMAND 0x4 > #define COMMAND_RAISE_LEGACY_IRQ BIT(0) > #define COMMAND_RAISE_MSI_IRQ BIT(1) > -#define MSI_NUMBER_SHIFT 2 > -/* 6 bits for MSI number */ > -#define COMMAND_READ BIT(8) > -#define COMMAND_WRITE BIT(9) > -#define COMMAND_COPY BIT(10) > +#define COMMAND_RAISE_MSIX_IRQ BIT(2) > +#define IRQ_TYPE_SHIFT 3 > +#define IRQ_TYPE_LEGACY 0 > +#define IRQ_TYPE_MSI 1 > +#define IRQ_TYPE_MSIX 2 > +#define MSI_NUMBER_SHIFT 5 Now that you are anyways fixing this, add a new register entry for MSI numbers. Let's not keep COMMAND and MSI's together. > +/* 12 bits for MSI number */ > +#define COMMAND_READ BIT(17) > +#define COMMAND_WRITE BIT(18) > +#define COMMAND_COPY BIT(19) This change should be done along with the pci-epf-test in a single patch. > > #define PCI_ENDPOINT_TEST_STATUS 0x8 > #define STATUS_READ_SUCCESS BIT(0) > @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida); > #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \ > miscdev) > > -static bool no_msi; > -module_param(no_msi, bool, 0444); > -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test"); Let's not remove this just to make sure existing users doesn't get affected. > +static int irq_type = IRQ_TYPE_MSIX; > +module_param(irq_type, int, 0444); > +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)"); > > enum pci_barno { > BAR_0, > @@ -103,7 +108,7 @@ struct pci_endpoint_test { > struct pci_endpoint_test_data { > enum pci_barno test_reg_bar; > size_t alignment; > - bool no_msi; > + int irq_type; > }; > > static inline u32 pci_endpoint_test_readl(struct pci_endpoint_test *test, > @@ -177,10 +182,10 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test, > > static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test) > { > - u32 val; > + u32 val = COMMAND_RAISE_LEGACY_IRQ; > > - pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, > - COMMAND_RAISE_LEGACY_IRQ); > + val |= (IRQ_TYPE_LEGACY << IRQ_TYPE_SHIFT); > + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val); > val = wait_for_completion_timeout(&test->irq_raised, > msecs_to_jiffies(1000)); > if (!val) > @@ -192,12 +197,12 @@ static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test) > static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test, > u8 msi_num) > { > - u32 val; > + u32 val = COMMAND_RAISE_MSI_IRQ; > struct pci_dev *pdev = test->pdev; > > - pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, > - msi_num << MSI_NUMBER_SHIFT | > - COMMAND_RAISE_MSI_IRQ); > + val |= (msi_num << MSI_NUMBER_SHIFT); > + val |= (IRQ_TYPE_MSI << IRQ_TYPE_SHIFT); > + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val); > val = wait_for_completion_timeout(&test->irq_raised, > msecs_to_jiffies(1000)); > if (!val) > @@ -209,6 +214,26 @@ static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test, > return false; > } > > +static bool pci_endpoint_test_msix_irq(struct pci_endpoint_test *test, > + u16 msix_num) > +{ > + u32 val = COMMAND_RAISE_MSIX_IRQ; > + struct pci_dev *pdev = test->pdev; > + > + val |= (msix_num << MSI_NUMBER_SHIFT); > + val |= (IRQ_TYPE_MSIX << IRQ_TYPE_SHIFT); > + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val); > + val = wait_for_completion_timeout(&test->irq_raised, > + msecs_to_jiffies(1000)); > + if (!val) > + return false; > + > + if (test->last_irq - pdev->irq == msix_num - 1) > + return true; > + > + return false; I think you can have a single function for msix_irq and msi_irq. Thanks Kishon From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-5.6 required=5.0 tests=DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id D9BBC7DE74 for ; Tue, 17 Apr 2018 10:33:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751857AbeDQKdl (ORCPT ); Tue, 17 Apr 2018 06:33:41 -0400 Received: from lelnx194.ext.ti.com ([198.47.27.80]:58214 "EHLO lelnx194.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751230AbeDQKdk (ORCPT ); Tue, 17 Apr 2018 06:33:40 -0400 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by lelnx194.ext.ti.com (8.15.1/8.15.1) with ESMTP id w3HAXV7x001902; Tue, 17 Apr 2018 05:33:31 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com; s=ti-com-17Q1; t=1523961211; bh=JTwxJWngvmRE+7zSMps6Vjvjks763lQZzN8uwL93uos=; h=Subject:To:References:CC:From:Date:In-Reply-To; b=tI+47Szx+8c9/n4F6DSvzU3w6mVjURCAYtujd7ztl7N5aJAKEFwmaPTmEE8mfDyd5 bPanpFHTQeQntOutt4/iasd30Rnxpo6WJyQ1TV4n9TdFRoL3e/EC1o2TJ0mccQ9itC rG5lmGF6fTffnBZn0hB4I+S4QYUxTNA5SHNuGaLI= Received: from DLEE100.ent.ti.com (dlee100.ent.ti.com [157.170.170.30]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id w3HAXVhX017943; Tue, 17 Apr 2018 05:33:31 -0500 Received: from DLEE114.ent.ti.com (157.170.170.25) by DLEE100.ent.ti.com (157.170.170.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1261.35; Tue, 17 Apr 2018 05:33:31 -0500 Received: from dflp33.itg.ti.com (10.64.6.16) by DLEE114.ent.ti.com (157.170.170.25) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.1.1261.35 via Frontend Transport; Tue, 17 Apr 2018 05:33:31 -0500 Received: from [172.24.190.233] (ileax41-snat.itg.ti.com [10.172.224.153]) by dflp33.itg.ti.com (8.14.3/8.13.8) with ESMTP id w3HAXRNC025326; Tue, 17 Apr 2018 05:33:28 -0500 Subject: Re: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support To: Gustavo Pimentel , , , , , , , References: <8b88f8c2b766f36c71659deb0fce635154b2b39f.1523379766.git.gustavo.pimentel@synopsys.com> CC: , , From: Kishon Vijay Abraham I Message-ID: Date: Tue, 17 Apr 2018 16:03:26 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <8b88f8c2b766f36c71659deb0fce635154b2b39f.1523379766.git.gustavo.pimentel@synopsys.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org Hi, On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote: > Adds the MSI-X support and updates driver documentation accordingly. > > Changes the driver parameter in order to allow the interruption type > selection. > > Signed-off-by: Gustavo Pimentel > --- > Documentation/misc-devices/pci-endpoint-test.txt | 3 + > drivers/misc/pci_endpoint_test.c | 102 +++++++++++++++++------ > 2 files changed, 79 insertions(+), 26 deletions(-) > > diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt > index 4ebc359..fdfa0f6 100644 > --- a/Documentation/misc-devices/pci-endpoint-test.txt > +++ b/Documentation/misc-devices/pci-endpoint-test.txt > @@ -10,6 +10,7 @@ The PCI driver for the test device performs the following tests > *) verifying addresses programmed in BAR > *) raise legacy IRQ > *) raise MSI IRQ > + *) raise MSI-X IRQ > *) read data > *) write data > *) copy data > @@ -25,6 +26,8 @@ ioctl > PCITEST_LEGACY_IRQ: Tests legacy IRQ > PCITEST_MSI: Tests message signalled interrupts. The MSI number > to be tested should be passed as argument. > + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number > + to be tested should be passed as argument. > PCITEST_WRITE: Perform write tests. The size of the buffer should be passed > as argument. > PCITEST_READ: Perform read tests. The size of the buffer should be passed > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c > index 37db0fc..a7d9354 100644 > --- a/drivers/misc/pci_endpoint_test.c > +++ b/drivers/misc/pci_endpoint_test.c > @@ -42,11 +42,16 @@ > #define PCI_ENDPOINT_TEST_COMMAND 0x4 > #define COMMAND_RAISE_LEGACY_IRQ BIT(0) > #define COMMAND_RAISE_MSI_IRQ BIT(1) > -#define MSI_NUMBER_SHIFT 2 > -/* 6 bits for MSI number */ > -#define COMMAND_READ BIT(8) > -#define COMMAND_WRITE BIT(9) > -#define COMMAND_COPY BIT(10) > +#define COMMAND_RAISE_MSIX_IRQ BIT(2) > +#define IRQ_TYPE_SHIFT 3 > +#define IRQ_TYPE_LEGACY 0 > +#define IRQ_TYPE_MSI 1 > +#define IRQ_TYPE_MSIX 2 > +#define MSI_NUMBER_SHIFT 5 Now that you are anyways fixing this, add a new register entry for MSI numbers. Let's not keep COMMAND and MSI's together. > +/* 12 bits for MSI number */ > +#define COMMAND_READ BIT(17) > +#define COMMAND_WRITE BIT(18) > +#define COMMAND_COPY BIT(19) This change should be done along with the pci-epf-test in a single patch. > > #define PCI_ENDPOINT_TEST_STATUS 0x8 > #define STATUS_READ_SUCCESS BIT(0) > @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida); > #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \ > miscdev) > > -static bool no_msi; > -module_param(no_msi, bool, 0444); > -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test"); Let's not remove this just to make sure existing users doesn't get affected. > +static int irq_type = IRQ_TYPE_MSIX; > +module_param(irq_type, int, 0444); > +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)"); > > enum pci_barno { > BAR_0, > @@ -103,7 +108,7 @@ struct pci_endpoint_test { > struct pci_endpoint_test_data { > enum pci_barno test_reg_bar; > size_t alignment; > - bool no_msi; > + int irq_type; > }; > > static inline u32 pci_endpoint_test_readl(struct pci_endpoint_test *test, > @@ -177,10 +182,10 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test, > > static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test) > { > - u32 val; > + u32 val = COMMAND_RAISE_LEGACY_IRQ; > > - pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, > - COMMAND_RAISE_LEGACY_IRQ); > + val |= (IRQ_TYPE_LEGACY << IRQ_TYPE_SHIFT); > + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val); > val = wait_for_completion_timeout(&test->irq_raised, > msecs_to_jiffies(1000)); > if (!val) > @@ -192,12 +197,12 @@ static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test) > static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test, > u8 msi_num) > { > - u32 val; > + u32 val = COMMAND_RAISE_MSI_IRQ; > struct pci_dev *pdev = test->pdev; > > - pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, > - msi_num << MSI_NUMBER_SHIFT | > - COMMAND_RAISE_MSI_IRQ); > + val |= (msi_num << MSI_NUMBER_SHIFT); > + val |= (IRQ_TYPE_MSI << IRQ_TYPE_SHIFT); > + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val); > val = wait_for_completion_timeout(&test->irq_raised, > msecs_to_jiffies(1000)); > if (!val) > @@ -209,6 +214,26 @@ static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test, > return false; > } > > +static bool pci_endpoint_test_msix_irq(struct pci_endpoint_test *test, > + u16 msix_num) > +{ > + u32 val = COMMAND_RAISE_MSIX_IRQ; > + struct pci_dev *pdev = test->pdev; > + > + val |= (msix_num << MSI_NUMBER_SHIFT); > + val |= (IRQ_TYPE_MSIX << IRQ_TYPE_SHIFT); > + pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val); > + val = wait_for_completion_timeout(&test->irq_raised, > + msecs_to_jiffies(1000)); > + if (!val) > + return false; > + > + if (test->last_irq - pdev->irq == msix_num - 1) > + return true; > + > + return false; I think you can have a single function for msix_irq and msi_irq. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html