[PATCH V3 1/5] powerpc/fsl-pci: Unify pci/pcie initialization code
Jia Hongtao-B38951
B38951 at freescale.com
Fri Jul 27 20:10:10 EST 2012
Hi kumar,
I know "duplicate code from pci_process_bridge_OF_ranges()" is
hard to accept but "refactor the code to have a shared function"
is knotty. Actually this is the reason I didn't do the refactor.
Here is the situation:
First, pci_process_bridge_OF_ranges() is a common code using by
so many pci client.
Second, the contents of pci_process_bridge_OF_ranges() twisted
together. It's hard to decouple the function I need for determining
swiotlb with other contents. I tried and found a way to do this
but the shared function need so many parameters which is also
unacceptable.
Third, my function to determine swiotlb should know the start
and the end of pci mem space address for all the controllers.
Note that the end of address is for determining where to map
PEXCSRBAR.
If you have any idea for this please let me know.
Thanks.
-Hongtao.
> -----Original Message-----
> From: Kumar Gala [mailto:galak at kernel.crashing.org]
> Sent: Friday, July 27, 2012 1:53 AM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev at lists.ozlabs.org; Wood Scott-B07421; Li Yang-R58472
> Subject: Re: [PATCH V3 1/5] powerpc/fsl-pci: Unify pci/pcie
> initialization code
>
>
> On Jul 26, 2012, at 7:30 AM, Jia Hongtao wrote:
>
> > We unified the Freescale pci/pcie initialization by changing the
> fsl_pci
> > to a platform driver. In previous PCI code architecture the
> initialization
> > routine is called at board_setup_arch stage. Now the initialization is
> done
> > in probe function which is architectural better. Also It's convenient
> for
> > adding PM support for PCI controller in later patch.
> >
> > One issue introduced by this architecture is the timing of swiotlb_init.
> > During PCI initialization the need of swiotlb is determined and this
> should
> > be done before swiotlb_init. So a new function to determine swiotlb by
> > parsing pci ranges is made. This function is called at board_setup_arch
> > stage which is earlier than swiotlb_init.
> >
> > Signed-off-by: Jia Hongtao <B38951 at freescale.com>
> > Signed-off-by: Li Yang <leoli at freescale.com>
> > ---
> > Changed for V3:
> > - Rebase the patch set on the latest tree
> > - merge PCI unify and swiotlb patch into one
> >
> > arch/powerpc/sysdev/fsl_pci.c | 155 ++++++++++++++++++++++++++++++++--
> -------
> > arch/powerpc/sysdev/fsl_pci.h | 9 +--
> > 2 files changed, 125 insertions(+), 39 deletions(-)
> >
> > diff --git a/arch/powerpc/sysdev/fsl_pci.c
> b/arch/powerpc/sysdev/fsl_pci.c
> > index a7b2a60..5228b6b 100644
> > --- a/arch/powerpc/sysdev/fsl_pci.c
> > +++ b/arch/powerpc/sysdev/fsl_pci.c
> > @@ -823,56 +823,143 @@ static const struct of_device_id pci_ids[] = {
> > {},
> > };
> >
> > -struct device_node *fsl_pci_primary;
> > -
> > -void __devinit fsl_pci_init(void)
> > +#ifdef CONFIG_SWIOTLB
> > +void pci_determine_swiotlb(void)
> > {
> > + const u32 *ranges;
> > + int rlen;
> > + int pna;
> > + int np;
> > struct device_node *node;
> > - struct pci_controller *hose;
> > - dma_addr_t max = 0xffffffff;
> > -
> > - /* Callers can specify the primary bus using other means. */
> > - if (!fsl_pci_primary) {
> > - /* If a PCI host bridge contains an ISA node, it's primary.
> */
> > - node = of_find_node_by_type(NULL, "isa");
> > - while ((fsl_pci_primary = of_get_parent(node))) {
> > - of_node_put(node);
> > - node = fsl_pci_primary;
> > -
> > - if (of_match_node(pci_ids, node))
> > - break;
> > - }
> > - }
> > + int memno;
> > + u32 pci_space;
> > + unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
> > + unsigned long long pci_addr_lo = ULLONG_MAX;
> > + unsigned long long pci_addr_hi = 0x0;
> > + dma_addr_t pci_dma_sz;
> >
> > - node = NULL;
> > for_each_node_by_type(node, "pci") {
> > if (of_match_node(pci_ids, node)) {
> > - /*
> > - * If there's no PCI host bridge with ISA, arbitrarily
> > - * designate one as primary. This can go away once
> > - * various bugs with primary-less systems are fixed.
> > - */
> > - if (!fsl_pci_primary)
> > - fsl_pci_primary = node;
> > -
> > - fsl_add_bridge(node, fsl_pci_primary == node);
> > - hose = pci_find_hose_for_OF_device(node);
> > - max = min(max, hose->dma_window_base_cur +
> > - hose->dma_window_size);
> > + memno = 0;
> > + pna = of_n_addr_cells(node);
> > + np = pna + 5;
>
> Don't duplicate code from pci_process_bridge_OF_ranges(), refactor the
> code to have a shared function:
>
> > + /* Get ranges property */
> > + ranges = of_get_property(node, "ranges", &rlen);
> > + if (ranges == NULL)
> > + return;
> > +
> > + /* Parse outbound MEM window range */
> > + while ((rlen -= np * 4) >= 0) {
> > + /* Read next ranges element */
> > + pci_space = ranges[0];
> > + if (!((pci_space >> 24) & 0x2)) {
> > + ranges += np;
> > + break;
> > + }
> > + pci_addr = of_read_number(ranges + 1, 2);
> > + cpu_addr = of_translate_address(
> > + node, ranges + 3);
> > + size = of_read_number(ranges + pna + 3, 2);
> > + ranges += np;
> > +
> > + /*
> > + * If we failed translation or got a zero-sized
> > + * region (some FW try to feed us with non
> > + * sensical zero sized regions such as power3
> > + * which look like some kind of attempt at
> > + * exposing the VGA memory hole)
> > + */
> > + if (cpu_addr == OF_BAD_ADDR || size == 0)
> > + continue;
> > +
> > + /*
> > + * Now consume following elements while they
> > + * are contiguous
> > + */
> > + for (; rlen >= np * sizeof(u32);
> > + ranges += np, rlen -= np * 4) {
> > + if (ranges[0] != pci_space)
> > + break;
> > + pci_next = of_read_number(ranges + 1,
> > + 2);
> > + cpu_next = of_translate_address(node,
> > + ranges + 3);
> > + if (pci_next != pci_addr + size ||
> > + cpu_next != cpu_addr + size)
> > + break;
> > + size += of_read_number(
> > + ranges + pna + 3, 2);
> > + }
> > +
> > + /* We support only 3 memory ranges */
> > + if (memno >= 3) {
> > + printk(KERN_INFO
> > + " \\--> Skipped (too
> many) !\n");
> > + continue;
> > + }
> > +
> > + pci_addr_lo = min(pci_addr, pci_addr_lo);
> > + pci_addr_hi = max(pci_addr + size, pci_addr_hi);
> > + memno++;
> > + }
> > }
> > }
> >
> > -#ifdef CONFIG_SWIOTLB
> > + /* Get PEXCSRBAR size (equal to CCSR size) */
> > + node = of_find_node_by_type(NULL, "soc");
> > + ranges = of_get_property(node, "ranges", &rlen);
> > + if (ranges == NULL)
> > + return;
> > +
> > + size = of_read_number(ranges + 3, 1);
> > + of_node_put(node);
> > +
> > + if (pci_addr_hi < (0x100000000ull - size))
> > + pci_dma_sz = pci_addr_lo;
> > + else
> > + pci_dma_sz = pci_addr_lo - size;
> > +
> > /*
> > * if we couldn't map all of DRAM via the dma windows
> > * we need SWIOTLB to handle buffers located outside of
> > * dma capable memory region
> > */
> > - if (memblock_end_of_DRAM() - 1 > max) {
> > + if (memblock_end_of_DRAM() > pci_dma_sz) {
> > ppc_swiotlb_enable = 1;
> > set_pci_dma_ops(&swiotlb_dma_ops);
> > - ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
> > + ppc_md.pci_dma_dev_setup =
> > + pci_dma_dev_setup_swiotlb;
>
> why the line wrap change?
>
> > }
> > +}
> > #endif
> > +
> > +int primary_phb_addr;
> > +static int __devinit fsl_pci_probe(struct platform_device *pdev)
> > +{
> > + struct pci_controller *hose;
> > + bool is_primary;
> > +
> > + 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);
> > + fsl_add_bridge(pdev->dev.of_node, is_primary);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver fsl_pci_driver = {
> > + .driver = {
> > + .name = "fsl-pci",
> > + .of_match_table = pci_ids,
> > + },
> > + .probe = fsl_pci_probe,
> > +};
> > +
> > +static int __init fsl_pci_init(void)
> > +{
> > + return platform_driver_register(&fsl_pci_driver);
> > }
> > +arch_initcall(fsl_pci_init);
> > #endif
>
>
More information about the Linuxppc-dev
mailing list