On Thu, Oct 08, 2015 at 02:38:22PM -0700, Yinghai Lu wrote:
> We register regions for legacy and iommu and all have open code.
> 
> Unify them to pci_register_region() and call it accordingly.
> 
> Signed-off-by: Yinghai Lu <ying...@kernel.org>
> ---
>  arch/sparc/kernel/pci_common.c | 83 
> +++++++++++++++++++-----------------------
>  1 file changed, 37 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/sparc/kernel/pci_common.c b/arch/sparc/kernel/pci_common.c
> index 28e976a..c4b6989 100644
> --- a/arch/sparc/kernel/pci_common.c
> +++ b/arch/sparc/kernel/pci_common.c
> @@ -328,41 +328,49 @@ void pci_get_pbm_props(struct pci_pbm_info *pbm)
>       }
>  }
>  
> -static void pci_register_legacy_regions(struct resource *io_res,
> -                                     struct resource *mem_res)
> +static void pci_register_region(struct pci_pbm_info *pbm, const char *name,
> +                             resource_size_t rstart, resource_size_t size)
>  {
> -     struct resource *p;
> +     struct resource *res, *conflict;
> +     struct resource *mem_res = &pbm->mem_space;
> +     resource_size_t offset = pbm->mem_offset;
> +     resource_size_t mem_rstart, mem_rend;
> +     resource_size_t rend = rstart + size - 1UL;
>  
> -     /* VGA Video RAM. */
> -     p = kzalloc(sizeof(*p), GFP_KERNEL);
> -     if (!p)
> +     if (!mem_res->flags)
>               return;
>  
> -     p->name = "Video RAM area";
> -     p->start = mem_res->start + 0xa0000UL;
> -     p->end = p->start + 0x1ffffUL;
> -     p->flags = IORESOURCE_BUSY;
> -     request_resource(mem_res, p);
> +     mem_rstart = mem_res->start - offset;
> +     mem_rend = mem_res->end - offset;

This is essentially doing pcibios_bus_to_resource(), except in an
unnecessarily arch-dependent way.  Can you rework it to use
pcibios_bus_to_resource()?  I think you'll have to do
pci_register_legacy_regions() in pci_scan_one_pbm() instead of in
pci_determine_mem_io_space(), so that you have a struct pci_bus to
use, but your next patch does that anyway.

>  
> -     p = kzalloc(sizeof(*p), GFP_KERNEL);
> -     if (!p)
> +     /* contain checking */
> +     if (!(mem_rstart <= rstart && mem_rend >= rend))
>               return;
>  
> -     p->name = "System ROM";
> -     p->start = mem_res->start + 0xf0000UL;
> -     p->end = p->start + 0xffffUL;
> -     p->flags = IORESOURCE_BUSY;
> -     request_resource(mem_res, p);
> -
> -     p = kzalloc(sizeof(*p), GFP_KERNEL);
> -     if (!p)
> +     res = kzalloc(sizeof(*res), GFP_KERNEL);
> +     if (!res)
>               return;
>  
> -     p->name = "Video ROM";
> -     p->start = mem_res->start + 0xc0000UL;
> -     p->end = p->start + 0x7fffUL;
> -     p->flags = IORESOURCE_BUSY;
> -     request_resource(mem_res, p);
> +     res->name = name;
> +     res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> +     res->start = rstart + offset;
> +     res->end = rend + offset;
> +     conflict = request_resource_conflict(mem_res, res);
> +     if (conflict) {
> +             printk(KERN_DEBUG "PCI: %s can't claim %s %pR: address conflict 
> with %s %pR\n",
> +                     pbm->name, res->name, res, conflict->name, conflict);
> +             kfree(res);
> +     }
> +}
> +
> +static void pci_register_legacy_regions(struct pci_pbm_info *pbm)
> +{
> +     /* VGA Video RAM. */
> +     pci_register_region(pbm, "Video RAM area", 0xa0000UL, 0x20000UL);
> +
> +     pci_register_region(pbm, "System ROM",     0xf0000UL, 0x10000UL);
> +
> +     pci_register_region(pbm, "Video ROM",      0xc0000UL,  0x8000UL);
>  }
>  
>  static void pci_register_iommu_region(struct pci_pbm_info *pbm)
> @@ -370,24 +378,8 @@ static void pci_register_iommu_region(struct 
> pci_pbm_info *pbm)
>       const u32 *vdma = of_get_property(pbm->op->dev.of_node, "virtual-dma",
>                                         NULL);
>  
> -     if (vdma) {
> -             struct resource *rp = kzalloc(sizeof(*rp), GFP_KERNEL);
> -
> -             if (!rp) {
> -                     pr_info("%s: Cannot allocate IOMMU resource.\n",
> -                             pbm->name);
> -                     return;
> -             }
> -             rp->name = "IOMMU";
> -             rp->start = pbm->mem_space.start + (unsigned long) vdma[0];
> -             rp->end = rp->start + (unsigned long) vdma[1] - 1UL;
> -             rp->flags = IORESOURCE_BUSY;
> -             if (request_resource(&pbm->mem_space, rp)) {
> -                     pr_info("%s: Unable to request IOMMU resource.\n",
> -                             pbm->name);
> -                     kfree(rp);
> -             }
> -     }
> +     if (vdma)
> +             pci_register_region(pbm, "IOMMU", vdma[0], vdma[1]);
>  }
>  
>  void pci_determine_mem_io_space(struct pci_pbm_info *pbm)
> @@ -506,8 +498,7 @@ void pci_determine_mem_io_space(struct pci_pbm_info *pbm)
>       if (pbm->mem64_space.flags)
>               request_resource(&iomem_resource, &pbm->mem64_space);
>  
> -     pci_register_legacy_regions(&pbm->io_space,
> -                                 &pbm->mem_space);
> +     pci_register_legacy_regions(pbm);
>       pci_register_iommu_region(pbm);
>  }
>  
> -- 
> 1.8.4.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to