[U-Boot] [PATCH v3 00/13] dm: pci: Support native driver model calls

At present driver model supports PCI, but most of the code in U-Boot still uses the old API.
This series changes the 'pci' command so that the new API is used. The old API is placed behind a 'compatibility' option. The overall goal is to deprecate the old API and remove all use of it. The auto-config and device finding will be the subject of future patches.
Changes in v3: - Use 'Show' instead of 'Shows' in the the pci_header_show() function comment - Fix mix-up between PCI_SIZE_xx and byte size - Fix 'switch (cmd)' merge error
Changes in v2: - Squash in the patch to fix pci_field_width() for 32-bit values - Rename '_byte' to class in pci_header_show() - Fix 'autoconfiguration' typo - Fix merge error in pciauto_setup_device() which removed needed code - Add a new patch to update the function comments - Add back the leading 0 in the printf() statements - Refactor the patch based on dropping the earlier refactor patch - Make pciinfo() static - Drop patch: pci: Move 'pci scan' code in with other commands - Move dm_pciauto_config_device() out of CONFIG_DM_PCI_COMPAT
Simon Glass (13): pci: Use a common return in command processing pci: Use a separate variable for the bus number pci: Refactor the pciinfo() function dm: pci: Add a comment about how to find struct pci_controller dm: pci: Rename pci_auto.c to pci_auto_old.c dm: pci: Move common auto-config functions to a common file dm: pci: Reorder functions in cmd_pci.c pci: Tidy up function comments in cmd_pci.c pci: Use common functions to read/write config pci: Use a separate 'dev' variable for the PCI device pci: Move PCI header output code into its own function dm: pci: Convert 'pci' command to driver model dm: pci: Disable PCI compatibility functions by default
arch/arm/mach-tegra/Kconfig | 1 + arch/x86/Kconfig | 3 + common/cmd_pci.c | 631 ++++++++++++++++++----------- configs/sandbox_defconfig | 1 + drivers/pci/Kconfig | 9 + drivers/pci/Makefile | 5 +- drivers/pci/pci_auto_common.c | 128 ++++++ drivers/pci/{pci_auto.c => pci_auto_old.c} | 116 ------ include/common.h | 1 - include/pci.h | 32 +- 10 files changed, 576 insertions(+), 351 deletions(-) create mode 100644 drivers/pci/pci_auto_common.c rename drivers/pci/{pci_auto.c => pci_auto_old.c} (81%)

Adjust the commands to return from the same place.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: None
common/cmd_pci.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 2eafd5c..f3148a3 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -410,6 +410,7 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) ulong addr = 0, value = 0, size = 0; pci_dev_t bdf = 0; char cmd = 's'; + int ret = 0;
if (argc > 1) cmd = argv[1][0]; @@ -453,7 +454,7 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) switch (argv[1][0]) { case 'h': /* header */ pci_header_show(bdf); - return 0; + break; case 'd': /* display */ return pci_cfg_display(bdf, addr, size, value); #ifdef CONFIG_CMD_PCI_ENUM @@ -463,23 +464,29 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) # else pci_init(); # endif - return 0; + break; #endif case 'n': /* next */ if (argc < 4) goto usage; - return pci_cfg_modify(bdf, addr, size, value, 0); + ret = pci_cfg_modify(bdf, addr, size, value, 0); + break; case 'm': /* modify */ if (argc < 4) goto usage; - return pci_cfg_modify(bdf, addr, size, value, 1); + ret = pci_cfg_modify(bdf, addr, size, value, 1); + break; case 'w': /* write */ if (argc < 5) goto usage; - return pci_cfg_write(bdf, addr, size, value); + ret = pci_cfg_write(bdf, addr, size, value); + break; + default: + ret = CMD_RET_USAGE; + break; }
- return 1; + return ret; usage: return CMD_RET_USAGE; }

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
Adjust the commands to return from the same place.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2: None
common/cmd_pci.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
Applied to u-boot-dm.

At present in do_pci(), bdf can either mean a bus number or a PCI bus number. Use separate variables instead to reduce confusion.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: None
common/cmd_pci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index f3148a3..bed880c 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -408,6 +408,7 @@ pci_cfg_modify (pci_dev_t bdf, ulong addr, ulong size, ulong value, int incrflag static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { ulong addr = 0, value = 0, size = 0; + int busnum = 0; pci_dev_t bdf = 0; char cmd = 's'; int ret = 0; @@ -438,16 +439,15 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) #endif default: /* scan bus */ value = 1; /* short listing */ - bdf = 0; /* bus number */ if (argc > 1) { if (argv[argc-1][0] == 'l') { value = 0; argc--; } if (argc > 1) - bdf = simple_strtoul(argv[1], NULL, 16); + busnum = simple_strtoul(argv[1], NULL, 16); } - pciinfo(bdf, value); + pciinfo(busnum, value); return 0; }

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
At present in do_pci(), bdf can either mean a bus number or a PCI bus number. Use separate variables instead to reduce confusion.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2: None
common/cmd_pci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Applied to u-boot-dm.

This function uses macros to output data. It seems better to use a table of registers rather than macro-based code generation. It also reduces the code/data size by 2KB on ARM.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: - Squash in the patch to fix pci_field_width() for 32-bit values - Rename '_byte' to class in pci_header_show()
common/cmd_pci.c | 237 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 148 insertions(+), 89 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index bed880c..190e3b6 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -131,6 +131,145 @@ void pci_header_show_brief(pci_dev_t dev) pci_class_str(class), subclass); }
+struct pci_reg_info { + const char *name; + enum pci_size_t size; + u8 offset; +}; + +static int pci_field_width(enum pci_size_t size) +{ + switch (size) { + case PCI_SIZE_8: + return 2; + case PCI_SIZE_16: + return 4; + case PCI_SIZE_32: + default: + return 8; + } +} + +static unsigned long pci_read_config(pci_dev_t dev, int offset, + enum pci_size_t size) +{ + u32 val32; + u16 val16; + u8 val8; + + switch (size) { + case PCI_SIZE_8: + pci_read_config_byte(dev, offset, &val8); + return val8; + case PCI_SIZE_16: + pci_read_config_word(dev, offset, &val16); + return val16; + case PCI_SIZE_32: + default: + pci_read_config_dword(dev, offset, &val32); + return val32; + } +} + +static void pci_show_regs(pci_dev_t dev, struct pci_reg_info *regs) +{ + for (; regs->name; regs++) { + printf(" %s =%*s%#.*lx\n", regs->name, + (int)(28 - strlen(regs->name)), "", + pci_field_width(regs->size), + pci_read_config(dev, regs->offset, regs->size)); + } +} + +static struct pci_reg_info regs_start[] = { + { "vendor ID", PCI_SIZE_16, PCI_VENDOR_ID }, + { "device ID", PCI_SIZE_16, PCI_DEVICE_ID }, + { "command register ID", PCI_SIZE_16, PCI_COMMAND }, + { "status register", PCI_SIZE_16, PCI_STATUS }, + { "revision ID", PCI_SIZE_8, PCI_REVISION_ID }, + {}, +}; + +static struct pci_reg_info regs_rest[] = { + { "sub class code", PCI_SIZE_8, PCI_CLASS_SUB_CODE }, + { "programming interface", PCI_SIZE_8, PCI_CLASS_PROG }, + { "cache line", PCI_SIZE_8, PCI_CACHE_LINE_SIZE }, + { "latency time", PCI_SIZE_8, PCI_LATENCY_TIMER }, + { "header type", PCI_SIZE_8, PCI_HEADER_TYPE }, + { "BIST", PCI_SIZE_8, PCI_BIST }, + { "base address 0", PCI_SIZE_32, PCI_BASE_ADDRESS_0 }, + {}, +}; + +static struct pci_reg_info regs_normal[] = { + { "base address 1", PCI_SIZE_32, PCI_BASE_ADDRESS_1 }, + { "base address 2", PCI_SIZE_32, PCI_BASE_ADDRESS_2 }, + { "base address 3", PCI_SIZE_32, PCI_BASE_ADDRESS_3 }, + { "base address 4", PCI_SIZE_32, PCI_BASE_ADDRESS_4 }, + { "base address 5", PCI_SIZE_32, PCI_BASE_ADDRESS_5 }, + { "cardBus CIS pointer", PCI_SIZE_32, PCI_CARDBUS_CIS }, + { "sub system vendor ID", PCI_SIZE_16, PCI_SUBSYSTEM_VENDOR_ID }, + { "sub system ID", PCI_SIZE_16, PCI_SUBSYSTEM_ID }, + { "expansion ROM base address", PCI_SIZE_32, PCI_ROM_ADDRESS }, + { "interrupt line", PCI_SIZE_8, PCI_INTERRUPT_LINE }, + { "interrupt pin", PCI_SIZE_8, PCI_INTERRUPT_PIN }, + { "min Grant", PCI_SIZE_8, PCI_MIN_GNT }, + { "max Latency", PCI_SIZE_8, PCI_MAX_LAT }, + {}, +}; + +static struct pci_reg_info regs_bridge[] = { + { "base address 1", PCI_SIZE_32, PCI_BASE_ADDRESS_1 }, + { "primary bus number", PCI_SIZE_8, PCI_PRIMARY_BUS }, + { "secondary bus number", PCI_SIZE_8, PCI_SECONDARY_BUS }, + { "subordinate bus number", PCI_SIZE_8, PCI_SUBORDINATE_BUS }, + { "secondary latency timer", PCI_SIZE_8, PCI_SEC_LATENCY_TIMER }, + { "IO base", PCI_SIZE_8, PCI_IO_BASE }, + { "IO limit", PCI_SIZE_8, PCI_IO_LIMIT }, + { "secondary status", PCI_SIZE_16, PCI_SEC_STATUS }, + { "memory base", PCI_SIZE_16, PCI_MEMORY_BASE }, + { "memory limit", PCI_SIZE_16, PCI_MEMORY_LIMIT }, + { "prefetch memory base", PCI_SIZE_16, PCI_PREF_MEMORY_BASE }, + { "prefetch memory limit", PCI_SIZE_16, PCI_PREF_MEMORY_LIMIT }, + { "prefetch memory base upper", PCI_SIZE_32, PCI_PREF_BASE_UPPER32 }, + { "prefetch memory limit upper", PCI_SIZE_32, PCI_PREF_LIMIT_UPPER32 }, + { "IO base upper 16 bits", PCI_SIZE_16, PCI_IO_BASE_UPPER16 }, + { "IO limit upper 16 bits", PCI_SIZE_16, PCI_IO_LIMIT_UPPER16 }, + { "expansion ROM base address", PCI_SIZE_32, PCI_ROM_ADDRESS1 }, + { "interrupt line", PCI_SIZE_8, PCI_INTERRUPT_LINE }, + { "interrupt pin", PCI_SIZE_8, PCI_INTERRUPT_PIN }, + { "bridge control", PCI_SIZE_16, PCI_BRIDGE_CONTROL }, + {}, +}; + +static struct pci_reg_info regs_cardbus[] = { + { "capabilities", PCI_SIZE_8, PCI_CB_CAPABILITY_LIST }, + { "secondary status", PCI_SIZE_16, PCI_CB_SEC_STATUS }, + { "primary bus number", PCI_SIZE_8, PCI_CB_PRIMARY_BUS }, + { "CardBus number", PCI_SIZE_8, PCI_CB_CARD_BUS }, + { "subordinate bus number", PCI_SIZE_8, PCI_CB_SUBORDINATE_BUS }, + { "CardBus latency timer", PCI_SIZE_8, PCI_CB_LATENCY_TIMER }, + { "CardBus memory base 0", PCI_SIZE_32, PCI_CB_MEMORY_BASE_0 }, + { "CardBus memory limit 0", PCI_SIZE_32, PCI_CB_MEMORY_LIMIT_0 }, + { "CardBus memory base 1", PCI_SIZE_32, PCI_CB_MEMORY_BASE_1 }, + { "CardBus memory limit 1", PCI_SIZE_32, PCI_CB_MEMORY_LIMIT_1 }, + { "CardBus IO base 0", PCI_SIZE_16, PCI_CB_IO_BASE_0 }, + { "CardBus IO base high 0", PCI_SIZE_16, PCI_CB_IO_BASE_0_HI }, + { "CardBus IO limit 0", PCI_SIZE_16, PCI_CB_IO_LIMIT_0 }, + { "CardBus IO limit high 0", PCI_SIZE_16, PCI_CB_IO_LIMIT_0_HI }, + { "CardBus IO base 1", PCI_SIZE_16, PCI_CB_IO_BASE_1 }, + { "CardBus IO base high 1", PCI_SIZE_16, PCI_CB_IO_BASE_1_HI }, + { "CardBus IO limit 1", PCI_SIZE_16, PCI_CB_IO_LIMIT_1 }, + { "CardBus IO limit high 1", PCI_SIZE_16, PCI_CB_IO_LIMIT_1_HI }, + { "interrupt line", PCI_SIZE_8, PCI_INTERRUPT_LINE }, + { "interrupt pin", PCI_SIZE_8, PCI_INTERRUPT_PIN }, + { "bridge control", PCI_SIZE_16, PCI_CB_BRIDGE_CONTROL }, + { "subvendor ID", PCI_SIZE_16, PCI_CB_SUBSYSTEM_VENDOR_ID }, + { "subdevice ID", PCI_SIZE_16, PCI_CB_SUBSYSTEM_ID }, + { "PC Card 16bit base address", PCI_SIZE_32, PCI_CB_LEGACY_MODE_BASE }, + {}, +}; + /* * Subroutine: PCI_Header_Show * @@ -143,111 +282,31 @@ void pci_header_show_brief(pci_dev_t dev) */ void pci_header_show(pci_dev_t dev) { - u8 _byte, header_type; - u16 _word; - u32 _dword; - -#define PRINT(msg, type, reg) \ - pci_read_config_##type(dev, reg, &_##type); \ - printf(msg, _##type) - -#define PRINT2(msg, type, reg, func) \ - pci_read_config_##type(dev, reg, &_##type); \ - printf(msg, _##type, func(_##type)) + u8 class, header_type;
pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type); + pci_show_regs(dev, regs_start);
- PRINT (" vendor ID = 0x%.4x\n", word, PCI_VENDOR_ID); - PRINT (" device ID = 0x%.4x\n", word, PCI_DEVICE_ID); - PRINT (" command register = 0x%.4x\n", word, PCI_COMMAND); - PRINT (" status register = 0x%.4x\n", word, PCI_STATUS); - PRINT (" revision ID = 0x%.2x\n", byte, PCI_REVISION_ID); - PRINT2(" class code = 0x%.2x (%s)\n", byte, PCI_CLASS_CODE, - pci_class_str); - PRINT (" sub class code = 0x%.2x\n", byte, PCI_CLASS_SUB_CODE); - PRINT (" programming interface = 0x%.2x\n", byte, PCI_CLASS_PROG); - PRINT (" cache line = 0x%.2x\n", byte, PCI_CACHE_LINE_SIZE); - PRINT (" latency time = 0x%.2x\n", byte, PCI_LATENCY_TIMER); - PRINT (" header type = 0x%.2x\n", byte, PCI_HEADER_TYPE); - PRINT (" BIST = 0x%.2x\n", byte, PCI_BIST); - PRINT (" base address 0 = 0x%.8x\n", dword, PCI_BASE_ADDRESS_0); + pci_read_config_byte(dev, PCI_CLASS_CODE, &class); + printf(" class code = 0x%.2x (%s)\n", class, + pci_class_str(class)); + pci_show_regs(dev, regs_rest);
switch (header_type & 0x03) { case PCI_HEADER_TYPE_NORMAL: /* "normal" PCI device */ - PRINT (" base address 1 = 0x%.8x\n", dword, PCI_BASE_ADDRESS_1); - PRINT (" base address 2 = 0x%.8x\n", dword, PCI_BASE_ADDRESS_2); - PRINT (" base address 3 = 0x%.8x\n", dword, PCI_BASE_ADDRESS_3); - PRINT (" base address 4 = 0x%.8x\n", dword, PCI_BASE_ADDRESS_4); - PRINT (" base address 5 = 0x%.8x\n", dword, PCI_BASE_ADDRESS_5); - PRINT (" cardBus CIS pointer = 0x%.8x\n", dword, PCI_CARDBUS_CIS); - PRINT (" sub system vendor ID = 0x%.4x\n", word, PCI_SUBSYSTEM_VENDOR_ID); - PRINT (" sub system ID = 0x%.4x\n", word, PCI_SUBSYSTEM_ID); - PRINT (" expansion ROM base address = 0x%.8x\n", dword, PCI_ROM_ADDRESS); - PRINT (" interrupt line = 0x%.2x\n", byte, PCI_INTERRUPT_LINE); - PRINT (" interrupt pin = 0x%.2x\n", byte, PCI_INTERRUPT_PIN); - PRINT (" min Grant = 0x%.2x\n", byte, PCI_MIN_GNT); - PRINT (" max Latency = 0x%.2x\n", byte, PCI_MAX_LAT); + pci_show_regs(dev, regs_normal); break; - case PCI_HEADER_TYPE_BRIDGE: /* PCI-to-PCI bridge */ - - PRINT (" base address 1 = 0x%.8x\n", dword, PCI_BASE_ADDRESS_1); - PRINT (" primary bus number = 0x%.2x\n", byte, PCI_PRIMARY_BUS); - PRINT (" secondary bus number = 0x%.2x\n", byte, PCI_SECONDARY_BUS); - PRINT (" subordinate bus number = 0x%.2x\n", byte, PCI_SUBORDINATE_BUS); - PRINT (" secondary latency timer = 0x%.2x\n", byte, PCI_SEC_LATENCY_TIMER); - PRINT (" IO base = 0x%.2x\n", byte, PCI_IO_BASE); - PRINT (" IO limit = 0x%.2x\n", byte, PCI_IO_LIMIT); - PRINT (" secondary status = 0x%.4x\n", word, PCI_SEC_STATUS); - PRINT (" memory base = 0x%.4x\n", word, PCI_MEMORY_BASE); - PRINT (" memory limit = 0x%.4x\n", word, PCI_MEMORY_LIMIT); - PRINT (" prefetch memory base = 0x%.4x\n", word, PCI_PREF_MEMORY_BASE); - PRINT (" prefetch memory limit = 0x%.4x\n", word, PCI_PREF_MEMORY_LIMIT); - PRINT (" prefetch memory base upper = 0x%.8x\n", dword, PCI_PREF_BASE_UPPER32); - PRINT (" prefetch memory limit upper = 0x%.8x\n", dword, PCI_PREF_LIMIT_UPPER32); - PRINT (" IO base upper 16 bits = 0x%.4x\n", word, PCI_IO_BASE_UPPER16); - PRINT (" IO limit upper 16 bits = 0x%.4x\n", word, PCI_IO_LIMIT_UPPER16); - PRINT (" expansion ROM base address = 0x%.8x\n", dword, PCI_ROM_ADDRESS1); - PRINT (" interrupt line = 0x%.2x\n", byte, PCI_INTERRUPT_LINE); - PRINT (" interrupt pin = 0x%.2x\n", byte, PCI_INTERRUPT_PIN); - PRINT (" bridge control = 0x%.4x\n", word, PCI_BRIDGE_CONTROL); + pci_show_regs(dev, regs_bridge); break; - case PCI_HEADER_TYPE_CARDBUS: /* PCI-to-CardBus bridge */ - - PRINT (" capabilities = 0x%.2x\n", byte, PCI_CB_CAPABILITY_LIST); - PRINT (" secondary status = 0x%.4x\n", word, PCI_CB_SEC_STATUS); - PRINT (" primary bus number = 0x%.2x\n", byte, PCI_CB_PRIMARY_BUS); - PRINT (" CardBus number = 0x%.2x\n", byte, PCI_CB_CARD_BUS); - PRINT (" subordinate bus number = 0x%.2x\n", byte, PCI_CB_SUBORDINATE_BUS); - PRINT (" CardBus latency timer = 0x%.2x\n", byte, PCI_CB_LATENCY_TIMER); - PRINT (" CardBus memory base 0 = 0x%.8x\n", dword, PCI_CB_MEMORY_BASE_0); - PRINT (" CardBus memory limit 0 = 0x%.8x\n", dword, PCI_CB_MEMORY_LIMIT_0); - PRINT (" CardBus memory base 1 = 0x%.8x\n", dword, PCI_CB_MEMORY_BASE_1); - PRINT (" CardBus memory limit 1 = 0x%.8x\n", dword, PCI_CB_MEMORY_LIMIT_1); - PRINT (" CardBus IO base 0 = 0x%.4x\n", word, PCI_CB_IO_BASE_0); - PRINT (" CardBus IO base high 0 = 0x%.4x\n", word, PCI_CB_IO_BASE_0_HI); - PRINT (" CardBus IO limit 0 = 0x%.4x\n", word, PCI_CB_IO_LIMIT_0); - PRINT (" CardBus IO limit high 0 = 0x%.4x\n", word, PCI_CB_IO_LIMIT_0_HI); - PRINT (" CardBus IO base 1 = 0x%.4x\n", word, PCI_CB_IO_BASE_1); - PRINT (" CardBus IO base high 1 = 0x%.4x\n", word, PCI_CB_IO_BASE_1_HI); - PRINT (" CardBus IO limit 1 = 0x%.4x\n", word, PCI_CB_IO_LIMIT_1); - PRINT (" CardBus IO limit high 1 = 0x%.4x\n", word, PCI_CB_IO_LIMIT_1_HI); - PRINT (" interrupt line = 0x%.2x\n", byte, PCI_INTERRUPT_LINE); - PRINT (" interrupt pin = 0x%.2x\n", byte, PCI_INTERRUPT_PIN); - PRINT (" bridge control = 0x%.4x\n", word, PCI_CB_BRIDGE_CONTROL); - PRINT (" subvendor ID = 0x%.4x\n", word, PCI_CB_SUBSYSTEM_VENDOR_ID); - PRINT (" subdevice ID = 0x%.4x\n", word, PCI_CB_SUBSYSTEM_ID); - PRINT (" PC Card 16bit base address = 0x%.8x\n", dword, PCI_CB_LEGACY_MODE_BASE); + pci_show_regs(dev, regs_cardbus); break;
default: printf("unknown header\n"); break; } - -#undef PRINT -#undef PRINT2 }
/* Convert the "bus.device.function" identifier into a number.

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
This function uses macros to output data. It seems better to use a table of registers rather than macro-based code generation. It also reduces the code/data size by 2KB on ARM.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2:
- Squash in the patch to fix pci_field_width() for 32-bit values
- Rename '_byte' to class in pci_header_show()
common/cmd_pci.c | 237 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 148 insertions(+), 89 deletions(-)
Applied to u-boot-dm.

With driver mode, struct pci_controller is stored as uclass-private data. Add a comment to that effect.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: None
include/pci.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/pci.h b/include/pci.h index 9c19482..c4f6577 100644 --- a/include/pci.h +++ b/include/pci.h @@ -537,6 +537,8 @@ extern void pci_cfgfunc_config_device(struct pci_controller* hose, pci_dev_t dev
/* * Structure of a PCI controller (host bridge) + * + * With driver model this is dev_get_uclass_priv(bus) */ struct pci_controller { #ifdef CONFIG_DM_PCI

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
With driver mode, struct pci_controller is stored as uclass-private data. Add a comment to that effect.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2: None
include/pci.h | 2 ++ 1 file changed, 2 insertions(+)
Applied to u-boot-dm.

This file should not be used with driver model as it has lots of legacy/ compatibility functions. Rename it to make this clear.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: None
drivers/pci/Makefile | 2 +- drivers/pci/{pci_auto.c => pci_auto_old.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename drivers/pci/{pci_auto.c => pci_auto_old.c} (100%)
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index bcf8127..dee844f 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -13,7 +13,7 @@ obj-$(CONFIG_X86) += pci_x86.o else obj-$(CONFIG_PCI) += pci.o endif -obj-$(CONFIG_PCI) += pci_common.o pci_auto.o pci_rom.o +obj-$(CONFIG_PCI) += pci_common.o pci_auto_old.o pci_rom.o
obj-$(CONFIG_FSL_PCI_INIT) += fsl_pci_init.o obj-$(CONFIG_PCI_INDIRECT_BRIDGE) += pci_indirect.o diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto_old.c similarity index 100% rename from drivers/pci/pci_auto.c rename to drivers/pci/pci_auto_old.c

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
This file should not be used with driver model as it has lots of legacy/ compatibility functions. Rename it to make this clear.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2: None
drivers/pci/Makefile | 2 +- drivers/pci/{pci_auto.c => pci_auto_old.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename drivers/pci/{pci_auto.c => pci_auto_old.c} (100%)
Applied to u-boot-dm.

Some functions will be used by driver model and legacy PCI code. To avoid duplication, put these in a separate, shared file.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: - Fix 'autoconfiguration' typo - Fix merge error in pciauto_setup_device() which removed needed code
drivers/pci/Makefile | 2 +- drivers/pci/pci_auto_common.c | 128 ++++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci_auto_old.c | 116 -------------------------------------- 3 files changed, 129 insertions(+), 117 deletions(-) create mode 100644 drivers/pci/pci_auto_common.c
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index dee844f..1f8f86f 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -13,7 +13,7 @@ obj-$(CONFIG_X86) += pci_x86.o else obj-$(CONFIG_PCI) += pci.o endif -obj-$(CONFIG_PCI) += pci_common.o pci_auto_old.o pci_rom.o +obj-$(CONFIG_PCI) += pci_auto_common.o pci_auto_old.o pci_common.o pci_rom.o
obj-$(CONFIG_FSL_PCI_INIT) += fsl_pci_init.o obj-$(CONFIG_PCI_INDIRECT_BRIDGE) += pci_indirect.o diff --git a/drivers/pci/pci_auto_common.c b/drivers/pci/pci_auto_common.c new file mode 100644 index 0000000..85c419e --- /dev/null +++ b/drivers/pci/pci_auto_common.c @@ -0,0 +1,128 @@ +/* + * PCI auto-configuration library + * + * Author: Matt Porter mporter@mvista.com + * + * Copyright 2000 MontaVista Software Inc. + * + * Modifications for driver model: + * Copyright 2015 Google, Inc + * Written by Simon Glass sjg@chromium.org + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <errno.h> +#include <pci.h> + +void pciauto_region_init(struct pci_region *res) +{ + /* + * Avoid allocating PCI resources from address 0 -- this is illegal + * according to PCI 2.1 and moreover, this is known to cause Linux IDE + * drivers to fail. Use a reasonable starting value of 0x1000 instead. + */ + res->bus_lower = res->bus_start ? res->bus_start : 0x1000; +} + +void pciauto_region_align(struct pci_region *res, pci_size_t size) +{ + res->bus_lower = ((res->bus_lower - 1) | (size - 1)) + 1; +} + +int pciauto_region_allocate(struct pci_region *res, pci_size_t size, + pci_addr_t *bar) +{ + pci_addr_t addr; + + if (!res) { + debug("No resource"); + goto error; + } + + addr = ((res->bus_lower - 1) | (size - 1)) + 1; + + if (addr - res->bus_start + size > res->size) { + debug("No room in resource"); + goto error; + } + + res->bus_lower = addr + size; + + debug("address=0x%llx bus_lower=0x%llx", (unsigned long long)addr, + (unsigned long long)res->bus_lower); + + *bar = addr; + return 0; + + error: + *bar = (pci_addr_t)-1; + return -1; +} + +void pciauto_config_init(struct pci_controller *hose) +{ + int i; + + hose->pci_io = NULL; + hose->pci_mem = NULL; + hose->pci_prefetch = NULL; + + for (i = 0; i < hose->region_count; i++) { + switch (hose->regions[i].flags) { + case PCI_REGION_IO: + if (!hose->pci_io || + hose->pci_io->size < hose->regions[i].size) + hose->pci_io = hose->regions + i; + break; + case PCI_REGION_MEM: + if (!hose->pci_mem || + hose->pci_mem->size < hose->regions[i].size) + hose->pci_mem = hose->regions + i; + break; + case (PCI_REGION_MEM | PCI_REGION_PREFETCH): + if (!hose->pci_prefetch || + hose->pci_prefetch->size < hose->regions[i].size) + hose->pci_prefetch = hose->regions + i; + break; + } + } + + + if (hose->pci_mem) { + pciauto_region_init(hose->pci_mem); + + debug("PCI Autoconfig: Bus Memory region: [0x%llx-0x%llx],\n" + "\t\tPhysical Memory [%llx-%llxx]\n", + (u64)hose->pci_mem->bus_start, + (u64)(hose->pci_mem->bus_start + hose->pci_mem->size - 1), + (u64)hose->pci_mem->phys_start, + (u64)(hose->pci_mem->phys_start + hose->pci_mem->size - 1)); + } + + if (hose->pci_prefetch) { + pciauto_region_init(hose->pci_prefetch); + + debug("PCI Autoconfig: Bus Prefetchable Mem: [0x%llx-0x%llx],\n" + "\t\tPhysical Memory [%llx-%llx]\n", + (u64)hose->pci_prefetch->bus_start, + (u64)(hose->pci_prefetch->bus_start + + hose->pci_prefetch->size - 1), + (u64)hose->pci_prefetch->phys_start, + (u64)(hose->pci_prefetch->phys_start + + hose->pci_prefetch->size - 1)); + } + + if (hose->pci_io) { + pciauto_region_init(hose->pci_io); + + debug("PCI Autoconfig: Bus I/O region: [0x%llx-0x%llx],\n" + "\t\tPhysical Memory: [%llx-%llx]\n", + (u64)hose->pci_io->bus_start, + (u64)(hose->pci_io->bus_start + hose->pci_io->size - 1), + (u64)hose->pci_io->phys_start, + (u64)(hose->pci_io->phys_start + hose->pci_io->size - 1)); + } +} diff --git a/drivers/pci/pci_auto_old.c b/drivers/pci/pci_auto_old.c index 0412bf3..932eab8 100644 --- a/drivers/pci/pci_auto_old.c +++ b/drivers/pci/pci_auto_old.c @@ -23,55 +23,6 @@ * */
-void pciauto_region_init(struct pci_region *res) -{ - /* - * Avoid allocating PCI resources from address 0 -- this is illegal - * according to PCI 2.1 and moreover, this is known to cause Linux IDE - * drivers to fail. Use a reasonable starting value of 0x1000 instead. - */ - res->bus_lower = res->bus_start ? res->bus_start : 0x1000; -} - -void pciauto_region_align(struct pci_region *res, pci_size_t size) -{ - res->bus_lower = ((res->bus_lower - 1) | (size - 1)) + 1; -} - -int pciauto_region_allocate(struct pci_region *res, pci_size_t size, - pci_addr_t *bar) -{ - pci_addr_t addr; - - if (!res) { - debug("No resource"); - goto error; - } - - addr = ((res->bus_lower - 1) | (size - 1)) + 1; - - if (addr - res->bus_start + size > res->size) { - debug("No room in resource"); - goto error; - } - - res->bus_lower = addr + size; - - debug("address=0x%llx bus_lower=0x%llx", (unsigned long long)addr, - (unsigned long long)res->bus_lower); - - *bar = addr; - return 0; - - error: - *bar = (pci_addr_t)-1; - return -1; -} - -/* - * - */ - void pciauto_setup_device(struct pci_controller *hose, pci_dev_t dev, int bars_num, struct pci_region *mem, @@ -385,73 +336,6 @@ void pciauto_postscan_setup_bridge(struct pci_controller *hose, } }
-/* - * - */ - -void pciauto_config_init(struct pci_controller *hose) -{ - int i; - - hose->pci_io = hose->pci_mem = hose->pci_prefetch = NULL; - - for (i = 0; i < hose->region_count; i++) { - switch(hose->regions[i].flags) { - case PCI_REGION_IO: - if (!hose->pci_io || - hose->pci_io->size < hose->regions[i].size) - hose->pci_io = hose->regions + i; - break; - case PCI_REGION_MEM: - if (!hose->pci_mem || - hose->pci_mem->size < hose->regions[i].size) - hose->pci_mem = hose->regions + i; - break; - case (PCI_REGION_MEM | PCI_REGION_PREFETCH): - if (!hose->pci_prefetch || - hose->pci_prefetch->size < hose->regions[i].size) - hose->pci_prefetch = hose->regions + i; - break; - } - } - - - if (hose->pci_mem) { - pciauto_region_init(hose->pci_mem); - - debug("PCI Autoconfig: Bus Memory region: [0x%llx-0x%llx],\n" - "\t\tPhysical Memory [%llx-%llxx]\n", - (u64)hose->pci_mem->bus_start, - (u64)(hose->pci_mem->bus_start + hose->pci_mem->size - 1), - (u64)hose->pci_mem->phys_start, - (u64)(hose->pci_mem->phys_start + hose->pci_mem->size - 1)); - } - - if (hose->pci_prefetch) { - pciauto_region_init(hose->pci_prefetch); - - debug("PCI Autoconfig: Bus Prefetchable Mem: [0x%llx-0x%llx],\n" - "\t\tPhysical Memory [%llx-%llx]\n", - (u64)hose->pci_prefetch->bus_start, - (u64)(hose->pci_prefetch->bus_start + - hose->pci_prefetch->size - 1), - (u64)hose->pci_prefetch->phys_start, - (u64)(hose->pci_prefetch->phys_start + - hose->pci_prefetch->size - 1)); - } - - if (hose->pci_io) { - pciauto_region_init(hose->pci_io); - - debug("PCI Autoconfig: Bus I/O region: [0x%llx-0x%llx],\n" - "\t\tPhysical Memory: [%llx-%llx]\n", - (u64)hose->pci_io->bus_start, - (u64)(hose->pci_io->bus_start + hose->pci_io->size - 1), - (u64)hose->pci_io->phys_start, - (u64)(hose->pci_io->phys_start + hose->pci_io->size - 1)); - - } -}
/* * HJF: Changed this to return int. I think this is required

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
Some functions will be used by driver model and legacy PCI code. To avoid duplication, put these in a separate, shared file.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2:
- Fix 'autoconfiguration' typo
- Fix merge error in pciauto_setup_device() which removed needed code
drivers/pci/Makefile | 2 +- drivers/pci/pci_auto_common.c | 128 ++++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci_auto_old.c | 116 -------------------------------------- 3 files changed, 129 insertions(+), 117 deletions(-) create mode 100644 drivers/pci/pci_auto_common.c
Applied to u-boot-dm.

Before converting this to driver model, reorder the code to avoid forward function declarations.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: None
common/cmd_pci.c | 216 +++++++++++++++++++++++++++---------------------------- 1 file changed, 106 insertions(+), 110 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 190e3b6..f8faa31 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -22,115 +22,6 @@ #include <asm/io.h> #include <pci.h>
-/* - * Follows routines for the output of infos about devices on PCI bus. - */ - -void pci_header_show(pci_dev_t dev); -void pci_header_show_brief(pci_dev_t dev); - -/* - * Subroutine: pciinfo - * - * Description: Show information about devices on PCI bus. - * Depending on the define CONFIG_SYS_SHORT_PCI_LISTING - * the output will be more or less exhaustive. - * - * Inputs: bus_no the number of the bus to be scanned. - * - * Return: None - * - */ -void pciinfo(int BusNum, int ShortPCIListing) -{ - struct pci_controller *hose = pci_bus_to_hose(BusNum); - int Device; - int Function; - unsigned char HeaderType; - unsigned short VendorID; - pci_dev_t dev; - int ret; - - if (!hose) - return; - - printf("Scanning PCI devices on bus %d\n", BusNum); - - if (ShortPCIListing) { - printf("BusDevFun VendorId DeviceId Device Class Sub-Class\n"); - printf("_____________________________________________________________\n"); - } - - for (Device = 0; Device < PCI_MAX_PCI_DEVICES; Device++) { - HeaderType = 0; - VendorID = 0; - for (Function = 0; Function < PCI_MAX_PCI_FUNCTIONS; Function++) { - /* - * If this is not a multi-function device, we skip the rest. - */ - if (Function && !(HeaderType & 0x80)) - break; - - dev = PCI_BDF(BusNum, Device, Function); - - if (pci_skip_dev(hose, dev)) - continue; - - ret = pci_read_config_word(dev, PCI_VENDOR_ID, - &VendorID); - if (ret) - goto error; - if ((VendorID == 0xFFFF) || (VendorID == 0x0000)) - continue; - - if (!Function) pci_read_config_byte(dev, PCI_HEADER_TYPE, &HeaderType); - - if (ShortPCIListing) - { - printf("%02x.%02x.%02x ", BusNum, Device, Function); - pci_header_show_brief(dev); - } - else - { - printf("\nFound PCI device %02x.%02x.%02x:\n", - BusNum, Device, Function); - pci_header_show(dev); - } - } - } - - return; -error: - printf("Cannot read bus configuration: %d\n", ret); -} - - -/* - * Subroutine: pci_header_show_brief - * - * Description: Reads and prints the header of the - * specified PCI device in short form. - * - * Inputs: dev Bus+Device+Function number - * - * Return: None - * - */ -void pci_header_show_brief(pci_dev_t dev) -{ - u16 vendor, device; - u8 class, subclass; - - pci_read_config_word(dev, PCI_VENDOR_ID, &vendor); - pci_read_config_word(dev, PCI_DEVICE_ID, &device); - pci_read_config_byte(dev, PCI_CLASS_CODE, &class); - pci_read_config_byte(dev, PCI_CLASS_SUB_CODE, &subclass); - - printf("0x%.4x 0x%.4x %-23s 0x%.2x\n", - vendor, device, - pci_class_str(class), subclass); -} - struct pci_reg_info { const char *name; enum pci_size_t size; @@ -284,10 +175,10 @@ void pci_header_show(pci_dev_t dev) { u8 class, header_type;
+ pci_read_config_byte(dev, PCI_CLASS_CODE, &class); pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type); pci_show_regs(dev, regs_start);
- pci_read_config_byte(dev, PCI_CLASS_CODE, &class); printf(" class code = 0x%.2x (%s)\n", class, pci_class_str(class)); pci_show_regs(dev, regs_rest); @@ -309,6 +200,111 @@ void pci_header_show(pci_dev_t dev) } }
+/* + * Subroutine: pci_header_show_brief + * + * Description: Reads and prints the header of the + * specified PCI device in short form. + * + * Inputs: dev Bus+Device+Function number + * + * Return: None + * + */ +void pci_header_show_brief(pci_dev_t dev) +{ + u16 vendor, device; + u8 class, subclass; + + pci_read_config_word(dev, PCI_VENDOR_ID, &vendor); + pci_read_config_word(dev, PCI_DEVICE_ID, &device); + pci_read_config_byte(dev, PCI_CLASS_CODE, &class); + pci_read_config_byte(dev, PCI_CLASS_SUB_CODE, &subclass); + + printf("0x%.4x 0x%.4x %-23s 0x%.2x\n", + vendor, device, + pci_class_str(class), subclass); +} + +/* + * Subroutine: pciinfo + * + * Description: Show information about devices on PCI bus. + * Depending on the defineCONFIG_SYS_SHORT_PCI_LISTING + * the output will be more or less exhaustive. + * + * Inputs: bus_no the number of the bus to be scanned. + * + * Return: None + * + */ +void pciinfo(int bus_num, int short_pci_listing) +{ + struct pci_controller *hose = pci_bus_to_hose(bus_num); + int Device; + int Function; + unsigned char HeaderType; + unsigned short VendorID; + pci_dev_t dev; + int ret; + + if (!hose) + return; + + printf("Scanning PCI devices on bus %d\n", bus_num); + + if (short_pci_listing) { + printf("BusDevFun VendorId DeviceId Device Class Sub-Class\n"); + printf("_____________________________________________________________\n"); + } + + for (Device = 0; Device < PCI_MAX_PCI_DEVICES; Device++) { + HeaderType = 0; + VendorID = 0; + for (Function = 0; Function < PCI_MAX_PCI_FUNCTIONS; + Function++) { + /* + * If this is not a multi-function device, we skip + * the rest. + */ + if (Function && !(HeaderType & 0x80)) + break; + + dev = PCI_BDF(bus_num, Device, Function); + + if (pci_skip_dev(hose, dev)) + continue; + + ret = pci_read_config_word(dev, PCI_VENDOR_ID, + &VendorID); + if (ret) + goto error; + if ((VendorID == 0xFFFF) || (VendorID == 0x0000)) + continue; + + if (!Function) { + pci_read_config_byte(dev, PCI_HEADER_TYPE, + &HeaderType); + } + + if (short_pci_listing) { + printf("%02x.%02x.%02x ", bus_num, Device, + Function); + pci_header_show_brief(dev); + } else { + printf("\nFound PCI device %02x.%02x.%02x:\n", + bus_num, Device, Function); + pci_header_show(dev); + } + } + } + + return; +error: + printf("Cannot read bus configuration: %d\n", ret); +} + + /* Convert the "bus.device.function" identifier into a number. */ static pci_dev_t get_pci_dev(char* name)

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
Before converting this to driver model, reorder the code to avoid forward function declarations.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2: None
common/cmd_pci.c | 216 +++++++++++++++++++++++++++---------------------------- 1 file changed, 106 insertions(+), 110 deletions(-)
Applied to u-boot-dm.

The function comments use an old style and some are incorrect. Update them.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: - Use 'Show' instead of 'Shows' in the the pci_header_show() function comment
Changes in v2: - Add a new patch to update the function comments
common/cmd_pci.c | 84 ++++++++++++++++++++++++++------------------------------ 1 file changed, 39 insertions(+), 45 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index f8faa31..7fdd3d8 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -161,15 +161,10 @@ static struct pci_reg_info regs_cardbus[] = { {}, };
-/* - * Subroutine: PCI_Header_Show - * - * Description: Reads the header of the specified PCI device. - * - * Inputs: BusDevFunc Bus+Device+Function number - * - * Return: None +/** + * @pci_header_show() - Show the header of the specified PCI device. * + * @dev: Bus+Device+Function number */ void pci_header_show(pci_dev_t dev) { @@ -200,16 +195,12 @@ void pci_header_show(pci_dev_t dev) } }
-/* - * Subroutine: pci_header_show_brief - * - * Description: Reads and prints the header of the - * specified PCI device in short form. +/** + * pci_header_show_brief() - Show the short-form PCI device header * - * Inputs: dev Bus+Device+Function number - * - * Return: None + * Reads and prints the header of the specified PCI device in short form. * + * @dev: Bus+Device+Function number */ void pci_header_show_brief(pci_dev_t dev) { @@ -226,25 +217,23 @@ void pci_header_show_brief(pci_dev_t dev) pci_class_str(class), subclass); }
-/* - * Subroutine: pciinfo - * - * Description: Show information about devices on PCI bus. - * Depending on the defineCONFIG_SYS_SHORT_PCI_LISTING - * the output will be more or less exhaustive. +/** + * pciinfo() - Show a list of devices on the PCI bus * - * Inputs: bus_no the number of the bus to be scanned. - * - * Return: None + * Show information about devices on PCI bus. Depending on @short_pci_listing + * the output will be more or less exhaustive. * + * @bus_num: The number of the bus to be scanned + * @short_pci_listing: true to use short form, showing only a brief header + * for each device */ void pciinfo(int bus_num, int short_pci_listing) { struct pci_controller *hose = pci_bus_to_hose(bus_num); - int Device; - int Function; - unsigned char HeaderType; - unsigned short VendorID; + int device; + int function; + unsigned char header_type; + unsigned short vendor_id; pci_dev_t dev; int ret;
@@ -258,42 +247,42 @@ void pciinfo(int bus_num, int short_pci_listing) printf("_____________________________________________________________\n"); }
- for (Device = 0; Device < PCI_MAX_PCI_DEVICES; Device++) { - HeaderType = 0; - VendorID = 0; - for (Function = 0; Function < PCI_MAX_PCI_FUNCTIONS; - Function++) { + for (device = 0; device < PCI_MAX_PCI_DEVICES; device++) { + header_type = 0; + vendor_id = 0; + for (function = 0; function < PCI_MAX_PCI_FUNCTIONS; + function++) { /* * If this is not a multi-function device, we skip * the rest. */ - if (Function && !(HeaderType & 0x80)) + if (function && !(header_type & 0x80)) break;
- dev = PCI_BDF(bus_num, Device, Function); + dev = PCI_BDF(bus_num, device, function);
if (pci_skip_dev(hose, dev)) continue;
ret = pci_read_config_word(dev, PCI_VENDOR_ID, - &VendorID); + &vendor_id); if (ret) goto error; - if ((VendorID == 0xFFFF) || (VendorID == 0x0000)) + if ((vendor_id == 0xFFFF) || (vendor_id == 0x0000)) continue;
- if (!Function) { + if (!function) { pci_read_config_byte(dev, PCI_HEADER_TYPE, - &HeaderType); + &header_type); }
if (short_pci_listing) { - printf("%02x.%02x.%02x ", bus_num, Device, - Function); + printf("%02x.%02x.%02x ", bus_num, device, + function); pci_header_show_brief(dev); } else { printf("\nFound PCI device %02x.%02x.%02x:\n", - bus_num, Device, Function); + bus_num, device, function); pci_header_show(dev); } } @@ -305,9 +294,13 @@ error: }
-/* Convert the "bus.device.function" identifier into a number. +/** + * get_pci_dev() - Convert the "bus.device.function" identifier into a number + * + * @name: Device string in the form "bus.device.function" where each is in hex + * @return encoded pci_dev_t or -1 if the string was invalid */ -static pci_dev_t get_pci_dev(char* name) +static pci_dev_t get_pci_dev(char *name) { char cnum[12]; int len, i, iold, n; @@ -328,6 +321,7 @@ static pci_dev_t get_pci_dev(char* name) if (n == 0) n = 1; bdfs[n] = simple_strtoul(cnum, NULL, 16); + return PCI_BDF(bdfs[0], bdfs[1], bdfs[2]); }

Hi Simon,
On Fri, Nov 27, 2015 at 10:51 AM, Simon Glass sjg@chromium.org wrote:
The function comments use an old style and some are incorrect. Update them.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3:
- Use 'Show' instead of 'Shows' in the the pci_header_show() function comment
Changes in v2:
- Add a new patch to update the function comments
common/cmd_pci.c | 84 ++++++++++++++++++++++++++------------------------------ 1 file changed, 39 insertions(+), 45 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index f8faa31..7fdd3d8 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -161,15 +161,10 @@ static struct pci_reg_info regs_cardbus[] = { {}, };
-/*
- Subroutine: PCI_Header_Show
- Description: Reads the header of the specified PCI device.
- Inputs: BusDevFunc Bus+Device+Function number
- Return: None
+/**
- @pci_header_show() - Show the header of the specified PCI device.
nits: please remove @ before pci_header_show(). Sorry I missed this in previous review. Please fix it when applying.
*/
- @dev: Bus+Device+Function number
void pci_header_show(pci_dev_t dev) { @@ -200,16 +195,12 @@ void pci_header_show(pci_dev_t dev) } }
[snip]
Regards, Bin

On 26 November 2015 at 23:37, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Nov 27, 2015 at 10:51 AM, Simon Glass sjg@chromium.org wrote:
The function comments use an old style and some are incorrect. Update them.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3:
- Use 'Show' instead of 'Shows' in the the pci_header_show() function comment
Changes in v2:
- Add a new patch to update the function comments
common/cmd_pci.c | 84 ++++++++++++++++++++++++++------------------------------ 1 file changed, 39 insertions(+), 45 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index f8faa31..7fdd3d8 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -161,15 +161,10 @@ static struct pci_reg_info regs_cardbus[] = { {}, };
-/*
- Subroutine: PCI_Header_Show
- Description: Reads the header of the specified PCI device.
- Inputs: BusDevFunc Bus+Device+Function number
- Return: None
+/**
- @pci_header_show() - Show the header of the specified PCI device.
nits: please remove @ before pci_header_show(). Sorry I missed this in previous review. Please fix it when applying.
Fixed, and
Applied to u-boot-dm.

Currently we use switch() and access PCI configuration via several functions, one for each data size. Adjust the code to use generic functions, where the data size is a parameter.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Fix mix-up between PCI_SIZE_xx and byte size
Changes in v2: - Add back the leading 0 in the printf() statements
common/cmd_pci.c | 74 ++++++++++++++++++++++++-------------------------------- 1 file changed, 31 insertions(+), 43 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 7fdd3d8..4874033 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -28,19 +28,24 @@ struct pci_reg_info { u8 offset; };
-static int pci_field_width(enum pci_size_t size) +static int pci_byte_size(enum pci_size_t size) { switch (size) { case PCI_SIZE_8: - return 2; + return 1; case PCI_SIZE_16: - return 4; + return 2; case PCI_SIZE_32: default: - return 8; + return 4; } }
+static int pci_field_width(enum pci_size_t size) +{ + return pci_byte_size(size) * 2; +} + static unsigned long pci_read_config(pci_dev_t dev, int offset, enum pci_size_t size) { @@ -325,38 +330,31 @@ static pci_dev_t get_pci_dev(char *name) return PCI_BDF(bdfs[0], bdfs[1], bdfs[2]); }
-static int pci_cfg_display(pci_dev_t bdf, ulong addr, ulong size, ulong length) +static int pci_cfg_display(pci_dev_t bdf, ulong addr, enum pci_size_t size, + ulong length) { #define DISP_LINE_LEN 16 ulong i, nbytes, linebytes; + int byte_size; int rc = 0;
+ byte_size = pci_byte_size(size); if (length == 0) - length = 0x40 / size; /* Standard PCI configuration space */ + length = 0x40 / byte_size; /* Standard PCI config space */
/* Print the lines. * once, and all accesses are with the specified bus width. */ - nbytes = length * size; + nbytes = length * byte_size; do { - uint val4; - ushort val2; - u_char val1; - printf("%08lx:", addr); - linebytes = (nbytes>DISP_LINE_LEN)?DISP_LINE_LEN:nbytes; - for (i=0; i<linebytes; i+= size) { - if (size == 4) { - pci_read_config_dword(bdf, addr, &val4); - printf(" %08x", val4); - } else if (size == 2) { - pci_read_config_word(bdf, addr, &val2); - printf(" %04x", val2); - } else { - pci_read_config_byte(bdf, addr, &val1); - printf(" %02x", val1); - } - addr += size; + linebytes = (nbytes > DISP_LINE_LEN) ? DISP_LINE_LEN : nbytes; + for (i = 0; i < linebytes; i += byte_size) { + unsigned long val; + + val = pci_read_config(bdf, addr, size); + printf(" %0*lx", pci_field_width(size), val); + addr += byte_size; } printf("\n"); nbytes -= linebytes; @@ -385,32 +383,20 @@ static int pci_cfg_write (pci_dev_t bdf, ulong addr, ulong size, ulong value) return 0; }
-static int -pci_cfg_modify (pci_dev_t bdf, ulong addr, ulong size, ulong value, int incrflag) +static int pci_cfg_modify(pci_dev_t bdf, ulong addr, enum pci_size_t size, + ulong value, int incrflag) { ulong i; int nbytes; - uint val4; - ushort val2; - u_char val1; + ulong val;
/* Print the address, followed by value. Then accept input for * the next value. A non-converted value exits. */ do { printf("%08lx:", addr); - if (size == 4) { - pci_read_config_dword(bdf, addr, &val4); - printf(" %08x", val4); - } - else if (size == 2) { - pci_read_config_word(bdf, addr, &val2); - printf(" %04x", val2); - } - else { - pci_read_config_byte(bdf, addr, &val1); - printf(" %02x", val1); - } + val = pci_read_config(bdf, addr, size); + printf(" %0*lx", pci_field_width(size), val);
nbytes = cli_readline(" ? "); if (nbytes == 0 || (nbytes == 1 && console_buffer[0] == '-')) { @@ -456,7 +442,8 @@ pci_cfg_modify (pci_dev_t bdf, ulong addr, ulong size, ulong value, int incrflag */ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - ulong addr = 0, value = 0, size = 0; + ulong addr = 0, value = 0, cmd_size = 0; + enum pci_size_t size = PCI_SIZE_32; int busnum = 0; pci_dev_t bdf = 0; char cmd = 's'; @@ -471,7 +458,8 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) case 'm': /* modify */ case 'w': /* write */ /* Check for a size specification. */ - size = cmd_get_data_size(argv[1], 4); + cmd_size = cmd_get_data_size(argv[1], 4); + size = (cmd_size == 4) ? PCI_SIZE_32 : cmd_size - 1; if (argc > 3) addr = simple_strtoul(argv[3], NULL, 16); if (argc > 4)

On Fri, Nov 27, 2015 at 10:51 AM, Simon Glass sjg@chromium.org wrote:
Currently we use switch() and access PCI configuration via several functions, one for each data size. Adjust the code to use generic functions, where the data size is a parameter.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Fix mix-up between PCI_SIZE_xx and byte size
Changes in v2:
- Add back the leading 0 in the printf() statements
common/cmd_pci.c | 74 ++++++++++++++++++++++++-------------------------------- 1 file changed, 31 insertions(+), 43 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 7fdd3d8..4874033 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -28,19 +28,24 @@ struct pci_reg_info { u8 offset; };
-static int pci_field_width(enum pci_size_t size) +static int pci_byte_size(enum pci_size_t size) { switch (size) { case PCI_SIZE_8:
return 2;
return 1; case PCI_SIZE_16:
return 4;
return 2; case PCI_SIZE_32: default:
return 8;
return 4; }
}
+static int pci_field_width(enum pci_size_t size) +{
return pci_byte_size(size) * 2;
+}
static unsigned long pci_read_config(pci_dev_t dev, int offset, enum pci_size_t size) { @@ -325,38 +330,31 @@ static pci_dev_t get_pci_dev(char *name) return PCI_BDF(bdfs[0], bdfs[1], bdfs[2]); }
-static int pci_cfg_display(pci_dev_t bdf, ulong addr, ulong size, ulong length) +static int pci_cfg_display(pci_dev_t bdf, ulong addr, enum pci_size_t size,
ulong length)
{ #define DISP_LINE_LEN 16 ulong i, nbytes, linebytes;
int byte_size; int rc = 0;
byte_size = pci_byte_size(size); if (length == 0)
length = 0x40 / size; /* Standard PCI configuration space */
length = 0x40 / byte_size; /* Standard PCI config space */ /* Print the lines. * once, and all accesses are with the specified bus width. */
nbytes = length * size;
nbytes = length * byte_size; do {
uint val4;
ushort val2;
u_char val1;
printf("%08lx:", addr);
linebytes = (nbytes>DISP_LINE_LEN)?DISP_LINE_LEN:nbytes;
for (i=0; i<linebytes; i+= size) {
if (size == 4) {
pci_read_config_dword(bdf, addr, &val4);
printf(" %08x", val4);
} else if (size == 2) {
pci_read_config_word(bdf, addr, &val2);
printf(" %04x", val2);
} else {
pci_read_config_byte(bdf, addr, &val1);
printf(" %02x", val1);
}
addr += size;
linebytes = (nbytes > DISP_LINE_LEN) ? DISP_LINE_LEN : nbytes;
for (i = 0; i < linebytes; i += byte_size) {
unsigned long val;
val = pci_read_config(bdf, addr, size);
printf(" %0*lx", pci_field_width(size), val);
addr += byte_size; } printf("\n"); nbytes -= linebytes;
@@ -385,32 +383,20 @@ static int pci_cfg_write (pci_dev_t bdf, ulong addr, ulong size, ulong value) return 0; }
-static int -pci_cfg_modify (pci_dev_t bdf, ulong addr, ulong size, ulong value, int incrflag) +static int pci_cfg_modify(pci_dev_t bdf, ulong addr, enum pci_size_t size,
ulong value, int incrflag)
{ ulong i; int nbytes;
uint val4;
ushort val2;
u_char val1;
ulong val; /* Print the address, followed by value. Then accept input for * the next value. A non-converted value exits. */ do { printf("%08lx:", addr);
if (size == 4) {
pci_read_config_dword(bdf, addr, &val4);
printf(" %08x", val4);
}
else if (size == 2) {
pci_read_config_word(bdf, addr, &val2);
printf(" %04x", val2);
}
else {
pci_read_config_byte(bdf, addr, &val1);
printf(" %02x", val1);
}
val = pci_read_config(bdf, addr, size);
printf(" %0*lx", pci_field_width(size), val); nbytes = cli_readline(" ? "); if (nbytes == 0 || (nbytes == 1 && console_buffer[0] == '-')) {
@@ -456,7 +442,8 @@ pci_cfg_modify (pci_dev_t bdf, ulong addr, ulong size, ulong value, int incrflag */ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) {
ulong addr = 0, value = 0, size = 0;
ulong addr = 0, value = 0, cmd_size = 0;
enum pci_size_t size = PCI_SIZE_32; int busnum = 0; pci_dev_t bdf = 0; char cmd = 's';
@@ -471,7 +458,8 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) case 'm': /* modify */ case 'w': /* write */ /* Check for a size specification. */
size = cmd_get_data_size(argv[1], 4);
cmd_size = cmd_get_data_size(argv[1], 4);
size = (cmd_size == 4) ? PCI_SIZE_32 : cmd_size - 1; if (argc > 3) addr = simple_strtoul(argv[3], NULL, 16); if (argc > 4)
--
Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com

On 26 November 2015 at 23:37, Bin Meng bmeng.cn@gmail.com wrote:
On Fri, Nov 27, 2015 at 10:51 AM, Simon Glass sjg@chromium.org wrote:
Currently we use switch() and access PCI configuration via several functions, one for each data size. Adjust the code to use generic functions, where the data size is a parameter.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Fix mix-up between PCI_SIZE_xx and byte size
Changes in v2:
- Add back the leading 0 in the printf() statements
common/cmd_pci.c | 74 ++++++++++++++++++++++++-------------------------------- 1 file changed, 31 insertions(+), 43 deletions(-)
[snip]
Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot-dm.

In the 'pci' command, add a separate variable to hold the PCI device. When this code is converted to driver model, this variable will be used to hold a struct udevice instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Fix 'switch (cmd)' merge error
Changes in v2: - Refactor the patch based on dropping the earlier refactor patch
common/cmd_pci.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 4874033..857e3e5 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -444,6 +444,7 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { ulong addr = 0, value = 0, cmd_size = 0; enum pci_size_t size = PCI_SIZE_32; + pci_dev_t dev; int busnum = 0; pci_dev_t bdf = 0; char cmd = 's'; @@ -488,12 +489,14 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 0; }
+ dev = bdf; + switch (argv[1][0]) { case 'h': /* header */ - pci_header_show(bdf); + pci_header_show(dev); break; case 'd': /* display */ - return pci_cfg_display(bdf, addr, size, value); + return pci_cfg_display(dev, addr, size, value); #ifdef CONFIG_CMD_PCI_ENUM case 'e': # ifdef CONFIG_DM_PCI @@ -506,17 +509,17 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) case 'n': /* next */ if (argc < 4) goto usage; - ret = pci_cfg_modify(bdf, addr, size, value, 0); + ret = pci_cfg_modify(dev, addr, size, value, 0); break; case 'm': /* modify */ if (argc < 4) goto usage; - ret = pci_cfg_modify(bdf, addr, size, value, 1); + ret = pci_cfg_modify(dev, addr, size, value, 1); break; case 'w': /* write */ if (argc < 5) goto usage; - ret = pci_cfg_write(bdf, addr, size, value); + ret = pci_cfg_write(dev, addr, size, value); break; default: ret = CMD_RET_USAGE;

On Fri, Nov 27, 2015 at 10:51 AM, Simon Glass sjg@chromium.org wrote:
In the 'pci' command, add a separate variable to hold the PCI device. When this code is converted to driver model, this variable will be used to hold a struct udevice instead.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Fix 'switch (cmd)' merge error
Changes in v2:
- Refactor the patch based on dropping the earlier refactor patch
common/cmd_pci.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 4874033..857e3e5 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -444,6 +444,7 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { ulong addr = 0, value = 0, cmd_size = 0; enum pci_size_t size = PCI_SIZE_32;
pci_dev_t dev; int busnum = 0; pci_dev_t bdf = 0; char cmd = 's';
@@ -488,12 +489,14 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 0; }
dev = bdf;
switch (argv[1][0]) { case 'h': /* header */
pci_header_show(bdf);
pci_header_show(dev); break; case 'd': /* display */
return pci_cfg_display(bdf, addr, size, value);
return pci_cfg_display(dev, addr, size, value);
#ifdef CONFIG_CMD_PCI_ENUM case 'e': # ifdef CONFIG_DM_PCI @@ -506,17 +509,17 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) case 'n': /* next */ if (argc < 4) goto usage;
ret = pci_cfg_modify(bdf, addr, size, value, 0);
ret = pci_cfg_modify(dev, addr, size, value, 0); break; case 'm': /* modify */ if (argc < 4) goto usage;
ret = pci_cfg_modify(bdf, addr, size, value, 1);
ret = pci_cfg_modify(dev, addr, size, value, 1); break; case 'w': /* write */ if (argc < 5) goto usage;
ret = pci_cfg_write(bdf, addr, size, value);
ret = pci_cfg_write(dev, addr, size, value); break; default: ret = CMD_RET_USAGE;
--
Reviewed-by: Bin Meng bmeng.cn@gmail.com

On 26 November 2015 at 23:38, Bin Meng bmeng.cn@gmail.com wrote:
On Fri, Nov 27, 2015 at 10:51 AM, Simon Glass sjg@chromium.org wrote:
In the 'pci' command, add a separate variable to hold the PCI device. When this code is converted to driver model, this variable will be used to hold a struct udevice instead.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Fix 'switch (cmd)' merge error
Changes in v2:
- Refactor the patch based on dropping the earlier refactor patch
common/cmd_pci.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
[snip]
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Thanks for the review.
Applied to u-boot-dm.

We want to share this code with the driver model version, so put it in a separate function.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: None
common/cmd_pci.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 857e3e5..fcda7f6 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -200,6 +200,16 @@ void pci_header_show(pci_dev_t dev) } }
+void pciinfo_header(int busnum, 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"); + } +} + /** * pci_header_show_brief() - Show the short-form PCI device header * @@ -245,12 +255,7 @@ void pciinfo(int bus_num, int short_pci_listing) if (!hose) return;
- printf("Scanning PCI devices on bus %d\n", bus_num); - - if (short_pci_listing) { - printf("BusDevFun VendorId DeviceId Device Class Sub-Class\n"); - printf("_____________________________________________________________\n"); - } + pciinfo_header(bus_num, short_pci_listing);
for (device = 0; device < PCI_MAX_PCI_DEVICES; device++) { header_type = 0;

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
We want to share this code with the driver model version, so put it in a separate function.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2: None
common/cmd_pci.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
Applied to u-boot-dm.

Adjust this command to use the correct PCI functions, instead of the compatibility layer.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: - Make pciinfo() static
common/cmd_pci.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- include/common.h | 1 - 2 files changed, 128 insertions(+), 6 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index fcda7f6..1bd6001 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -18,6 +18,7 @@ #include <cli.h> #include <command.h> #include <console.h> +#include <dm.h> #include <asm/processor.h> #include <asm/io.h> #include <pci.h> @@ -46,6 +47,19 @@ static int pci_field_width(enum pci_size_t size) return pci_byte_size(size) * 2; }
+#ifdef CONFIG_DM_PCI +static void pci_show_regs(struct udevice *dev, struct pci_reg_info *regs) +{ + for (; regs->name; regs++) { + unsigned long val; + + dm_pci_read_config(dev, regs->offset, &val, regs->size); + printf(" %s =%*s%#.*lx\n", regs->name, + (int)(28 - strlen(regs->name)), "", + pci_field_width(regs->size), val); + } +} +#else static unsigned long pci_read_config(pci_dev_t dev, int offset, enum pci_size_t size) { @@ -76,6 +90,7 @@ static void pci_show_regs(pci_dev_t dev, struct pci_reg_info *regs) pci_read_config(dev, regs->offset, regs->size)); } } +#endif
static struct pci_reg_info regs_start[] = { { "vendor ID", PCI_SIZE_16, PCI_VENDOR_ID }, @@ -171,15 +186,25 @@ static struct pci_reg_info regs_cardbus[] = { * * @dev: Bus+Device+Function number */ +#ifdef CONFIG_DM_PCI +void pci_header_show(struct udevice *dev) +#else void pci_header_show(pci_dev_t dev) +#endif { +#ifdef CONFIG_DM_PCI + unsigned long class, header_type; + + dm_pci_read_config(dev, PCI_CLASS_CODE, &class, PCI_SIZE_8); + dm_pci_read_config(dev, PCI_HEADER_TYPE, &header_type, PCI_SIZE_8); +#else u8 class, header_type;
pci_read_config_byte(dev, PCI_CLASS_CODE, &class); pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type); +#endif pci_show_regs(dev, regs_start); - - printf(" class code = 0x%.2x (%s)\n", class, + printf(" class code = 0x%.2x (%s)\n", (int)class, pci_class_str(class)); pci_show_regs(dev, regs_rest);
@@ -210,6 +235,55 @@ void pciinfo_header(int busnum, bool short_listing) } }
+#ifdef CONFIG_DM_PCI +/** + * pci_header_show_brief() - Show the short-form PCI device header + * + * Reads and prints the header of the specified PCI device in short form. + * + * @dev: PCI device to show + */ +static void pci_header_show_brief(struct udevice *dev) +{ + ulong vendor, device; + ulong class, subclass; + + dm_pci_read_config(dev, PCI_VENDOR_ID, &vendor, PCI_SIZE_16); + dm_pci_read_config(dev, PCI_DEVICE_ID, &device, PCI_SIZE_16); + dm_pci_read_config(dev, PCI_CLASS_CODE, &class, PCI_SIZE_8); + dm_pci_read_config(dev, PCI_CLASS_SUB_CODE, &subclass, PCI_SIZE_8); + + printf("0x%.4lx 0x%.4lx %-23s 0x%.2lx\n", + vendor, device, + pci_class_str(class), subclass); +} + +static void pciinfo(struct udevice *bus, bool short_listing) +{ + struct udevice *dev; + + pciinfo_header(bus->seq, short_listing); + + for (device_find_first_child(bus, &dev); + dev; + device_find_next_child(&dev)) { + struct pci_child_platdata *pplat; + + pplat = dev_get_parent_platdata(dev); + if (short_listing) { + printf("%02x.%02x.%02x ", bus->seq, + PCI_DEV(pplat->devfn), PCI_FUNC(pplat->devfn)); + pci_header_show_brief(dev); + } else { + printf("\nFound PCI device %02x.%02x.%02x:\n", bus->seq, + PCI_DEV(pplat->devfn), PCI_FUNC(pplat->devfn)); + pci_header_show(dev); + } + } +} + +#else + /** * pci_header_show_brief() - Show the short-form PCI device header * @@ -302,7 +376,7 @@ void pciinfo(int bus_num, int short_pci_listing) error: printf("Cannot read bus configuration: %d\n", ret); } - +#endif
/** * get_pci_dev() - Convert the "bus.device.function" identifier into a number @@ -335,8 +409,13 @@ static pci_dev_t get_pci_dev(char *name) return PCI_BDF(bdfs[0], bdfs[1], bdfs[2]); }
+#ifdef CONFIG_DM_PCI +static int pci_cfg_display(struct udevice *dev, ulong addr, + enum pci_size_t size, ulong length) +#else static int pci_cfg_display(pci_dev_t bdf, ulong addr, enum pci_size_t size, ulong length) +#endif { #define DISP_LINE_LEN 16 ulong i, nbytes, linebytes; @@ -357,7 +436,11 @@ static int pci_cfg_display(pci_dev_t bdf, ulong addr, enum pci_size_t size, for (i = 0; i < linebytes; i += byte_size) { unsigned long val;
+#ifdef CONFIG_DM_PCI + dm_pci_read_config(dev, addr, &val, size); +#else val = pci_read_config(bdf, addr, size); +#endif printf(" %0*lx", pci_field_width(size), val); addr += byte_size; } @@ -372,6 +455,7 @@ static int pci_cfg_display(pci_dev_t bdf, ulong addr, enum pci_size_t size, return (rc); }
+#ifndef CONFIG_DM_PCI static int pci_cfg_write (pci_dev_t bdf, ulong addr, ulong size, ulong value) { if (size == 4) { @@ -387,9 +471,15 @@ static int pci_cfg_write (pci_dev_t bdf, ulong addr, ulong size, ulong value) } return 0; } +#endif
-static int pci_cfg_modify(pci_dev_t bdf, ulong addr, enum pci_size_t size, +#ifdef CONFIG_DM_PCI +static int pci_cfg_modify(struct udevice *dev, ulong addr, ulong size, ulong value, int incrflag) +#else +static int pci_cfg_modify(pci_dev_t bdf, ulong addr, ulong size, ulong value, + int incrflag) +#endif { ulong i; int nbytes; @@ -400,7 +490,11 @@ static int pci_cfg_modify(pci_dev_t bdf, ulong addr, enum pci_size_t size, */ do { printf("%08lx:", addr); +#ifdef CONFIG_DM_PCI + dm_pci_read_config(dev, addr, &val, size); +#else val = pci_read_config(bdf, addr, size); +#endif printf(" %0*lx", pci_field_width(size), val);
nbytes = cli_readline(" ? "); @@ -427,7 +521,11 @@ static int pci_cfg_modify(pci_dev_t bdf, ulong addr, enum pci_size_t size, /* good enough to not time out */ bootretry_reset_cmd_timeout(); - pci_cfg_write (bdf, addr, size, i); +#ifdef CONFIG_DM_PCI + dm_pci_write_config(dev, addr, i, size); +#else + pci_cfg_write(bdf, addr, size, i); +#endif if (incrflag) addr += size; } @@ -449,7 +547,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { ulong addr = 0, value = 0, cmd_size = 0; enum pci_size_t size = PCI_SIZE_32; +#ifdef CONFIG_DM_PCI + struct udevice *dev, *bus; +#else pci_dev_t dev; +#endif int busnum = 0; pci_dev_t bdf = 0; char cmd = 's'; @@ -490,11 +592,28 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc > 1) busnum = simple_strtoul(argv[1], NULL, 16); } +#ifdef CONFIG_DM_PCI + ret = uclass_get_device_by_seq(UCLASS_PCI, busnum, &bus); + if (ret) { + printf("No such bus\n"); + return CMD_RET_FAILURE; + } + pciinfo(bus, value); +#else pciinfo(busnum, value); +#endif return 0; }
+#ifdef CONFIG_DM_PCI + ret = pci_bus_find_bdf(bdf, &dev); + if (ret) { + printf("No such device\n"); + return CMD_RET_FAILURE; + } +#else dev = bdf; +#endif
switch (argv[1][0]) { case 'h': /* header */ @@ -524,7 +643,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) case 'w': /* write */ if (argc < 5) goto usage; +#ifdef CONFIG_DM_PCI + ret = dm_pci_write_config(dev, addr, value, size); +#else ret = pci_cfg_write(dev, addr, size, value); +#endif break; default: ret = CMD_RET_USAGE; diff --git a/include/common.h b/include/common.h index a3831b7..75c78d5 100644 --- a/include/common.h +++ b/include/common.h @@ -432,7 +432,6 @@ int get_env_id (void);
void pci_init (void); void pci_init_board(void); -void pciinfo (int, int);
#if defined(CONFIG_PCI) && defined(CONFIG_4xx) int pci_pre_init (struct pci_controller *);

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
Adjust this command to use the correct PCI functions, instead of the compatibility layer.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2:
- Make pciinfo() static
common/cmd_pci.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- include/common.h | 1 - 2 files changed, 128 insertions(+), 6 deletions(-)
Applied to u-boot-dm.

We eventually need to drop the compatibility functions for driver model. As a first step, create a configuration option to enable them and hide them when the option is disabled.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: - Drop patch: pci: Move 'pci scan' code in with other commands - Move dm_pciauto_config_device() out of CONFIG_DM_PCI_COMPAT
arch/arm/mach-tegra/Kconfig | 1 + arch/x86/Kconfig | 3 +++ configs/sandbox_defconfig | 1 + drivers/pci/Kconfig | 9 +++++++++ drivers/pci/Makefile | 3 ++- include/pci.h | 30 ++++++++++++++++++++++++++---- 6 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index 8db0708..fbfb204 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -7,6 +7,7 @@ config TEGRA_COMMON select DM_I2C select DM_KEYBOARD select DM_PCI + select DM_PCI_COMPAT select DM_SERIAL select DM_SPI select DM_SPI_FLASH diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 16ba4f4..7e7cb61 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -93,6 +93,9 @@ config SYS_X86_START16 depends on X86_RESET_VECTOR default 0xfffff800
+config DM_PCI_COMPAT + default y # Until we finish moving over to the new API + config BOARD_ROMSIZE_KB_512 bool config BOARD_ROMSIZE_KB_1024 diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 0178674..27753cc 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -41,6 +41,7 @@ CONFIG_SPI_FLASH_SANDBOX=y CONFIG_SPI_FLASH=y CONFIG_DM_ETH=y CONFIG_DM_PCI=y +CONFIG_DM_PCI_COMPAT=y CONFIG_PCI_SANDBOX=y CONFIG_PINCTRL=y CONFIG_PINCONF=y diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index c219c19..26aa2b0 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -9,6 +9,15 @@ config DM_PCI available PCI devices, allows scanning of PCI buses and provides device configuration support.
+config DM_PCI_COMPAT + bool "Enable compatible functions for PCI" + depends on DM_PCI + help + Enable compatibility functions for PCI so that old code can be used + with CONFIG_DM_PCI enabled. This should be used as an interim + measure when porting a board to use driver model for PCI. Once the + board is fully supported, this option should be disabled. + config PCI_SANDBOX bool "Sandbox PCI support" depends on SANDBOX && DM_PCI diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 1f8f86f..6b761b4 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -6,7 +6,8 @@ #
ifneq ($(CONFIG_DM_PCI),) -obj-$(CONFIG_PCI) += pci-uclass.o pci_compat.o +obj-$(CONFIG_PCI) += pci-uclass.o +obj-$(CONFIG_DM_PCI_COMPAT) += pci_compat.o obj-$(CONFIG_PCI_SANDBOX) += pci_sandbox.o obj-$(CONFIG_SANDBOX) += pci-emul-uclass.o obj-$(CONFIG_X86) += pci_x86.o diff --git a/include/pci.h b/include/pci.h index c4f6577..2adca85 100644 --- a/include/pci.h +++ b/include/pci.h @@ -656,6 +656,7 @@ extern pci_addr_t pci_hose_phys_to_bus(struct pci_controller* hose, pci_bus_to_virt((dev), (addr), PCI_REGION_IO, (len), (map_flags))
/* For driver model these are defined in macros in pci_compat.c */ +#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) extern int pci_hose_read_config_byte(struct pci_controller *hose, pci_dev_t dev, int where, u8 *val); extern int pci_hose_read_config_word(struct pci_controller *hose, @@ -668,6 +669,7 @@ extern int pci_hose_write_config_word(struct pci_controller *hose, pci_dev_t dev, int where, u16 val); extern int pci_hose_write_config_dword(struct pci_controller *hose, pci_dev_t dev, int where, u32 val); +#endif
#ifndef CONFIG_DM_PCI extern int pci_read_config_byte(pci_dev_t dev, int where, u8 *val); @@ -678,6 +680,13 @@ extern int pci_write_config_word(pci_dev_t dev, int where, u16 val); extern int pci_write_config_dword(pci_dev_t dev, int where, u32 val); #endif
+void pciauto_region_init(struct pci_region *res); +void pciauto_region_align(struct pci_region *res, pci_size_t size); +void pciauto_config_init(struct pci_controller *hose); +int pciauto_region_allocate(struct pci_region *res, pci_size_t size, + pci_addr_t *bar); + +#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) extern int pci_hose_read_config_byte_via_dword(struct pci_controller *hose, pci_dev_t dev, int where, u8 *val); extern int pci_hose_read_config_word_via_dword(struct pci_controller *hose, @@ -696,9 +705,6 @@ extern int pci_skip_dev(struct pci_controller *hose, pci_dev_t dev); extern int pci_hose_scan(struct pci_controller *hose); extern int pci_hose_scan_bus(struct pci_controller *hose, int bus);
-extern void pciauto_region_init(struct pci_region* res); -extern void pciauto_region_align(struct pci_region *res, pci_size_t size); -extern int pciauto_region_allocate(struct pci_region* res, pci_size_t size, pci_addr_t *bar); extern void pciauto_setup_device(struct pci_controller *hose, pci_dev_t dev, int bars_num, struct pci_region *mem, @@ -708,7 +714,6 @@ extern void pciauto_prescan_setup_bridge(struct pci_controller *hose, pci_dev_t dev, int sub_bus); extern void pciauto_postscan_setup_bridge(struct pci_controller *hose, pci_dev_t dev, int sub_bus); -extern void pciauto_config_init(struct pci_controller *hose); extern int pciauto_config_device(struct pci_controller *hose, pci_dev_t dev);
extern pci_dev_t pci_find_device (unsigned int vendor, unsigned int device, int index); @@ -739,6 +744,7 @@ extern void board_pci_fixup_dev(struct pci_controller *hose, pci_dev_t dev, unsigned short device, unsigned short class); #endif +#endif /* !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) */
const char * pci_class_str(u8 class); int pci_last_busno(void); @@ -747,6 +753,7 @@ int pci_last_busno(void); extern void pci_mpc85xx_init (struct pci_controller *hose); #endif
+#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) /** * pci_write_bar32() - Write the address of a BAR including control bits * @@ -783,6 +790,7 @@ u32 pci_read_bar32(struct pci_controller *hose, pci_dev_t dev, int barnum); */ pci_dev_t pci_hose_find_devices(struct pci_controller *hose, int busnum, struct pci_device_id *ids, int *indexp); +#endif /* !CONFIG_DM_PCI || CONFIG_DM_PCI_COMPAT */
/* Access sizes for PCI reads and writes */ enum pci_size_t { @@ -1041,6 +1049,7 @@ int dm_pci_write_config32(struct udevice *dev, int offset, u32 value); */ int pci_write_config32(pci_dev_t pcidev, int offset, u32 value);
+#ifdef CONFIG_DM_PCI_COMPAT /* Compatibility with old naming */ static inline int pci_write_config_dword(pci_dev_t pcidev, int offset, u32 value) @@ -1093,6 +1102,19 @@ static inline int pci_read_config_byte(pci_dev_t pcidev, int offset, return pci_read_config8(pcidev, offset, valuep); }
+#endif /* CONFIG_DM_PCI_COMPAT */ + +/** + * dm_pciauto_config_device() - configure a device ready for use + * + * Space is allocated for each PCI base address register (BAR) so that the + * devices are mapped into memory and I/O space ready for use. + * + * @dev: Device to configure + * @return 0 if OK, -ve on error + */ +int dm_pciauto_config_device(struct udevice *dev); + /** * pci_conv_32_to_size() - convert a 32-bit read value to the given size *

On 26 November 2015 at 18:51, Simon Glass sjg@chromium.org wrote:
We eventually need to drop the compatibility functions for driver model. As a first step, create a configuration option to enable them and hide them when the option is disabled.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2:
- Drop patch: pci: Move 'pci scan' code in with other commands
- Move dm_pciauto_config_device() out of CONFIG_DM_PCI_COMPAT
arch/arm/mach-tegra/Kconfig | 1 + arch/x86/Kconfig | 3 +++ configs/sandbox_defconfig | 1 + drivers/pci/Kconfig | 9 +++++++++ drivers/pci/Makefile | 3 ++- include/pci.h | 30 ++++++++++++++++++++++++++---- 6 files changed, 42 insertions(+), 5 deletions(-)
Applied to u-boot-dm.
participants (2)
-
Bin Meng
-
Simon Glass