Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1485655 > unrolled thread

Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none MSI/MSI-X/INTx mode

Started byShawn Guo <shawnguo@kernel.org>
First post2016-09-18 03:00 +0200
Last post2016-09-26 12:10 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none  MSI/MSI-X/INTx mode Shawn Guo <shawnguo@kernel.org> - 2016-09-18 03:00 +0200
    RE: [PATCH v5 3/3] pci:aer: add support aer interrupt with none  MSI/MSI-X/INTx mode Po Liu <po.liu@nxp.com> - 2016-09-18 05:40 +0200
      Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none  MSI/MSI-X/INTx mode Shawn Guo <shawnguo@kernel.org> - 2016-09-20 14:50 +0200
        RE: [PATCH v5 3/3] pci:aer: add support aer interrupt with none  MSI/MSI-X/INTx mode Po Liu <po.liu@nxp.com> - 2016-09-21 13:30 +0200
          Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none  MSI/MSI-X/INTx mode Shawn Guo <shawnguo@kernel.org> - 2016-10-01 00:20 +0200
      Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none  MSI/MSI-X/INTx mode Rob Herring <robh@kernel.org> - 2016-09-23 15:10 +0200
        RE: [PATCH v5 3/3] pci:aer: add support aer interrupt with none  MSI/MSI-X/INTx mode Po Liu <po.liu@nxp.com> - 2016-09-26 12:10 +0200

#1485655 — Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none MSI/MSI-X/INTx mode

FromShawn Guo <shawnguo@kernel.org>
Date2016-09-18 03:00 +0200
SubjectRe: [PATCH v5 3/3] pci:aer: add support aer interrupt with none MSI/MSI-X/INTx mode
Message-ID<siyCB-17w-1@gated-at.bofh.it>
On Tue, Sep 13, 2016 at 12:40:59PM +0800, Po Liu wrote:
> On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> maybe there is interrupt line for aer pme etc. Search the interrupt
> number in the fdt file. Then fixup the dev->irq with it.
> 
> Signed-off-by: Po Liu <po.liu@nxp.com>

Will the new kernel work with existing/old DTB?  I'm trying to
understand the dependency between driver and DTS changes.

Shawn

> ---
> changes for v5:
> 	- Add clear 'aer' interrup-names description
> 
>  .../devicetree/bindings/pci/layerscape-pci.txt     | 11 +++++---
>  drivers/pci/pcie/portdrv_core.c                    | 31 +++++++++++++++++++---
>  2 files changed, 35 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 41e9f55..101d0a7 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -18,8 +18,10 @@ Required properties:
>  - reg: base addresses and lengths of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> -- interrupt-names: Must include the following entries:
> -  "intr": The interrupt that is asserted for controller interrupts
> +- interrupt-names: It may be include the following entries:
> +  "aer": The interrupt that is asserted for aer interrupt
> +  "pme": The interrupt that is asserted for pme interrupt
> +  ......
>  - fsl,pcie-scfg: Must include two entries.
>    The first entry must be a link to the SCFG device node
>    The second entry must be '0' or '1' based on physical PCIe controller index.
> @@ -35,8 +37,9 @@ Example:
>  		reg = <0x00 0x03400000 0x0 0x00010000   /* controller registers */
>  		       0x40 0x00000000 0x0 0x00002000>; /* configuration space */
>  		reg-names = "regs", "config";
> -		interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* controller interrupt */
> -		interrupt-names = "intr";
> +		interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, /* aer interrupt */
> +			<GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* pme interrupt */
> +		interrupt-names = "aer", "pme";
>  		fsl,pcie-scfg = <&scfg 0>;
>  		#address-cells = <3>;
>  		#size-cells = <2>;
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index e9270b4..7c4943d 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -16,6 +16,7 @@
>  #include <linux/slab.h>
>  #include <linux/pcieport_if.h>
>  #include <linux/aer.h>
> +#include <linux/of_irq.h>
>  
>  #include "../pci.h"
>  #include "portdrv.h"
> @@ -200,6 +201,28 @@ static int pcie_port_enable_msix(struct pci_dev *dev, int *vectors, int mask)
>  static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
>  {
>  	int i, irq = -1;
> +	int ret;
> +	struct device_node *np = NULL;
> +
> +	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
> +		irqs[i] = 0;
> +
> +	if (dev->bus->dev.of_node)
> +		np = dev->bus->dev.of_node;
> +
> +	/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
> +	 * request irq for aer
> +	 */
> +	if (IS_ENABLED(CONFIG_OF_IRQ) && np &&
> +			(mask & PCIE_PORT_SERVICE_PME)) {
> +		ret = of_irq_get_byname(np, "aer");
> +		if (ret > 0) {
> +			irqs[PCIE_PORT_SERVICE_AER_SHIFT] = ret;
> +			if (dev->irq)
> +				irq = dev->irq;
> +			goto no_msi;
> +		}
> +	}
>  
>  	/*
>  	 * If MSI cannot be used for PCIe PME or hotplug, we have to use
> @@ -225,11 +248,13 @@ static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
>  		irq = dev->irq;
>  
>   no_msi:
> -	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
> -		irqs[i] = irq;
> +	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++) {
> +		if (!irqs[i])
> +			irqs[i] = irq;
> +	}
>  	irqs[PCIE_PORT_SERVICE_VC_SHIFT] = -1;
>  
> -	if (irq < 0)
> +	if (irq < 0 && irqs[PCIE_PORT_SERVICE_AER_SHIFT] < 0)
>  		return -ENODEV;
>  	return 0;
>  }
> -- 
> 2.1.0.27.g96db324
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[toc] | [next] | [standalone]


#1485681

FromPo Liu <po.liu@nxp.com>
Date2016-09-18 05:40 +0200
Message-ID<siB7r-2Pf-3@gated-at.bofh.it>
In reply to#1485655
Hi Shawn,


>  -----Original Message-----
>  From: Shawn Guo [mailto:shawnguo@kernel.org]
>  Sent: Sunday, September 18, 2016 8:52 AM
>  To: Po Liu
>  Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>  linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Roy Zang; Arnd
>  Bergmann; Marc Zyngier; Stuart Yoder; Leo Li; M.H. Lian; Murali
>  Karicheri; Bjorn Helgaas; Mingkai Hu
>  Subject: Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none
>  MSI/MSI-X/INTx mode
>  
>  On Tue, Sep 13, 2016 at 12:40:59PM +0800, Po Liu wrote:
>  > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
>  > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
>  > maybe there is interrupt line for aer pme etc. Search the interrupt
>  > number in the fdt file. Then fixup the dev->irq with it.
>  >
>  > Signed-off-by: Po Liu <po.liu@nxp.com>
>  
>  Will the new kernel work with existing/old DTB?  I'm trying to
>  understand the dependency between driver and DTS changes.

Yes, We've never use name 'intr' before. So we remove it is ok. 
'aer' is a dts name for researching it's true interrupt number by kernel. This patch is first time to use name 'aer'. So it must be compatible with existing/old DTB.

>  
>  Shawn
>  
>  > ---
>  > changes for v5:
>  > 	- Add clear 'aer' interrup-names description
>  >
>  >  .../devicetree/bindings/pci/layerscape-pci.txt     | 11 +++++---
>  >  drivers/pci/pcie/portdrv_core.c                    | 31
>  +++++++++++++++++++---
>  >  2 files changed, 35 insertions(+), 7 deletions(-)
>  >
>  > diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > index 41e9f55..101d0a7 100644
>  > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > @@ -18,8 +18,10 @@ Required properties:
>  >  - reg: base addresses and lengths of the PCIe controller
>  >  - interrupts: A list of interrupt outputs of the controller. Must
>  contain an
>  >    entry for each entry in the interrupt-names property.
>  > -- interrupt-names: Must include the following entries:
>  > -  "intr": The interrupt that is asserted for controller interrupts
>  > +- interrupt-names: It may be include the following entries:
>  > +  "aer": The interrupt that is asserted for aer interrupt
>  > +  "pme": The interrupt that is asserted for pme interrupt
>  > +  ......
>  >  - fsl,pcie-scfg: Must include two entries.
>  >    The first entry must be a link to the SCFG device node
>  >    The second entry must be '0' or '1' based on physical PCIe
>  controller index.
>  > @@ -35,8 +37,9 @@ Example:
>  >  		reg = <0x00 0x03400000 0x0 0x00010000   /* controller
>  registers */
>  >  		       0x40 0x00000000 0x0 0x00002000>; /* configuration
>  space */
>  >  		reg-names = "regs", "config";
>  > -		interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /*
>  controller interrupt */
>  > -		interrupt-names = "intr";
>  > +		interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, /* aer
>  interrupt */
>  > +			<GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* pme interrupt */
>  > +		interrupt-names = "aer", "pme";
>  >  		fsl,pcie-scfg = <&scfg 0>;
>  >  		#address-cells = <3>;
>  >  		#size-cells = <2>;
>  > diff --git a/drivers/pci/pcie/portdrv_core.c
>  > b/drivers/pci/pcie/portdrv_core.c index e9270b4..7c4943d 100644
>  > --- a/drivers/pci/pcie/portdrv_core.c
>  > +++ b/drivers/pci/pcie/portdrv_core.c
>  > @@ -16,6 +16,7 @@
>  >  #include <linux/slab.h>
>  >  #include <linux/pcieport_if.h>
>  >  #include <linux/aer.h>
>  > +#include <linux/of_irq.h>
>  >
>  >  #include "../pci.h"
>  >  #include "portdrv.h"
>  > @@ -200,6 +201,28 @@ static int pcie_port_enable_msix(struct pci_dev
>  > *dev, int *vectors, int mask)  static int init_service_irqs(struct
>  > pci_dev *dev, int *irqs, int mask)  {
>  >  	int i, irq = -1;
>  > +	int ret;
>  > +	struct device_node *np = NULL;
>  > +
>  > +	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
>  > +		irqs[i] = 0;
>  > +
>  > +	if (dev->bus->dev.of_node)
>  > +		np = dev->bus->dev.of_node;
>  > +
>  > +	/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
>  > +	 * request irq for aer
>  > +	 */
>  > +	if (IS_ENABLED(CONFIG_OF_IRQ) && np &&
>  > +			(mask & PCIE_PORT_SERVICE_PME)) {
>  > +		ret = of_irq_get_byname(np, "aer");
>  > +		if (ret > 0) {
>  > +			irqs[PCIE_PORT_SERVICE_AER_SHIFT] = ret;
>  > +			if (dev->irq)
>  > +				irq = dev->irq;
>  > +			goto no_msi;
>  > +		}
>  > +	}
>  >
>  >  	/*
>  >  	 * If MSI cannot be used for PCIe PME or hotplug, we have to use
>  @@
>  > -225,11 +248,13 @@ static int init_service_irqs(struct pci_dev *dev,
>  int *irqs, int mask)
>  >  		irq = dev->irq;
>  >
>  >   no_msi:
>  > -	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
>  > -		irqs[i] = irq;
>  > +	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++) {
>  > +		if (!irqs[i])
>  > +			irqs[i] = irq;
>  > +	}
>  >  	irqs[PCIE_PORT_SERVICE_VC_SHIFT] = -1;
>  >
>  > -	if (irq < 0)
>  > +	if (irq < 0 && irqs[PCIE_PORT_SERVICE_AER_SHIFT] < 0)
>  >  		return -ENODEV;
>  >  	return 0;
>  >  }
>  > --
>  > 2.1.0.27.g96db324
>  >
>  >
>  > _______________________________________________
>  > linux-arm-kernel mailing list
>  > linux-arm-kernel@lists.infradead.org
>  > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[toc] | [prev] | [next] | [standalone]


#1487297

FromShawn Guo <shawnguo@kernel.org>
Date2016-09-20 14:50 +0200
Message-ID<sjsEN-31h-5@gated-at.bofh.it>
In reply to#1485681
On Sun, Sep 18, 2016 at 03:37:27AM +0000, Po Liu wrote:
> Hi Shawn,
> 
> 
> >  -----Original Message-----
> >  From: Shawn Guo [mailto:shawnguo@kernel.org]
> >  Sent: Sunday, September 18, 2016 8:52 AM
> >  To: Po Liu
> >  Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >  linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Roy Zang; Arnd
> >  Bergmann; Marc Zyngier; Stuart Yoder; Leo Li; M.H. Lian; Murali
> >  Karicheri; Bjorn Helgaas; Mingkai Hu
> >  Subject: Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none
> >  MSI/MSI-X/INTx mode
> >  
> >  On Tue, Sep 13, 2016 at 12:40:59PM +0800, Po Liu wrote:
> >  > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> >  > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> >  > maybe there is interrupt line for aer pme etc. Search the interrupt
> >  > number in the fdt file. Then fixup the dev->irq with it.
> >  >
> >  > Signed-off-by: Po Liu <po.liu@nxp.com>
> >  
> >  Will the new kernel work with existing/old DTB?  I'm trying to
> >  understand the dependency between driver and DTS changes.
> 
> Yes, We've never use name 'intr' before. So we remove it is ok. 
> 'aer' is a dts name for researching it's true interrupt number by kernel. This patch is first time to use name 'aer'. So it must be compatible with existing/old DTB.

Does that mean driver and DTS changes can go through separate trees,
i.e. PCI and arm-soc, without introducing regressions on either tree?
Or does the patch series needs to go in as a whole?

Shawn

[toc] | [prev] | [next] | [standalone]


#1488052

FromPo Liu <po.liu@nxp.com>
Date2016-09-21 13:30 +0200
Message-ID<sjNSV-8oB-3@gated-at.bofh.it>
In reply to#1487297
Hi Shawn,


>  -----Original Message-----
>  From: Shawn Guo [mailto:shawnguo@kernel.org]
>  Sent: Tuesday, September 20, 2016 8:39 PM
>  To: Po Liu
>  Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>  linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Roy Zang; Arnd
>  Bergmann; Marc Zyngier; Stuart Yoder; Leo Li; M.H. Lian; Murali
>  Karicheri; Bjorn Helgaas; Mingkai Hu
>  Subject: Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none
>  MSI/MSI-X/INTx mode
>  
>  On Sun, Sep 18, 2016 at 03:37:27AM +0000, Po Liu wrote:
>  > Hi Shawn,
>  >
>  >
>  > >  -----Original Message-----
>  > >  From: Shawn Guo [mailto:shawnguo@kernel.org]
>  > >  Sent: Sunday, September 18, 2016 8:52 AM
>  > >  To: Po Liu
>  > >  Cc: linux-pci@vger.kernel.org;
>  > > linux-arm-kernel@lists.infradead.org;
>  > >  linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Roy Zang;
>  > > Arnd  Bergmann; Marc Zyngier; Stuart Yoder; Leo Li; M.H. Lian;
>  > > Murali  Karicheri; Bjorn Helgaas; Mingkai Hu
>  > >  Subject: Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with
>  > > none  MSI/MSI-X/INTx mode
>  > >
>  > >  On Tue, Sep 13, 2016 at 12:40:59PM +0800, Po Liu wrote:
>  > >  > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC
>  mode.
>  > >  > When chip support the aer interrupt with none MSI/MSI-X/INTx
>  > > mode,  > maybe there is interrupt line for aer pme etc. Search the
>  > > interrupt  > number in the fdt file. Then fixup the dev->irq with it.
>  > >  >
>  > >  > Signed-off-by: Po Liu <po.liu@nxp.com>
>  > >
>  > >  Will the new kernel work with existing/old DTB?  I'm trying to
>  > > understand the dependency between driver and DTS changes.
>  >
>  > Yes, We've never use name 'intr' before. So we remove it is ok.
>  > 'aer' is a dts name for researching it's true interrupt number by
>  kernel. This patch is first time to use name 'aer'. So it must be
>  compatible with existing/old DTB.
>  
>  Does that mean driver and DTS changes can go through separate trees, i.e.
>  PCI and arm-soc, without introducing regressions on either tree?
>  Or does the patch series needs to go in as a whole?

Should be as a whole. The driver base on the dts. Or else, the driver would not found the 'aer' point.
Thanks!

Po

>  
>  Shawn

[toc] | [prev] | [next] | [standalone]


#1494312

FromShawn Guo <shawnguo@kernel.org>
Date2016-10-01 00:20 +0200
Message-ID<snejT-819-1@gated-at.bofh.it>
In reply to#1488052
On Wed, Sep 21, 2016 at 06:54:59AM +0000, Po Liu wrote:
> >  > >  Will the new kernel work with existing/old DTB?  I'm trying to
> >  > > understand the dependency between driver and DTS changes.
> >  >
> >  > Yes, We've never use name 'intr' before. So we remove it is ok.
> >  > 'aer' is a dts name for researching it's true interrupt number by
> >  kernel. This patch is first time to use name 'aer'. So it must be
> >  compatible with existing/old DTB.
> >  
> >  Does that mean driver and DTS changes can go through separate trees, i.e.
> >  PCI and arm-soc, without introducing regressions on either tree?
> >  Or does the patch series needs to go in as a whole?
> 
> Should be as a whole. The driver base on the dts. Or else, the driver would not found the 'aer' point.

Let me try to understand this one more time.  So if the new kernel boots
with an existing DTB, the driver will fail to find 'aer' interrupt,
right?  My question is that in this case, the driver will just stop
working or keep working in the same way as old kernel.  The former case
is a regression which breaks old DTB support, and the latter is pretty
much like that we are adding a new support, and the DTS change can go
independently with driver part through different subsystem tree.

Hope I make my question clear this time.

Shawn

[toc] | [prev] | [next] | [standalone]


#1490045

FromRob Herring <robh@kernel.org>
Date2016-09-23 15:10 +0200
Message-ID<skyoN-44f-3@gated-at.bofh.it>
In reply to#1485681
On Sun, Sep 18, 2016 at 03:37:27AM +0000, Po Liu wrote:
> Hi Shawn,
> 
> 
> >  -----Original Message-----
> >  From: Shawn Guo [mailto:shawnguo@kernel.org]
> >  Sent: Sunday, September 18, 2016 8:52 AM
> >  To: Po Liu
> >  Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >  linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Roy Zang; Arnd
> >  Bergmann; Marc Zyngier; Stuart Yoder; Leo Li; M.H. Lian; Murali
> >  Karicheri; Bjorn Helgaas; Mingkai Hu
> >  Subject: Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none
> >  MSI/MSI-X/INTx mode
> >  
> >  On Tue, Sep 13, 2016 at 12:40:59PM +0800, Po Liu wrote:
> >  > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> >  > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> >  > maybe there is interrupt line for aer pme etc. Search the interrupt
> >  > number in the fdt file. Then fixup the dev->irq with it.
> >  >
> >  > Signed-off-by: Po Liu <po.liu@nxp.com>
> >  
> >  Will the new kernel work with existing/old DTB?  I'm trying to
> >  understand the dependency between driver and DTS changes.
> 
> Yes, We've never use name 'intr' before. So we remove it is ok. 
> 'aer' is a dts name for researching it's true interrupt number by 
> kernel. This patch is first time to use name 'aer'. So it must be 
> compatible with existing/old DTB.

Please explain why you are not breaking compatibility in the commit 
message. I asked for this on v2.

> >  > ---
> >  > changes for v5:
> >  > 	- Add clear 'aer' interrup-names description
> >  >
> >  >  .../devicetree/bindings/pci/layerscape-pci.txt     | 11 +++++---
> >  >  drivers/pci/pcie/portdrv_core.c                    | 31
> >  +++++++++++++++++++---
> >  >  2 files changed, 35 insertions(+), 7 deletions(-)
> >  >
> >  > diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> >  > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> >  > index 41e9f55..101d0a7 100644
> >  > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> >  > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> >  > @@ -18,8 +18,10 @@ Required properties:
> >  >  - reg: base addresses and lengths of the PCIe controller
> >  >  - interrupts: A list of interrupt outputs of the controller. Must
> >  contain an
> >  >    entry for each entry in the interrupt-names property.
> >  > -- interrupt-names: Must include the following entries:
> >  > -  "intr": The interrupt that is asserted for controller interrupts
> >  > +- interrupt-names: It may be include the following entries:

"may be" is not okay. It should be "must" or explain when an interrupt 
would not be present. Really, differences in interrupts means you need 
different compatible strings.

Rob

> >  > +  "aer": The interrupt that is asserted for aer interrupt
> >  > +  "pme": The interrupt that is asserted for pme interrupt
> >  > +  ......

[toc] | [prev] | [next] | [standalone]


#1491193

FromPo Liu <po.liu@nxp.com>
Date2016-09-26 12:10 +0200
Message-ID<slB1f-2w1-7@gated-at.bofh.it>
In reply to#1490045
Hi Rob,

>  -----Original Message-----
>  From: Rob Herring [mailto:robh@kernel.org]
>  Sent: Friday, September 23, 2016 9:06 PM
>  To: Po Liu
>  Cc: Shawn Guo; linux-pci@vger.kernel.org; linux-arm-
>  kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
>  devicetree@vger.kernel.org; Roy Zang; Arnd Bergmann; Marc Zyngier;
>  Stuart Yoder; Leo Li; M.H. Lian; Murali Karicheri; Bjorn Helgaas;
>  Mingkai Hu
>  Subject: Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none
>  MSI/MSI-X/INTx mode
>  
>  On Sun, Sep 18, 2016 at 03:37:27AM +0000, Po Liu wrote:
>  > Hi Shawn,
>  >
>  >
>  > >  -----Original Message-----
>  > >  From: Shawn Guo [mailto:shawnguo@kernel.org]
>  > >  Sent: Sunday, September 18, 2016 8:52 AM
>  > >  To: Po Liu
>  > >  Cc: linux-pci@vger.kernel.org;
>  > > linux-arm-kernel@lists.infradead.org;
>  > >  linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Roy Zang;
>  > > Arnd  Bergmann; Marc Zyngier; Stuart Yoder; Leo Li; M.H. Lian;
>  > > Murali  Karicheri; Bjorn Helgaas; Mingkai Hu
>  > >  Subject: Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with
>  > > none  MSI/MSI-X/INTx mode
>  > >
>  > >  On Tue, Sep 13, 2016 at 12:40:59PM +0800, Po Liu wrote:
>  > >  > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC
>  mode.
>  > >  > When chip support the aer interrupt with none MSI/MSI-X/INTx
>  > > mode,  > maybe there is interrupt line for aer pme etc. Search the
>  > > interrupt  > number in the fdt file. Then fixup the dev->irq with it.
>  > >  >
>  > >  > Signed-off-by: Po Liu <po.liu@nxp.com>
>  > >
>  > >  Will the new kernel work with existing/old DTB?  I'm trying to
>  > > understand the dependency between driver and DTS changes.
>  >
>  > Yes, We've never use name 'intr' before. So we remove it is ok.
>  > 'aer' is a dts name for researching it's true interrupt number by
>  > kernel. This patch is first time to use name 'aer'. So it must be
>  > compatible with existing/old DTB.
>  
>  Please explain why you are not breaking compatibility in the commit
>  message. I asked for this on v2.

Sorry, I didn't really catch what your means. Do you mean I should add why I remove the 'intr'?

>  
>  > >  > ---
>  > >  > changes for v5:
>  > >  > 	- Add clear 'aer' interrup-names description
>  > >  >
>  > >  >  .../devicetree/bindings/pci/layerscape-pci.txt     | 11 +++++---
>  > >  >  drivers/pci/pcie/portdrv_core.c                    | 31
>  > >  +++++++++++++++++++---
>  > >  >  2 files changed, 35 insertions(+), 7 deletions(-)  >  > diff
>  > > --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > >  > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > >  > index 41e9f55..101d0a7 100644
>  > >  > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > >  > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > >  > @@ -18,8 +18,10 @@ Required properties:
>  > >  >  - reg: base addresses and lengths of the PCIe controller  >  -
>  > > interrupts: A list of interrupt outputs of the controller. Must
>  > > contain an
>  > >  >    entry for each entry in the interrupt-names property.
>  > >  > -- interrupt-names: Must include the following entries:
>  > >  > -  "intr": The interrupt that is asserted for controller
>  > > interrupts  > +- interrupt-names: It may be include the following
>  entries:
>  
>  "may be" is not okay. It should be "must" or explain when an interrupt
>  would not be present. Really, differences in interrupts means you need
>  different compatible strings.

How about changing "must" to "should" or "could" and also add when to add after "aer": to explain when to add it?

Thanks!

>  
>  Rob
>  
>  > >  > +  "aer": The interrupt that is asserted for aer interrupt  > +
>  > > "pme": The interrupt that is asserted for pme interrupt  > +  ......

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web