[PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Shengzhou Liu]

On some platforms, in RC mode, root port has neither MSI/MSI-X nor INTx
interrupt generated, which are available only in EP mode on those platform.
In this case, we try to use other interrupt if supported (i.e. there is the
shared error interrupt on platform P1010, P3041, P4080, etc) to have AER,
Hot-plug, etc, services to work.

Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
---
 Documentation/kernel-parameters.txt |    4 ++++
 drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index a92c5eb..af97c81 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 		nomsi	Do not use MSI for native PCIe PME signaling (this makes
 			all PCIe root ports use INTx for all services).
 
+	pcie_irq=	[PCIE] Native PCIe root port interrupt options:
+		other	Try to use other interrupt when root port has
+			neither MSI/MSI-X nor INTx support.
+
 	pcmv=		[HW,PCMCIA] BadgePAD 4
 
 	pd.		[PARIDE]
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 75915b3..653679e 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -30,6 +30,17 @@ static int __init pciehp_setup(char *str)
 }
 __setup("pcie_hp=", pciehp_setup);
 
+bool port_other_interrupt_enabled;
+
+static int __init portservice_setup(char *str)
+{
+	if (!strncmp(str, "other", 5))
+		port_other_interrupt_enabled = true;
+
+	return 1;
+}
+__setup("pcie_irq=", portservice_setup);
+
 /**
  * release_pcie_device - free PCI Express port service device structure
  * @dev: Port service device to release
@@ -216,6 +227,14 @@ static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
 	if (!pci_enable_msi(dev) || dev->pin)
 		irq = dev->irq;
 
+	/*
+	 * On some platforms, root port has neither MSI/MSI-X nor INTx
+	 * interrupt support in RC mode, so try to use other interrupt(i.e.
+	 * shared interrupt if supported).
+	 */
+	else if (port_other_interrupt_enabled && dev->irq)
+		irq = dev->irq;
+
  no_msi:
 	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
 		irqs[i] = irq;
-- 
1.6.4

[PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Shengzhou Liu]

On some platforms, in RC mode, root port has neither MSI/MSI-X nor INTx
interrupt generated, which are available only in EP mode on those platform.
In this case, we try to use other interrupt if supported (i.e. there is the
shared error interrupt on platform P1010, P3041, P4080, etc) to have AER,
Hot-plug, etc, services to work.

Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
---
 Documentation/kernel-parameters.txt |    4 ++++
 drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index a92c5eb..af97c81 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 		nomsi	Do not use MSI for native PCIe PME signaling (this makes
 			all PCIe root ports use INTx for all services).
 
+	pcie_irq=	[PCIE] Native PCIe root port interrupt options:
+		other	Try to use other interrupt when root port has
+			neither MSI/MSI-X nor INTx support.
+
 	pcmv=		[HW,PCMCIA] BadgePAD 4
 
 	pd.		[PARIDE]
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 75915b3..653679e 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -30,6 +30,17 @@ static int __init pciehp_setup(char *str)
 }
 __setup("pcie_hp=", pciehp_setup);
 
+bool port_other_interrupt_enabled;
+
+static int __init portservice_setup(char *str)
+{
+	if (!strncmp(str, "other", 5))
+		port_other_interrupt_enabled = true;
+
+	return 1;
+}
+__setup("pcie_irq=", portservice_setup);
+
 /**
  * release_pcie_device - free PCI Express port service device structure
  * @dev: Port service device to release
@@ -216,6 +227,14 @@ static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
 	if (!pci_enable_msi(dev) || dev->pin)
 		irq = dev->irq;
 
+	/*
+	 * On some platforms, root port has neither MSI/MSI-X nor INTx
+	 * interrupt support in RC mode, so try to use other interrupt(i.e.
+	 * shared interrupt if supported).
+	 */
+	else if (port_other_interrupt_enabled && dev->irq)
+		irq = dev->irq;
+
  no_msi:
 	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
 		irqs[i] = irq;
-- 
1.6.4

Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Scott Wood]

On 07/09/2012 05:49 AM, Shengzhou Liu wrote:
> On some platforms, in RC mode, root port has neither MSI/MSI-X nor INTx
> interrupt generated, which are available only in EP mode on those platform.
> In this case, we try to use other interrupt if supported (i.e. there is the
> shared error interrupt on platform P1010, P3041, P4080, etc) to have AER,
> Hot-plug, etc, services to work.
> 
> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
> ---
>  Documentation/kernel-parameters.txt |    4 ++++
>  drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index a92c5eb..af97c81 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  		nomsi	Do not use MSI for native PCIe PME signaling (this makes
>  			all PCIe root ports use INTx for all services).
>  
> +	pcie_irq=	[PCIE] Native PCIe root port interrupt options:
> +		other	Try to use other interrupt when root port has
> +			neither MSI/MSI-X nor INTx support.

Why does the user need to specify this?  Shouldn't this be a matter of
communication between kernel internals?

> @@ -216,6 +227,14 @@ static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
>  	if (!pci_enable_msi(dev) || dev->pin)
>  		irq = dev->irq;
>  
> +	/*
> +	 * On some platforms, root port has neither MSI/MSI-X nor INTx
> +	 * interrupt support in RC mode, so try to use other interrupt(i.e.
> +	 * shared interrupt if supported).
> +	 */
> +	else if (port_other_interrupt_enabled && dev->irq)
> +		irq = dev->irq;

Is there any reason to not use dev->irq if it is non-zero?

-Scott

Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Scott Wood]

On 07/09/2012 05:49 AM, Shengzhou Liu wrote:
> On some platforms, in RC mode, root port has neither MSI/MSI-X nor INTx
> interrupt generated, which are available only in EP mode on those platform.
> In this case, we try to use other interrupt if supported (i.e. there is the
> shared error interrupt on platform P1010, P3041, P4080, etc) to have AER,
> Hot-plug, etc, services to work.
> 
> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
> ---
>  Documentation/kernel-parameters.txt |    4 ++++
>  drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index a92c5eb..af97c81 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  		nomsi	Do not use MSI for native PCIe PME signaling (this makes
>  			all PCIe root ports use INTx for all services).
>  
> +	pcie_irq=	[PCIE] Native PCIe root port interrupt options:
> +		other	Try to use other interrupt when root port has
> +			neither MSI/MSI-X nor INTx support.

Why does the user need to specify this?  Shouldn't this be a matter of
communication between kernel internals?

> @@ -216,6 +227,14 @@ static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
>  	if (!pci_enable_msi(dev) || dev->pin)
>  		irq = dev->irq;
>  
> +	/*
> +	 * On some platforms, root port has neither MSI/MSI-X nor INTx
> +	 * interrupt support in RC mode, so try to use other interrupt(i.e.
> +	 * shared interrupt if supported).
> +	 */
> +	else if (port_other_interrupt_enabled && dev->irq)
> +		irq = dev->irq;

Is there any reason to not use dev->irq if it is non-zero?

-Scott

RE: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Liu Shengzhou-B36685]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVHVlc2RheSwgSnVseSAxMCwgMjAxMiAxMjozOSBBTQ0KPiBUbzogTGl1IFNo
ZW5nemhvdS1CMzY2ODUNCj4gQ2M6IGJoZWxnYWFzQGdvb2dsZS5jb207IGxpbnV4LXBjaUB2Z2Vy
Lmtlcm5lbC5vcmc7IGxpbnV4cHBjLQ0KPiBkZXZAbGlzdHMub3psYWJzLm9yZw0KPiBTdWJqZWN0
OiBSZTogW1BBVENIXSBQQ0k6IEFkZCBwY2llX2lycT1vdGhlciB0byBlbmFibGUgbm9uIE1TSS9J
TlR4IGludGVycnVwdA0KPiBmb3IgcG9ydCBzZXJ2aWNlIGRyaXZlcg0KPiANCj4gT24gMDcvMDkv
MjAxMiAwNTo0OSBBTSwgU2hlbmd6aG91IExpdSB3cm90ZToNCj4gPiBPbiBzb21lIHBsYXRmb3Jt
cywgaW4gUkMgbW9kZSwgcm9vdCBwb3J0IGhhcyBuZWl0aGVyIE1TSS9NU0ktWCBub3INCj4gPiBJ
TlR4IGludGVycnVwdCBnZW5lcmF0ZWQsIHdoaWNoIGFyZSBhdmFpbGFibGUgb25seSBpbiBFUCBt
b2RlIG9uIHRob3NlDQo+IHBsYXRmb3JtLg0KPiA+IEluIHRoaXMgY2FzZSwgd2UgdHJ5IHRvIHVz
ZSBvdGhlciBpbnRlcnJ1cHQgaWYgc3VwcG9ydGVkIChpLmUuIHRoZXJlDQo+ID4gaXMgdGhlIHNo
YXJlZCBlcnJvciBpbnRlcnJ1cHQgb24gcGxhdGZvcm0gUDEwMTAsIFAzMDQxLCBQNDA4MCwgZXRj
KSB0bw0KPiA+IGhhdmUgQUVSLCBIb3QtcGx1ZywgZXRjLCBzZXJ2aWNlcyB0byB3b3JrLg0KPiA+
DQo+ID4gU2lnbmVkLW9mZi1ieTogU2hlbmd6aG91IExpdSA8U2hlbmd6aG91LkxpdUBmcmVlc2Nh
bGUuY29tPg0KPiA+IC0tLQ0KPiA+ICBEb2N1bWVudGF0aW9uL2tlcm5lbC1wYXJhbWV0ZXJzLnR4
dCB8ICAgIDQgKysrKw0KPiA+ICBkcml2ZXJzL3BjaS9wY2llL3BvcnRkcnZfY29yZS5jICAgICB8
ICAgMTkgKysrKysrKysrKysrKysrKysrKw0KPiA+ICAyIGZpbGVzIGNoYW5nZWQsIDIzIGluc2Vy
dGlvbnMoKyksIDAgZGVsZXRpb25zKC0pDQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvRG9jdW1lbnRh
dGlvbi9rZXJuZWwtcGFyYW1ldGVycy50eHQNCj4gPiBiL0RvY3VtZW50YXRpb24va2VybmVsLXBh
cmFtZXRlcnMudHh0DQo+ID4gaW5kZXggYTkyYzVlYi4uYWY5N2M4MSAxMDA2NDQNCj4gPiAtLS0g
YS9Eb2N1bWVudGF0aW9uL2tlcm5lbC1wYXJhbWV0ZXJzLnR4dA0KPiA+ICsrKyBiL0RvY3VtZW50
YXRpb24va2VybmVsLXBhcmFtZXRlcnMudHh0DQo+ID4gQEAgLTIyMTgsNiArMjIxOCwxMCBAQCBi
eXRlcyByZXNwZWN0aXZlbHkuIFN1Y2ggbGV0dGVyIHN1ZmZpeGVzIGNhbiBhbHNvIGJlDQo+IGVu
dGlyZWx5IG9taXR0ZWQuDQo+ID4gIAkJbm9tc2kJRG8gbm90IHVzZSBNU0kgZm9yIG5hdGl2ZSBQ
Q0llIFBNRSBzaWduYWxpbmcgKHRoaXMgbWFrZXMNCj4gPiAgCQkJYWxsIFBDSWUgcm9vdCBwb3J0
cyB1c2UgSU5UeCBmb3IgYWxsIHNlcnZpY2VzKS4NCj4gPg0KPiA+ICsJcGNpZV9pcnE9CVtQQ0lF
XSBOYXRpdmUgUENJZSByb290IHBvcnQgaW50ZXJydXB0IG9wdGlvbnM6DQo+ID4gKwkJb3RoZXIJ
VHJ5IHRvIHVzZSBvdGhlciBpbnRlcnJ1cHQgd2hlbiByb290IHBvcnQgaGFzDQo+ID4gKwkJCW5l
aXRoZXIgTVNJL01TSS1YIG5vciBJTlR4IHN1cHBvcnQuDQo+IA0KPiBXaHkgZG9lcyB0aGUgdXNl
ciBuZWVkIHRvIHNwZWNpZnkgdGhpcz8gIFNob3VsZG4ndCB0aGlzIGJlIGEgbWF0dGVyIG9mDQo+
IGNvbW11bmljYXRpb24gYmV0d2VlbiBrZXJuZWwgaW50ZXJuYWxzPw0KPiANCg0KVGhlICJvdGhl
ciBpbnRlcnJ1cHQiIGFwcGVhcnMgYSBub24tc3RhbmRhcmQgaW50ZXJydXB0IHdheSBjb21wYXJl
ZCB0byBNU0kvTVNJLVggYW5kIElOVHggaW4gcG9pbnQgb2YgUENJZSBzcGVjLg0KVGhlIGludGVu
dCBvZiBzcGVjaWZ5aW5nIHRoaXMgaXMgdG8gaGF2ZSBhbiBpbnRlcnZlbnRpb24gYW5kIGNvbmZp
cm1hdGlvbiBtYW51YWxseSB0byBhdm9pZCBjYXVzaW5nIHVuZXhwZWN0ZWQgaXNzdWUgb24gc29t
ZSB1bmtub3duIHBsYXRmb3Jtcy4NCkknbSBnbGFkIHRvIHJlbW92ZSB0aGUgc3BlY2lmaWVkIGtl
cm5lbCBwYXJhbWV0ZXIgaWYgaXQgd291bGQgYmUgYWNjZXB0ZWQgdXBzdHJlYW0uDQoNCj4gPiBA
QCAtMjE2LDYgKzIyNywxNCBAQCBzdGF0aWMgaW50IGluaXRfc2VydmljZV9pcnFzKHN0cnVjdCBw
Y2lfZGV2ICpkZXYsIGludA0KPiAqaXJxcywgaW50IG1hc2spDQo+ID4gIAlpZiAoIXBjaV9lbmFi
bGVfbXNpKGRldikgfHwgZGV2LT5waW4pDQo+ID4gIAkJaXJxID0gZGV2LT5pcnE7DQo+ID4NCj4g
PiArCS8qDQo+ID4gKwkgKiBPbiBzb21lIHBsYXRmb3Jtcywgcm9vdCBwb3J0IGhhcyBuZWl0aGVy
IE1TSS9NU0ktWCBub3IgSU5UeA0KPiA+ICsJICogaW50ZXJydXB0IHN1cHBvcnQgaW4gUkMgbW9k
ZSwgc28gdHJ5IHRvIHVzZSBvdGhlciBpbnRlcnJ1cHQoaS5lLg0KPiA+ICsJICogc2hhcmVkIGlu
dGVycnVwdCBpZiBzdXBwb3J0ZWQpLg0KPiA+ICsJICovDQo+ID4gKwllbHNlIGlmIChwb3J0X290
aGVyX2ludGVycnVwdF9lbmFibGVkICYmIGRldi0+aXJxKQ0KPiA+ICsJCWlycSA9IGRldi0+aXJx
Ow0KPiANCj4gSXMgdGhlcmUgYW55IHJlYXNvbiB0byBub3QgdXNlIGRldi0+aXJxIGlmIGl0IGlz
IG5vbi16ZXJvPw0KPiANCj4gLVNjb3R0DQoNCkkgZ3Vlc3MgYSB0aGlua2luZyBvZiB0aGUgb3Jp
Z2luYWwgYXV0aG9yIGRpZCBzbyBpcyBiYXNlZCBvbiB0aGUgcG9pbnQgb2YgUENJZSBzcGVjLCB3
aGV0aGVyIElOVHggaXMgc3VwcG9ydGVkIG9yIG5vdCBpcyBkZWNpZGVkIGJ5IHRoZSAiUENJIEV4
cHJlc3MgSW50ZXJydXB0IFBpbiBSZWdpc3RlcihSZWFkIG9ubHkpIi4NCg0KSXMgdGhlcmUgdGhl
IGNhc2Ugb24gc29tZSBvdGhlciB1bmtub3duIHBsYXRmb3JtcyB0aGF0IGRldi0+aXJxIGlzIG5v
bi16ZXJvIGR1ZSB0byBpbmNvcnJlY3QgaW5pdGlhbGl6YXRpb24gb2YgaXJxIGFsbG9jYXRpb24s
IGJ1dCBhY3R1YWxseSBpdCBkb2Vzbid0IHN1cHBvcnQgTVNJL01TSVgvSU5UeCBvciBhbnkgb3Ro
ZXIgaW50ZXJydXB0PyAgDQpJZiB3ZSBkb24ndCBuZWVkIHRvIGNvbnNpZGVyIHRoaXMgY2FzZSwg
d2UgY2FuIHVzZSBkZXYtPmlycSBkaXJlY3RseSBpbnN0ZWFkIG9mIGRldi0+cGluIHRvIHN1cHBv
cnQgIm90aGVyIGludGVycnVwdCIuDQoNClRoYW5rcywNClNoZW5nemhvdQ0K

Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Scott Wood]

On 07/10/2012 01:13 AM, Liu Shengzhou-B36685 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Tuesday, July 10, 2012 12:39 AM
>> To: Liu Shengzhou-B36685
>> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linuxppc-
>> dev@lists.ozlabs.org
>> Subject: Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt
>> for port service driver
>>
>> On 07/09/2012 05:49 AM, Shengzhou Liu wrote:
>>> On some platforms, in RC mode, root port has neither MSI/MSI-X nor
>>> INTx interrupt generated, which are available only in EP mode on those
>> platform.
>>> In this case, we try to use other interrupt if supported (i.e. there
>>> is the shared error interrupt on platform P1010, P3041, P4080, etc) to
>>> have AER, Hot-plug, etc, services to work.
>>>
>>> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
>>> ---
>>>  Documentation/kernel-parameters.txt |    4 ++++
>>>  drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
>>>  2 files changed, 23 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/kernel-parameters.txt
>>> b/Documentation/kernel-parameters.txt
>>> index a92c5eb..af97c81 100644
>>> --- a/Documentation/kernel-parameters.txt
>>> +++ b/Documentation/kernel-parameters.txt
>>> @@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can also be
>> entirely omitted.
>>>  		nomsi	Do not use MSI for native PCIe PME signaling (this makes
>>>  			all PCIe root ports use INTx for all services).
>>>
>>> +	pcie_irq=	[PCIE] Native PCIe root port interrupt options:
>>> +		other	Try to use other interrupt when root port has
>>> +			neither MSI/MSI-X nor INTx support.
>>
>> Why does the user need to specify this?  Shouldn't this be a matter of
>> communication between kernel internals?
>>
> 
> The "other interrupt" appears a non-standard interrupt way compared to MSI/MSI-X and INTx in point of PCIe spec.

It still shouldn't be the user's responsibility to pass this in.

> The intent of specifying this is to have an intervention and
> confirmation manually to avoid causing unexpected issue on some
> unknown platforms.
>
> I'm glad to remove the specified kernel parameter if it would be accepted upstream.

Hopefully someone will comment if there is harm in doing this
unconditionally.  If there is, then we should handle this via a quirk or
similar mechanism.

-Scott

Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Scott Wood]

On 07/10/2012 01:13 AM, Liu Shengzhou-B36685 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Tuesday, July 10, 2012 12:39 AM
>> To: Liu Shengzhou-B36685
>> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linuxppc-
>> dev@lists.ozlabs.org
>> Subject: Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt
>> for port service driver
>>
>> On 07/09/2012 05:49 AM, Shengzhou Liu wrote:
>>> On some platforms, in RC mode, root port has neither MSI/MSI-X nor
>>> INTx interrupt generated, which are available only in EP mode on those
>> platform.
>>> In this case, we try to use other interrupt if supported (i.e. there
>>> is the shared error interrupt on platform P1010, P3041, P4080, etc) to
>>> have AER, Hot-plug, etc, services to work.
>>>
>>> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
>>> ---
>>>  Documentation/kernel-parameters.txt |    4 ++++
>>>  drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
>>>  2 files changed, 23 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/kernel-parameters.txt
>>> b/Documentation/kernel-parameters.txt
>>> index a92c5eb..af97c81 100644
>>> --- a/Documentation/kernel-parameters.txt
>>> +++ b/Documentation/kernel-parameters.txt
>>> @@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can also be
>> entirely omitted.
>>>  		nomsi	Do not use MSI for native PCIe PME signaling (this makes
>>>  			all PCIe root ports use INTx for all services).
>>>
>>> +	pcie_irq=	[PCIE] Native PCIe root port interrupt options:
>>> +		other	Try to use other interrupt when root port has
>>> +			neither MSI/MSI-X nor INTx support.
>>
>> Why does the user need to specify this?  Shouldn't this be a matter of
>> communication between kernel internals?
>>
> 
> The "other interrupt" appears a non-standard interrupt way compared to MSI/MSI-X and INTx in point of PCIe spec.

It still shouldn't be the user's responsibility to pass this in.

> The intent of specifying this is to have an intervention and
> confirmation manually to avoid causing unexpected issue on some
> unknown platforms.
>
> I'm glad to remove the specified kernel parameter if it would be accepted upstream.

Hopefully someone will comment if there is harm in doing this
unconditionally.  If there is, then we should handle this via a quirk or
similar mechanism.

-Scott

Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Kumar Gala]

On Jul 10, 2012, at 10:31 AM, Scott Wood wrote:

> On 07/10/2012 01:13 AM, Liu Shengzhou-B36685 wrote:
>> 
>> 
>>> -----Original Message-----
>>> From: Wood Scott-B07421
>>> Sent: Tuesday, July 10, 2012 12:39 AM
>>> To: Liu Shengzhou-B36685
>>> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linuxppc-
>>> dev@lists.ozlabs.org
>>> Subject: Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt
>>> for port service driver
>>> 
>>> On 07/09/2012 05:49 AM, Shengzhou Liu wrote:
>>>> On some platforms, in RC mode, root port has neither MSI/MSI-X nor
>>>> INTx interrupt generated, which are available only in EP mode on those
>>> platform.
>>>> In this case, we try to use other interrupt if supported (i.e. there
>>>> is the shared error interrupt on platform P1010, P3041, P4080, etc) to
>>>> have AER, Hot-plug, etc, services to work.
>>>> 
>>>> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
>>>> ---
>>>> Documentation/kernel-parameters.txt |    4 ++++
>>>> drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
>>>> 2 files changed, 23 insertions(+), 0 deletions(-)
>>>> 
>>>> diff --git a/Documentation/kernel-parameters.txt
>>>> b/Documentation/kernel-parameters.txt
>>>> index a92c5eb..af97c81 100644
>>>> --- a/Documentation/kernel-parameters.txt
>>>> +++ b/Documentation/kernel-parameters.txt
>>>> @@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can also be
>>> entirely omitted.
>>>> 		nomsi	Do not use MSI for native PCIe PME signaling (this makes
>>>> 			all PCIe root ports use INTx for all services).
>>>> 
>>>> +	pcie_irq=	[PCIE] Native PCIe root port interrupt options:
>>>> +		other	Try to use other interrupt when root port has
>>>> +			neither MSI/MSI-X nor INTx support.
>>> 
>>> Why does the user need to specify this?  Shouldn't this be a matter of
>>> communication between kernel internals?
>>> 
>> 
>> The "other interrupt" appears a non-standard interrupt way compared to MSI/MSI-X and INTx in point of PCIe spec.
> 
> It still shouldn't be the user's responsibility to pass this in.

I agree with Scott, this should be done in board code or as a PCI fixup, not as a command line arg.

>> The intent of specifying this is to have an intervention and
>> confirmation manually to avoid causing unexpected issue on some
>> unknown platforms.
>> 
>> I'm glad to remove the specified kernel parameter if it would be accepted upstream.
> 
> Hopefully someone will comment if there is harm in doing this
> unconditionally.  If there is, then we should handle this via a quirk or
> similar mechanism.
> 
> -Scott

If we need to do it via quirk, wondering if we can use the pci_dev_flags and set the flag in a quirk.

- k

Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Kumar Gala]

On Jul 10, 2012, at 10:31 AM, Scott Wood wrote:

> On 07/10/2012 01:13 AM, Liu Shengzhou-B36685 wrote:
>>=20
>>=20
>>> -----Original Message-----
>>> From: Wood Scott-B07421
>>> Sent: Tuesday, July 10, 2012 12:39 AM
>>> To: Liu Shengzhou-B36685
>>> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linuxppc-
>>> dev@lists.ozlabs.org
>>> Subject: Re: [PATCH] PCI: Add pcie_irq=3Dother to enable non =
MSI/INTx interrupt
>>> for port service driver
>>>=20
>>> On 07/09/2012 05:49 AM, Shengzhou Liu wrote:
>>>> On some platforms, in RC mode, root port has neither MSI/MSI-X nor
>>>> INTx interrupt generated, which are available only in EP mode on =
those
>>> platform.
>>>> In this case, we try to use other interrupt if supported (i.e. =
there
>>>> is the shared error interrupt on platform P1010, P3041, P4080, etc) =
to
>>>> have AER, Hot-plug, etc, services to work.
>>>>=20
>>>> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
>>>> ---
>>>> Documentation/kernel-parameters.txt |    4 ++++
>>>> drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
>>>> 2 files changed, 23 insertions(+), 0 deletions(-)
>>>>=20
>>>> diff --git a/Documentation/kernel-parameters.txt
>>>> b/Documentation/kernel-parameters.txt
>>>> index a92c5eb..af97c81 100644
>>>> --- a/Documentation/kernel-parameters.txt
>>>> +++ b/Documentation/kernel-parameters.txt
>>>> @@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can =
also be
>>> entirely omitted.
>>>> 		nomsi	Do not use MSI for native PCIe PME signaling =
(this makes
>>>> 			all PCIe root ports use INTx for all services).
>>>>=20
>>>> +	pcie_irq=3D	[PCIE] Native PCIe root port interrupt options:
>>>> +		other	Try to use other interrupt when root port has
>>>> +			neither MSI/MSI-X nor INTx support.
>>>=20
>>> Why does the user need to specify this?  Shouldn't this be a matter =
of
>>> communication between kernel internals?
>>>=20
>>=20
>> The "other interrupt" appears a non-standard interrupt way compared =
to MSI/MSI-X and INTx in point of PCIe spec.
>=20
> It still shouldn't be the user's responsibility to pass this in.

I agree with Scott, this should be done in board code or as a PCI fixup, =
not as a command line arg.

>> The intent of specifying this is to have an intervention and
>> confirmation manually to avoid causing unexpected issue on some
>> unknown platforms.
>>=20
>> I'm glad to remove the specified kernel parameter if it would be =
accepted upstream.
>=20
> Hopefully someone will comment if there is harm in doing this
> unconditionally.  If there is, then we should handle this via a quirk =
or
> similar mechanism.
>=20
> -Scott

If we need to do it via quirk, wondering if we can use the pci_dev_flags =
and set the flag in a quirk.

- k=

RE: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Liu Shengzhou-B36685]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, July 10, 2012 11:50 PM
> To: Wood Scott-B07421
> Cc: Liu Shengzhou-B36685; bhelgaas@google.com; Wood Scott-B07421; linuxppc-
> dev@lists.ozlabs.org; linux-pci@vger.kernel.org
> Subject: Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt
> for port service driver
> 
> 
> On Jul 10, 2012, at 10:31 AM, Scott Wood wrote:
> 
> > On 07/10/2012 01:13 AM, Liu Shengzhou-B36685 wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Wood Scott-B07421
> >>> Sent: Tuesday, July 10, 2012 12:39 AM
> >>> To: Liu Shengzhou-B36685
> >>> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linuxppc-
> >>> dev@lists.ozlabs.org
> >>> Subject: Re: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx
> >>> interrupt for port service driver
> >>>
> >>> On 07/09/2012 05:49 AM, Shengzhou Liu wrote:
> >>>> On some platforms, in RC mode, root port has neither MSI/MSI-X nor
> >>>> INTx interrupt generated, which are available only in EP mode on
> >>>> those
> >>> platform.
> >>>> In this case, we try to use other interrupt if supported (i.e.
> >>>> there is the shared error interrupt on platform P1010, P3041,
> >>>> P4080, etc) to have AER, Hot-plug, etc, services to work.
> >>>>
> >>>> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
> >>>> ---
> >>>> Documentation/kernel-parameters.txt |    4 ++++
> >>>> drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
> >>>> 2 files changed, 23 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/kernel-parameters.txt
> >>>> b/Documentation/kernel-parameters.txt
> >>>> index a92c5eb..af97c81 100644
> >>>> --- a/Documentation/kernel-parameters.txt
> >>>> +++ b/Documentation/kernel-parameters.txt
> >>>> @@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can
> >>>> also be
> >>> entirely omitted.
> >>>> 		nomsi	Do not use MSI for native PCIe PME signaling (this makes
> >>>> 			all PCIe root ports use INTx for all services).
> >>>>
> >>>> +	pcie_irq=	[PCIE] Native PCIe root port interrupt options:
> >>>> +		other	Try to use other interrupt when root port has
> >>>> +			neither MSI/MSI-X nor INTx support.
> >>>
> >>> Why does the user need to specify this?  Shouldn't this be a matter
> >>> of communication between kernel internals?
> >>>
> >>
> >> The "other interrupt" appears a non-standard interrupt way compared to
> MSI/MSI-X and INTx in point of PCIe spec.
> >
> > It still shouldn't be the user's responsibility to pass this in.
> 
> I agree with Scott, this should be done in board code or as a PCI fixup, not
> as a command line arg.
> 
> >> The intent of specifying this is to have an intervention and
> >> confirmation manually to avoid causing unexpected issue on some
> >> unknown platforms.
> >>
> >> I'm glad to remove the specified kernel parameter if it would be accepted
> upstream.
> >
> > Hopefully someone will comment if there is harm in doing this
> > unconditionally.  If there is, then we should handle this via a quirk
> > or similar mechanism.
> >
> > -Scott
> 
> If we need to do it via quirk, wondering if we can use the pci_dev_flags and
> set the flag in a quirk.
> 
> - k
 
Yes, it's better to do that with quirt way, I'm going to submit a new patch with that way.

Thanks,
Shengzhou

RE: [PATCH] PCI: Add pcie_irq=other to enable non MSI/INTx interrupt for port service driver

[Liu Shengzhou-B36685]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, July 10, 2012 11:50 PM
> To: Wood Scott-B07421
> Cc: Liu Shengzhou-B36685; bhelgaas@google.com; Wood Scott-B07421; linuxpp=
c-
> dev@lists.ozlabs.org; linux-pci@vger.kernel.org
> Subject: Re: [PATCH] PCI: Add pcie_irq=3Dother to enable non MSI/INTx int=
errupt
> for port service driver
>=20
>=20
> On Jul 10, 2012, at 10:31 AM, Scott Wood wrote:
>=20
> > On 07/10/2012 01:13 AM, Liu Shengzhou-B36685 wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Wood Scott-B07421
> >>> Sent: Tuesday, July 10, 2012 12:39 AM
> >>> To: Liu Shengzhou-B36685
> >>> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linuxppc-
> >>> dev@lists.ozlabs.org
> >>> Subject: Re: [PATCH] PCI: Add pcie_irq=3Dother to enable non MSI/INTx
> >>> interrupt for port service driver
> >>>
> >>> On 07/09/2012 05:49 AM, Shengzhou Liu wrote:
> >>>> On some platforms, in RC mode, root port has neither MSI/MSI-X nor
> >>>> INTx interrupt generated, which are available only in EP mode on
> >>>> those
> >>> platform.
> >>>> In this case, we try to use other interrupt if supported (i.e.
> >>>> there is the shared error interrupt on platform P1010, P3041,
> >>>> P4080, etc) to have AER, Hot-plug, etc, services to work.
> >>>>
> >>>> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
> >>>> ---
> >>>> Documentation/kernel-parameters.txt |    4 ++++
> >>>> drivers/pci/pcie/portdrv_core.c     |   19 +++++++++++++++++++
> >>>> 2 files changed, 23 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/kernel-parameters.txt
> >>>> b/Documentation/kernel-parameters.txt
> >>>> index a92c5eb..af97c81 100644
> >>>> --- a/Documentation/kernel-parameters.txt
> >>>> +++ b/Documentation/kernel-parameters.txt
> >>>> @@ -2218,6 +2218,10 @@ bytes respectively. Such letter suffixes can
> >>>> also be
> >>> entirely omitted.
> >>>> 		nomsi	Do not use MSI for native PCIe PME signaling (this makes
> >>>> 			all PCIe root ports use INTx for all services).
> >>>>
> >>>> +	pcie_irq=3D	[PCIE] Native PCIe root port interrupt options:
> >>>> +		other	Try to use other interrupt when root port has
> >>>> +			neither MSI/MSI-X nor INTx support.
> >>>
> >>> Why does the user need to specify this?  Shouldn't this be a matter
> >>> of communication between kernel internals?
> >>>
> >>
> >> The "other interrupt" appears a non-standard interrupt way compared to
> MSI/MSI-X and INTx in point of PCIe spec.
> >
> > It still shouldn't be the user's responsibility to pass this in.
>=20
> I agree with Scott, this should be done in board code or as a PCI fixup, =
not
> as a command line arg.
>=20
> >> The intent of specifying this is to have an intervention and
> >> confirmation manually to avoid causing unexpected issue on some
> >> unknown platforms.
> >>
> >> I'm glad to remove the specified kernel parameter if it would be accep=
ted
> upstream.
> >
> > Hopefully someone will comment if there is harm in doing this
> > unconditionally.  If there is, then we should handle this via a quirk
> > or similar mechanism.
> >
> > -Scott
>=20
> If we need to do it via quirk, wondering if we can use the pci_dev_flags =
and
> set the flag in a quirk.
>=20
> - k
=20
Yes, it's better to do that with quirt way, I'm going to submit a new patch=
 with that way.

Thanks,
Shengzhou