[U-Boot] [PATCH v2 0/1] cmd: pci: add option to parse and display BAR information
From: Yehuda Yitschak yehuday@marvell.com
Updates to initial patch following comments from Stephan Rose I don't have access to a board with DM_PCI support so I tested only on non DM_PCI. Appreciate if someone can test on DM_PCI
v1->v2: - Added support for DM_PCI - Added print of memory type and prefetchable flag - Skipped printing of disabled BARs
Yehuda Yitschak (1): cmd: pci: add option to parse and display BAR information
cmd/pci.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+)
From: Yehuda Yitschak yehuday@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@marvell.com --- cmd/pci.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+)
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 + 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; + } + + 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); + 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", + bar_id, base, size, is_64 ? 64 : 32, + is_io ? "I/O" : "MEM", + prefetchable ? "Prefetchable" : ""); + } + + 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"
On 06.11.2016 15:31, yehuday@marvell.com wrote:
From: Yehuda Yitschak yehuday@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@marvell.com
I've tested this patch on a DM based PCI driver. And it looks good so far:
=> pci bar 0.0.0 ID Base Size Width Type ---------------------------------------------------------- 0 0x00000000f9000000 0x0000000000100000 32 MEM => pci bar 0.0.1 No such device => pci bar 1.0.0 ID Base Size Width Type ---------------------------------------------------------- 0 0x00000000f8000000 0x0000000000020000 32 MEM 1 0x00000000f8020000 0x0000000000020000 32 MEM 2 0x00000000ffffffe0 0x0000000000000020 32 I/O
So:
Tested-by: Stefan Roese sr@denx.de
Thanks, Stefan
Hi,
On 6 November 2016 at 07:31, yehuday@marvell.com wrote:
From: Yehuda Yitschak yehuday@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@marvell.com
cmd/pci.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+)
Reviewed-by: Simon Glass sjg@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
Hi Simon
-----Original Message----- From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass Sent: Friday, November 11, 2016 18:17 To: Yehuda Yitschak Cc: Bin Meng; Heiko Schocher; Przemyslaw Marczak; Stefan Roese; Stephen Warren; U-Boot Mailing List Subject: Re: [PATCH v2 1/1] cmd: pci: add option to parse and display BAR information
Hi,
On 6 November 2016 at 07:31, yehuday@marvell.com wrote:
From: Yehuda Yitschak yehuday@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@marvell.com
cmd/pci.c | 95
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +++++
1 file changed, 95 insertions(+)
Reviewed-by: Simon Glass sjg@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.
Cool. Less clutter
dm_pci_read_config8(dev, PCI_HEADER_TYPE, &header_type); #elsepci_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); #elsepci_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.
I prefer to keep them. It makes debugging much simpler I will change the format to %#016llx.
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" : ""); ^
Strange, I can't see that. What compiler are you using when you get the warning ? I am using gcc-4.8 for armv8, maybe that's why I don't see the warnings I might come down to the built-in definition of __UINT64_TYPE__ which the sandbox arch uses
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
Hi,
On 22 November 2016 at 03:49, Yehuda Yitschak yehuday@marvell.com wrote:
Hi Simon
-----Original Message----- From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass Sent: Friday, November 11, 2016 18:17 To: Yehuda Yitschak Cc: Bin Meng; Heiko Schocher; Przemyslaw Marczak; Stefan Roese; Stephen Warren; U-Boot Mailing List Subject: Re: [PATCH v2 1/1] cmd: pci: add option to parse and display BAR information
Hi,
On 6 November 2016 at 07:31, yehuday@marvell.com wrote:
From: Yehuda Yitschak yehuday@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@marvell.com
cmd/pci.c | 95
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +++++
1 file changed, 95 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
[...]
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" : ""); ^
Strange, I can't see that. What compiler are you using when you get the warning ? I am using gcc-4.8 for armv8, maybe that's why I don't see the warnings I might come down to the built-in definition of __UINT64_TYPE__ which the sandbox arch uses
This is sandbox, perhaps this:
gcc version 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04.3) [...]
Regards, Simon
participants (4)
-
Simon Glass -
Stefan Roese -
Yehuda Yitschak -
yehuday＠marvell.com