LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Kishon Vijay Abraham I <kishon@ti.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>, Rob Herring <robh@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Lokesh Vutla <lokeshvutla@ti.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Tom Joseph <tjoseph@cadence.com>, <linux-pci@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>, <nadeem@cadence.com>
Subject: Re: [PATCH v2 5/6] misc: pci_endpoint_test: Do not request or allocate IRQs in probe
Date: Wed, 4 Aug 2021 19:32:44 +0530	[thread overview]
Message-ID: <02c1ddb7-539e-20a0-1bef-e10e76922a0e@ti.com> (raw)
In-Reply-To: <20210803095839.GA11252@lpieralisi>

Hi Lorenzo,

On 03/08/21 3:28 pm, Lorenzo Pieralisi wrote:
> On Tue, Aug 03, 2021 at 01:19:31PM +0530, Kishon Vijay Abraham I wrote:
>> Allocation of IRQ vectors and requesting IRQ is done as part of
>> PCITEST_SET_IRQTYPE. Do not request or allocate IRQs in probe for
>> AM654 and J721E so that the user space test script has better control
>> of the devices for which the IRQs are configured. Since certain user
>> space scripts could rely on allocation of IRQ vectors during probe,
>> remove allocation of IRQs only for TI's K3 platform.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/misc/pci_endpoint_test.c | 19 +++++++++++++------
>>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> I don't claim to understand the inner details of the endpoint test
> device but it looks like this approach should be redesigned.
> 
> I don't believe using devices quirks is the best approach to
> expose/remove a feature to userspace, this can soon become
> unmaintenable.
> 
> Maybe you can elaborate a bit more on what the real issue is please ?

The actual reason for introducing this patch (affects only AM654 and
J721E) is due to Errata ID #i2101 GIC: ITS Misbehavior
(https://www.ti.com/lit/er/sprz455a/sprz455a.pdf). So if more than 5
devices use GIC ITS simultaneously, GIC fails to raise interrupts.

Though this patch is not an actual workaround for the issue (the
workaround is in GIC ITS driver provided in the errata document), it
helps to keep testing PCIe RC/EP using pci-endpoint-test even when
multiple pci-epf-test endpoint devices are connected (Normal test-setup
having J721E-J721E back to back connection can support 21 pci-epf-test
devices). So this patch lets user to individually enable interrupts for
each of the devices and could disable after the interrupt test.

Since pci_endpoint_test is used only for testing PCIE RC/EP
communication and pci-endpoint-test has already implemented
PCITEST_SET_IRQTYPE for the userspace to enable interrupt, tried to not
enable the interrupts of all the devices by default in the probe (for
AM654 and J721E where this errata applies).

Thanks,
Kishon

> 
> Thanks,
> Lorenzo
> 
>> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
>> index c7ee34013485..9740f2a0e7cd 100644
>> --- a/drivers/misc/pci_endpoint_test.c
>> +++ b/drivers/misc/pci_endpoint_test.c
>> @@ -79,6 +79,9 @@
>>  #define PCI_DEVICE_ID_RENESAS_R8A774C0		0x002d
>>  #define PCI_DEVICE_ID_RENESAS_R8A774E1		0x0025
>>  
>> +#define is_j721e_pci_dev(pdev)         \
>> +		((pdev)->device == PCI_DEVICE_ID_TI_J721E)
>> +
>>  static DEFINE_IDA(pci_endpoint_test_ida);
>>  
>>  #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \
>> @@ -810,9 +813,11 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
>>  
>>  	pci_set_master(pdev);
>>  
>> -	if (!pci_endpoint_test_alloc_irq_vectors(test, irq_type)) {
>> -		err = -EINVAL;
>> -		goto err_disable_irq;
>> +	if (!(is_am654_pci_dev(pdev) || is_j721e_pci_dev(pdev))) {
>> +		if (!pci_endpoint_test_alloc_irq_vectors(test, irq_type)) {
>> +			err = -EINVAL;
>> +			goto err_disable_irq;
>> +		}
>>  	}
>>  
>>  	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
>> @@ -850,9 +855,11 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
>>  		goto err_ida_remove;
>>  	}
>>  
>> -	if (!pci_endpoint_test_request_irq(test)) {
>> -		err = -EINVAL;
>> -		goto err_kfree_test_name;
>> +	if (!(is_am654_pci_dev(pdev) || is_j721e_pci_dev(pdev))) {
>> +		if (!pci_endpoint_test_request_irq(test)) {
>> +			err = -EINVAL;
>> +			goto err_kfree_test_name;
>> +		}
>>  	}
>>  
>>  	misc_device = &test->miscdev;
>> -- 
>> 2.17.1
>>

  reply	other threads:[~2021-08-04 14:03 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-03  7:49 [PATCH v2 0/6] PCI: Add support for J7200 and AM64 Kishon Vijay Abraham I
2021-08-03  7:49 ` [PATCH v2 1/6] PCI: cadence: Use bitfield for *quirk_retrain_flag* instead of bool Kishon Vijay Abraham I
2021-08-03  7:49 ` [PATCH v2 2/6] PCI: cadence: Add quirk flag to set minimum delay in LTSSM Detect.Quiet state Kishon Vijay Abraham I
2021-08-03 10:08   ` Lorenzo Pieralisi
2021-08-03  7:49 ` [PATCH v2 3/6] PCI: j721e: Add PCIe support for J7200 Kishon Vijay Abraham I
2021-08-03 10:14   ` Lorenzo Pieralisi
2021-08-03  7:49 ` [PATCH v2 4/6] PCI: j721e: Add PCIe support for AM64 Kishon Vijay Abraham I
2021-08-03  7:49 ` [PATCH v2 5/6] misc: pci_endpoint_test: Do not request or allocate IRQs in probe Kishon Vijay Abraham I
2021-08-03  9:58   ` Lorenzo Pieralisi
2021-08-04 14:02     ` Kishon Vijay Abraham I [this message]
2021-08-05 11:26       ` Lorenzo Pieralisi
2021-08-09  4:36         ` Kishon Vijay Abraham I
2021-08-03  7:49 ` [PATCH v2 6/6] misc: pci_endpoint_test: Add deviceID for AM64 and J7200 Kishon Vijay Abraham I
2021-08-03 10:52 ` [PATCH v2 0/6] PCI: Add support for J7200 and AM64 Lorenzo Pieralisi
2021-08-11 12:26   ` Kishon Vijay Abraham I

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=02c1ddb7-539e-20a0-1bef-e10e76922a0e@ti.com \
    --to=kishon@ti.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lokeshvutla@ti.com \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=nadeem@cadence.com \
    --cc=robh@kernel.org \
    --cc=tjoseph@cadence.com \
    --subject='Re: [PATCH v2 5/6] misc: pci_endpoint_test: Do not request or allocate IRQs in probe' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).