[parisc-linux] System bus walk patch

[parisc-linux] System bus walk patch

[Ryan Bradetich]

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

Hello parisc-hackers,

This is the first of a series of patches to impliment an I/O tree for
parsic-linux.  I am
throwing this patch out to the list for discussion, and feedback before
committing it
to CVS because if this patch does not work, it will probably prevent
your machine from
booting. :)

The goal of this first patch is to simply add a system bus walk to the
end of  the firmware
device discovery to add additional devices not reported by the
firmware.  This patch does
not attempt to start organizing the pa_devices into a tree strucuture,
nor walk any other
native bus besides the system bus.  These features will be included in
additional patches,
after this one has been tested and committed to CVS.

I have tested this patch fairly extensively on the C200, but have not
tried it on different
machines. I would be interested in any feedback, concerns, questions
anyone has about
this patch.

Thanks,

- Ryan

[-- Attachment #2: iotree.patch --]
[-- Type: text/plain, Size: 4824 bytes --]

--- arch/parisc/kernel/inventory.orig	Fri Feb  2 12:46:00 2001
+++ arch/parisc/kernel/inventory.c	Fri Feb  2 12:37:36 2001
@@ -171,59 +171,88 @@
 }
 #endif /* __LP64__ */
 
+#ifdef __LP64__
+#define FPA 0xFFFFFFFFFFF80000 /* Fixed Physical Address - Location of the Central Bus */
+#else /* !__LP64__ */
+#define FPA 0xFFF80000         /* Fixed Physical Address - Location of the Central Bus */
+#endif /* __LP64__ */
+
+/* The fixed portion is contained in hpa[14..19] for 32 bit and hpa[46..51] for 64 bit.
+** The maximum number of native devices is 2^6 (64) and the offset between devices is
+** 2^12 (0x1000).
+** - Ryan
+*/
+#define MAX_NATIVE_DEVICES 64
+#define NATIVE_DEVICE_OFFSET 0x1000
 
-
-static int do_newer_workstation_inventory(void)
+static int do_native_bus_walk(unsigned long hpa)
 {
-    long status, mod_index, addr_index;
-    struct hp_device *hp_device;
-    int num;
+	int num = 0;
+	struct hp_device *hp_device;
+	unsigned long hpa_end = hpa + (MAX_NATIVE_DEVICES * NATIVE_DEVICE_OFFSET);
+
+	for(; hpa < hpa_end; hpa += NATIVE_DEVICE_OFFSET) {
+		hp_device = alloc_pa_dev(hpa);
+		if (!hp_device)
+			continue;
+		
+		register_pa_dev(hp_device);
+		++num;
+	}
+	return num;
+}
 
-    /* So the idea here is to simply try one SYSTEM_MAP call.  If 
-       that one works, great, otherwise do it another way */
 
-    status = pdc_system_map_find_mods(&module_result, &module_path, 0);
-    if (status != PDC_RET_OK) 
-	return 0;
-    
-    /* This is for newer non-PDC-PAT boxes */
-    printk("a newer box...\n");
-    num = 0;
-    for (mod_index = 0, status = PDC_RET_OK; 
-	 status != PDC_RET_NE_PROC && status != PDC_RET_NE_MOD;
-	 mod_index++) 
-    {
-        status = pdc_system_map_find_mods(&module_result,
-				  &module_path,  mod_index);
+static int do_newer_workstation_inventory(void)
+{
+	long status, mod_index, addr_index;
+	struct hp_device *hp_device;
+	int num;
+	
+	/* So the idea here is to simply try one SYSTEM_MAP call.  If 
+	   that one works, great, otherwise do it another way */
+	
+	status = pdc_system_map_find_mods(&module_result, &module_path, 0);
 	if (status != PDC_RET_OK) 
-	    continue;
-	    
-	hp_device = alloc_pa_dev((unsigned long) module_result.mod_addr);
-	if (!hp_device)
-	    continue;
-	    
-	(void) register_pa_dev(hp_device);
-	num++;
-
-	/* if available, get the additional addresses for a module */
-	if (!module_result.add_addrs) 
-	    continue;
+		return 0;
 	
-	for (addr_index = 1; addr_index <= module_result.add_addrs; addr_index++) {
-	    status = pdc_system_map_find_addrs(
-			&addr_result, mod_index, addr_index);
-			
-	    if (status == PDC_RET_OK)
-		add_pa_dev_addr(hp_device, (unsigned long)addr_result.mod_addr);
-	    else {
-		printk("Bad PDC_FIND_ADDRESS status return (%ld) for index %ld\n",
-		    status, addr_index);
-	        status = PDC_RET_OK; /* reset status for outer loop */
-	    }
-	}
-    } /* end of main loop */
-
-    return (num > 0);
+	/* This is for newer non-PDC-PAT boxes */
+	printk("a newer box...\n");
+	num = 0;
+	for (mod_index = 0, status = PDC_RET_OK; 
+	     status != PDC_RET_NE_PROC && status != PDC_RET_NE_MOD;
+	     mod_index++) {
+		status = pdc_system_map_find_mods(&module_result,
+						  &module_path,  mod_index);
+		if (status != PDC_RET_OK) 
+			continue;
+		
+		hp_device = alloc_pa_dev((unsigned long) module_result.mod_addr);
+		if (!hp_device)
+			continue;
+		
+		(void) register_pa_dev(hp_device);
+		num++;
+		
+		/* if available, get the additional addresses for a module */
+		if (!module_result.add_addrs) 
+			continue;
+		
+		for (addr_index = 1; addr_index <= module_result.add_addrs; addr_index++) {
+			status = pdc_system_map_find_addrs(&addr_result, mod_index, addr_index);
+			if (status == PDC_RET_OK) {
+				add_pa_dev_addr(hp_device, (unsigned long)addr_result.mod_addr);
+			} else {
+				printk("Bad PDC_FIND_ADDRESS status return (%ld) for index %ld\n",
+				       status, addr_index);
+				status = PDC_RET_OK; /* reset status for outer loop */
+			}
+		}
+	} /* end of main loop */
+	
+	/* Walk the system bus */
+	num += do_native_bus_walk(FPA);
+	return (num > 0);
 }
 
 
--- arch/parisc/kernel/drivers.orig	Fri Feb  2 12:46:10 2001
+++ arch/parisc/kernel/drivers.c	Fri Feb  2 12:01:28 2001
@@ -103,15 +103,20 @@
 
 struct hp_device *alloc_pa_dev(unsigned long hpa)
 {
-
+	int i, status;
 	struct hp_device * d;
-	int status;
 
 	d = &pa_devices[num_devices];
 	status = pdc_iodc_read(&pdc_result, (void *)hpa, 0, &iodc_data, 32);
 	if (status != PDC_RET_OK) {
 		/* There is no device here, so we'll skip it */
 		return NULL;
+	}
+
+	/* Check to make sure this device has not already been added -Ryan */
+	for(i = 0; i < num_devices; ++i) {
+		if((unsigned long)pa_devices[i].hpa == hpa)
+			return NULL;
 	}
 
 	d->hw_type = iodc_data[3]&0x1f;

Re: [parisc-linux] System bus walk patch

[Matthew Wilcox]

On Fri, Feb 02, 2001 at 01:05:40PM -0700, Ryan Bradetich wrote:
> I would be interested in any feedback, concerns, questions
> anyone has about this patch.

> --- arch/parisc/kernel/inventory.orig	Fri Feb  2 12:46:00 2001
> +++ arch/parisc/kernel/inventory.c	Fri Feb  2 12:37:36 2001
> @@ -171,59 +171,88 @@
>  }
>  #endif /* __LP64__ */
>  
> +#ifdef __LP64__
> +#define FPA 0xFFFFFFFFFFF80000 /* Fixed Physical Address - Location of the Central Bus */
> +#else /* !__LP64__ */
> +#define FPA 0xFFF80000         /* Fixed Physical Address - Location of the Central Bus */
> +#endif /* __LP64__ */

I'm sure there's a way to do this without #ifdef's... something like

#define FPA (unsigned long)-80000

but perhaps that feels obfuscated -- after all we're after an address here.

#define FPA ((unsigned long)(signed int)0xfff80000)

produces the right result (just tested it).  or perhaps we should have
a macro to give us IO space addresses.

most of what i'm about to comment on is just a reindentation to
CodingStyle of preexisting code, yes?

> +		(void) register_pa_dev(hp_device);

unnecessary cast to void.

> +		for (addr_index = 1; addr_index <= module_result.add_addrs; addr_index++) {
> +			status = pdc_system_map_find_addrs(&addr_result, mod_index, addr_index);
> +			if (status == PDC_RET_OK) {
> +				add_pa_dev_addr(hp_device, (unsigned long)addr_result.mod_addr);
> +			} else {
> +				printk("Bad PDC_FIND_ADDRESS status return (%ld) for index %ld\n",
> +				       status, addr_index);
> +				status = PDC_RET_OK; /* reset status for outer loop */
> +			}
> +		}

that's just ugly.  don't reuse the `status' variable.

		for (addr_index = 1; addr_index <= module_result.add_addrs; addr_index++) {
			int mod_status = pdc_system_map_find_addrs(&addr_result, mod_index, addr_index);
			if (mod_status) == PDC_RET_OK) {
				add_pa_dev_addr(hp_device, (unsigned long)addr_result.mod_addr);
			} else {
				printk("Bad PDC_FIND_ADDRESS status return (%ld) for index %ld\n",
					mod_status, addr_index);
			}
		}

-- 
Revolutions do not require corporate support.

Re: [parisc-linux] System bus walk patch

[Helge Deller]

> > +#ifdef __LP64__
> > +#define FPA 0xFFFFFFFFFFF80000 /* Fixed Physical Address - Location of
> > the Central Bus */ +#else /* !__LP64__ */
> > +#define FPA 0xFFF80000         /* Fixed Physical Address - Location of
> > the Central Bus */ +#endif /* __LP64__ */
>
> I'm sure there's a way to do this without #ifdef's... something like
>
> #define FPA (unsigned long)-80000
>
> but perhaps that feels obfuscated -- after all we're after an address here.
>
> #define FPA ((unsigned long)(signed int)0xfff80000)

or just as I did it in led.c
#define KITTYHAWK_LCD_CMD  (0xfffffffff0190000UL) /* 64bit-ready */ 
which is automatically casted by the compiler ?

Helge

Re: [parisc-linux] System bus walk patch

[Ryan Bradetich]

After revewing the feedback I received from this patch, I
took the liberty to cleanup arch/parisc/kernel/inventory.c
to meet the CodingStyle requirments.  I also committed my
changes to include the System Bus Walking code into cvs
head.  If this commit causes any problems for anyone, let me
know, and I'd be glad to work through the issues with them.

Thanks,

- Ryan


Ryan Bradetich wrote:

> Hello parisc-hackers,
>
> This is the first of a series of patches to impliment an I/O tree for
> parsic-linux.  I am
> throwing this patch out to the list for discussion, and feedback before
> committing it
> to CVS because if this patch does not work, it will probably prevent
> your machine from
> booting. :)
>
> The goal of this first patch is to simply add a system bus walk to the
> end of  the firmware
> device discovery to add additional devices not reported by the
> firmware.  This patch does
> not attempt to start organizing the pa_devices into a tree strucuture,
> nor walk any other
> native bus besides the system bus.  These features will be included in
> additional patches,
> after this one has been tested and committed to CVS.
>
> I have tested this patch fairly extensively on the C200, but have not
> tried it on different
> machines. I would be interested in any feedback, concerns, questions
> anyone has about
> this patch.
>
> Thanks,
>
> - Ryan
>
>

[Patch Removed]