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

[Simon Glass]

Hi Bin,

On 22 March 2017 at 03:27, Bin Meng <bmeng.cn at gmail.com> wrote:
> 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"?

Oops, will fix.

>
>> +};
>> +
>> +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?

Possibly...can you give me an example so I can understand your idea?

>
>> +
>>  /* 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

Regards,
Simon