diff mbox

[U-Boot,V3] mpc83xx: fix pcie configuration space read/write

Message ID 000001cbb0c3$dd923910$6401a8c0@LENOVOE5CA6843
State Changes Requested
Headers show

Commit Message

Baidu Boy Jan. 10, 2011, 12:42 p.m. UTC
This patch fix a problem for the pcie enumeration when the mpc83xx pcie controller is 
connected with switch or we use both of the two pcie controller

Signed-off-by: Baidu Boy <liucai.lfn@gmail.com>
---
Changes for V2:
     - Avoid line wrap in the patch
Changes for V3
	- Add space between ) and {

 arch/powerpc/cpu/mpc83xx/pcie.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

Comments

Scott Wood Jan. 10, 2011, 6:51 p.m. UTC | #1
On Mon, 10 Jan 2011 20:42:28 +0800
Baidu Boy <liucai.lfn@gmail.com> wrote:

> This patch fix a problem for the pcie enumeration when the mpc83xx pcie controller is 
> connected with switch or we use both of the two pcie controller
> 
> Signed-off-by: Baidu Boy <liucai.lfn@gmail.com>
> ---
> Changes for V2:
>      - Avoid line wrap in the patch
> Changes for V3
> 	- Add space between ) and {

What about my comments?

http://lists.denx.de/pipermail/u-boot/2011-January/084758.html

-Scott
Baidu Boy Jan. 11, 2011, 11:51 a.m. UTC | #2
Hi, Scott

2011/1/11 Scott Wood <scottwood@freescale.com>:
> On Mon, 10 Jan 2011 20:42:28 +0800
> Baidu Boy <liucai.lfn@gmail.com> wrote:
>
>> This patch fix a problem for the pcie enumeration when the mpc83xx pcie controller is
>> connected with switch or we use both of the two pcie controller
>>
>> Signed-off-by: Baidu Boy <liucai.lfn@gmail.com>
>> ---
>> Changes for V2:
>>      - Avoid line wrap in the patch
>> Changes for V3
>>       - Add space between ) and {
>
> What about my comments?
>
> http://lists.denx.de/pipermail/u-boot/2011-January/084758.html
>

Actually, Your suggestion is better. But it will touch the common
structure pci_controller.
And I just want to make the change only in the mpc83xx.
Thanks
Scott Wood Jan. 11, 2011, 6:25 p.m. UTC | #3
On Tue, 11 Jan 2011 19:51:41 +0800
Baidu Boy <liucai.lfn@gmail.com> wrote:

> Hi, Scott
> 
> 2011/1/11 Scott Wood <scottwood@freescale.com>:
> > On Mon, 10 Jan 2011 20:42:28 +0800
> > Baidu Boy <liucai.lfn@gmail.com> wrote:
> >
> >> This patch fix a problem for the pcie enumeration when the mpc83xx pcie controller is
> >> connected with switch or we use both of the two pcie controller
> >>
> >> Signed-off-by: Baidu Boy <liucai.lfn@gmail.com>
> >> ---
> >> Changes for V2:
> >>      - Avoid line wrap in the patch
> >> Changes for V3
> >>       - Add space between ) and {
> >
> > What about my comments?
> >
> > http://lists.denx.de/pipermail/u-boot/2011-January/084758.html
> >
> 
> Actually, Your suggestion is better. But it will touch the common
> structure pci_controller.

That's OK, it's been touched by other PCI implementations too.  Ideally
these custom fields would be replaced by a private-data pointer.

> And I just want to make the change only in the mpc83xx.

Don't be afraid to refactor at a larger scale when needed.

Your current approach only works during initial scanning.  It won't
work if config accesses are done later.

-Scott
Baidu Boy Jan. 15, 2011, 11:26 a.m. UTC | #4
Hi,Scott

2011/1/12 Scott Wood <scottwood@freescale.com>:
> On Tue, 11 Jan 2011 19:51:41 +0800
> Baidu Boy <liucai.lfn@gmail.com> wrote:
>
>> Hi, Scott
>>
>> 2011/1/11 Scott Wood <scottwood@freescale.com>:
>> > On Mon, 10 Jan 2011 20:42:28 +0800
>> > Baidu Boy <liucai.lfn@gmail.com> wrote:
>> >
>> >> This patch fix a problem for the pcie enumeration when the mpc83xx pcie controller is
>> >> connected with switch or we use both of the two pcie controller
>> >>
>> >> Signed-off-by: Baidu Boy <liucai.lfn@gmail.com>
>> >> ---
>> >> Changes for V2:
>> >>      - Avoid line wrap in the patch
>> >> Changes for V3
>> >>       - Add space between ) and {
>> >
>> > What about my comments?
>> >
>> > http://lists.denx.de/pipermail/u-boot/2011-January/084758.html
>> >
>>
>> Actually, Your suggestion is better. But it will touch the common
>> structure pci_controller.
>
> That's OK, it's been touched by other PCI implementations too.  Ideally
> these custom fields would be replaced by a private-data pointer.
>
>> And I just want to make the change only in the mpc83xx.
>
> Don't be afraid to refactor at a larger scale when needed.
>
> Your current approach only works during initial scanning.  It won't
> work if config accesses are done later.
>

I have re-submit another patch:
http://patchwork.ozlabs.org/patch/79047/

Please discard previous patches.

Thanks
diff mbox

Patch

diff --git a/arch/powerpc/cpu/mpc83xx/pcie.c b/arch/powerpc/cpu/mpc83xx/pcie.c
index 46a706d..8429848 100644
--- a/arch/powerpc/cpu/mpc83xx/pcie.c
+++ b/arch/powerpc/cpu/mpc83xx/pcie.c
@@ -46,13 +46,15 @@  static struct {
 #endif
 };
 
+static u8 pcie_index = 0;
+
 #ifdef CONFIG_83XX_GENERIC_PCIE_REGISTER_HOSES
 
 static int mpc83xx_pcie_remap_cfg(struct pci_controller *hose, pci_dev_t dev)
 {
 	int bus = PCI_BUS(dev) - hose->first_busno;
 	immap_t *immr = (immap_t *)CONFIG_SYS_IMMR;
-	pex83xx_t *pex = &immr->pciexp[bus];
+	pex83xx_t *pex = &immr->pciexp[pcie_index];
 	struct pex_outbound_window *out_win = &pex->bridge.pex_outbound_win[0];
 	u8 devfn = PCI_DEV(dev) << 3 | PCI_FUNC(dev);
 	u32 dev_base = bus << 24 | devfn << 16;
@@ -324,6 +326,8 @@  void mpc83xx_pcie_init(int num_buses, struct pci_region **reg)
 		num_buses = ARRAY_SIZE(mpc83xx_pcie_cfg_space);
 	}
 
-	for (i = 0; i < num_buses; i++)
+	for (i = 0; i < num_buses; i++) {
+		pcie_index = i;
 		mpc83xx_pcie_init_bus(i, reg[i]);
+	}
 }