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

2012-07-11 Thread Liu Shengzhou-B36685


 -Original Message-
 From: Kumar Gala [mailto:ga...@kernel.crashing.org]
 Sent: Tuesday, July 10, 2012 11:50 PM
 To: Wood Scott-B07421
 Cc: Liu Shengzhou-B36685; bhelg...@google.com; Wood Scott-B07421; linuxppc-
 d...@lists.ozlabs.org; linux-...@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: bhelg...@google.com; linux-...@vger.kernel.org; linuxppc-
  d...@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@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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2012-07-10 Thread Liu Shengzhou-B36685


 -Original Message-
 From: Wood Scott-B07421
 Sent: Tuesday, July 10, 2012 12:39 AM
 To: Liu Shengzhou-B36685
 Cc: bhelg...@google.com; linux-...@vger.kernel.org; linuxppc-
 d...@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@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.
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.

  @@ -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

I guess a thinking of the original author did so is based on the point of PCIe 
spec, whether INTx is supported or not is decided by the PCI Express Interrupt 
Pin Register(Read only).

Is there the case on some other unknown platforms that dev-irq is non-zero due 
to incorrect initialization of irq allocation, but actually it doesn't support 
MSI/MSIX/INTx or any other interrupt?  
If we don't need to consider this case, we can use dev-irq directly instead of 
dev-pin to support other interrupt.

Thanks,
Shengzhou
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2012-07-10 Thread 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: bhelg...@google.com; linux-...@vger.kernel.org; linuxppc-
 d...@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@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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2012-07-10 Thread 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: bhelg...@google.com; linux-...@vger.kernel.org; linuxppc-
 d...@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@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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2012-07-09 Thread 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@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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev