[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.