linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] layerscape-pci binding updates
@ 2021-11-20  0:16 Li Yang
  2021-11-20  0:16 ` [PATCH 1/4] dt-bindings: pci: layerscape-pci: Add a optional property big-endian Li Yang
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Li Yang @ 2021-11-20  0:16 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, linux-pci, devicetree, linux-kernel,
	Hou Zhiqiang
  Cc: Li Yang

This series includes two binding changes from Zhiqiang's previous
submission rebased to latest 5.16-rc1:
[PATCHv5 0/6] PCI: layerscape: Add power management support

They describe the hardware and are not necessarily connected with the PM
driver changes.  The series also includes two other binding updates to
better describe the pcie hardware.

Hou Zhiqiang (2):
  dt-bindings: pci: layerscape-pci: Add a optional property big-endian
  dt-bindings: pci: layerscape-pci: Update the description of SCFG
    property

Li Yang (1):
  dt-bindings: pci: layerscape-pci: define aer/pme interrupts

Xiaowei Bao (1):
  dt-bindings: pci: layerscape-pci: Add EP mode compatible strings for
    ls1028a

 .../bindings/pci/layerscape-pci.txt           | 21 ++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] dt-bindings: pci: layerscape-pci: Add a optional property big-endian
  2021-11-20  0:16 [PATCH 0/4] layerscape-pci binding updates Li Yang
@ 2021-11-20  0:16 ` Li Yang
  2021-11-20  0:16 ` [PATCH 2/4] dt-bindings: pci: layerscape-pci: Update the description of SCFG property Li Yang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Li Yang @ 2021-11-20  0:16 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, linux-pci, devicetree, linux-kernel,
	Hou Zhiqiang
  Cc: Rob Herring

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

This property is to indicate the endianness when accessing the
PEX_LUT and PF register block, so if these registers are
implemented in big-endian, specify this property.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/pci/layerscape-pci.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
index f36efa73a470..215d2ee65c83 100644
--- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
+++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
@@ -40,6 +40,10 @@ Required properties:
   of the data transferred from/to the IP block. This can avoid the software
   cache flush/invalid actions, and improve the performance significantly.
 
+Optional properties:
+- big-endian: If the PEX_LUT and PF register block is in big-endian, specify
+  this property.
+
 Example:
 
 	pcie@3400000 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] dt-bindings: pci: layerscape-pci: Update the description of SCFG property
  2021-11-20  0:16 [PATCH 0/4] layerscape-pci binding updates Li Yang
  2021-11-20  0:16 ` [PATCH 1/4] dt-bindings: pci: layerscape-pci: Add a optional property big-endian Li Yang
@ 2021-11-20  0:16 ` Li Yang
  2021-11-20  0:16 ` [PATCH 3/4] dt-bindings: pci: layerscape-pci: Add EP mode compatible strings for ls1028a Li Yang
  2021-11-20  0:16 ` [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts Li Yang
  3 siblings, 0 replies; 11+ messages in thread
From: Li Yang @ 2021-11-20  0:16 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, linux-pci, devicetree, linux-kernel,
	Hou Zhiqiang
  Cc: Rob Herring

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

Update the description of the second entry of 'fsl,pcie-scfg' property,
as the LS1043A PCIe controller also has some control registers in SCFG
block, while it has 3 controllers.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/pci/layerscape-pci.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
index 215d2ee65c83..f1115fcd8088 100644
--- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
+++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
@@ -34,7 +34,7 @@ Required properties:
   "intr": The interrupt that is asserted for controller interrupts
 - 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.
+  The second entry is the physical PCIe controller index starting from '0'.
   This is used to get SCFG PEXN registers
 - dma-coherent: Indicates that the hardware IP block can ensure the coherency
   of the data transferred from/to the IP block. This can avoid the software
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] dt-bindings: pci: layerscape-pci: Add EP mode compatible strings for ls1028a
  2021-11-20  0:16 [PATCH 0/4] layerscape-pci binding updates Li Yang
  2021-11-20  0:16 ` [PATCH 1/4] dt-bindings: pci: layerscape-pci: Add a optional property big-endian Li Yang
  2021-11-20  0:16 ` [PATCH 2/4] dt-bindings: pci: layerscape-pci: Update the description of SCFG property Li Yang
@ 2021-11-20  0:16 ` Li Yang
  2021-11-30  1:59   ` Rob Herring
  2021-11-20  0:16 ` [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts Li Yang
  3 siblings, 1 reply; 11+ messages in thread
From: Li Yang @ 2021-11-20  0:16 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, linux-pci, devicetree, linux-kernel,
	Hou Zhiqiang
  Cc: Xiaowei Bao, Li Yang

From: Xiaowei Bao <xiaowei.bao@nxp.com>

Add EP mode compatible string for ls1028a.

Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
Signed-off-by: Li Yang <leoyang.li@nxp.com>
---
 Documentation/devicetree/bindings/pci/layerscape-pci.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
index f1115fcd8088..8fd6039a826b 100644
--- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
+++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
@@ -23,6 +23,7 @@ Required properties:
         "fsl,ls1012a-pcie"
         "fsl,ls1028a-pcie"
   EP mode:
+	"fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep"
 	"fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
 	"fsl,ls1088a-pcie-ep", "fsl,ls-pcie-ep"
 	"fsl,ls2088a-pcie-ep", "fsl,ls-pcie-ep"
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts
  2021-11-20  0:16 [PATCH 0/4] layerscape-pci binding updates Li Yang
                   ` (2 preceding siblings ...)
  2021-11-20  0:16 ` [PATCH 3/4] dt-bindings: pci: layerscape-pci: Add EP mode compatible strings for ls1028a Li Yang
@ 2021-11-20  0:16 ` Li Yang
  2021-11-30  2:02   ` Rob Herring
  2021-11-30 14:22   ` Bjorn Helgaas
  3 siblings, 2 replies; 11+ messages in thread
From: Li Yang @ 2021-11-20  0:16 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, linux-pci, devicetree, linux-kernel,
	Hou Zhiqiang
  Cc: Li Yang

Some platforms using this controller have separated interrupt lines for
aer or pme events instead of having a single interrupt line for
miscellaneous events.  Define interrupts in the binding for these
interrupt lines.

Signed-off-by: Li Yang <leoyang.li@nxp.com>
---
 .../devicetree/bindings/pci/layerscape-pci.txt     | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
index 8fd6039a826b..bcf11bfc4bab 100644
--- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
+++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
@@ -31,8 +31,13 @@ Required properties:
 - reg: base addresses and lengths of the PCIe controller register blocks.
 - 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 could include the following entries:
+  "aer": For interrupt line reporting aer events when non MSI/MSI-X/INTx mode
+		is used
+  "pme": For interrupt line reporting pme events when non MSI/MSI-X/INTx mode
+		is used
+  "intr": For interrupt line reporting miscellaneous controller events
+  ......
 - fsl,pcie-scfg: Must include two entries.
   The first entry must be a link to the SCFG device node
   The second entry is the physical PCIe controller index starting from '0'.
@@ -52,8 +57,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>;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] dt-bindings: pci: layerscape-pci: Add EP mode compatible strings for ls1028a
  2021-11-20  0:16 ` [PATCH 3/4] dt-bindings: pci: layerscape-pci: Add EP mode compatible strings for ls1028a Li Yang
@ 2021-11-30  1:59   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-11-30  1:59 UTC (permalink / raw)
  To: Li Yang
  Cc: linux-pci, Xiaowei Bao, Rob Herring, Hou Zhiqiang, devicetree,
	Bjorn Helgaas, linux-kernel

On Fri, 19 Nov 2021 18:16:20 -0600, Li Yang wrote:
> From: Xiaowei Bao <xiaowei.bao@nxp.com>
> 
> Add EP mode compatible string for ls1028a.
> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> Signed-off-by: Li Yang <leoyang.li@nxp.com>
> ---
>  Documentation/devicetree/bindings/pci/layerscape-pci.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts
  2021-11-20  0:16 ` [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts Li Yang
@ 2021-11-30  2:02   ` Rob Herring
  2021-11-30  3:35     ` Leo Li
  2021-11-30 14:22   ` Bjorn Helgaas
  1 sibling, 1 reply; 11+ messages in thread
From: Rob Herring @ 2021-11-30  2:02 UTC (permalink / raw)
  To: Li Yang; +Cc: Bjorn Helgaas, linux-pci, devicetree, linux-kernel, Hou Zhiqiang

On Fri, Nov 19, 2021 at 06:16:21PM -0600, Li Yang wrote:
> Some platforms using this controller have separated interrupt lines for
> aer or pme events instead of having a single interrupt line for
> miscellaneous events.  Define interrupts in the binding for these
> interrupt lines.
> 
> Signed-off-by: Li Yang <leoyang.li@nxp.com>
> ---
>  .../devicetree/bindings/pci/layerscape-pci.txt     | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 8fd6039a826b..bcf11bfc4bab 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -31,8 +31,13 @@ Required properties:
>  - reg: base addresses and lengths of the PCIe controller register blocks.
>  - 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 could include the following entries:
> +  "aer": For interrupt line reporting aer events when non MSI/MSI-X/INTx mode
> +		is used
> +  "pme": For interrupt line reporting pme events when non MSI/MSI-X/INTx mode
> +		is used
> +  "intr": For interrupt line reporting miscellaneous controller events
> +  ......
>  - fsl,pcie-scfg: Must include two entries.
>    The first entry must be a link to the SCFG device node
>    The second entry is the physical PCIe controller index starting from '0'.
> @@ -52,8 +57,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";

This isn't a compatible change. The h/w suddenly has no 'intr' 
interrupt?

>  		fsl,pcie-scfg = <&scfg 0>;
>  		#address-cells = <3>;
>  		#size-cells = <2>;
> -- 
> 2.25.1
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts
  2021-11-30  2:02   ` Rob Herring
@ 2021-11-30  3:35     ` Leo Li
  2021-11-30 13:47       ` Rob Herring
  0 siblings, 1 reply; 11+ messages in thread
From: Leo Li @ 2021-11-30  3:35 UTC (permalink / raw)
  To: Rob Herring; +Cc: Bjorn Helgaas, linux-pci, devicetree, linux-kernel, Z.Q. Hou



> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Monday, November 29, 2021 8:02 PM
> To: Leo Li <leoyang.li@nxp.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Z.Q. Hou
> <zhiqiang.hou@nxp.com>
> Subject: Re: [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme
> interrupts
> 
> On Fri, Nov 19, 2021 at 06:16:21PM -0600, Li Yang wrote:
> > Some platforms using this controller have separated interrupt lines
> > for aer or pme events instead of having a single interrupt line for
> > miscellaneous events.  Define interrupts in the binding for these
> > interrupt lines.
> >
> > Signed-off-by: Li Yang <leoyang.li@nxp.com>
> > ---
> >  .../devicetree/bindings/pci/layerscape-pci.txt     | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > index 8fd6039a826b..bcf11bfc4bab 100644
> > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > @@ -31,8 +31,13 @@ Required properties:
> >  - reg: base addresses and lengths of the PCIe controller register blocks.
> >  - 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 could include the following entries:
> > +  "aer": For interrupt line reporting aer events when non MSI/MSI-X/INTx
> mode
> > +		is used
> > +  "pme": For interrupt line reporting pme events when non MSI/MSI-
> X/INTx mode
> > +		is used
> > +  "intr": For interrupt line reporting miscellaneous controller
> > +events
> > +  ......
> >  - fsl,pcie-scfg: Must include two entries.
> >    The first entry must be a link to the SCFG device node
> >    The second entry is the physical PCIe controller index starting from '0'.
> > @@ -52,8 +57,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";
> 
> This isn't a compatible change. The h/w suddenly has no 'intr'
> interrupt?

The original 'intr' was just a place holder for a HW interrupt signal without a clear definition of events associated.  Some later SoC has more interrupt signals to associate with more specific events.

If needed, we can keep the "intr" interrupt-name there just for backward compatibility although it was never used in Linux.

> 
> >  		fsl,pcie-scfg = <&scfg 0>;
> >  		#address-cells = <3>;
> >  		#size-cells = <2>;
> > --
> > 2.25.1
> >
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts
  2021-11-30  3:35     ` Leo Li
@ 2021-11-30 13:47       ` Rob Herring
  2021-12-01 23:34         ` Leo Li
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2021-11-30 13:47 UTC (permalink / raw)
  To: Leo Li; +Cc: Bjorn Helgaas, linux-pci, devicetree, linux-kernel, Z.Q. Hou

On Mon, Nov 29, 2021 at 9:35 PM Leo Li <leoyang.li@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: Monday, November 29, 2021 8:02 PM
> > To: Leo Li <leoyang.li@nxp.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>; linux-pci@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Z.Q. Hou
> > <zhiqiang.hou@nxp.com>
> > Subject: Re: [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme
> > interrupts
> >
> > On Fri, Nov 19, 2021 at 06:16:21PM -0600, Li Yang wrote:
> > > Some platforms using this controller have separated interrupt lines
> > > for aer or pme events instead of having a single interrupt line for
> > > miscellaneous events.  Define interrupts in the binding for these
> > > interrupt lines.
> > >
> > > Signed-off-by: Li Yang <leoyang.li@nxp.com>
> > > ---
> > >  .../devicetree/bindings/pci/layerscape-pci.txt     | 14 ++++++++++----
> > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > > index 8fd6039a826b..bcf11bfc4bab 100644
> > > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > > @@ -31,8 +31,13 @@ Required properties:
> > >  - reg: base addresses and lengths of the PCIe controller register blocks.
> > >  - 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 could include the following entries:
> > > +  "aer": For interrupt line reporting aer events when non MSI/MSI-X/INTx
> > mode
> > > +           is used
> > > +  "pme": For interrupt line reporting pme events when non MSI/MSI-
> > X/INTx mode
> > > +           is used
> > > +  "intr": For interrupt line reporting miscellaneous controller
> > > +events
> > > +  ......
> > >  - fsl,pcie-scfg: Must include two entries.
> > >    The first entry must be a link to the SCFG device node
> > >    The second entry is the physical PCIe controller index starting from '0'.
> > > @@ -52,8 +57,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";
> >
> > This isn't a compatible change. The h/w suddenly has no 'intr'
> > interrupt?
>
> The original 'intr' was just a place holder for a HW interrupt signal without a clear definition of events associated.  Some later SoC has more interrupt signals to associate with more specific events.

'Later SoC' means new compatible, but you're not changing the
compatible. If it was just wrong for all SoCs, then state that in the
commit message. Please define all the interrupts on all SoCs, so it is
not changing again.

> If needed, we can keep the "intr" interrupt-name there just for backward compatibility although it was never used in Linux.

What about other OSs?

Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts
  2021-11-20  0:16 ` [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts Li Yang
  2021-11-30  2:02   ` Rob Herring
@ 2021-11-30 14:22   ` Bjorn Helgaas
  1 sibling, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2021-11-30 14:22 UTC (permalink / raw)
  To: Li Yang
  Cc: Bjorn Helgaas, Rob Herring, linux-pci, devicetree, linux-kernel,
	Hou Zhiqiang

On Fri, Nov 19, 2021 at 06:16:21PM -0600, Li Yang wrote:
> Some platforms using this controller have separated interrupt lines for
> aer or pme events instead of having a single interrupt line for
> miscellaneous events.  Define interrupts in the binding for these
> interrupt lines.

s/separated/separate/

In subject, commit log, and description and comments below:

s/aer/AER/
s/pme/PME/

These are acronyms, not words, and capitalizing them matches usage in
the specs.

It's OK to keep them lower-case in code-like things like variable
names and interrupt-names.

> Signed-off-by: Li Yang <leoyang.li@nxp.com>
> ---
>  .../devicetree/bindings/pci/layerscape-pci.txt     | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 8fd6039a826b..bcf11bfc4bab 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -31,8 +31,13 @@ Required properties:
>  - reg: base addresses and lengths of the PCIe controller register blocks.
>  - 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 could include the following entries:
> +  "aer": For interrupt line reporting aer events when non MSI/MSI-X/INTx mode
> +		is used
> +  "pme": For interrupt line reporting pme events when non MSI/MSI-X/INTx mode
> +		is used
> +  "intr": For interrupt line reporting miscellaneous controller events
> +  ......
>  - fsl,pcie-scfg: Must include two entries.
>    The first entry must be a link to the SCFG device node
>    The second entry is the physical PCIe controller index starting from '0'.
> @@ -52,8 +57,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>;
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts
  2021-11-30 13:47       ` Rob Herring
@ 2021-12-01 23:34         ` Leo Li
  0 siblings, 0 replies; 11+ messages in thread
From: Leo Li @ 2021-12-01 23:34 UTC (permalink / raw)
  To: Rob Herring; +Cc: Bjorn Helgaas, linux-pci, devicetree, linux-kernel, Z.Q. Hou



> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, November 30, 2021 7:47 AM
> To: Leo Li <leoyang.li@nxp.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Z.Q. Hou
> <zhiqiang.hou@nxp.com>
> Subject: Re: [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme
> interrupts
> 
> On Mon, Nov 29, 2021 at 9:35 PM Leo Li <leoyang.li@nxp.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Rob Herring <robh@kernel.org>
> > > Sent: Monday, November 29, 2021 8:02 PM
> > > To: Leo Li <leoyang.li@nxp.com>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>; linux-pci@vger.kernel.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Z.Q. Hou
> > > <zhiqiang.hou@nxp.com>
> > > Subject: Re: [PATCH 4/4] dt-bindings: pci: layerscape-pci: define
> > > aer/pme interrupts
> > >
> > > On Fri, Nov 19, 2021 at 06:16:21PM -0600, Li Yang wrote:
> > > > Some platforms using this controller have separated interrupt
> > > > lines for aer or pme events instead of having a single interrupt
> > > > line for miscellaneous events.  Define interrupts in the binding
> > > > for these interrupt lines.
> > > >
> > > > Signed-off-by: Li Yang <leoyang.li@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/pci/layerscape-pci.txt     | 14 ++++++++++----
> > > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > > > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > > > index 8fd6039a826b..bcf11bfc4bab 100644
> > > > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > > > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> > > > @@ -31,8 +31,13 @@ Required properties:
> > > >  - reg: base addresses and lengths of the PCIe controller register blocks.
> > > >  - 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 could include the following entries:
> > > > +  "aer": For interrupt line reporting aer events when non
> > > > +MSI/MSI-X/INTx
> > > mode
> > > > +           is used
> > > > +  "pme": For interrupt line reporting pme events when non
> > > > + MSI/MSI-
> > > X/INTx mode
> > > > +           is used
> > > > +  "intr": For interrupt line reporting miscellaneous controller
> > > > +events
> > > > +  ......
> > > >  - fsl,pcie-scfg: Must include two entries.
> > > >    The first entry must be a link to the SCFG device node
> > > >    The second entry is the physical PCIe controller index starting from '0'.
> > > > @@ -52,8 +57,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";
> > >
> > > This isn't a compatible change. The h/w suddenly has no 'intr'
> > > interrupt?
> >
> > The original 'intr' was just a place holder for a HW interrupt signal without a
> clear definition of events associated.  Some later SoC has more interrupt
> signals to associate with more specific events.
> 
> 'Later SoC' means new compatible, but you're not changing the compatible. If
> it was just wrong for all SoCs, then state that in the commit message. Please
> define all the interrupts on all SoCs, so it is not changing again.

Different SoCs could have different number of interrupt lines and the events routing could also be different among SoCs.  It is really hard to name the interrupt lines properly that works for all SoCs.  That is probably why we chose to name key events instead of interrupt lines.

The HW documentation is also not very clear on this part.  But I will try to summarize the situation better in next version, for example:

"aer": Used for interrupt line which reports AER events when non MSI/MSI-X/INTx mode is used
"pme": Used for interrupt line which reports PME events when non MSI/MSI-X/INTx mode is used
"intr": Used for SoC(like ls2080a, lx2160, ls2080, ls2088, ls1088) which has a single interrupt line for miscellaneous controller events(which could include AER and PME events).

> 
> > If needed, we can keep the "intr" interrupt-name there just for backward
> compatibility although it was never used in Linux.
> 
> What about other OSs?
> 
> Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-12-01 23:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-20  0:16 [PATCH 0/4] layerscape-pci binding updates Li Yang
2021-11-20  0:16 ` [PATCH 1/4] dt-bindings: pci: layerscape-pci: Add a optional property big-endian Li Yang
2021-11-20  0:16 ` [PATCH 2/4] dt-bindings: pci: layerscape-pci: Update the description of SCFG property Li Yang
2021-11-20  0:16 ` [PATCH 3/4] dt-bindings: pci: layerscape-pci: Add EP mode compatible strings for ls1028a Li Yang
2021-11-30  1:59   ` Rob Herring
2021-11-20  0:16 ` [PATCH 4/4] dt-bindings: pci: layerscape-pci: define aer/pme interrupts Li Yang
2021-11-30  2:02   ` Rob Herring
2021-11-30  3:35     ` Leo Li
2021-11-30 13:47       ` Rob Herring
2021-12-01 23:34         ` Leo Li
2021-11-30 14:22   ` Bjorn Helgaas

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).