Re: [ACPI] [RFC/PATCH 2/3] ACPI based I/O APIC hot-plug

From: Bjorn Helgaas <bjorn.helgaas_at_hp.com>
Date: 2005-04-22 03:21:53
> +acpi_status __devinit
>  acpi_map_iosapic (acpi_handle handle, u32 depth, void *context, void **ret)

I think "acpi_map_iosapic" is poorly named.  It's really associating
an iosapic with a locality domain.

And there's nothing ia64-specific in acpi_map_iosapic().  It'd be
nice to figure out a way to move it into generic ACPI code.

But your patch didn't introduce either of these problems, so
I don't think you have to fix them now.

>  	unsigned short 	num_rte;	/* number of RTE in this IOSAPIC */
> +	int		count;		/* # of RTEs in use on this IOSAPIC */

"count" isn't very descriptive.  Maybe "rtes_inuse" or similar? 

> -void __init
> +static inline int iosapic_alloc (void)

Nitpick: should be

	static inline int
	iosapic_alloc (void)

to match the style of the rest of the file.

> +static inline void free_iosapic (int index)

Nitpick: follow style again.

> +	memset(&iosapic_lists[index], 0, sizeof(struct iosapic));

What about

	memset(&iosapic_lists[index], 0, sizeof(iosapic_lists[0]));

so you can tell this is correct without looking up the declaration of
iosapic_lists[]?

> +static inline int iosapic_check (unsigned int gsi_base, unsigned int ver)

Nitpick: follow style again.  And maybe a more descriptive name?


-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Received on Thu Apr 21 13:23:19 2005

This archive was generated by hypermail 2.1.8 : 2005-08-02 09:20:37 EST