[U-Boot] [PATCH] pci: Add a command to show PCI regions

[Bin Meng]

Hi Simon,

On Fri, Mar 17, 2017 at 7:50 PM, Simon Glass <sjg at chromium.org> wrote:
> Add 'pci regions' which lists the I/O and memory regions accessible from
> the PCI controller.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>  cmd/pci.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/pci.c b/cmd/pci.c
> index 7993c1a099..84d663d442 100644
> --- a/cmd/pci.c
> +++ b/cmd/pci.c
> @@ -606,6 +606,45 @@ static int pci_cfg_modify(pci_dev_t bdf, ulong addr, ulong size, ulong value,
>         return 0;
>  }
>
> +static const struct pci_flag_info {
> +       uint flag;
> +       const char *name;
> +} pci_flag_info[] = {
> +       { PCI_REGION_IO, "io" },
> +       { PCI_REGION_PREFETCH, "prefetch" },
> +       { PCI_REGION_SYS_MEMORY, "sysmem" },
> +       { PCI_REGION_RO, "readonly" },
> +       { PCI_REGION_IO, "io" },

Duplicated entry for "io"?

> +};
> +
> +static void pci_show_regions(struct udevice *bus)
> +{
> +       struct pci_controller *hose = dev_get_uclass_priv(bus);
> +       const struct pci_region *reg;
> +       int i, j;
> +
> +       if (!hose) {
> +               printf("Bus '%s' is not a PCI controller\n", bus->name);
> +               return;
> +       }
> +
> +       printf("#   %-16s %-16s %-16s  %s\n", "Bus start", "Phys start", "Size",
> +              "Flags");
> +       for (i = 0, reg = hose->regions; i < hose->region_count; i++, reg++) {
> +               printf("%d   %#016llx %#016llx %#016llx  ", i,
> +                      (unsigned long long)reg->bus_start,
> +                      (unsigned long long)reg->phys_start,
> +                      (unsigned long long)reg->size);
> +               if (!(reg->flags & PCI_REGION_TYPE))
> +                       printf("mem ");
> +               for (j = 0; j < ARRAY_SIZE(pci_flag_info); j++) {
> +                       if (reg->flags & pci_flag_info[j].flag)
> +                               printf("%s ", pci_flag_info[j].name);
> +               }
> +               printf("\n");
> +       }
> +}

Is it a good idea to explicitly distinguish the regions of inbound
(sysmem) and outbound (mem, io, prefetch) in the output?

> +
>  /* PCI Configuration Space access commands
>   *
>   * Syntax:
> @@ -657,10 +696,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                 pci_init();
>                 return 0;
>  #endif
> +       case 'r': /* no break */
>         default:                /* scan bus */
>                 value = 1; /* short listing */
>                 if (argc > 1) {
> -                       if (argv[argc-1][0] == 'l') {
> +                       if (cmd != 'r' && argv[argc-1][0] == 'l') {
>                                 value = 0;
>                                 argc--;
>                         }
> @@ -673,7 +713,10 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                         printf("No such bus\n");
>                         return CMD_RET_FAILURE;
>                 }
> -               pciinfo(bus, value);
> +               if (cmd == 'r')
> +                       pci_show_regions(bus);
> +               else
> +                       pciinfo(bus, value);
>  #else
>                 pciinfo(busnum, value);
>  #endif
> @@ -745,6 +788,8 @@ static char pci_help_text[] =
>  #ifdef CONFIG_DM_PCI
>         "pci bar b.d.f\n"
>         "    - show BARs base and size for device b.d.f'\n"
> +       "pci regions\n"
> +       "    - show PCI regions\n"
>  #endif
>         "pci display[.b, .w, .l] b.d.f [address] [# of objects]\n"
>         "    - display PCI configuration space (CFG)\n"
> --

Regards,
Bin