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

[Simon Glass]

Hi Bin,

On 22 March 2017 at 03:27, Bin Meng  wrote:
> Hi Simon,
>
> On Fri, Mar 17, 2017 at 7:50 PM, Simon Glass  wrote:
>> Add 'pci regions' which lists the I/O and memory regions accessible from
>> the PCI controller.
>>
>> Signed-off-by: Simon Glass 
>> ---
>>
>>  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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [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  wrote:
> Add 'pci regions' which lists the I/O and memory regions accessible from
> the PCI controller.
>
> Signed-off-by: Simon Glass 
> ---
>
>  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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Simon Glass]

Add 'pci regions' which lists the I/O and memory regions accessible from
the PCI controller.

Signed-off-by: Simon Glass 
---

 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" },
+};
+
+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");
+   }
+}
+
 /* 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"
-- 
2.12.0.367.g23dc2f6d3c-goog

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot