All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 0/1] cmd: pci: add option to parse and display BAR information
@ 2016-11-06 14:31 yehuday at marvell.com
  2016-11-06 14:31 ` [U-Boot] [PATCH v2 1/1] " yehuday at marvell.com
  0 siblings, 1 reply; 6+ messages in thread
From: yehuday at marvell.com @ 2016-11-06 14:31 UTC (permalink / raw)
  To: u-boot

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(+)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v2 1/1] cmd: pci: add option to parse and display BAR information
  2016-11-06 14:31 [U-Boot] [PATCH v2 0/1] cmd: pci: add option to parse and display BAR information yehuday at marvell.com
@ 2016-11-06 14:31 ` yehuday at marvell.com
  2016-11-07  6:55   ` Stefan Roese
  2016-11-11 16:16   ` Simon Glass
  0 siblings, 2 replies; 6+ messages in thread
From: yehuday at marvell.com @ 2016-11-06 14:31 UTC (permalink / raw)
  To: u-boot

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"
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v2 1/1] cmd: pci: add option to parse and display BAR information
  2016-11-06 14:31 ` [U-Boot] [PATCH v2 1/1] " yehuday at marvell.com
@ 2016-11-07  6:55   ` Stefan Roese
  2016-11-11 16:16   ` Simon Glass
  1 sibling, 0 replies; 6+ messages in thread
From: Stefan Roese @ 2016-11-07  6:55 UTC (permalink / raw)
  To: u-boot

On 06.11.2016 15:31, yehuday at 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v2 1/1] cmd: pci: add option to parse and display BAR information
  2016-11-06 14:31 ` [U-Boot] [PATCH v2 1/1] " yehuday at marvell.com
  2016-11-07  6:55   ` Stefan Roese
@ 2016-11-11 16:16   ` Simon Glass
  2016-11-22 10:49     ` Yehuda Yitschak
  1 sibling, 1 reply; 6+ messages in thread
From: Simon Glass @ 2016-11-11 16:16 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v2 1/1] cmd: pci: add option to parse and display BAR information
  2016-11-11 16:16   ` Simon Glass
@ 2016-11-22 10:49     ` Yehuda Yitschak
  2016-11-24  2:21       ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Yehuda Yitschak @ 2016-11-22 10:49 UTC (permalink / raw)
  To: u-boot

Hi Simon

> -----Original Message-----
> From: sjg at google.com [mailto:sjg at 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); #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.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v2 1/1] cmd: pci: add option to parse and display BAR information
  2016-11-22 10:49     ` Yehuda Yitschak
@ 2016-11-24  2:21       ` Simon Glass
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2016-11-24  2:21 UTC (permalink / raw)
  To: u-boot

Hi,

On 22 November 2016 at 03:49, Yehuda Yitschak <yehuday@marvell.com> wrote:
> Hi Simon
>
>> -----Original Message-----
>> From: sjg at google.com [mailto:sjg at 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-11-24  2:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-06 14:31 [U-Boot] [PATCH v2 0/1] cmd: pci: add option to parse and display BAR information yehuday at marvell.com
2016-11-06 14:31 ` [U-Boot] [PATCH v2 1/1] " yehuday at marvell.com
2016-11-07  6:55   ` Stefan Roese
2016-11-11 16:16   ` Simon Glass
2016-11-22 10:49     ` Yehuda Yitschak
2016-11-24  2:21       ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.