[U-Boot] [PATCH 08/23] pci: Make PCI bridge memory alignment configurable

[Simon Glass]

Hi Paul,

On 26 September 2016 at 12:29, Paul Burton <paul.burton at imgtec.com> wrote:
> On some systems aligning PCI memory to a 1MB boundary every time a
> bridge is encountered may lead to exhausting the available memory space
> before all devices have been assigned addresses.
>
> For example on the MIPS Boston development board we have an Intel EG20T
> Platform Controller Hub connected to a Xilinx AXI to PCIe root port
> which is only assigned a 1MB memory region. The Intel EG20T contains a
> bridge device beneath which all of its peripheral devices can be found,
> and that bridge device contains a ROM. If we align to 1MB when we
> encounter each bridge device we therefore do something like this:
>
>   - Start with bus_lower at 0x16000000.
>
>   - Find the Xilinx root bridge, which has no visible BARs so we do very
>     little to it.
>
>   - Probe the bus beneath the Xilinx bridge device, aligning bus_lower
>     to a 1MB boundary first. That leaves it still at 0x16000000.
>
>   - Find the EG20T bridge device, which we find has a 64KiB ROM. We
>     assign it the address range 0x16000000-0x1600ffff which leaves
>     bus_lower at 0x16010000.
>
>   - Probe the bus beneath the EG20T bridge device, aligning bus_lower to
>     a 1MB boundary first. This leaves bus_lower at 0x16100000, which is
>     the end of the available memory space.
>
>   - Find the various peripheral devices the EG20T contains, but fail to
>     assign any memory space to them since bus_lower is at the end of the
>     memory space available to the PCI bus.
>
> This patch allows that 1MB alignment to be changed or disabled via
> Kconfig, so that systems such as this can select an alignment which
> works for them.
>
> Signed-off-by: Paul Burton <paul.burton at imgtec.com>
>
> ---
>
>  drivers/pci/Kconfig    |  7 +++++++
>  drivers/pci/pci_auto.c | 24 ++++++++++++++++--------
>  2 files changed, 23 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

Please see below

>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 9a7c187..35bf8fe 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -18,6 +18,13 @@ config DM_PCI_COMPAT
>           measure when porting a board to use driver model for PCI. Once the
>           board is fully supported, this option should be disabled.
>
> +config DM_PCI_BRIDGE_MEM_ALIGN
> +       hex "PCI bridge memory alignment"
> +       default 0x100000
> +       help
> +         PCI memory space will be aligned to be a multiple of this value
> +         whenever a PCI-to-PCI bridge is encountered.

Can you also add why this is useful, and why 1MB is a suitable default?

> +
>  config PCI_SANDBOX
>         bool "Sandbox PCI support"
>         depends on SANDBOX && DM_PCI
> diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
> index ee9a854..0326316 100644
> --- a/drivers/pci/pci_auto.c
> +++ b/drivers/pci/pci_auto.c
> @@ -186,8 +186,10 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
>         dm_pci_write_config8(dev, PCI_SUBORDINATE_BUS, 0xff);
>
>         if (pci_mem) {
> -               /* Round memory allocator to 1MB boundary */
> -               pciauto_region_align(pci_mem, 0x100000);
> +               /* Round memory allocator to a suitable boundary */
> +               if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN)

Can you update the pciauto_region_align() so you can omit this if () ?

> +                       pciauto_region_align(pci_mem,
> +                                            CONFIG_DM_PCI_BRIDGE_MEM_ALIGN);
>
>                 /*
>                  * Set up memory and I/O filter limits, assume 32-bit
> @@ -200,8 +202,10 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
>         }
>
>         if (pci_prefetch) {
> -               /* Round memory allocator to 1MB boundary */
> -               pciauto_region_align(pci_prefetch, 0x100000);
> +               /* Round memory allocator to a suitable boundary */
> +               if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN)
> +                       pciauto_region_align(pci_prefetch,
> +                                            CONFIG_DM_PCI_BRIDGE_MEM_ALIGN);
>
>                 /*
>                  * Set up memory and I/O filter limits, assume 32-bit
> @@ -260,8 +264,10 @@ void dm_pciauto_postscan_setup_bridge(struct udevice *dev, int sub_bus)
>         dm_pci_write_config8(dev, PCI_SUBORDINATE_BUS, sub_bus);
>
>         if (pci_mem) {
> -               /* Round memory allocator to 1MB boundary */
> -               pciauto_region_align(pci_mem, 0x100000);
> +               /* Round memory allocator to a suitable boundary */
> +               if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN)
> +                       pciauto_region_align(pci_mem,
> +                                            CONFIG_DM_PCI_BRIDGE_MEM_ALIGN);
>
>                 dm_pci_write_config16(dev, PCI_MEMORY_LIMIT,
>                                       (pci_mem->bus_lower - 1) >> 16);
> @@ -274,8 +280,10 @@ void dm_pciauto_postscan_setup_bridge(struct udevice *dev, int sub_bus)
>                                      &prefechable_64);
>                 prefechable_64 &= PCI_PREF_RANGE_TYPE_MASK;
>
> -               /* Round memory allocator to 1MB boundary */
> -               pciauto_region_align(pci_prefetch, 0x100000);
> +               /* Round memory allocator to a suitable boundary */
> +               if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN)
> +                       pciauto_region_align(pci_prefetch,
> +                                            CONFIG_DM_PCI_BRIDGE_MEM_ALIGN);
>
>                 dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT,
>                                       (pci_prefetch->bus_lower - 1) >> 16);
> --
> 2.10.0
>

Regards,
Simon