All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls
@ 2015-11-12 21:45 Simon Glass
  2015-11-12 21:45 ` [U-Boot] [PATCH 01/14] pci: Move 'pci scan' code in with other commands Simon Glass
                   ` (13 more replies)
  0 siblings, 14 replies; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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%)

-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 01/14] pci: Move 'pci scan' code in with other commands
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  4:26   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 02/14] pci: Use a common return in command processing Simon Glass
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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;
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 02/14] pci: Use a common return in command processing
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
  2015-11-12 21:45 ` [U-Boot] [PATCH 01/14] pci: Move 'pci scan' code in with other commands Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  4:26   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 03/14] pci: Use a separate variable for the bus number Simon Glass
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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;
 }
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 03/14] pci: Use a separate variable for the bus number
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
  2015-11-12 21:45 ` [U-Boot] [PATCH 01/14] pci: Move 'pci scan' code in with other commands Simon Glass
  2015-11-12 21:45 ` [U-Boot] [PATCH 02/14] pci: Use a common return in command processing Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  4:26   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 04/14] pci: Refactor the pciinfo() function Simon Glass
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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)
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 04/14] pci: Refactor the pciinfo() function
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (2 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 03/14] pci: Use a separate variable for the bus number Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  4:26   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 05/14] dm: pci: Add a comment about how to find struct pci_controller Simon Glass
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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.
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 05/14] dm: pci: Add a comment about how to find struct pci_controller
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (3 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 04/14] pci: Refactor the pciinfo() function Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  5:29   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 06/14] dm: pci: Rename pci_auto.c to pci_auto_old.c Simon Glass
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 06/14] dm: pci: Rename pci_auto.c to pci_auto_old.c
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (4 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 05/14] dm: pci: Add a comment about how to find struct pci_controller Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  5:29   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 07/14] dm: pci: Move common auto-config functions to a common file Simon Glass
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 07/14] dm: pci: Move common auto-config functions to a common file
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (5 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 06/14] dm: pci: Rename pci_auto.c to pci_auto_old.c Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  5:29   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 08/14] dm: pci: Reorder functions in cmd_pci.c Simon Glass
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 08/14] dm: pci: Reorder functions in cmd_pci.c
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (6 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 07/14] dm: pci: Move common auto-config functions to a common file Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  7:11   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 09/14] pci: Use common functions to read/write config Simon Glass
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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)
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 09/14] pci: Use common functions to read/write config
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (7 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 08/14] dm: pci: Reorder functions in cmd_pci.c Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  7:11   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 10/14] pci: Fix pci_field_width() for 32-bit values Simon Glass
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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)
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 10/14] pci: Fix pci_field_width() for 32-bit values
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (8 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 09/14] pci: Use common functions to read/write config Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  7:11   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 11/14] pci: Use a separate 'dev' variable for the PCI device Simon Glass
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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;
 	}
 }
 
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 11/14] pci: Use a separate 'dev' variable for the PCI device
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (9 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 10/14] pci: Fix pci_field_width() for 32-bit values Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  7:11   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 12/14] pci: Move PCI header output code into its own function Simon Glass
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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;
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 12/14] pci: Move PCI header output code into its own function
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (10 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 11/14] pci: Use a separate 'dev' variable for the PCI device Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  7:11   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 13/14] dm: pci: Convert 'pci' command to driver model Simon Glass
  2015-11-12 21:45 ` [U-Boot] [PATCH 14/14] dm: pci: Disable PCI compatibility functions by default Simon Glass
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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;
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 13/14] dm: pci: Convert 'pci' command to driver model
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (11 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 12/14] pci: Move PCI header output code into its own function Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  7:11   ` Bin Meng
  2015-11-12 21:45 ` [U-Boot] [PATCH 14/14] dm: pci: Disable PCI compatibility functions by default Simon Glass
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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 *);
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 14/14] dm: pci: Disable PCI compatibility functions by default
  2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
                   ` (12 preceding siblings ...)
  2015-11-12 21:45 ` [U-Boot] [PATCH 13/14] dm: pci: Convert 'pci' command to driver model Simon Glass
@ 2015-11-12 21:45 ` Simon Glass
  2015-11-13  7:11   ` Bin Meng
  13 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2015-11-12 21:45 UTC (permalink / raw)
  To: u-boot

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
  *
-- 
2.6.0.rc2.230.g3dd15c0

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

* [U-Boot] [PATCH 01/14] pci: Move 'pci scan' code in with other commands
  2015-11-12 21:45 ` [U-Boot] [PATCH 01/14] pci: Move 'pci scan' code in with other commands Simon Glass
@ 2015-11-13  4:26   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  4:26 UTC (permalink / raw)
  To: u-boot

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>

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

* [U-Boot] [PATCH 02/14] pci: Use a common return in command processing
  2015-11-12 21:45 ` [U-Boot] [PATCH 02/14] pci: Use a common return in command processing Simon Glass
@ 2015-11-13  4:26   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  4:26 UTC (permalink / raw)
  To: u-boot

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;
>   usage:
>         return CMD_RET_USAGE;
>  }
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

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

* [U-Boot] [PATCH 03/14] pci: Use a separate variable for the bus number
  2015-11-12 21:45 ` [U-Boot] [PATCH 03/14] pci: Use a separate variable for the bus number Simon Glass
@ 2015-11-13  4:26   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  4:26 UTC (permalink / raw)
  To: u-boot

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>

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

* [U-Boot] [PATCH 04/14] pci: Refactor the pciinfo() function
  2015-11-12 21:45 ` [U-Boot] [PATCH 04/14] pci: Refactor the pciinfo() function Simon Glass
@ 2015-11-13  4:26   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  4:26 UTC (permalink / raw)
  To: u-boot

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

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

* [U-Boot] [PATCH 05/14] dm: pci: Add a comment about how to find struct pci_controller
  2015-11-12 21:45 ` [U-Boot] [PATCH 05/14] dm: pci: Add a comment about how to find struct pci_controller Simon Glass
@ 2015-11-13  5:29   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  5:29 UTC (permalink / raw)
  To: u-boot

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>

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

* [U-Boot] [PATCH 06/14] dm: pci: Rename pci_auto.c to pci_auto_old.c
  2015-11-12 21:45 ` [U-Boot] [PATCH 06/14] dm: pci: Rename pci_auto.c to pci_auto_old.c Simon Glass
@ 2015-11-13  5:29   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  5:29 UTC (permalink / raw)
  To: u-boot

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>

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

* [U-Boot] [PATCH 07/14] dm: pci: Move common auto-config functions to a common file
  2015-11-12 21:45 ` [U-Boot] [PATCH 07/14] dm: pci: Move common auto-config functions to a common file Simon Glass
@ 2015-11-13  5:29   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  5:29 UTC (permalink / raw)
  To: u-boot

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

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

* [U-Boot] [PATCH 08/14] dm: pci: Reorder functions in cmd_pci.c
  2015-11-12 21:45 ` [U-Boot] [PATCH 08/14] dm: pci: Reorder functions in cmd_pci.c Simon Glass
@ 2015-11-13  7:11   ` Bin Meng
  2015-11-16  1:35     ` Simon Glass
  0 siblings, 1 reply; 30+ messages in thread
From: Bin Meng @ 2015-11-13  7:11 UTC (permalink / raw)
  To: u-boot

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

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

* [U-Boot] [PATCH 09/14] pci: Use common functions to read/write config
  2015-11-12 21:45 ` [U-Boot] [PATCH 09/14] pci: Use common functions to read/write config Simon Glass
@ 2015-11-13  7:11   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  7:11 UTC (permalink / raw)
  To: u-boot

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

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

* [U-Boot] [PATCH 10/14] pci: Fix pci_field_width() for 32-bit values
  2015-11-12 21:45 ` [U-Boot] [PATCH 10/14] pci: Fix pci_field_width() for 32-bit values Simon Glass
@ 2015-11-13  7:11   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  7:11 UTC (permalink / raw)
  To: u-boot

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

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

* [U-Boot] [PATCH 11/14] pci: Use a separate 'dev' variable for the PCI device
  2015-11-12 21:45 ` [U-Boot] [PATCH 11/14] pci: Use a separate 'dev' variable for the PCI device Simon Glass
@ 2015-11-13  7:11   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  7:11 UTC (permalink / raw)
  To: u-boot

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

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

* [U-Boot] [PATCH 12/14] pci: Move PCI header output code into its own function
  2015-11-12 21:45 ` [U-Boot] [PATCH 12/14] pci: Move PCI header output code into its own function Simon Glass
@ 2015-11-13  7:11   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  7:11 UTC (permalink / raw)
  To: u-boot

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>

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

* [U-Boot] [PATCH 13/14] dm: pci: Convert 'pci' command to driver model
  2015-11-12 21:45 ` [U-Boot] [PATCH 13/14] dm: pci: Convert 'pci' command to driver model Simon Glass
@ 2015-11-13  7:11   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  7:11 UTC (permalink / raw)
  To: u-boot

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

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

* [U-Boot] [PATCH 14/14] dm: pci: Disable PCI compatibility functions by default
  2015-11-12 21:45 ` [U-Boot] [PATCH 14/14] dm: pci: Disable PCI compatibility functions by default Simon Glass
@ 2015-11-13  7:11   ` Bin Meng
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Meng @ 2015-11-13  7:11 UTC (permalink / raw)
  To: u-boot

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_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);

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

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

* [U-Boot] [PATCH 08/14] dm: pci: Reorder functions in cmd_pci.c
  2015-11-13  7:11   ` Bin Meng
@ 2015-11-16  1:35     ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2015-11-16  1:35 UTC (permalink / raw)
  To: u-boot

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

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

end of thread, other threads:[~2015-11-16  1:35 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-12 21:45 [U-Boot] [PATCH 00/14] dm: pci: Support native driver model calls Simon Glass
2015-11-12 21:45 ` [U-Boot] [PATCH 01/14] pci: Move 'pci scan' code in with other commands Simon Glass
2015-11-13  4:26   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 02/14] pci: Use a common return in command processing Simon Glass
2015-11-13  4:26   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 03/14] pci: Use a separate variable for the bus number Simon Glass
2015-11-13  4:26   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 04/14] pci: Refactor the pciinfo() function Simon Glass
2015-11-13  4:26   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 05/14] dm: pci: Add a comment about how to find struct pci_controller Simon Glass
2015-11-13  5:29   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 06/14] dm: pci: Rename pci_auto.c to pci_auto_old.c Simon Glass
2015-11-13  5:29   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 07/14] dm: pci: Move common auto-config functions to a common file Simon Glass
2015-11-13  5:29   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 08/14] dm: pci: Reorder functions in cmd_pci.c Simon Glass
2015-11-13  7:11   ` Bin Meng
2015-11-16  1:35     ` Simon Glass
2015-11-12 21:45 ` [U-Boot] [PATCH 09/14] pci: Use common functions to read/write config Simon Glass
2015-11-13  7:11   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 10/14] pci: Fix pci_field_width() for 32-bit values Simon Glass
2015-11-13  7:11   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 11/14] pci: Use a separate 'dev' variable for the PCI device Simon Glass
2015-11-13  7:11   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 12/14] pci: Move PCI header output code into its own function Simon Glass
2015-11-13  7:11   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 13/14] dm: pci: Convert 'pci' command to driver model Simon Glass
2015-11-13  7:11   ` Bin Meng
2015-11-12 21:45 ` [U-Boot] [PATCH 14/14] dm: pci: Disable PCI compatibility functions by default Simon Glass
2015-11-13  7:11   ` Bin Meng

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