public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] IXP425: Fixing PCI access Part 1/1
@ 2008-12-04 21:09 Stefan Althoefer
  2008-12-06 22:41 ` Stefan Althoefer
  2008-12-15 23:24 ` Wolfgang Denk
  0 siblings, 2 replies; 5+ messages in thread
From: Stefan Althoefer @ 2008-12-04 21:09 UTC (permalink / raw)
  To: u-boot

[PATCH] IXP425: Fixing PCI access

This patch fixes the PCI handling routines of the IXP port.
It seems that this hasn't been touch for quite a while and
u-boot PCI handling has changed since then (but nobody
update IXP). Not even access to configuration space
did work.

It was tested with Janz emPC-A400.


The patch is against "latest" u-boot git-repository

Please (still) be patient if style of submission or patches are
offending.

Signed-off-by: Stefan Althoefer <stefan.althoefer@web.de>
----

diff -uprN u-boot-orig//cpu/ixp/pci.c u-boot/cpu/ixp/pci.c
--- u-boot-orig//cpu/ixp/pci.c	2008-12-02 17:25:31.000000000 +0100
+++ u-boot/cpu/ixp/pci.c	2008-12-02 22:00:59.000000000 +0100
@@ -33,16 +33,15 @@
 #include <asm/arch/ixp425.h>
 #include <asm/arch/ixp425pci.h>
 
-static void non_prefetch_read (unsigned int addr, unsigned int cmd,
+static int non_prefetch_read (unsigned int addr, unsigned int cmd,
 			       unsigned int *data);
-static void non_prefetch_write (unsigned int addr, unsigned int cmd,
+static int non_prefetch_write (unsigned int addr, unsigned int cmd,
 				unsigned int data);
 static void configure_pins (void);
 static void sys_pci_gpio_clock_config (void);
-static void pci_bus_scan (void);
-static int pci_device_exists (unsigned int deviceNo);
-static void sys_pci_bar_info_get (unsigned int devnum, unsigned int bus,
-				  unsigned int dev, unsigned int func);
+void pci_bus_scan (void);
+static int pci_device_exists (pci_dev_t dev);
+static void sys_pci_bar_info_get (unsigned int devnum, pci_dev_t dev);
 static void sys_pci_device_bars_write (void);
 static void calc_bars (PciBar * Bars[], unsigned int nBars,
 		       unsigned int startAddr);
@@ -68,6 +67,23 @@ PciBar *memBars[IXP425_PCI_MAX_BAR];
 PciBar *ioBars[IXP425_PCI_MAX_BAR];
 PciDevice devices[IXP425_PCI_MAX_FUNC_ON_BUS];
 
+extern int pciTranslateIrq(pci_dev_t dev, int intPin);
+
+static u32 ixp4xx_config_addr(u8 bus_num, pci_dev_t devfn, int where)
+{
+	u32 addr;
+	if (!bus_num) {
+		/* type 0 */
+		addr = BIT(32-PCI_DEV(devfn)) | ((PCI_FUNC(devfn)) << 8) |
+		    (where & ~3);
+	} else {
+		/* type 1 */
+		addr = (bus_num << 16) | ((PCI_DEV(devfn)) << 11) |
+		        ((PCI_FUNC(devfn)) << 8) | (where & ~3) | 1;
+	}
+	return addr;
+}
+
 int pci_read_config_dword (pci_dev_t dev, int where, unsigned int *val)
 {
 	unsigned int retval;
@@ -75,8 +91,11 @@ int pci_read_config_dword (pci_dev_t dev
 
 	/*address bits 31:28 specify the device 10:8 specify the function */
 	/*Set the address to be read */
-	addr = BIT ((31 - dev)) | (where & ~3);
-	non_prefetch_read (addr, NP_CMD_CONFIGREAD, &retval);
+	//addr = BIT ((31 - dev)) | (where & ~3);
+	addr = ixp4xx_config_addr(PCI_BUS(dev),dev, where & ~3);
+	if( non_prefetch_read (addr, NP_CMD_CONFIGREAD, &retval) != OK ){
+		return ERROR;
+	}
 
 	*val = retval;
 
@@ -99,8 +118,11 @@ int pci_read_config_word (pci_dev_t dev,
 	byteEnables = byteEnables << PCI_NP_CBE_BESL;
 	/*address bits 31:28 specify the device 10:8 specify the function */
 	/*Set the address to be read */
-	addr = BIT ((31 - dev)) | (where & ~3);
-	non_prefetch_read (addr, byteEnables | NP_CMD_CONFIGREAD, &retval);
+	//addr = BIT ((31 - dev)) | (where & ~3);
+	addr = ixp4xx_config_addr(PCI_BUS(dev),dev, where & ~3);
+	if( non_prefetch_read (addr, byteEnables | NP_CMD_CONFIGREAD, &retval) != OK ){
+		return ERROR;
+	}
 
 	/*Pick out the word we are interested in */
 	*val = (retval >> (8 * n));
@@ -123,8 +145,11 @@ int pci_read_config_byte (pci_dev_t dev,
 
 	/*address bits 31:28 specify the device, 10:8 specify the function */
 	/*Set the address to be read */
-	addr = BIT ((31 - dev)) | (where & ~3);
-	non_prefetch_read (addr, byteEnables | NP_CMD_CONFIGREAD, &retval);
+	//addr = BIT ((31 - dev)) | (where & ~3);
+	addr = ixp4xx_config_addr(PCI_BUS(dev),dev, where & ~3);
+	if( non_prefetch_read (addr, byteEnables | NP_CMD_CONFIGREAD, &retval) != OK ){
+		return ERROR;
+	}
 	/*Pick out the byte we are interested in */
 	*val = (retval >> (8 * n));
 
@@ -146,8 +171,11 @@ int pci_write_config_byte (pci_dev_t dev
 	ldata = val << (8 * n);
 	/*address bits 31:28 specify the device 10:8 specify the function */
 	/*Set the address to be written */
-	addr = BIT ((31 - dev)) | (where & ~3);
-	non_prefetch_write (addr, byteEnables | NP_CMD_CONFIGWRITE, ldata);
+	//addr = BIT ((31 - dev)) | (where & ~3);
+	addr = ixp4xx_config_addr(PCI_BUS(dev),dev, where & ~3);
+	if( non_prefetch_write (addr, byteEnables | NP_CMD_CONFIGWRITE, ldata) != OK ){
+		return ERROR;
+	}
 
 	return (OK);
 }
@@ -169,8 +197,11 @@ int pci_write_config_word (pci_dev_t dev
 	ldata = val << (8 * n);
 	/*address bits 31:28 specify the device 10:8 specify the function */
 	/*Set the address to be written */
-	addr = BIT (31 - dev) | (where & ~3);
-	non_prefetch_write (addr, byteEnables | NP_CMD_CONFIGWRITE, ldata);
+	//addr = BIT (31 - dev) | (where & ~3);
+	addr = ixp4xx_config_addr(PCI_BUS(dev),dev, where & ~3);
+	if( non_prefetch_write (addr, byteEnables | NP_CMD_CONFIGWRITE, ldata) != OK ){
+		return ERROR;
+	}
 
 	return (OK);
 }
@@ -181,29 +212,41 @@ int pci_write_config_dword (pci_dev_t de
 
 	/*address bits 31:28 specify the device 10:8 specify the function */
 	/*Set the address to be written */
-	addr = BIT (31 - dev) | (where & ~3);
-	non_prefetch_write (addr, NP_CMD_CONFIGWRITE, val);
+	//addr = BIT (31 - dev) | (where & ~3);
+	addr = ixp4xx_config_addr(PCI_BUS(dev),dev, where & ~3);
+	if( non_prefetch_write (addr, NP_CMD_CONFIGWRITE, val) != OK ){
+		return ERROR;
+	}
 
 	return (OK);
 }
 
-void non_prefetch_read (unsigned int addr,
+int non_prefetch_read (unsigned int addr,
 			unsigned int cmd, unsigned int *data)
 {
-	REG_WRITE (PCI_CSR_BASE, PCI_NP_AD_OFFSET, addr);
+	unsigned int   isr;
 
+	REG_WRITE (PCI_CSR_BASE, PCI_NP_AD_OFFSET, addr);
 	/*set up and execute the read */
 	REG_WRITE (PCI_CSR_BASE, PCI_NP_CBE_OFFSET, cmd);
-
 	/*The result of the read is now in np_rdata */
 	REG_READ (PCI_CSR_BASE, PCI_NP_RDATA_OFFSET, *data);
 
-	return;
+	/* Check for abort */
+	REG_READ (PCI_CSR_BASE, PCI_ISR_OFFSET, isr);
+	if( isr & PCI_ISR_PFE ){
+		/* clear the bit */
+		REG_WRITE (PCI_CSR_BASE, PCI_ISR_OFFSET, PCI_ISR_PFE);
+		return ERROR;
+	}
+
+	return OK;
 }
 
-void non_prefetch_write (unsigned int addr,
+int non_prefetch_write (unsigned int addr,
 			 unsigned int cmd, unsigned int data)
 {
+	unsigned int   isr;
 
 	REG_WRITE (PCI_CSR_BASE, PCI_NP_AD_OFFSET, addr);
 	/*set up the write */
@@ -211,7 +254,15 @@ void non_prefetch_write (unsigned int ad
 	/*Execute the write by writing to NP_WDATA */
 	REG_WRITE (PCI_CSR_BASE, PCI_NP_WDATA_OFFSET, data);
 
-	return;
+	/* Check for abort */
+	REG_READ (PCI_CSR_BASE, PCI_ISR_OFFSET, isr);
+	if( isr & PCI_ISR_PFE ){
+		/* clear the bit */
+		REG_WRITE (PCI_CSR_BASE, PCI_ISR_OFFSET, PCI_ISR_PFE);
+		return ERROR;
+	}
+
+	return OK;
 }
 
 /*
@@ -259,7 +310,7 @@ void pci_ixp_init (struct pci_controller
 
 /*
  ==========================================================
-		Init IXP PCI
+ 		Init IXP PCI
  ==========================================================
 */
 	REG_READ (PCI_CSR_BASE, PCI_CSR_OFFSET, regval);
@@ -304,9 +355,7 @@ void pci_ixp_init (struct pci_controller
 	pci_write_config_word (0, PCI_CFG_COMMAND, INITIAL_PCI_CMD);
 	REG_WRITE (PCI_CSR_BASE, PCI_ISR_OFFSET, PCI_ISR_PSE
 		   | PCI_ISR_PFE | PCI_ISR_PPE | PCI_ISR_AHBE);
-#ifdef CONFIG_PCI_SCAN_SHOW
-	printf ("Device  bus  dev  func  deviceID  vendorID \n");
-#endif
+
 	pci_bus_scan ();
 }
 
@@ -349,15 +398,17 @@ void sys_pci_gpio_clock_config (void)
 
 void pci_bus_scan (void)
 {
-	unsigned int bus = 0, dev, func = 0;
+        int       busno, devno, funcno;
+	pci_dev_t dev;
 	unsigned short data16;
 	unsigned int data32;
 	unsigned char intPin;
 
+	unsigned int vendorId;
+	unsigned char HeaderType;
+
 	/* Assign first device to ourselves */
-	devices[0].bus = 0;
-	devices[0].device = 0;
-	devices[0].func = 0;
+	devices[0].device = PCI_BDF(0,0,0);
 
 	crp_read (PCI_CFG_VENDOR_ID, &data32);
 
@@ -371,22 +422,30 @@ void pci_bus_scan (void)
 	nMBars = 0;
 	nIOBars = 0;
 
-	for (dev = 0; dev < IXP425_PCI_MAX_DEV; dev++) {
-
-		/*Check whether a device is present */
-		if (pci_device_exists (dev) != TRUE) {
+	for(busno=0; busno<2; busno++){
+	  for(devno=0; devno<PCI_MAX_PCI_DEVICES; devno++){
+	    for(funcno=0; funcno<PCI_MAX_PCI_FUNCTIONS; funcno++) {
+		dev = PCI_BDF(busno,devno,funcno);
+
+		if( pci_read_config_dword (dev, PCI_CFG_VENDOR_ID, &vendorId) == ERROR ){
+		    funcno=PCI_MAX_PCI_FUNCTIONS;
+		    continue;
+		}
 
-			/*Clear error bits in ISR, write 1 to clear */
-			REG_WRITE (PCI_CSR_BASE, PCI_ISR_OFFSET, PCI_ISR_PSE
-				   | PCI_ISR_PFE | PCI_ISR_PPE |
-				   PCI_ISR_AHBE);
-			continue;
+		if ( vendorId == 0x0 ) {
+		    funcno=PCI_MAX_PCI_FUNCTIONS;
+		    continue;
+		}
+		
+		if( funcno == 0 ){
+		    pci_read_config_byte(dev, PCI_HEADER_TYPE, &HeaderType);
+		    if ( !(HeaderType & 0x80) ){
+			funcno=PCI_MAX_PCI_FUNCTIONS;
+		    }
 		}
 
 		/*A device is present, add an entry to the array */
-		devices[nDevices].bus = bus;
 		devices[nDevices].device = dev;
-		devices[nDevices].func = func;
 
 		pci_read_config_word (dev, PCI_CFG_VENDOR_ID, &data16);
 
@@ -399,24 +458,22 @@ void pci_bus_scan (void)
 		devices[nDevices].error = FALSE;
 
 		/*Figure out what BARs are on this device */
-		sys_pci_bar_info_get (nDevices, bus, dev, func);
+		sys_pci_bar_info_get (nDevices, dev);
 		/*Figure out what INTX# line the card uses */
-		pci_read_config_byte (dev, PCI_CFG_DEV_INT_PIN, &intPin);
+	pci_read_config_byte (dev, PCI_CFG_DEV_INT_PIN, &intPin);
 
 		/*assign the appropriate irq line */
 		if (intPin > PCI_IRQ_LINES) {
 			devices[nDevices].error = TRUE;
 		} else if (intPin != 0) {
 			/*This device uses an interrupt line */
-			/*devices[nDevices].irq = ixp425PciIntTranslate[dev][intPin-1]; */
-			devices[nDevices].irq = intPin;
+			//devices[nDevices].irq = ixp425PciIntTranslate[devno][intPin-1];
+			devices[nDevices].irq = pciTranslateIrq(dev,intPin);
+			//devices[nDevices].irq = intPin;
 		}
-#ifdef CONFIG_PCI_SCAN_SHOW
-		printf ("%06d    %03d %03d %04d  %08d      %08x\n", nDevices,
-			devices[nDevices].vendor_id);
-#endif
 		nDevices++;
-
+	    }
+	  }
 	}
 
 	calc_bars (memBars, nMBars, IXP425_PCI_BAR_MEM_BASE);
@@ -426,44 +483,43 @@ void pci_bus_scan (void)
 		   | PCI_ISR_PFE | PCI_ISR_PPE | PCI_ISR_AHBE);
 }
 
-void sys_pci_bar_info_get (unsigned int devnum,
-			   unsigned int bus,
-			   unsigned int dev, unsigned int func)
+void sys_pci_bar_info_get (unsigned int devnum, pci_dev_t dev)
 {
 	unsigned int data32;
 	unsigned int tmp;
 	unsigned int size;
+	int          bar;
 
-	pci_write_config_dword (devnum,
-				PCI_CFG_BASE_ADDRESS_0, IXP425_PCI_BAR_QUERY);
-	pci_read_config_dword (devnum, PCI_CFG_BASE_ADDRESS_0, &data32);
-
-	devices[devnum].bar[0].address = (data32 & 1);
-
-	if (data32 & 1) {
+	for(bar=0; bar<=5; bar++){
+	    pci_write_config_dword (dev,
+				    PCI_CFG_BASE_ADDRESS_0+(4*bar), IXP425_PCI_BAR_QUERY);
+	    pci_read_config_dword (dev, PCI_CFG_BASE_ADDRESS_0+(4*bar), &data32);
+	    
+	    devices[devnum].bar[dev].address = (data32 & 1);
+	    
+	    if (data32 & 1) {
 		/* IO space */
 		tmp = data32 & ~0x3;
 		size = ~(tmp - 1);
-		devices[devnum].bar[0].size = size;
-
+		devices[devnum].bar[bar].size = size;
+		
 		if (nIOBars < IXP425_PCI_MAX_BAR) {
-			ioBars[nIOBars++] = &devices[devnum].bar[0];
+		    ioBars[nIOBars++] = &devices[devnum].bar[bar];
 		}
-	} else {
+	    } else {
 		/* Mem space */
 		tmp = data32 & ~IXP425_PCI_BOTTOM_NIBBLE_OF_LONG_MASK;
 		size = ~(tmp - 1);
-		devices[devnum].bar[0].size = size;
-
+		devices[devnum].bar[bar].size = size;
+		
 		if (nMBars < IXP425_PCI_MAX_BAR) {
-			memBars[nMBars++] = &devices[devnum].bar[0];
+			memBars[nMBars++] = &devices[devnum].bar[bar];
 		} else {
 			devices[devnum].error = TRUE;
 		}
-
+		
+	    }
 	}
-
-	devices[devnum].bar[1].size = 0;
 }
 
 void sortBars (PciBar * Bars[], unsigned int nBars)
@@ -497,28 +553,33 @@ void calc_bars (PciBar * Bars[], unsigne
 	}
 
 	for (i = 0; i < nBars; i++) {
+	    if( Bars[i]->size > 0 ){
+		if( startAddr & ((Bars[i]->size)-1) ){
+		    startAddr |= ((Bars[i]->size)-1);
+		    startAddr += 1;
+		}
 		Bars[i]->address |= startAddr;
 		startAddr += Bars[i]->size;
+	    }
 	}
 }
 
 void sys_pci_device_bars_write (void)
 {
-	unsigned int i;
-	int addr;
+	unsigned int i, bar;
 
 	for (i = 1; i < nDevices; i++) {
 		if (devices[i].error) {
 			continue;
 		}
 
-		pci_write_config_dword (devices[i].device,
-					PCI_CFG_BASE_ADDRESS_0,
-					devices[i].bar[0].address);
-		addr = BIT (31 - devices[i].device) |
-			(0 << PCI_NP_AD_FUNCSL) |
-			(PCI_CFG_BASE_ADDRESS_0 & ~3);
-		pci_write_config_dword (devices[i].device,
+		for(bar=0; bar<5; bar++){
+		    pci_write_config_dword (devices[i].device,
+					    PCI_CFG_BASE_ADDRESS_0+(4*bar),
+					    devices[i].bar[bar].address);
+		}
+
+		pci_write_config_byte (devices[i].device,
 					PCI_CFG_DEV_INT_LINE, devices[i].irq);
 
 		pci_write_config_word (devices[i].device,
@@ -528,24 +589,18 @@ void sys_pci_device_bars_write (void)
 }
 
 
-int pci_device_exists (unsigned int deviceNo)
+int pci_device_exists (pci_dev_t dev)
 {
 	unsigned int vendorId;
-	unsigned int regval;
 
-	pci_read_config_dword (deviceNo, PCI_CFG_VENDOR_ID, &vendorId);
+	if( pci_read_config_dword (dev, PCI_CFG_VENDOR_ID, &vendorId) == ERROR ){
+	    return FALSE;
+	}
 
-	/* There are two ways to find out an empty device.
-	 *   1. check Master Abort bit after the access.
-	 *   2. check whether the vendor id read back is 0x0.
-	 */
-	REG_READ (PCI_CSR_BASE, PCI_ISR_OFFSET, regval);
-	if ((vendorId != 0x0) && ((regval & PCI_ISR_PFE) == 0)) {
+	if ( vendorId != 0x0 ) {
 		return TRUE;
 	}
-	/*no device present, make sure that the master abort bit is reset */
 
-	REG_WRITE (PCI_CSR_BASE, PCI_ISR_OFFSET, PCI_ISR_PFE);
 	return FALSE;
 }
 
diff -uprN u-boot-orig//board/ixdp425/ixdp425.c u-boot/board/ixdp425/ixdp425.c
--- u-boot-orig//board/ixdp425/ixdp425.c	2008-12-02 17:25:31.000000000 +0100
+++ u-boot/board/ixdp425/ixdp425.c	2008-12-03 14:13:36.000000000 +0100
@@ -118,9 +118,18 @@ void pci_init_board(void)
 
 	pci_ixp_init(&hose);
 }
+
+#include <pci.h>
+int pciTranslateIrq(pci_dev_t dev, int intPin)
+{
+	/* FIXME: This needs to be implemented by someone
+	   who onws an IXDP425 board */
+	return 0;
+}
 #endif
 
 int board_eth_init(bd_t *bis)
 {
 	return pci_eth_init(bis);
 }
+
diff -uprN u-boot-orig//drivers/pci/pci_indirect.c u-boot/drivers/pci/pci_indirect.c
--- u-boot-orig//drivers/pci/pci_indirect.c	2008-12-02 17:25:31.000000000 +0100
+++ u-boot/drivers/pci/pci_indirect.c	2008-12-02 23:03:09.000000000 +0100
@@ -11,7 +11,7 @@
 
 #include <common.h>
 
-#if (!defined(__I386__) && !defined(CONFIG_IXDP425))
+#if (!defined(__I386__) && !defined(CONFIG_IXP425))
 
 #include <asm/processor.h>
 #include <asm/io.h>
@@ -133,4 +133,4 @@ void pci_setup_indirect(struct pci_contr
 	hose->cfg_data = (unsigned char *) cfg_data;
 }
 
-#endif	/* !__I386__ && !CONFIG_IXDP425 */
+#endif	/* !__I386__ && !CONFIG_IXP425 */
Dateien u-boot-orig//.git/index und u-boot/.git/index sind verschieden.
diff -uprN u-boot-orig//include/asm-arm/arch-ixp/ixp425pci.h u-boot/include/asm-arm/arch-ixp/ixp425pci.h
--- u-boot-orig//include/asm-arm/arch-ixp/ixp425pci.h	2008-12-02 17:25:32.000000000 +0100
+++ u-boot/include/asm-arm/arch-ixp/ixp425pci.h	2008-12-02 22:04:57.000000000 +0100
@@ -52,9 +52,7 @@ typedef struct
 
 typedef struct
 {
-	unsigned int bus;
-	unsigned int device;
-	unsigned int func;
+	pci_dev_t    device;
 	unsigned int irq;
 	BOOL error;
 	unsigned short vendor_id;
@@ -74,13 +72,13 @@ typedef struct
 
 #define IXP425_PCI_BAR_QUERY			0xffffffff
 
-#define IXP425_PCI_BAR_MEM_BASE 0x100000
-#define IXP425_PCI_BAR_IO_BASE	0x000000
+#define IXP425_PCI_BAR_MEM_BASE 0x48000000
+#define IXP425_PCI_BAR_IO_BASE	0x00000000    /* not supported */
 
 /*define the maximum number of bus segments - we support a single segment*/
-#define IXP425_PCI_MAX_BUS  1
+#define IXP425_PCI_MAX_BUS  4
 /*define the maximum number of cards per bus segment*/
-#define IXP425_PCI_MAX_DEV  4
+#define IXP425_PCI_MAX_DEV  8
 /*define the maximum number of functions per device*/
 #define IXP425_PCI_MAX_FUNC 8
 /* define the maximum number of separate functions that we can
@@ -153,7 +151,7 @@ typedef struct
 /*define the default setting of the AHB memory base reg*/
 #define IXP425_PCI_AHBMEMBASE_DEFAULT 0x00010203
 #define IXP425_PCI_AHBIOBASE_DEFAULT  0x0
-#define IXP425_PCI_PCIMEMBASE_DEFAULT 0x0
+#define IXP425_PCI_PCIMEMBASE_DEFAULT 0x48494a4b
 
 /*define the default settings for the controller's BARs*/
 #ifdef IXP425_PCI_SIMPLE_MAPPING

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

* [U-Boot] [PATCH] IXP425: Fixing PCI access Part 1/1
  2008-12-04 21:09 [U-Boot] [PATCH] IXP425: Fixing PCI access Part 1/1 Stefan Althoefer
@ 2008-12-06 22:41 ` Stefan Althoefer
  2008-12-15 23:25   ` Wolfgang Denk
  2008-12-15 23:24 ` Wolfgang Denk
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Althoefer @ 2008-12-06 22:41 UTC (permalink / raw)
  To: u-boot

I posted new version of patch to fix some more errors and style, so
this is obsolete.

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

* [U-Boot] [PATCH] IXP425: Fixing PCI access Part 1/1
  2008-12-04 21:09 [U-Boot] [PATCH] IXP425: Fixing PCI access Part 1/1 Stefan Althoefer
  2008-12-06 22:41 ` Stefan Althoefer
@ 2008-12-15 23:24 ` Wolfgang Denk
  2008-12-17 23:52   ` Stefan Althoefer
  1 sibling, 1 reply; 5+ messages in thread
From: Wolfgang Denk @ 2008-12-15 23:24 UTC (permalink / raw)
  To: u-boot

Dear Stefan Althoefer,

In message <4938471d.iPiarO7PylTL2jNa%stefan.althoefer@web.de> you wrote:
> [PATCH] IXP425: Fixing PCI access
> 
> This patch fixes the PCI handling routines of the IXP port.
> It seems that this hasn't been touch for quite a while and
> u-boot PCI handling has changed since then (but nobody
> update IXP). Not even access to configuration space
> did work.
> 
> It was tested with Janz emPC-A400.

Coding style, and comments below "---", please.

> +static u32 ixp4xx_config_addr(u8 bus_num, pci_dev_t devfn, int where)
> +{
> +	u32 addr;

Empty line after declarations, please.

> +	if (!bus_num) {
> +		/* type 0 */
> +		addr = BIT(32-PCI_DEV(devfn)) | ((PCI_FUNC(devfn)) << 8) |
> +		    (where & ~3);

Indentation by TAB, please.

...
> +	addr = ixp4xx_config_addr(PCI_BUS(dev),dev, where & ~3);
> +	if( non_prefetch_read (addr, NP_CMD_CONFIGREAD, &retval) != OK ){

Space between '0' and '{', please.

> +	//addr = BIT ((31 - dev)) | (where & ~3);

No C++ comments, please.

And don't add dead code.

> +	if( isr & PCI_ISR_PFE ){

Write this as:

	if (isr & PCI_ISR_PFE) {

> @@ -304,9 +355,7 @@ void pci_ixp_init (struct pci_controller
>  	pci_write_config_word (0, PCI_CFG_COMMAND, INITIAL_PCI_CMD);
>  	REG_WRITE (PCI_CSR_BASE, PCI_ISR_OFFSET, PCI_ISR_PSE
>  		   | PCI_ISR_PFE | PCI_ISR_PPE | PCI_ISR_AHBE);
> -#ifdef CONFIG_PCI_SCAN_SHOW
> -	printf ("Device  bus  dev  func  deviceID  vendorID \n");
> -#endif
> +
>  	pci_bus_scan ();

Why do you remove the message?

> +	for(busno=0; busno<2; busno++){
> +	  for(devno=0; devno<PCI_MAX_PCI_DEVICES; devno++){
> +	    for(funcno=0; funcno<PCI_MAX_PCI_FUNCTIONS; funcno++) {
> +		dev = PCI_BDF(busno,devno,funcno);
> +
> +		if( pci_read_config_dword (dev, PCI_CFG_VENDOR_ID, &vendorId) == ERROR ){
> +		    funcno=PCI_MAX_PCI_FUNCTIONS;
> +		    continue;
> +		}

Indentation by TAB, please.

> -			/*devices[nDevices].irq = ixp425PciIntTranslate[dev][intPin-1]; */
> -			devices[nDevices].irq = intPin;
> +			//devices[nDevices].irq = ixp425PciIntTranslate[devno][intPin-1];
> +			devices[nDevices].irq = pciTranslateIrq(dev,intPin);
> +			//devices[nDevices].irq = intPin;

No C++ comments, no dead code.

> -#ifdef CONFIG_PCI_SCAN_SHOW
> -		printf ("%06d    %03d %03d %04d  %08d      %08x\n", nDevices,
> -			devices[nDevices].vendor_id);
> -#endif

???


and so on and on...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Success in marriage is not so much finding the right person as it  is
being the right person.

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

* [U-Boot] [PATCH] IXP425: Fixing PCI access Part 1/1
  2008-12-06 22:41 ` Stefan Althoefer
@ 2008-12-15 23:25   ` Wolfgang Denk
  0 siblings, 0 replies; 5+ messages in thread
From: Wolfgang Denk @ 2008-12-15 23:25 UTC (permalink / raw)
  To: u-boot

Dear Stefan Althoefer,

In message <ghev21$fjd$1@ger.gmane.org> you wrote:
> I posted new version of patch to fix some more errors and style, so
> this is obsolete.

It would be nice if you keept the threading in place, so one can
actually see the new patch as part of the old thread.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Spock, did you see the looks on their faces?"
"Yes, Captain, a sort of vacant contentment."

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

* [U-Boot] [PATCH] IXP425: Fixing PCI access Part 1/1
  2008-12-15 23:24 ` Wolfgang Denk
@ 2008-12-17 23:52   ` Stefan Althoefer
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Althoefer @ 2008-12-17 23:52 UTC (permalink / raw)
  To: u-boot

Hi Wolfgang Denk,

> 
>> @@ -304,9 +355,7 @@ void pci_ixp_init (struct pci_controller
>>  	pci_write_config_word (0, PCI_CFG_COMMAND, INITIAL_PCI_CMD);
>>  	REG_WRITE (PCI_CSR_BASE, PCI_ISR_OFFSET, PCI_ISR_PSE
>>  		   | PCI_ISR_PFE | PCI_ISR_PPE | PCI_ISR_AHBE);
>> -#ifdef CONFIG_PCI_SCAN_SHOW
>> -	printf ("Device  bus  dev  func  deviceID  vendorID \n");
>> -#endif
>> +
>>  	pci_bus_scan ();
> 
> Why do you remove the message?
> 
>> -#ifdef CONFIG_PCI_SCAN_SHOW
>> -		printf ("%06d    %03d %03d %04d  %08d      %08x\n", nDevices,
>> -			devices[nDevices].vendor_id);
>> -#endif
> 
> ???
> 

I found the following initialization sequence for the IXP425:
     cpu_init() -> pci_init() -> pci_init_board() ->
	        pci_ixp_init() -> pci_bus_scan()
and later:
     serial_init()

If printf()->...->serial_putc() is called in pci_bus_scan()
before serial_init() it hung up the IXP425.

Assuming that there is good reason to call pci_init() so early,
CONFIG_PCI_SCAN_SHOW cannot be used.

Therefore I removed the dead code.

I think I noticed this in the revised version of the patch.
I'm working on a beautified version.

-- Stefan

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

end of thread, other threads:[~2008-12-17 23:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-04 21:09 [U-Boot] [PATCH] IXP425: Fixing PCI access Part 1/1 Stefan Althoefer
2008-12-06 22:41 ` Stefan Althoefer
2008-12-15 23:25   ` Wolfgang Denk
2008-12-15 23:24 ` Wolfgang Denk
2008-12-17 23:52   ` Stefan Althoefer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox