diff mbox

[RFC] powerpc/fsl-pci: Document the "fsl, has-isa" property for Freescale PCI

Message ID 1333263396-23932-1-git-send-email-B38951@freescale.com (mailing list archive)
State RFC
Headers show

Commit Message

Hongtao Jia April 1, 2012, 6:56 a.m. UTC
If PCI is primary bus we should set isa_io/mem_base when parsing PCI bridge
resources from device tree. The previous way to check the primary bus based
on a hard-coded address named primary_phb_addr. Now we add a property named
"fsl,has-isa" into device tree. In kernel we use this property to find out
the bus is primary or not. This way is more flexible.

Signed-off-by: Jia Hongtao <B38951@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 .../devicetree/bindings/powerpc/fsl/pci.txt        |   36 ++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/pci.txt

Comments

Kumar Gala April 4, 2012, 1:08 p.m. UTC | #1
On Apr 1, 2012, at 1:56 AM, Jia Hongtao wrote:

> If PCI is primary bus we should set isa_io/mem_base when parsing PCI bridge
> resources from device tree. The previous way to check the primary bus based
> on a hard-coded address named primary_phb_addr. Now we add a property named
> "fsl,has-isa" into device tree. In kernel we use this property to find out
> the bus is primary or not. This way is more flexible.
> 
> Signed-off-by: Jia Hongtao <B38951@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> .../devicetree/bindings/powerpc/fsl/pci.txt        |   36 ++++++++++++++++++++
> 1 files changed, 36 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/pci.txt

This isn't freescale specific, its linux specific.  If anything the property should be linux,has-isa.

But in general I dont think this is a good idea.  In truth one could search the device tree for:

	device_type = "isa";

to try and set this dynamically.

- k
Hongtao Jia April 6, 2012, 3:04 a.m. UTC | #2
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Wednesday, April 04, 2012 9:09 PM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; devicetree-
> discuss@lists.ozlabs.org
> Subject: Re: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property
> for Freescale PCI
> 
> 
> On Apr 1, 2012, at 1:56 AM, Jia Hongtao wrote:
> 
> > If PCI is primary bus we should set isa_io/mem_base when parsing PCI
> bridge
> > resources from device tree. The previous way to check the primary bus
> based
> > on a hard-coded address named primary_phb_addr. Now we add a property
> named
> > "fsl,has-isa" into device tree. In kernel we use this property to find
> out
> > the bus is primary or not. This way is more flexible.
> >
> > Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> > .../devicetree/bindings/powerpc/fsl/pci.txt        |   36
> ++++++++++++++++++++
> > 1 files changed, 36 insertions(+), 0 deletions(-)
> > create mode 100644
> Documentation/devicetree/bindings/powerpc/fsl/pci.txt
> 
> This isn't freescale specific, its linux specific.  If anything the
> property should be linux,has-isa.
> 
> But in general I dont think this is a good idea.  In truth one could
> search the device tree for:
> 
> 	device_type = "isa";
> 
> to try and set this dynamically.
> 
> - k
> 

Yes, it's not Freescale specific, but it's only used by Freescale now in
the kernel. This is why I named the property as "fsl,has-isa".

To indicate PCI bus is primary we have three ways to go and we now like
the 2nd solution:

1. As this patch said, we add a property to device tree manually.

2. Set this property dynamically in uboot when scanning PCI bridge.
Actually we have already done this. The problem is users should update
uboot and kernel together or it's not work. To support old uboot we decide
to add this property into device tree too temporarily. We will remove it
from device tree at an appropriate future.

3. Just as you said we could search the device tree by device_type = "isa".
   There are two problems here:
	* There is no OF API for searching just in PCI node now. That means
	  we can't easily find whether there is "isa" bridge or not under
	  this PCI controller while scanning it.
	* Boards MPC8541CDS and MPC8555CDS has no "isa" node in device tree
	  but has ISA bridge under PCI controller.

- Jia Hongtao
Hongtao Jia April 10, 2012, 9:17 a.m. UTC | #3
> -----Original Message-----
> From: linuxppc-dev-bounces+b38951=freescale.com@lists.ozlabs.org
> [mailto:linuxppc-dev-bounces+b38951=freescale.com@lists.ozlabs.org] On
> Behalf Of Jia Hongtao-B38951
> Sent: Friday, April 06, 2012 11:05 AM
> To: Kumar Gala
> Cc: devicetree-discuss@lists.ozlabs.org; linuxppc-dev@lists.ozlabs.org;
> Li Yang-R58472
> Subject: RE: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property
> for Freescale PCI
> 
> 
> > -----Original Message-----
> > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > Sent: Wednesday, April 04, 2012 9:09 PM
> > To: Jia Hongtao-B38951
> > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; devicetree-
> > discuss@lists.ozlabs.org
> > Subject: Re: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property
> > for Freescale PCI
> >
> >
> > On Apr 1, 2012, at 1:56 AM, Jia Hongtao wrote:
> >
> > > If PCI is primary bus we should set isa_io/mem_base when parsing PCI
> > bridge
> > > resources from device tree. The previous way to check the primary bus
> > based
> > > on a hard-coded address named primary_phb_addr. Now we add a property
> > named
> > > "fsl,has-isa" into device tree. In kernel we use this property to
> find
> > out
> > > the bus is primary or not. This way is more flexible.
> > >
> > > Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > ---
> > > .../devicetree/bindings/powerpc/fsl/pci.txt        |   36
> > ++++++++++++++++++++
> > > 1 files changed, 36 insertions(+), 0 deletions(-)
> > > create mode 100644
> > Documentation/devicetree/bindings/powerpc/fsl/pci.txt
> >
> > This isn't freescale specific, its linux specific.  If anything the
> > property should be linux,has-isa.
> >
> > But in general I dont think this is a good idea.  In truth one could
> > search the device tree for:
> >
> > 	device_type = "isa";
> >
> > to try and set this dynamically.
> >
> > - k
> >
> 
> Yes, it's not Freescale specific, but it's only used by Freescale now in
> the kernel. This is why I named the property as "fsl,has-isa".
> 
> To indicate PCI bus is primary we have three ways to go and we now like
> the 2nd solution:
> 
> 1. As this patch said, we add a property to device tree manually.
> 
> 2. Set this property dynamically in uboot when scanning PCI bridge.
> Actually we have already done this. The problem is users should update
> uboot and kernel together or it's not work. To support old uboot we
> decide
> to add this property into device tree too temporarily. We will remove it
> from device tree at an appropriate future.
> 
> 3. Just as you said we could search the device tree by device_type =
> "isa".
>    There are two problems here:
> 	* There is no OF API for searching just in PCI node now. That means
> 	  we can't easily find whether there is "isa" bridge or not under
> 	  this PCI controller while scanning it.
> 	* Boards MPC8541CDS and MPC8555CDS has no "isa" node in device tree
> 	  but has ISA bridge under PCI controller.
> 
> - Jia Hongtao
> 

Hi Kumar,

I agree with the name change from "fsl,has-isa" to "linux,has-isa".
If this is ok I will update this patch and send the patches based on this
new property.

Also, I have a question about "isa" node in dts. Is this node being used
by kernel? If not we can remove it from all boards and check the primary
bus by uboot. If "isa" is useful why MPC8541CDS and MPC8555CDS has no
"isa" node in device tree but has ISA bridge under PCI controller?

Thanks.
- Jia Hongtao
Hongtao Jia April 23, 2012, 3:34 a.m. UTC | #4
> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Tuesday, April 10, 2012 5:17 PM
> To: Kumar Gala
> Cc: devicetree-discuss@lists.ozlabs.org; linuxppc-dev@lists.ozlabs.org;
> Li Yang-R58472; Jia Hongtao-B38951
> Subject: RE: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property
> for Freescale PCI
> 
> 
> > -----Original Message-----
> > From: linuxppc-dev-bounces+b38951=freescale.com@lists.ozlabs.org
> > [mailto:linuxppc-dev-bounces+b38951=freescale.com@lists.ozlabs.org] On
> > Behalf Of Jia Hongtao-B38951
> > Sent: Friday, April 06, 2012 11:05 AM
> > To: Kumar Gala
> > Cc: devicetree-discuss@lists.ozlabs.org; linuxppc-dev@lists.ozlabs.org;
> > Li Yang-R58472
> > Subject: RE: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property
> > for Freescale PCI
> >
> >
> > > -----Original Message-----
> > > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > > Sent: Wednesday, April 04, 2012 9:09 PM
> > > To: Jia Hongtao-B38951
> > > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; devicetree-
> > > discuss@lists.ozlabs.org
> > > Subject: Re: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa"
> property
> > > for Freescale PCI
> > >
> > >
> > > On Apr 1, 2012, at 1:56 AM, Jia Hongtao wrote:
> > >
> > > > If PCI is primary bus we should set isa_io/mem_base when parsing
> PCI
> > > bridge
> > > > resources from device tree. The previous way to check the primary
> bus
> > > based
> > > > on a hard-coded address named primary_phb_addr. Now we add a
> property
> > > named
> > > > "fsl,has-isa" into device tree. In kernel we use this property to
> > find
> > > out
> > > > the bus is primary or not. This way is more flexible.
> > > >
> > > > Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > > ---
> > > > .../devicetree/bindings/powerpc/fsl/pci.txt        |   36
> > > ++++++++++++++++++++
> > > > 1 files changed, 36 insertions(+), 0 deletions(-)
> > > > create mode 100644
> > > Documentation/devicetree/bindings/powerpc/fsl/pci.txt
> > >
> > > This isn't freescale specific, its linux specific.  If anything the
> > > property should be linux,has-isa.
> > >
> > > But in general I dont think this is a good idea.  In truth one could
> > > search the device tree for:
> > >
> > > 	device_type = "isa";
> > >
> > > to try and set this dynamically.
> > >
> > > - k
> > >
> >
> > Yes, it's not Freescale specific, but it's only used by Freescale now
> in
> > the kernel. This is why I named the property as "fsl,has-isa".
> >
> > To indicate PCI bus is primary we have three ways to go and we now like
> > the 2nd solution:
> >
> > 1. As this patch said, we add a property to device tree manually.
> >
> > 2. Set this property dynamically in uboot when scanning PCI bridge.
> > Actually we have already done this. The problem is users should update
> > uboot and kernel together or it's not work. To support old uboot we
> > decide
> > to add this property into device tree too temporarily. We will remove
> it
> > from device tree at an appropriate future.
> >
> > 3. Just as you said we could search the device tree by device_type =
> > "isa".
> >    There are two problems here:
> > 	* There is no OF API for searching just in PCI node now. That means
> > 	  we can't easily find whether there is "isa" bridge or not under
> > 	  this PCI controller while scanning it.
> > 	* Boards MPC8541CDS and MPC8555CDS has no "isa" node in device tree
> > 	  but has ISA bridge under PCI controller.
> >
> > - Jia Hongtao
> >
> 
> Hi Kumar,
> 
> I agree with the name change from "fsl,has-isa" to "linux,has-isa".
> If this is ok I will update this patch and send the patches based on this
> new property.
> 
> Also, I have a question about "isa" node in dts. Is this node being used
> by kernel? If not we can remove it from all boards and check the primary
> bus by uboot. If "isa" is useful why MPC8541CDS and MPC8555CDS has no
> "isa" node in device tree but has ISA bridge under PCI controller?
> 
> Thanks.
> - Jia Hongtao

Hi Kumar and Ben,

Do you have any idea on this?
Thanks.

 - Jia Hongtao
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/powerpc/fsl/pci.txt b/Documentation/devicetree/bindings/powerpc/fsl/pci.txt
new file mode 100644
index 0000000..7b18090
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/pci.txt
@@ -0,0 +1,36 @@ 
+* Freescale PCI
+
+Freescale PCI specific property:
+- fsl,has-isa : If PCI is primary bus we should set isa_io/mem_base when
+                parsing PCI bridge resources. This property is an indicator
+                to inform kernel the PCI is primary.
+
+Example (MPC8572DS)
+	&pci0 {
+		compatible = "fsl,mpc8548-pcie";
+		device_type = "pci";
+		#size-cells = <2>;
+		#address-cells = <3>;
+		bus-range = <0 255>;
+		clock-frequency = <33333333>;
+		interrupts = <24 2 0 0>;
+		fsl,has-isa;
+
+		pcie@0 {
+			reg = <0 0 0 0 0>;
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+			device_type = "pci";
+			interrupts = <24 2 0 0>;
+			interrupt-map-mask = <0xf800 0 0 7>;
+
+			interrupt-map = <
+				/* IDSEL 0x0 */
+				0000 0x0 0x0 0x1 &mpic 0x8 0x1 0x0 0x0
+				0000 0x0 0x0 0x2 &mpic 0x9 0x1 0x0 0x0
+				0000 0x0 0x0 0x3 &mpic 0xa 0x1 0x0 0x0
+				0000 0x0 0x0 0x4 &mpic 0xb 0x1 0x0 0x0
+				>;
+		};
+	};