Re: [PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-03 Thread Li Li
On Thu, 2008-01-03 at 18:14 +0800, Arnd Bergmann wrote:

> After some more research, I noticed that the distinction between 
> primary and secondary host bridges will go away _anyway_, so 
> I guess we shouldn't worry about it too much.
> 
> On powerpc64, we already don't care, as the
> arch/powerpc/kernel/isa-bridge.c 
> code takes care of legacy I/O areas there, instead of the PCI code. 
> It's only needed on machines with legacy ISA devices (PC keyboard, 
> floppy, ...), and those can be converted to use the isa-bridge code 
> at some point.
> 

This is really a good new :)

- Tony

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


Re: [PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-03 Thread Arnd Bergmann
On Thursday 03 January 2008, Li Li wrote:
> 
> > It's easy enough to just panic() if you find more than one primary
> > bus, 
> > since the information from the device tree is static enough that one 
> > will know how to fix this after seeing the error.
> > 
> 
> It is just a case. what about if do not find primary pci host.
> The current code can prevent this potential error and make system
> robust.

After some more research, I noticed that the distinction between
primary and secondary host bridges will go away _anyway_, so
I guess we shouldn't worry about it too much.

On powerpc64, we already don't care, as the arch/powerpc/kernel/isa-bridge.c
code takes care of legacy I/O areas there, instead of the PCI code.
It's only needed on machines with legacy ISA devices (PC keyboard,
floppy, ...), and those can be converted to use the isa-bridge code
at some point.

Arnd <><
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-03 Thread Li Li
On Thu, 2008-01-03 at 16:14 +0800, Arnd Bergmann wrote:
> On Thursday 03 January 2008, Li Li wrote: 
> > > * The detection method for the primary bus is somewhat fragile, 
> > > because 
> > > we depend on the order of the nodes in the device tree, which is
> not 
> > > specified to have a meaning. /Maybe/ there could be a property in 
> > > (at most) one of the PCI host bridge nodes saying that this
> specific 
> > > bus 
> > > is the primary one. 
> > > 
> > 
> > Put this primary in code is more save than in dts. 
> > The dts lacks error checking function. If provide error dts such as
> two 
> > pci hosts both has primary property will occur chaos.
> 
> It's easy enough to just panic() if you find more than one primary
> bus, 
> since the information from the device tree is static enough that one 
> will know how to fix this after seeing the error.
> 

It is just a case. what about if do not find primary pci host.
The current code can prevent this potential error and make system
robust.

- Tony


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


Re: [PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-03 Thread Arnd Bergmann
On Thursday 03 January 2008, Li Li wrote:
> > * The detection method for the primary bus is somewhat fragile,
> > because 
> > we depend on the order of the nodes in the device tree, which is not 
> > specified to have a meaning. /Maybe/ there could be a property in 
> > (at most) one of the PCI host bridge nodes saying that this specific
> > bus 
> > is the primary one.
> > 
> 
> Put this primary in code is more save than in dts.
> The dts lacks error checking function. If provide error dts such as two
> pci hosts both has primary property will occur chaos.

It's easy enough to just panic() if you find more than one primary bus,
since the information from the device tree is static enough that one
will know how to fix this after seeing the error.

Arnd <><
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-02 Thread Li Li
On Wed, 2008-01-02 at 19:53 +0800, Arnd Bergmann wrote:
> On Wednesday 02 January 2008, Li Li wrote: 
> >  #ifdef CONFIG_PCI 
> > -   for_each_compatible_node(np, "pci", "fsl,mpc8349-pci") 
> > -   mpc83xx_add_bridge(np); 
> > +   for_each_compatible_node(np, "pci", "fsl,mpc8349-pci") { 
> > +   if (primary_pci_bus) { 
> > +   mpc83xx_add_bridge(np, PPC_83XX_PCI |
> PPC_83XX_PCI_PRIMARY); 
> > +   primary_pci_bus = 0; 
> > +   } else 
> > +   mpc83xx_add_bridge(np, PPC_83XX_PCI); 
> > +   } 
> > + 
> > +   for_each_compatible_node(np, "pci", "fsl,mpc8377-pcie") { 
> > +   if (primary_pci_bus) { 
> > +   mpc83xx_add_bridge(np, PPC_83XX_PCIE |
> PPC_83XX_PCI_PRIMARY); 
> > +   primary_pci_bus = 0; 
> > +   } else 
> > +   mpc83xx_add_bridge(np, PPC_83XX_PCIE); 
> > +   } 
> > + 
> > +   ppc_md.pci_exclude_device = mpc837x_exclude_device; 
> >  #endif
> 
> A few comments on how I think this can be improved:
> 
> * Instead of duplicating this code across all platforms, make a
> single 
> function that does the probing in one place (including the #ifdef 
> CONFIG_PCI).
> 

This code appears in 834x board in first and then copied from one board
to another. It is a legacy issue. :(
Yes. it is the time to remove this duplicates.

> * Better yet, get rid of mpc83xx_add_bridge and make it possible for
> the 
> mpc83xx code to use the of_pci_phb_driver from 
> arch/powerpc/kernel/of_platform.c. This used to be impossible because 
> of the differences  between 32 and 64 bit code for PCI probing, but 
> I think with the work than benh put into unifying the two, we should 
> be pretty close now.
> 

This is really a huge task and almost affects all exist boards. We need
more opinions in here. 


> * The detection method for the primary bus is somewhat fragile,
> because 
> we depend on the order of the nodes in the device tree, which is not 
> specified to have a meaning. /Maybe/ there could be a property in 
> (at most) one of the PCI host bridge nodes saying that this specific
> bus 
> is the primary one.
> 

Put this primary in code is more save than in dts.
The dts lacks error checking function. If provide error dts such as two
pci hosts both has primary property will occur chaos.

> * Since you are using exactly the same probing code for pci and pcie, 
> you may want to check for a more generic "compatible" property than 
> the specific model, so you can use the same code for both.
> 
> Arnd <><
> 

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


Re: [PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-02 Thread Li Li
On Wed, 2008-01-02 at 23:23 +0800, Kumar Gala wrote:
> 
> On Jan 2, 2008, at 5:16 AM, Li Li wrote:
> 
> > * The MPC837x PCIE controller hardware resources and SerDes are  
> > initiated in u-boot. 
> > * Merge the MPC837x PCIE code into arch/powerpc/sysdev/fsl_pci.c 
> > * The MPC837x PCIE controller`s configure address bit field is
> uniqe: 
> >   bus number: bits 31-24 
> >   device number:  bits 23-19 
> >   function number:bits 18-16 
> >   ext reg number: bits 11-8 
> >   reg number: bits 7-2 
> > * Add mpc837x_exclude_device to fixup a controller bug.
> 
> what is the bug that is being worked around?
> 
For Type 0 configure transactions, the PCIE controller do not check the
device number bits and just assume the device number bits are 0.

- tony

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


Re: [PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-02 Thread Olof Johansson
On Wed, Jan 02, 2008 at 07:16:45PM +0800, Li Li wrote:
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 1ee009e..f84caa7 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2111,6 +2111,10 @@
>  #define PCI_DEVICE_ID_TDI_EHCI  0x0101
>  
>  #define PCI_VENDOR_ID_FREESCALE  0x1957
> +#define PCI_DEVICE_ID_MPC8378E   0x00c4
> +#define PCI_DEVICE_ID_MPC83780x00c5
> +#define PCI_DEVICE_ID_MPC8377E   0x00c6
> +#define PCI_DEVICE_ID_MPC83770x00c7
>  #define PCI_DEVICE_ID_MPC8548E   0x0012
>  #define PCI_DEVICE_ID_MPC85480x0013
>  #define PCI_DEVICE_ID_MPC8543E   0x0014

In general it's not needed to add device ID's to the global table,
especially when they're only used once or twice. Just use the hex constant
in the driver instead.


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


Re: [PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-02 Thread Kumar Gala

On Jan 2, 2008, at 5:16 AM, Li Li wrote:

> * The MPC837x PCIE controller hardware resources and SerDes are  
> initiated in u-boot.
> * Merge the MPC837x PCIE code into arch/powerpc/sysdev/fsl_pci.c
> * The MPC837x PCIE controller`s configure address bit field is uniqe:
>   bus number: bits 31-24
>   device number:  bits 23-19
>   function number:bits 18-16
>   ext reg number: bits 11-8
>   reg number: bits 7-2
> * Add mpc837x_exclude_device to fixup a controller bug.

what is the bug that is being worked around?

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


Re: [PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-02 Thread Arnd Bergmann
On Wednesday 02 January 2008, Li Li wrote:
>  #ifdef CONFIG_PCI
> -   for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
> -   mpc83xx_add_bridge(np);
> +   for_each_compatible_node(np, "pci", "fsl,mpc8349-pci") {
> +   if (primary_pci_bus) {
> +   mpc83xx_add_bridge(np, PPC_83XX_PCI | 
> PPC_83XX_PCI_PRIMARY);
> +   primary_pci_bus = 0;
> +   } else
> +   mpc83xx_add_bridge(np, PPC_83XX_PCI);
> +   }
> +
> +   for_each_compatible_node(np, "pci", "fsl,mpc8377-pcie") {
> +   if (primary_pci_bus) {
> +   mpc83xx_add_bridge(np, PPC_83XX_PCIE | 
> PPC_83XX_PCI_PRIMARY);
> +   primary_pci_bus = 0;
> +   } else
> +   mpc83xx_add_bridge(np, PPC_83XX_PCIE);
> +   }
> +
> +   ppc_md.pci_exclude_device = mpc837x_exclude_device;
>  #endif

A few comments on how I think this can be improved:

* Instead of duplicating this code across all platforms, make a single
function that does the probing in one place (including the #ifdef
CONFIG_PCI).

* Better yet, get rid of mpc83xx_add_bridge and make it possible for the
mpc83xx code to use the of_pci_phb_driver from
arch/powerpc/kernel/of_platform.c. This used to be impossible because
of the differences  between 32 and 64 bit code for PCI probing, but
I think with the work than benh put into unifying the two, we should
be pretty close now.

* The detection method for the primary bus is somewhat fragile, because
we depend on the order of the nodes in the device tree, which is not
specified to have a meaning. /Maybe/ there could be a property in
(at most) one of the PCI host bridge nodes saying that this specific bus
is the primary one.

* Since you are using exactly the same probing code for pci and pcie,
you may want to check for a more generic "compatible" property than
the specific model, so you can use the same code for both.

Arnd <><
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[PATCH] powerpc: Add MPC837x PCIE controller RC mode support

2008-01-02 Thread Li Li
* The MPC837x PCIE controller hardware resources and SerDes are initiated in 
u-boot.
* Merge the MPC837x PCIE code into arch/powerpc/sysdev/fsl_pci.c
* The MPC837x PCIE controller`s configure address bit field is uniqe:
bus number: bits 31-24
device number:  bits 23-19
function number:bits 18-16
ext reg number: bits 11-8
reg number: bits 7-2
* Add mpc837x_exclude_device to fixup a controller bug.
* Add flag variant to mpc83xx_add_bridge function.

Signed-off-by: Tony Li <[EMAIL PROTECTED]>
---
 arch/powerpc/boot/dts/mpc8377_mds.dts |   54 --
 arch/powerpc/boot/dts/mpc8378_mds.dts |   54 --
 arch/powerpc/platforms/83xx/Kconfig   |2 +
 arch/powerpc/platforms/83xx/mpc8313_rdb.c |   10 ++-
 arch/powerpc/platforms/83xx/mpc832x_mds.c |   12 ++-
 arch/powerpc/platforms/83xx/mpc832x_rdb.c |   10 ++-
 arch/powerpc/platforms/83xx/mpc834x_itx.c |   10 ++-
 arch/powerpc/platforms/83xx/mpc834x_mds.c |   10 ++-
 arch/powerpc/platforms/83xx/mpc836x_mds.c |   12 ++-
 arch/powerpc/platforms/83xx/mpc837x_mds.c |   39 +++-
 arch/powerpc/platforms/83xx/mpc83xx.h |6 +-
 arch/powerpc/platforms/83xx/pci.c |   29 --
 arch/powerpc/sysdev/fsl_pci.c |  159 +
 arch/powerpc/sysdev/fsl_pci.h |3 +
 include/asm-powerpc/pci-bridge.h  |1 +
 include/linux/pci_ids.h   |4 +
 16 files changed, 375 insertions(+), 40 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8377_mds.dts 
b/arch/powerpc/boot/dts/mpc8377_mds.dts
index 4402e39..4af3802 100644
--- a/arch/powerpc/boot/dts/mpc8377_mds.dts
+++ b/arch/powerpc/boot/dts/mpc8377_mds.dts
@@ -197,14 +197,6 @@
clock = ;
};
 
-   serdes2:[EMAIL PROTECTED] {
-   compatible = "fsl,serdes";
-   reg = ;
-   vdd-1v;
-   protocol = "pcie";
-   clock = ;
-   };
-
/* IPIC
 * interrupts cell = 
 * sense values match linux IORESOURCE_IRQ_* defines:
@@ -279,4 +271,50 @@
compatible = "fsl,mpc8349-pci";
device_type = "pci";
};
+
+   [EMAIL PROTECTED] {
+   interrupt-map-mask = ;
+   interrupt-map = <
+    0 0 1 &ipic 1 8
+    0 0 2 &ipic 1 8
+    0 0 3 &ipic 1 8
+    0 0 4 &ipic 1 8
+   >;
+   interrupt-parent = < &ipic >;
+   interrupts = <1 8>;
+   bus-range = <0 0>;
+   ranges = <0200 0 A000 A000 0 1000
+ 0100 0  B100 0 0080>;
+   clock-frequency = <0>;
+   #interrupt-cells = <1>;
+   #size-cells = <2>;
+   #address-cells = <3>;
+   reg = ;
+   compatible = "fsl,mpc8377-pcie";
+   device_type = "pci";
+   };
+
+   [EMAIL PROTECTED] {
+   interrupt-map-mask = ;
+   interrupt-map = <
+    0 0 1 &ipic 2 8
+    0 0 2 &ipic 2 8
+    0 0 3 &ipic 2 8
+    0 0 4 &ipic 2 8
+   >;
+   interrupt-parent = < &ipic >;
+   interrupts = <2 8>;
+   bus-range = <0 0>;
+   ranges = <0200 0 C000 C000 0 1000
+ 0100 0  D100 0 0080>;
+   clock-frequency = <0>;
+   #interrupt-cells = <1>;
+   #size-cells = <2>;
+   #address-cells = <3>;
+   reg = ;
+   compatible = "fsl,mpc8377-pcie";
+   device_type = "pci";
+   };
 };
diff --git a/arch/powerpc/boot/dts/mpc8378_mds.dts 
b/arch/powerpc/boot/dts/mpc8378_mds.dts
index 54171f4..de9d40c 100644
--- a/arch/powerpc/boot/dts/mpc8378_mds.dts
+++ b/arch/powerpc/boot/dts/mpc8378_mds.dts
@@ -179,14 +179,6 @@
clock = ;
};
 
-   serdes2:[EMAIL PROTECTED] {
-   compatible = "fsl,serdes";
-   reg = ;
-   vdd-1v;
-   protocol = "pcie";
-   clock = ;
-   };
-
/* IPIC
 * interrupts cell = 
 * sense values match linux IORESOURCE_IRQ_* defines:
@@ -261,4 +253,50 @@
compatible = "fsl,mpc8349-pci";
device_type = "pci";
};
+
+   [EMAIL PROTECTED] {
+   interrupt-map-mask = ;
+   interrupt-map = <
+    0 0 1 &ipic 1 8
+    0 0 2 &ipic 1 8
+