[U-Boot] [PATCH 00/14] 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.
Simon Glass (14): pci: Move 'pci scan' code in with other commands 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: Use common functions to read/write config pci: Fix pci_field_width() for 32-bit values 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 | 2 + arch/x86/Kconfig | 3 + common/cmd_pci.c | 579 +++++++++++++++++++---------- configs/sandbox_defconfig | 10 +- drivers/pci/Kconfig | 9 + drivers/pci/Makefile | 5 +- drivers/pci/pci_auto_common.c | 128 +++++++ drivers/pci/{pci_auto.c => pci_auto_old.c} | 122 ------ include/common.h | 1 - include/pci.h | 23 +- 10 files changed, 542 insertions(+), 340 deletions(-) create mode 100644 drivers/pci/pci_auto_common.c rename drivers/pci/{pci_auto.c => pci_auto_old.c} (79%)
At present the 'pci scan' code has its own code path. Adjust it so that it can be placed with the rest of the command processing code. This will allow us to use common set code for all commands.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/cmd_pci.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 69c5332..4f4c341 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -445,11 +445,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc > 1) bdf = simple_strtoul(argv[1], NULL, 16); } - pciinfo(bdf, value); - return 0; + cmd = 's'; + break; }
- switch (argv[1][0]) { + switch (cmd) { case 'h': /* header */ pci_header_show(bdf); return 0; @@ -472,6 +472,9 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 4) goto usage; return pci_cfg_modify(bdf, addr, size, value, 1); + case 's': + pciinfo(bdf, value); + return 0; case 'w': /* write */ if (argc < 5) goto usage;
On Fri, Nov 13, 2015 at 5:45 AM, Simon Glass sjg@chromium.org wrote:
At present the 'pci scan' code has its own code path. Adjust it so that it can be placed with the rest of the command processing code. This will allow us to use common set code for all commands.
Signed-off-by: Simon Glass sjg@chromium.org
common/cmd_pci.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 69c5332..4f4c341 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -445,11 +445,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc > 1) bdf = simple_strtoul(argv[1], NULL, 16); }
pciinfo(bdf, value);return 0;
cmd = 's';break; }
switch (argv[1][0]) {
switch (cmd) { case 'h': /* header */ pci_header_show(bdf); return 0;@@ -472,6 +472,9 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (argc < 4) goto usage; return pci_cfg_modify(bdf, addr, size, value, 1);
case 's':pciinfo(bdf, value);return 0; case 'w': /* write */ if (argc < 5) goto usage;--
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Adjust the commands to return from the same place.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/cmd_pci.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 4f4c341..5762769 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -409,6 +409,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]; @@ -452,7 +453,7 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) switch (cmd) { 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 @@ -462,26 +463,32 @@ 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 's': pciinfo(bdf, value); - return 0; + 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 Fri, Nov 13, 2015 at 5:45 AM, Simon Glass sjg@chromium.org wrote:
Adjust the commands to return from the same place.
Signed-off-by: Simon Glass sjg@chromium.org
common/cmd_pci.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 4f4c341..5762769 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -409,6 +409,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];@@ -452,7 +453,7 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) switch (cmd) { 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 @@ -462,26 +463,32 @@ 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 's': pciinfo(bdf, value);
return 0;
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;}
Reviewed-by: Bin Meng bmeng.cn@gmail.com
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 ---
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 5762769..6e28b70 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -407,6 +407,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; @@ -437,14 +438,13 @@ 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); } cmd = 's'; break; @@ -476,7 +476,7 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) ret = pci_cfg_modify(bdf, addr, size, value, 1); break; case 's': - pciinfo(bdf, value); + pciinfo(busnum, value); break; case 'w': /* write */ if (argc < 5)
On Fri, Nov 13, 2015 at 5:45 AM, 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
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 5762769..6e28b70 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -407,6 +407,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;@@ -437,14 +438,13 @@ 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); } cmd = 's'; break;@@ -476,7 +476,7 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) ret = pci_cfg_modify(bdf, addr, size, value, 1); break; case 's':
pciinfo(bdf, value);
pciinfo(busnum, value); break; case 'w': /* write */ if (argc < 5)--
Reviewed-by: Bin Meng bmeng.cn@gmail.com
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 ---
common/cmd_pci.c | 235 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 147 insertions(+), 88 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 6e28b70..debcd1c 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -130,6 +130,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 32; + } +} + +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,110 +282,30 @@ 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))
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, &_byte); + printf(" class code = 0x%.2x (%s)\n", _byte, + pci_class_str(_byte)); + 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.
Hi Simon,
On Fri, Nov 13, 2015 at 5:45 AM, 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
common/cmd_pci.c | 235 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 147 insertions(+), 88 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 6e28b70..debcd1c 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -130,6 +130,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 32;
This should be 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,110 +282,30 @@ void pci_header_show_brief(pci_dev_t dev) void pci_header_show(pci_dev_t dev) { u8 _byte, header_type;
While we are here, can we rename the variable "_byte" to "class" to indicate its actual purpose?
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)) 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, &_byte);printf(" class code = 0x%.2x (%s)\n", _byte,pci_class_str(_byte));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.
Regards, Bin
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 ---
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 Fri, Nov 13, 2015 at 5:45 AM, 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
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
Reviewed-by: Bin Meng bmeng.cn@gmail.com
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 ---
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 Fri, Nov 13, 2015 at 5:45 AM, 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
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 --
Reviewed-by: Bin Meng bmeng.cn@gmail.com
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 ---
drivers/pci/Makefile | 2 +- drivers/pci/pci_auto_common.c | 128 ++++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci_auto_old.c | 122 ---------------------------------------- 3 files changed, 129 insertions(+), 123 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..faf904e --- /dev/null +++ b/drivers/pci/pci_auto_common.c @@ -0,0 +1,128 @@ +/* + * PCI autoconfiguration 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..54c5e1d 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, @@ -89,7 +40,6 @@ void pciauto_setup_device(struct pci_controller *hose, struct pci_region *bar_res; int found_mem64 = 0; #endif - u16 class;
pci_hose_read_config_word(hose, dev, PCI_COMMAND, &cmdstat); cmdstat = (cmdstat & ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) | PCI_COMMAND_MASTER; @@ -207,11 +157,6 @@ void pciauto_setup_device(struct pci_controller *hose, } #endif
- /* PCI_COMMAND_IO must be set for VGA device */ - pci_hose_read_config_word(hose, dev, PCI_CLASS_DEVICE, &class); - if (class == PCI_CLASS_DISPLAY_VGA) - cmdstat |= PCI_COMMAND_IO; - pci_hose_write_config_word(hose, dev, PCI_COMMAND, cmdstat); pci_hose_write_config_byte(hose, dev, PCI_CACHE_LINE_SIZE, CONFIG_SYS_PCI_CACHE_LINE_SIZE); @@ -385,73 +330,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
Hi Simon,
On Fri, Nov 13, 2015 at 5:45 AM, 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
drivers/pci/Makefile | 2 +- drivers/pci/pci_auto_common.c | 128 ++++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci_auto_old.c | 122 ---------------------------------------- 3 files changed, 129 insertions(+), 123 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..faf904e --- /dev/null +++ b/drivers/pci/pci_auto_common.c @@ -0,0 +1,128 @@ +/*
- PCI autoconfiguration library
nits: auto configuration
- 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..54c5e1d 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, @@ -89,7 +40,6 @@ void pciauto_setup_device(struct pci_controller *hose, struct pci_region *bar_res; int found_mem64 = 0; #endif
u16 class; pci_hose_read_config_word(hose, dev, PCI_COMMAND, &cmdstat); cmdstat = (cmdstat & ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) | PCI_COMMAND_MASTER;@@ -207,11 +157,6 @@ void pciauto_setup_device(struct pci_controller *hose, } #endif
/* PCI_COMMAND_IO must be set for VGA device */pci_hose_read_config_word(hose, dev, PCI_CLASS_DEVICE, &class);if (class == PCI_CLASS_DISPLAY_VGA)cmdstat |= PCI_COMMAND_IO;
I guess this is a merge/rebase mistake? This should not be removed.
pci_hose_write_config_word(hose, dev, PCI_COMMAND, cmdstat); pci_hose_write_config_byte(hose, dev, PCI_CACHE_LINE_SIZE, CONFIG_SYS_PCI_CACHE_LINE_SIZE);@@ -385,73 +330,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
--
Regards, Bin
Before converting this to driver model, reorder the code to avoid forward function declarations.
Signed-off-by: Simon Glass sjg@chromium.org ---
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 debcd1c..53b0f42 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -21,115 +21,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; @@ -283,10 +174,10 @@ void pci_header_show(pci_dev_t dev) { u8 _byte, header_type;
+ pci_read_config_byte(dev, PCI_CLASS_CODE, &_byte); pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type); pci_show_regs(dev, regs_start);
- pci_read_config_byte(dev, PCI_CLASS_CODE, &_byte); printf(" class code = 0x%.2x (%s)\n", _byte, pci_class_str(_byte)); pci_show_regs(dev, regs_rest); @@ -308,6 +199,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)
Hi Simon,
On Fri, Nov 13, 2015 at 5:45 AM, 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
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 debcd1c..53b0f42 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -21,115 +21,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; @@ -283,10 +174,10 @@ void pci_header_show(pci_dev_t dev) { u8 _byte, header_type;
pci_read_config_byte(dev, PCI_CLASS_CODE, &_byte); pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type); pci_show_regs(dev, regs_start);
pci_read_config_byte(dev, PCI_CLASS_CODE, &_byte); printf(" class code = 0x%.2x (%s)\n", _byte, pci_class_str(_byte)); pci_show_regs(dev, regs_rest);@@ -308,6 +199,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
Can we use @dev, @return here?
- */
+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.
It should be 'bus_num'. 'short_pci_listing' is missing here. Also please use @bus_num, @return, etc.
- 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;
Please rename the above 4 variables to avoid CamelCase.
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) --
Regards, Bin
Hi Bin,
On 13 November 2015 at 00:11, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Nov 13, 2015 at 5:45 AM, 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
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 debcd1c..53b0f42 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -21,115 +21,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; @@ -283,10 +174,10 @@ void pci_header_show(pci_dev_t dev) { u8 _byte, header_type;
pci_read_config_byte(dev, PCI_CLASS_CODE, &_byte); pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type); pci_show_regs(dev, regs_start);
pci_read_config_byte(dev, PCI_CLASS_CODE, &_byte); printf(" class code = 0x%.2x (%s)\n", _byte, pci_class_str(_byte)); pci_show_regs(dev, regs_rest);@@ -308,6 +199,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
Can we use @dev, @return here?
Will add a new patch.
- */
+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.
It should be 'bus_num'. 'short_pci_listing' is missing here. Also please use @bus_num, @return, etc.
I don't like changing code that I move as it is hard to compare. I'll add a separate patch to clean these up.
- 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;Please rename the above 4 variables to avoid CamelCase.
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) --
Regards, Bin
Regards, Simon
Currently we using 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 ---
common/cmd_pci.c | 49 +++++++++++++++---------------------------------- 1 file changed, 15 insertions(+), 34 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 53b0f42..306e734 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -330,7 +330,8 @@ 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; @@ -344,23 +345,13 @@ static int pci_cfg_display(pci_dev_t bdf, ulong addr, ulong size, ulong length) */ nbytes = length * 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); - } + unsigned long val; + + val = pci_read_config(bdf, addr, size); + printf(" %*lx", pci_field_width(size), val); addr += size; } printf("\n"); @@ -390,32 +381,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(" %*lx", pci_field_width(size), val);
nbytes = cli_readline(" ? "); if (nbytes == 0 || (nbytes == 1 && console_buffer[0] == '-')) { @@ -461,7 +440,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; int busnum = 0; pci_dev_t bdf = 0; char cmd = 's'; @@ -476,7 +456,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 : size - 1; if (argc > 3) addr = simple_strtoul(argv[3], NULL, 16); if (argc > 4)
Hi Simon,
On Fri, Nov 13, 2015 at 5:45 AM, Simon Glass sjg@chromium.org wrote:
Currently we using switch() and access PCI configuration via several
using -> use
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
common/cmd_pci.c | 49 +++++++++++++++---------------------------------- 1 file changed, 15 insertions(+), 34 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 53b0f42..306e734 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -330,7 +330,8 @@ 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; @@ -344,23 +345,13 @@ static int pci_cfg_display(pci_dev_t bdf, ulong addr, ulong size, ulong length) */ nbytes = length * 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);}
unsigned long val;val = pci_read_config(bdf, addr, size);printf(" %*lx", pci_field_width(size), val);
I think this should be "%.*x"
addr += size; } printf("\n");@@ -390,32 +381,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(" %*lx", pci_field_width(size), val);
"%.*x"?
nbytes = cli_readline(" ? "); if (nbytes == 0 || (nbytes == 1 && console_buffer[0] == '-')) {@@ -461,7 +440,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; int busnum = 0; pci_dev_t bdf = 0; char cmd = 's';@@ -476,7 +456,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 : size - 1; if (argc > 3) addr = simple_strtoul(argv[3], NULL, 16); if (argc > 4)--
Regards, Bin
This should return 8, not 32. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/cmd_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 306e734..747d6b9 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -36,7 +36,7 @@ static int pci_field_width(enum pci_size_t size) return 4; case PCI_SIZE_32: default: - return 32; + return 8; } }
Hi Simon,
On Fri, Nov 13, 2015 at 5:45 AM, Simon Glass sjg@chromium.org wrote:
This should return 8, not 32. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org
This patch should be squashed into the patch#4 where this bug was introduced in the first place.
common/cmd_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 306e734..747d6b9 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -36,7 +36,7 @@ static int pci_field_width(enum pci_size_t size) return 4; case PCI_SIZE_32: default:
return 32;
return 8; }}
--
Regards, Bin
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 ---
common/cmd_pci.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 747d6b9..3d09beb 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -442,6 +442,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_dev_t dev; int busnum = 0; pci_dev_t bdf = 0; char cmd = 's'; @@ -482,16 +483,18 @@ 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); } - cmd = 's'; + pciinfo(busnum, value); break; }
+ dev = bdf; + switch (cmd) { 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 @@ -504,20 +507,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); - break; - case 's': - pciinfo(busnum, value); + 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;
Hi Simon,
On Fri, Nov 13, 2015 at 5:45 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
common/cmd_pci.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 747d6b9..3d09beb 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -442,6 +442,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_dev_t dev; int busnum = 0; pci_dev_t bdf = 0; char cmd = 's';@@ -482,16 +483,18 @@ 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); }
cmd = 's';
pciinfo(busnum, value);
This looks wrong to me, as patch#1 in this series did the changes to move "pciinfo" call to the switch..case below.
break; }
dev = bdf;switch (cmd) { 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 @@ -504,20 +507,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);break;case 's':pciinfo(busnum, value);
ret = pci_cfg_modify(dev, addr, size, value, 1);
Ditto.
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;--
Regards, Bin
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 ---
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 3d09beb..6303bed 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -199,6 +199,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"); + } +} + /* * Subroutine: pci_header_show_brief * @@ -250,12 +260,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++) { HeaderType = 0;
On Fri, Nov 13, 2015 at 5:45 AM, 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
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 3d09beb..6303bed 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -199,6 +199,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");}+}
/*
- Subroutine: pci_header_show_brief
@@ -250,12 +260,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++) { HeaderType = 0;--
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Adjust this command to use the correct PCI functions, instead of the compatibility layer.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/cmd_pci.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- include/common.h | 1 - 2 files changed, 121 insertions(+), 6 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 6303bed..4f71e57 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -17,6 +17,7 @@ #include <bootretry.h> #include <cli.h> #include <command.h> +#include <dm.h> #include <asm/processor.h> #include <asm/io.h> #include <pci.h> @@ -40,6 +41,19 @@ static int pci_field_width(enum pci_size_t size) } }
+#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) { @@ -70,6 +84,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 }, @@ -170,15 +185,25 @@ static struct pci_reg_info regs_cardbus[] = { * Return: None * */ +#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 _byte, header_type; + + dm_pci_read_config(dev, PCI_CLASS_CODE, &_byte, PCI_SIZE_8); + dm_pci_read_config(dev, PCI_HEADER_TYPE, &header_type, PCI_SIZE_8); +#else u8 _byte, header_type;
pci_read_config_byte(dev, PCI_CLASS_CODE, &_byte); pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type); +#endif pci_show_regs(dev, regs_start); - - printf(" class code = 0x%.2x (%s)\n", _byte, + printf(" class code = 0x%.2x (%s)\n", (int)_byte, pci_class_str(_byte)); pci_show_regs(dev, regs_rest);
@@ -209,6 +234,48 @@ void pciinfo_header(int busnum, bool short_listing) } }
+#ifdef CONFIG_DM_PCI +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); +} + +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 + /* * Subroutine: pci_header_show_brief * @@ -307,7 +374,7 @@ void pciinfo(int bus_num, int short_pci_listing) error: printf("Cannot read bus configuration: %d\n", ret); } - +#endif
/* Convert the "bus.device.function" identifier into a number. */ @@ -335,8 +402,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; @@ -355,7 +427,11 @@ static int pci_cfg_display(pci_dev_t bdf, ulong addr, enum pci_size_t size, for (i=0; i<linebytes; i+= 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(" %*lx", pci_field_width(size), val); addr += size; } @@ -370,6 +446,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) { @@ -385,9 +462,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; @@ -398,7 +481,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(" %*lx", pci_field_width(size), val);
nbytes = cli_readline(" ? "); @@ -425,7 +512,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; } @@ -447,7 +538,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; +#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'; @@ -488,11 +583,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 break; }
+#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 (cmd) { case 'h': /* header */ @@ -522,7 +634,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 09a131d..df7b03d 100644 --- a/include/common.h +++ b/include/common.h @@ -433,7 +433,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 *);
Hi Simon,
On Fri, Nov 13, 2015 at 5:45 AM, 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
common/cmd_pci.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- include/common.h | 1 - 2 files changed, 121 insertions(+), 6 deletions(-)
diff --git a/common/cmd_pci.c b/common/cmd_pci.c index 6303bed..4f71e57 100644 --- a/common/cmd_pci.c +++ b/common/cmd_pci.c @@ -17,6 +17,7 @@ #include <bootretry.h> #include <cli.h> #include <command.h> +#include <dm.h> #include <asm/processor.h> #include <asm/io.h> #include <pci.h> @@ -40,6 +41,19 @@ static int pci_field_width(enum pci_size_t size) } }
+#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) { @@ -70,6 +84,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 }, @@ -170,15 +185,25 @@ static struct pci_reg_info regs_cardbus[] = {
- Return: None
*/ +#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 _byte, header_type;dm_pci_read_config(dev, PCI_CLASS_CODE, &_byte, PCI_SIZE_8);dm_pci_read_config(dev, PCI_HEADER_TYPE, &header_type, PCI_SIZE_8);+#else u8 _byte, header_type;
pci_read_config_byte(dev, PCI_CLASS_CODE, &_byte); pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);+#endif pci_show_regs(dev, regs_start);
printf(" class code = 0x%.2x (%s)\n", _byte,
printf(" class code = 0x%.2x (%s)\n", (int)_byte, pci_class_str(_byte)); pci_show_regs(dev, regs_rest);@@ -209,6 +234,48 @@ void pciinfo_header(int busnum, bool short_listing) } }
+#ifdef CONFIG_DM_PCI +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);+}
+void pciinfo(struct udevice *bus, bool short_listing)
This function should be static?
+{
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
/*
- Subroutine: pci_header_show_brief
@@ -307,7 +374,7 @@ void pciinfo(int bus_num, int short_pci_listing) error: printf("Cannot read bus configuration: %d\n", ret); }
+#endif
/* Convert the "bus.device.function" identifier into a number. */ @@ -335,8 +402,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; @@ -355,7 +427,11 @@ static int pci_cfg_display(pci_dev_t bdf, ulong addr, enum pci_size_t size, for (i=0; i<linebytes; i+= 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(" %*lx", pci_field_width(size), val); addr += size; } @@ -370,6 +446,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) { @@ -385,9 +462,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; @@ -398,7 +481,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(" %*lx", pci_field_width(size), val);
nbytes = cli_readline(" ? ");@@ -425,7 +512,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; } @@ -447,7 +538,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; +#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'; @@ -488,11 +583,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 break; }
+#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 (cmd) { case 'h': /* header */@@ -522,7 +634,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 09a131d..df7b03d 100644 --- a/include/common.h +++ b/include/common.h @@ -433,7 +433,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 *); --
Regards, Bin
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 ---
arch/arm/mach-tegra/Kconfig | 2 ++ arch/x86/Kconfig | 3 +++ configs/sandbox_defconfig | 10 +++++----- drivers/pci/Kconfig | 9 +++++++++ drivers/pci/Makefile | 3 ++- include/pci.h | 21 +++++++++++++++++---- 6 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index e5215ab..3906fc1 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -13,6 +13,7 @@ config TEGRA_ARMV7_COMMON select DM_SPI select DM_GPIO select DM_PCI + select DM_PCI_COMPAT
choice prompt "Tegra SoC select" @@ -45,6 +46,7 @@ config TEGRA210 select DM_SPI select DM_GPIO select DM_PCI + select DM_PCI_COMPAT
endchoice
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index f92082d..e972973 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 94c8e68..92725d8 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -6,6 +6,7 @@ CONFIG_FIT_SIGNATURE=y # CONFIG_CMD_ELF is not set # CONFIG_CMD_IMLS is not set # CONFIG_CMD_FLASH is not set +CONFIG_CMD_REMOTEPROC=y # CONFIG_CMD_SETEXPR is not set CONFIG_CMD_SOUND=y CONFIG_BOOTSTAGE=y @@ -19,6 +20,8 @@ CONFIG_OF_HOSTFILE=y CONFIG_REGMAP=y CONFIG_SYSCON=y CONFIG_DEVRES=y +CONFIG_ADC=y +CONFIG_ADC_SANDBOX=y CONFIG_CLK=y CONFIG_SANDBOX_GPIO=y CONFIG_SYS_I2C_SANDBOX=y @@ -34,6 +37,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 @@ -43,12 +47,12 @@ CONFIG_DM_PMIC_SANDBOX=y CONFIG_DM_REGULATOR=y CONFIG_DM_REGULATOR_SANDBOX=y CONFIG_RAM=y +CONFIG_REMOTEPROC_SANDBOX=y CONFIG_DM_RTC=y CONFIG_SANDBOX_SERIAL=y CONFIG_SOUND=y CONFIG_SOUND_SANDBOX=y CONFIG_SANDBOX_SPI=y -CONFIG_DM_TPM=y CONFIG_TPM_TIS_SANDBOX=y CONFIG_USB=y CONFIG_DM_USB=y @@ -63,7 +67,3 @@ CONFIG_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y CONFIG_UT_ENV=y -CONFIG_REMOTEPROC_SANDBOX=y -CONFIG_CMD_REMOTEPROC=y -CONFIG_ADC=y -CONFIG_ADC_SANDBOX=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..fc7d494 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,10 @@ static inline int pci_read_config_byte(pci_dev_t pcidev, int offset, return pci_read_config8(pcidev, offset, valuep); }
+int dm_pciauto_config_device(struct udevice *dev); + +#endif /* CONFIG_DM_PCI_COMPAT */ + /** * pci_conv_32_to_size() - convert a 32-bit read value to the given size *
Hi Simon,
On Fri, Nov 13, 2015 at 5:45 AM, 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
arch/arm/mach-tegra/Kconfig | 2 ++ arch/x86/Kconfig | 3 +++ configs/sandbox_defconfig | 10 +++++----- drivers/pci/Kconfig | 9 +++++++++ drivers/pci/Makefile | 3 ++- include/pci.h | 21 +++++++++++++++++---- 6 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index e5215ab..3906fc1 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -13,6 +13,7 @@ config TEGRA_ARMV7_COMMON select DM_SPI select DM_GPIO select DM_PCI
select DM_PCI_COMPATchoice prompt "Tegra SoC select" @@ -45,6 +46,7 @@ config TEGRA210 select DM_SPI select DM_GPIO select DM_PCI
select DM_PCI_COMPATendchoice
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index f92082d..e972973 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 APIconfig BOARD_ROMSIZE_KB_512 bool config BOARD_ROMSIZE_KB_1024 diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 94c8e68..92725d8 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -6,6 +6,7 @@ CONFIG_FIT_SIGNATURE=y # CONFIG_CMD_ELF is not set # CONFIG_CMD_IMLS is not set # CONFIG_CMD_FLASH is not set +CONFIG_CMD_REMOTEPROC=y # CONFIG_CMD_SETEXPR is not set CONFIG_CMD_SOUND=y CONFIG_BOOTSTAGE=y @@ -19,6 +20,8 @@ CONFIG_OF_HOSTFILE=y CONFIG_REGMAP=y CONFIG_SYSCON=y CONFIG_DEVRES=y +CONFIG_ADC=y +CONFIG_ADC_SANDBOX=y CONFIG_CLK=y CONFIG_SANDBOX_GPIO=y CONFIG_SYS_I2C_SANDBOX=y @@ -34,6 +37,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 @@ -43,12 +47,12 @@ CONFIG_DM_PMIC_SANDBOX=y CONFIG_DM_REGULATOR=y CONFIG_DM_REGULATOR_SANDBOX=y CONFIG_RAM=y +CONFIG_REMOTEPROC_SANDBOX=y CONFIG_DM_RTC=y CONFIG_SANDBOX_SERIAL=y CONFIG_SOUND=y CONFIG_SOUND_SANDBOX=y CONFIG_SANDBOX_SPI=y -CONFIG_DM_TPM=y CONFIG_TPM_TIS_SANDBOX=y CONFIG_USB=y CONFIG_DM_USB=y @@ -63,7 +67,3 @@ CONFIG_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y CONFIG_UT_ENV=y -CONFIG_REMOTEPROC_SANDBOX=y -CONFIG_CMD_REMOTEPROC=y -CONFIG_ADC=y -CONFIG_ADC_SANDBOX=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_PCIhelpEnable compatibility functions for PCI so that old code can be usedwith CONFIG_DM_PCI enabled. This should be used as an interimmeasure when porting a board to use driver model for PCI. Once theboard 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..fc7d494 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,10 @@ static inline int pci_read_config_byte(pci_dev_t pcidev, int offset, return pci_read_config8(pcidev, offset, valuep); }
+int dm_pciauto_config_device(struct udevice *dev);
Why is this declaration put inside #ifdef CONFIG_DM_PCI_COMPAT #endif?
+#endif /* CONFIG_DM_PCI_COMPAT */
/**
- pci_conv_32_to_size() - convert a 32-bit read value to the given size
--
Regards, Bin
participants (2)
-
Bin Meng -
Simon Glass