Re: [PATCH 4/4] pci: Extend 'pci' command with bus option '*'

[Stefan Roese <sr@denx.de>]

On 1/17/22 16:38, Pali Rohár wrote:
> Allow to call 'pci' and 'pci regions' commands with bus option '*' which
> cause pci to process all buses.
> 
> PCIe is point-to-point HW and so on each bus is maximally one physical
> device. Therefore for PCIe it is common to have multiple buses.
> 
> This change allows to easily print all available PCIe devices in system.
> 
> Make '*' as default option when no bus argument is specified.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   cmd/pci.c | 45 +++++++++++++++++++++++++++++++++++----------
>   1 file changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/cmd/pci.c b/cmd/pci.c
> index 8d2c0c4b43dd..4415feb2225b 100644
> --- a/cmd/pci.c
> +++ b/cmd/pci.c
> @@ -256,10 +256,8 @@ static void pci_header_show(struct udevice *dev)
>       }
>   }
>   
> -static void pciinfo_header(int busnum, bool short_listing)
> +static void pciinfo_header(bool short_listing)
>   {
> -	printf("Scanning PCI devices on bus %d\n", busnum);
> -
>   	if (short_listing) {
>   		printf("BusDevFun  VendorId   DeviceId   Device Class       Sub-Class\n");
>   		printf("_____________________________________________________________\n");
> @@ -288,11 +286,15 @@ static void pci_header_show_brief(struct udevice *dev)
>   	       pci_class_str(class), subclass);
>   }
>   
> -static void pciinfo(struct udevice *bus, bool short_listing)
> +static void pciinfo(struct udevice *bus, bool short_listing, bool multi)
>   {
>   	struct udevice *dev;
>   
> -	pciinfo_header(dev_seq(bus), short_listing);
> +	if (!multi)
> +		printf("Scanning PCI devices on bus %d\n", dev_seq(bus));
> +
> +	if (!multi || dev_seq(bus) == 0)
> +		pciinfo_header(short_listing);
>   
>   	for (device_find_first_child(bus, &dev);
>   	     dev;
> @@ -483,10 +485,11 @@ static int do_pci(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   	ulong addr = 0, value = 0, cmd_size = 0;
>   	enum pci_size_t size = PCI_SIZE_32;
>   	struct udevice *dev, *bus;
> -	int busnum = 0;
> +	int busnum = -1;
>   	pci_dev_t bdf = 0;
>   	char cmd = 's';
>   	int ret = 0;
> +	char *endp;
>   
>   	if (argc > 1)
>   		cmd = argv[1][0];
> @@ -522,7 +525,11 @@ static int do_pci(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   				argc--;
>   			}
>   			if (argc > 2 || (argc > 1 && cmd != 'r' && argv[1][0] != 's')) {
> -				busnum = hextoul(argv[argc - 1], NULL);
> +				if (argv[argc - 1][0] != '*') {
> +					busnum = hextoul(argv[argc - 1], &endp);
> +					if (*endp)
> +						goto usage;
> +				}
>   				argc--;
>   			}
>   			if (cmd == 'r' && argc > 2)
> @@ -530,6 +537,24 @@ static int do_pci(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   			else if (cmd != 'r' && (argc > 2 || (argc == 2 && argv[1][0] != 's')))
>   				goto usage;
>   		}
> +		if (busnum == -1) {
> +			if (cmd != 'r') {
> +				for (busnum = 0;
> +				     uclass_get_device_by_seq(UCLASS_PCI, busnum, &bus) == 0;
> +				     busnum++)
> +					pciinfo(bus, value, true);
> +			} else {
> +				for (busnum = 0;
> +				     uclass_get_device_by_seq(UCLASS_PCI, busnum, &bus) == 0;
> +				     busnum++) {
> +					/* Regions are controller specific so skip non-root buses */
> +					if (device_is_on_pci_bus(bus))
> +						continue;
> +					pci_show_regions(bus);
> +				}
> +			}
> +			return 0;
> +		}
>   		ret = uclass_get_device_by_seq(UCLASS_PCI, busnum, &bus);
>   		if (ret) {
>   			printf("No such bus\n");
> @@ -538,7 +563,7 @@ static int do_pci(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   		if (cmd == 'r')
>   			pci_show_regions(bus);
>   		else
> -			pciinfo(bus, value);
> +			pciinfo(bus, value, false);
>   		return 0;
>   	}
>   
> @@ -585,7 +610,7 @@ static int do_pci(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   
>   #ifdef CONFIG_SYS_LONGHELP
>   static char pci_help_text[] =
> -	"[bus] [long]\n"
> +	"[bus|*] [long]\n"
>   	"    - short or long list of PCI devices on bus 'bus'\n"
>   	"pci enum\n"
>   	"    - Enumerate PCI buses\n"
> @@ -593,7 +618,7 @@ static char pci_help_text[] =
>   	"    - 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 regions [bus]\n"
> +	"pci regions [bus|*]\n"
>   	"    - show PCI regions\n"
>   	"pci display[.b, .w, .l] b.d.f [address] [# of objects]\n"
>   	"    - display PCI configuration space (CFG)\n"

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de