On Mon, May 3, 2021 at 11:16 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> This adds a new PCI controller driver for the Intel IXP4xx
> (IX425, IXP435 etc), based on the XScale microarchitecture.
>
> This replaces the old driver in arch/arm/mach-ixp4xx/common-pci.c
> which utilized the ARM-specific BIOS32 PCI framework,
> and all parameterization for such things as memory and
> IO space as well as interrupt swizzling is done from the
> device tree.
Nice work!
> The __raw_writel() and __raw_readl() are used for accessing
> the PCI controller for the same reason that these accessors
> are used in the timer, IRQ and GPIO drivers: the platform
> will alter its address bus pattern based on whether the
> system is booted in big- or little-endian mode. For this
> reason all register on IXP4xx must always be accessed in
> native (CPU) endianness.
Can you add a pair of inline function that wraps these into
a driver specific helper with a comment?
> This driver supports 64MB of PCI memory space, but not the
> indirect access of 1GB that is available in the old driver.
> We can address that later if and only if there are users
> that need all 1GB of PCI address space.
> +
> + ret = pci_scan_root_bus_bridge(host);
> + if (ret) {
> + dev_err(dev, "failed to scan host: %d\n", ret);
> + return ret;
> + }
> +
> + p->bus = host->bus;
I don't think you need to store the bus separately, just use
host->bus everywhere.
> + pci_bus_assign_resources(p->bus);
> + pci_bus_add_devices(p->bus);
Can you call pci_host_probe() instead of open-coding it here?
> +
> +static struct platform_driver ixp4xx_pci_driver = {
> + .driver = {
> + .name = "ixp4xx-pci",
> + .of_match_table = of_match_ptr(ixp4xx_pci_of_match),
> + .suppress_bind_attrs = true,
> + },
> + .probe = ixp4xx_pci_probe,
> +};
> +builtin_platform_driver(ixp4xx_pci_driver);
It should be possible to make it a loadable module, after Rob Herring
fixed some of the bugs around that.
Arnd
