[U-Boot] [PATCH v2 1/1] cmd: pci: add option to parse and display BAR information

Simon Glass sjg at chromium.org
Fri Nov 11 17:16:48 CET 2016


Hi,

On 6 November 2016 at 07:31,  <yehuday at marvell.com> wrote:
> From: Yehuda Yitschak <yehuday at marvell.com>
>
> Currently the PCI command only allows to see the BAR register
> values but not the size and actual base address.
> This little extension parses the BAR registers and displays
> the base, size and type of each BAR.
>
> Signed-off-by: Yehuda Yitschak <yehuday at marvell.com>
> ---
>  cmd/pci.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)

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

>
> diff --git a/cmd/pci.c b/cmd/pci.c
> index 2f4978a..51eb536 100644
> --- a/cmd/pci.c
> +++ b/cmd/pci.c
> @@ -92,6 +92,96 @@ static void pci_show_regs(pci_dev_t dev, struct pci_reg_info *regs)
>  }
>  #endif
>
> +#ifdef CONFIG_DM_PCI
> +int pci_bar_show(struct udevice *dev)
> +#else
> +int pci_bar_show(pci_dev_t dev)
> +#endif
> +{
> +       u8 header_type;
> +       int bar_cnt, bar_id, is_64, is_io, mem_type;
> +       u32 base_low, base_high;
> +       u32 size_low, size_high;
> +       u64 base, size;
> +       u32 reg_addr;
> +       int prefetchable;
> +
> +#ifdef CONFIG_DM_PCI

I think you can implement only the DM_PCI case, since we are trying to
move everything to DM_PCI.

> +       dm_pci_read_config8(dev, PCI_HEADER_TYPE, &header_type);
> +#else
> +       pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);
> +#endif
> +
> +       if (header_type == PCI_HEADER_TYPE_CARDBUS) {
> +               printf("CardBus doesn't support BARs\n");
> +               return -1;

-ENOSYS perhaps. This is -EPERM which seems wrong.

> +       }
> +
> +       bar_cnt = (header_type == PCI_HEADER_TYPE_NORMAL) ? 6 : 2;
> +
> +       printf("ID   Base                Size                Width  Type\n");
> +       printf("----------------------------------------------------------\n");
> +
> +       bar_id = 0;
> +       reg_addr = PCI_BASE_ADDRESS_0;
> +       while (bar_cnt) {
> +#ifdef CONFIG_DM_PCI
> +               dm_pci_read_config32(dev, reg_addr, &base_low);
> +               dm_pci_write_config32(dev, reg_addr, 0xFFFFFFFF);

Suggest lower-case hex.

> +               dm_pci_read_config32(dev, reg_addr, &size_low);
> +               dm_pci_write_config32(dev, reg_addr, base_low);
> +#else
> +               pci_read_config_dword(dev, reg_addr, &base_low);
> +               pci_write_config_dword(dev, reg_addr, 0xFFFFFFFF);
> +               pci_read_config_dword(dev, reg_addr, &size_low);
> +               pci_write_config_dword(dev, reg_addr, base_low);
> +#endif
> +               reg_addr += 4;
> +
> +               base = base_low & ~0xF;
> +               size = size_low & ~0xF;
> +               base_high = 0x0;
> +               size_high = 0xFFFFFFFF;
> +               is_64 = 0;
> +               prefetchable = base_low & PCI_BASE_ADDRESS_MEM_PREFETCH;
> +               is_io = base_low & PCI_BASE_ADDRESS_SPACE_IO;
> +               mem_type = base_low & PCI_BASE_ADDRESS_MEM_TYPE_MASK;
> +
> +               if (mem_type == PCI_BASE_ADDRESS_MEM_TYPE_64) {
> +#ifdef CONFIG_DM_PCI
> +                       dm_pci_read_config32(dev, reg_addr, &base_high);
> +                       dm_pci_write_config32(dev, reg_addr, 0xFFFFFFFF);
> +                       dm_pci_read_config32(dev, reg_addr, &size_high);
> +                       dm_pci_write_config32(dev, reg_addr, base_high);
> +#else
> +                       pci_read_config_dword(dev, reg_addr, &base_high);
> +                       pci_write_config_dword(dev, reg_addr, 0xFFFFFFFF);
> +                       pci_read_config_dword(dev, reg_addr, &size_high);
> +                       pci_write_config_dword(dev, reg_addr, base_high);
> +#endif
> +                       bar_cnt--;
> +                       reg_addr += 4;
> +                       is_64 = 1;
> +               }
> +
> +               base = base | ((u64)base_high << 32);
> +               size = size | ((u64)size_high << 32);
> +
> +               if ((!is_64 && size_low) || (is_64 && size)) {
> +                       size = ~size + 1;
> +                       printf(" %d   0x%016llx  0x%016llx  %d     %s   %s\n",

Can we drop the the address values? It is implied in U-Boot. If not,
let's use %#x.

> +                              bar_id, base, size, is_64 ? 64 : 32,
> +                              is_io ? "I/O" : "MEM",
> +                              prefetchable ? "Prefetchable" : "");

Check with sandbox, this gives a warning:

cmd/pci.c: In function ‘pci_bar_show’:
cmd/pci.c:175:11: warning: format ‘%llx’ expects argument of type
‘long long unsigned int’, but argument 3 has type ‘u64’ [-Wformat=]
           prefetchable ? "Prefetchable" : "");
           ^
cmd/pci.c:175:11: warning: format ‘%llx’ expects argument of type
‘long long unsigned int’, but argument 4 has type ‘u64’ [-Wformat=]

> +               }
> +
> +               bar_id++;
> +               bar_cnt--;
> +       }
> +
> +       return 0;
> +}
> +
>  static struct pci_reg_info regs_start[] = {
>         { "vendor ID", PCI_SIZE_16, PCI_VENDOR_ID },
>         { "device ID", PCI_SIZE_16, PCI_DEVICE_ID },
> @@ -573,6 +663,7 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                 if (argc > 4)
>                         value = simple_strtoul(argv[4], NULL, 16);
>         case 'h':               /* header */
> +       case 'b':               /* bars */
>                 if (argc < 3)
>                         goto usage;
>                 if ((bdf = get_pci_dev(argv[2])) == -1)
> @@ -641,6 +732,8 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                 ret = pci_cfg_write(dev, addr, size, value);
>  #endif
>                 break;
> +       case 'b':               /* bars */
> +               return pci_bar_show(dev);
>         default:
>                 ret = CMD_RET_USAGE;
>                 break;
> @@ -663,6 +756,8 @@ static char pci_help_text[] =
>  #endif
>         "pci header b.d.f\n"
>         "    - show header of PCI device 'bus.device.function'\n"
> +       "pci bar b.d.f\n"
> +       "    - show BARs base and size for device b.d.f'\n"
>         "pci display[.b, .w, .l] b.d.f [address] [# of objects]\n"
>         "    - display PCI configuration space (CFG)\n"
>         "pci next[.b, .w, .l] b.d.f address\n"
> --
> 1.9.1
>

Regards,
Simon


More information about the U-Boot mailing list