On Fri, May 02, 2014 at 05:41:15PM +0100, Will Deacon wrote:
> + bus_range = &pci->cfg.bus_range;
> + for (busn = bus_range->start; busn <= bus_range->end; ++busn) {
> + u32 idx = busn - bus_range->start;
> + u32 sz = 1 << pci->cfg.ops->bus_shift;
> +
> + pci->cfg.win[idx] = devm_ioremap(dev,
> + pci->cfg.res.start + busn * sz,
> + sz);
Why map each bus individually? Both CAM and ECAM define consecutive
busses consecutively in the address space, and I though ioremap was OK
with unaligned stuff?
> +out_unmap_cfg:
> + while (busn-- > bus_range->start)
> + devm_iounmap(dev, pci->cfg.win[busn - bus_range->start]);
Is there a reason to explicitly clean up devm resources? I guess it is
because this is in setup not probe?
It seems strange to me for a driver to do this sort of work in a setup
function, typically probe acquires as much stuff as it can, that way
defered probe can work properly.
Looking at pci-mvebu, 'setup' is only populating the resource list, I
would suggest the same split for this driver.
> +out_release_res:
> + release_child_resources(&iomem_resource);
> + release_child_resources(&ioport_resource);
This looks really off to me, doesn't this free all resources in the
system?
This isn't enough?:
> + list_for_each_entry(win, &sys->resources, list)
> + devm_kfree(dev, win->res);
> + pci_free_resource_list(&sys->resources);
At worst, I would guess 'release_child_resources(win->res);' in the
loop. But since no bus scan has been done, is there any chance of
child resources at this point?
Regards,
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
