Hi Bin,

On 22 March 2017 at 03:27, Bin Meng bmeng.cn@gmail.com wrote:

Hi Simon,

On Fri, Mar 17, 2017 at 7:50 PM, Simon Glass sjg@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@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