[U-Boot] [PATCH V2] mpc83xx: fix pcie configuration space read/write

[U-Boot] [PATCH V2] mpc83xx: fix pcie configuration space read/write

[Baidu Boy]

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

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

diff --git a/arch/powerpc/cpu/mpc83xx/pcie.c b/arch/powerpc/cpu/mpc83xx/pcie.c
index 46a706d..2a200b8 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]);
+	}
 }
-- 
1.7.3.1.msysgit.0

[U-Boot] [PATCH V2] mpc83xx: fix pcie configuration space read/write

[Scott Wood]

On Fri, 7 Jan 2011 20:42:51 +0800
Baidu Boy <liucai.lfn@gmail.com> wrote:

> Signed-off-by: Baidu Boy <liucai.lfn@gmail.com>

Sign-offs (and ideally the From: field as well) should contain your
real name.

> diff --git a/arch/powerpc/cpu/mpc83xx/pcie.c b/arch/powerpc/cpu/mpc83xx/pcie.c
> index 46a706d..2a200b8 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];

Maybe struct pci_controller should contain a field for the driver's
private use, where such things could be placed?

>  	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]);
> +	}
>  }

Won't this break if config space is accessed after initializion?

-Scott

[U-Boot] [PATCH V2] mpc83xx: fix pcie configuration space read/write

[Sergei Shtylyov]

Hello.

On 07-01-2011 15:42, Baidu Boy 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

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

> diff --git a/arch/powerpc/cpu/mpc83xx/pcie.c b/arch/powerpc/cpu/mpc83xx/pcie.c
> index 46a706d..2a200b8 100644
> --- a/arch/powerpc/cpu/mpc83xx/pcie.c
> +++ b/arch/powerpc/cpu/mpc83xx/pcie.c
[...]
> @@ -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++){

    There should be space between ) and {.

> +		pcie_index = i;
>   		mpc83xx_pcie_init_bus(i, reg[i]);
> +	}
>   }

WBR, Sergei

[U-Boot] [PATCH V2] mpc83xx:fix pcie configuration space read/write

[Baidu Boy]

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:
      - code refine
      
 arch/powerpc/cpu/mpc83xx/pcie.c |   20 +++++++++++++++++++-
 include/pci.h                   |    2 ++
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/cpu/mpc83xx/pcie.c b/arch/powerpc/cpu/mpc83xx/pcie.c
index 46a706d..ee94a8b 100644
--- a/arch/powerpc/cpu/mpc83xx/pcie.c
+++ b/arch/powerpc/cpu/mpc83xx/pcie.c
@@ -30,6 +30,21 @@ DECLARE_GLOBAL_DATA_PTR;
 
 #define PCIE_MAX_BUSES 2
 
+/*private structure for mpc83xx pcie hose*/
+static struct mpc83xx_pcie_priv {
+	u8 index;
+} pcie_priv[PCIE_MAX_BUSES] = {
+	{
+		/*pcie controller 1*/
+		.index = 0,
+	},
+	{
+		/*pcie controller 2*/
+		.index = 1,
+	},
+};
+
+
 static struct {
 	u32 base;
 	u32 size;
@@ -52,7 +67,8 @@ 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];
+	struct mpc83xx_pcie_priv *pcie_priv = hose->priv_data;
+	pex83xx_t *pex = &immr->pciexp[pcie_priv->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;
@@ -142,6 +158,8 @@ static void mpc83xx_pcie_register_hose(int bus, struct pci_region *reg,
 
 	hose->cfg_addr = (unsigned int *)mpc83xx_pcie_cfg_space[bus].base;
 
+	hose->priv_data = &pcie_priv[bus];
+
 	pci_set_ops(hose,
 			pcie_read_config_byte,
 			pcie_read_config_word,
diff --git a/include/pci.h b/include/pci.h
index c456006..8b3bdbb 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -420,6 +420,8 @@ struct pci_controller {
 	/* Used by ppc405 autoconfig*/
 	struct pci_region *pci_fb;
 	int current_busno;
+
+	void *priv_data;
 };
 
 extern __inline__ void pci_set_ops(struct pci_controller *hose,
-- 
1.7.3.1.msysgit.0

[U-Boot] [PATCH V2] mpc83xx:fix pcie configuration space read/write

[Wolfgang Denk]

Dear "Baidu Boy",

In message <000401cbb527$cc725cb0$6401a8c0@LENOVOE5CA6843> you 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:
>       - code refine

You submit this as "V2", when it is atleast V4 of this patch?

Please see
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

Do you think that "code refine" is a sufficient "description of what
you have changed compared to previous versions of this patch"?

It is not so for me, which is why I hereby NAK this patch. Sorry.

Also, Scott already pointed out that "SubmittingPatches" lists the
requirement of "using your real name (sorry, no pseudonyms or
anonymous contributions.)" for the Signed-off-by line.  Please confirm
that "Baidu Boy" is your real name.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The light at the end of the tunnel is usually a "No Exit" sign.