| Message ID | 1343305827-26734-2-git-send-email-B38951@freescale.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Kumar Gala |
| Headers | show |
On Jul 26, 2012, at 7:30 AM, Jia Hongtao wrote: > PCI host bridge is primary bus if it contains an ISA node. But not all boards > fit this rule. Device tree should be updated for all these boards. I don't really seen any reason for this patch. We can just use the code as Scott wrote it that sets fsl_pci_primary based on search for the isa node. > > Signed-off-by: Jia Hongtao <B38951@freescale.com> > Signed-off-by: Li Yang <leoli@freescale.com> > --- > Changed for V3: > - Using non-recursive function to find ISA under PCI > > arch/powerpc/include/asm/pci-bridge.h | 1 + > arch/powerpc/sysdev/fsl_pci.c | 31 ++++++++++++++++++++++++------- > arch/powerpc/sysdev/fsl_pci.h | 12 +++++++++++- > 3 files changed, 36 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h > index ac39e6a..b48fa7f 100644 > --- a/arch/powerpc/include/asm/pci-bridge.h > +++ b/arch/powerpc/include/asm/pci-bridge.h > @@ -20,6 +20,7 @@ struct device_node; > struct pci_controller { > struct pci_bus *bus; > char is_dynamic; > + int is_primary; > #ifdef CONFIG_PPC64 > int node; > #endif > diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c > index 5228b6b..97557c5 100644 > --- a/arch/powerpc/sysdev/fsl_pci.c > +++ b/arch/powerpc/sysdev/fsl_pci.c > @@ -453,6 +453,7 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary) > > hose->first_busno = bus_range ? bus_range[0] : 0x0; > hose->last_busno = bus_range ? bus_range[1] : 0xff; > + hose->is_primary = is_primary; > > setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4, > PPC_INDIRECT_TYPE_BIG_ENDIAN); > @@ -933,18 +934,34 @@ void pci_determine_swiotlb(void) > } > #endif > > -int primary_phb_addr; > +/* Checkout if PCI contains ISA node (Only scan the children of PCI) */ > +static int of_pci_has_isa(struct device_node *pci_node) > +{ > + struct device_node *np; > + > + read_lock(&devtree_lock); > + if (!pci_node) > + return 0; > + np = pci_node->allnext; > + for (; np != pci_node->sibling; np = np->allnext) { > + if (np->type && (of_node_cmp(np->type, "isa") == 0) > + && of_node_get(np)) { > + of_node_put(pci_node); > + return 1; > + } > + } > + of_node_put(pci_node); > + read_unlock(&devtree_lock); > + return 0; > +} > + > static int __devinit fsl_pci_probe(struct platform_device *pdev) > { > - struct pci_controller *hose; > bool is_primary; > + is_primary = of_pci_has_isa(pdev->dev.of_node); > > - if (of_match_node(pci_ids, pdev->dev.of_node)) { > - struct resource rsrc; > - of_address_to_resource(pdev->dev.of_node, 0, &rsrc); > - is_primary = ((rsrc.start & 0xfffff) == primary_phb_addr); > + if (of_match_node(pci_ids, pdev->dev.of_node)) > fsl_add_bridge(pdev->dev.of_node, is_primary); > - } > > return 0; > } > diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h > index 095392d..c884e06 100644 > --- a/arch/powerpc/sysdev/fsl_pci.h > +++ b/arch/powerpc/sysdev/fsl_pci.h > @@ -88,7 +88,17 @@ struct ccsr_pci { > __be32 pex_err_cap_r3; /* 0x.e34 - PCIE error capture register 0 */ > }; > > -extern int primary_phb_addr; > + > +#ifdef CONFIG_SUSPEND > +struct fsl_pci_private_data { > + int inbound_num; > + struct pci_outbound_window_regs __iomem *pci_pow; > + struct pci_inbound_window_regs __iomem *pci_piw; > + void *saved_regs; > +}; > +#endif > + This struct has nothing to do with this patch > +extern int is_has_isa_node(struct device_node *parent); Where is is_has_isa_node() defined or used? > extern int fsl_add_bridge(struct device_node *dev, int is_primary); > extern void fsl_pcibios_fixup_bus(struct pci_bus *bus); > extern int mpc83xx_add_bridge(struct device_node *dev); > -- > 1.7.5.1 >
Thanks for all your comments. Sorry for the mistakes in this patchset. I am just so eager to push them to upstream. I will work on the comments very carfully. Thanks. -Hongtao. > -----Original Message----- > From: Kumar Gala [mailto:galak@kernel.crashing.org] > Sent: Friday, July 27, 2012 2:22 AM > To: Jia Hongtao-B38951 > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li Yang-R58472 > Subject: Re: [PATCH V3 2/5] powerpc/fsl-pci: Determine primary bus by > looking for ISA node > > > On Jul 26, 2012, at 7:30 AM, Jia Hongtao wrote: > > > PCI host bridge is primary bus if it contains an ISA node. But not all > boards > > fit this rule. Device tree should be updated for all these boards. > > I don't really seen any reason for this patch. We can just use the code > as Scott wrote it that sets fsl_pci_primary based on search for the isa > node. > > > > > Signed-off-by: Jia Hongtao <B38951@freescale.com> > > Signed-off-by: Li Yang <leoli@freescale.com> > > --- > > Changed for V3: > > - Using non-recursive function to find ISA under PCI > > > > arch/powerpc/include/asm/pci-bridge.h | 1 + > > arch/powerpc/sysdev/fsl_pci.c | 31 ++++++++++++++++++++++++-- > ----- > > arch/powerpc/sysdev/fsl_pci.h | 12 +++++++++++- > > 3 files changed, 36 insertions(+), 8 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/pci-bridge.h > b/arch/powerpc/include/asm/pci-bridge.h > > index ac39e6a..b48fa7f 100644 > > --- a/arch/powerpc/include/asm/pci-bridge.h > > +++ b/arch/powerpc/include/asm/pci-bridge.h > > @@ -20,6 +20,7 @@ struct device_node; > > struct pci_controller { > > struct pci_bus *bus; > > char is_dynamic; > > + int is_primary; > > #ifdef CONFIG_PPC64 > > int node; > > #endif > > diff --git a/arch/powerpc/sysdev/fsl_pci.c > b/arch/powerpc/sysdev/fsl_pci.c > > index 5228b6b..97557c5 100644 > > --- a/arch/powerpc/sysdev/fsl_pci.c > > +++ b/arch/powerpc/sysdev/fsl_pci.c > > @@ -453,6 +453,7 @@ int __init fsl_add_bridge(struct device_node *dev, > int is_primary) > > > > hose->first_busno = bus_range ? bus_range[0] : 0x0; > > hose->last_busno = bus_range ? bus_range[1] : 0xff; > > + hose->is_primary = is_primary; > > > > setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4, > > PPC_INDIRECT_TYPE_BIG_ENDIAN); > > @@ -933,18 +934,34 @@ void pci_determine_swiotlb(void) > > } > > #endif > > > > -int primary_phb_addr; > > +/* Checkout if PCI contains ISA node (Only scan the children of PCI) > */ > > +static int of_pci_has_isa(struct device_node *pci_node) > > +{ > > + struct device_node *np; > > + > > + read_lock(&devtree_lock); > > + if (!pci_node) > > + return 0; > > + np = pci_node->allnext; > > + for (; np != pci_node->sibling; np = np->allnext) { > > + if (np->type && (of_node_cmp(np->type, "isa") == 0) > > + && of_node_get(np)) { > > + of_node_put(pci_node); > > + return 1; > > + } > > + } > > + of_node_put(pci_node); > > + read_unlock(&devtree_lock); > > + return 0; > > +} > > + > > static int __devinit fsl_pci_probe(struct platform_device *pdev) > > { > > - struct pci_controller *hose; > > bool is_primary; > > + is_primary = of_pci_has_isa(pdev->dev.of_node); > > > > - if (of_match_node(pci_ids, pdev->dev.of_node)) { > > - struct resource rsrc; > > - of_address_to_resource(pdev->dev.of_node, 0, &rsrc); > > - is_primary = ((rsrc.start & 0xfffff) == primary_phb_addr); > > + if (of_match_node(pci_ids, pdev->dev.of_node)) > > fsl_add_bridge(pdev->dev.of_node, is_primary); > > - } > > > > return 0; > > } > > diff --git a/arch/powerpc/sysdev/fsl_pci.h > b/arch/powerpc/sysdev/fsl_pci.h > > index 095392d..c884e06 100644 > > --- a/arch/powerpc/sysdev/fsl_pci.h > > +++ b/arch/powerpc/sysdev/fsl_pci.h > > @@ -88,7 +88,17 @@ struct ccsr_pci { > > __be32 pex_err_cap_r3; /* 0x.e34 - PCIE error capture > register 0 */ > > }; > > > > -extern int primary_phb_addr; > > + > > +#ifdef CONFIG_SUSPEND > > +struct fsl_pci_private_data { > > + int inbound_num; > > + struct pci_outbound_window_regs __iomem *pci_pow; > > + struct pci_inbound_window_regs __iomem *pci_piw; > > + void *saved_regs; > > +}; > > +#endif > > + > > This struct has nothing to do with this patch > > > +extern int is_has_isa_node(struct device_node *parent); > > Where is is_has_isa_node() defined or used? > > > extern int fsl_add_bridge(struct device_node *dev, int is_primary); > > extern void fsl_pcibios_fixup_bus(struct pci_bus *bus); > > extern int mpc83xx_add_bridge(struct device_node *dev); > > -- > > 1.7.5.1 > > >
> -----Original Message----- > From: Kumar Gala [mailto:galak@kernel.crashing.org] > Sent: Friday, July 27, 2012 2:22 AM > To: Jia Hongtao-B38951 > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li Yang-R58472 > Subject: Re: [PATCH V3 2/5] powerpc/fsl-pci: Determine primary bus by > looking for ISA node > > > On Jul 26, 2012, at 7:30 AM, Jia Hongtao wrote: > > > PCI host bridge is primary bus if it contains an ISA node. But not all > boards > > fit this rule. Device tree should be updated for all these boards. > > I don't really seen any reason for this patch. We can just use the code > as Scott wrote it that sets fsl_pci_primary based on search for the isa > node. > I change the way of searching ISA node just because the platform driver mechanism. Probe function of this driver will be called for each PCI controller which means more than once. I think the Scott's way is not perfectly match this situation. Anyway I will find a better way to solve this by refactoring the Scott's method or using my own way. Thanks. -Hongtao.
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h index ac39e6a..b48fa7f 100644 --- a/arch/powerpc/include/asm/pci-bridge.h +++ b/arch/powerpc/include/asm/pci-bridge.h @@ -20,6 +20,7 @@ struct device_node; struct pci_controller { struct pci_bus *bus; char is_dynamic; + int is_primary; #ifdef CONFIG_PPC64 int node; #endif diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index 5228b6b..97557c5 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -453,6 +453,7 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary) hose->first_busno = bus_range ? bus_range[0] : 0x0; hose->last_busno = bus_range ? bus_range[1] : 0xff; + hose->is_primary = is_primary; setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4, PPC_INDIRECT_TYPE_BIG_ENDIAN); @@ -933,18 +934,34 @@ void pci_determine_swiotlb(void) } #endif -int primary_phb_addr; +/* Checkout if PCI contains ISA node (Only scan the children of PCI) */ +static int of_pci_has_isa(struct device_node *pci_node) +{ + struct device_node *np; + + read_lock(&devtree_lock); + if (!pci_node) + return 0; + np = pci_node->allnext; + for (; np != pci_node->sibling; np = np->allnext) { + if (np->type && (of_node_cmp(np->type, "isa") == 0) + && of_node_get(np)) { + of_node_put(pci_node); + return 1; + } + } + of_node_put(pci_node); + read_unlock(&devtree_lock); + return 0; +} + static int __devinit fsl_pci_probe(struct platform_device *pdev) { - struct pci_controller *hose; bool is_primary; + is_primary = of_pci_has_isa(pdev->dev.of_node); - if (of_match_node(pci_ids, pdev->dev.of_node)) { - struct resource rsrc; - of_address_to_resource(pdev->dev.of_node, 0, &rsrc); - is_primary = ((rsrc.start & 0xfffff) == primary_phb_addr); + if (of_match_node(pci_ids, pdev->dev.of_node)) fsl_add_bridge(pdev->dev.of_node, is_primary); - } return 0; } diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h index 095392d..c884e06 100644 --- a/arch/powerpc/sysdev/fsl_pci.h +++ b/arch/powerpc/sysdev/fsl_pci.h @@ -88,7 +88,17 @@ struct ccsr_pci { __be32 pex_err_cap_r3; /* 0x.e34 - PCIE error capture register 0 */ }; -extern int primary_phb_addr; + +#ifdef CONFIG_SUSPEND +struct fsl_pci_private_data { + int inbound_num; + struct pci_outbound_window_regs __iomem *pci_pow; + struct pci_inbound_window_regs __iomem *pci_piw; + void *saved_regs; +}; +#endif + +extern int is_has_isa_node(struct device_node *parent); extern int fsl_add_bridge(struct device_node *dev, int is_primary); extern void fsl_pcibios_fixup_bus(struct pci_bus *bus); extern int mpc83xx_add_bridge(struct device_node *dev);