Re: [PATCH] 2.6.14: NSLU2 machine support

[Russell King]

On Fri, Oct 28, 2005 at 12:50:22AM -0700, John Bowler wrote:
> +static irqreturn_t nslu2_power_handler(int irq, void *dev_id, struct pt_regs *regs)
> +{
> +	/* Signal init to do the ctrlaltdel action */
> +	kill_proc(1, SIGINT, 1);

There is a function in the kernel for doing this.  It's called
ctrl_alt_del().

> +static struct resource nslu2_uart_resources[] = {
> +	{
> +		.start		= IXP4XX_UART1_BASE_PHYS,
> +		.end		= IXP4XX_UART1_BASE_PHYS + 0x0fff,
> +		.flags		= IORESOURCE_MEM
> +	},
> +	{
> +		.start		= IXP4XX_UART2_BASE_PHYS,
> +		.end		= IXP4XX_UART2_BASE_PHYS + 0x0fff,
> +		.flags		= IORESOURCE_MEM
> +	}

Initialisers should generally end with a ',' irrespective of whether
you have a following entry or not.  It makes maintainence and/or adding
new entries easier.

> +static struct platform_device *nslu2_devices[] __initdata = {
> +	&nslu2_i2c_controller,
> +	&nslu2_flash,
> +	&nslu2_uart

Ditto.

> +MACHINE_START(NSLU2, "Linksys NSLU2")
> +	/* Maintainer: www.nslu2-linux.org */
> +	.phys_ram	= PHYS_OFFSET,
> +	.phys_io	= IXP4XX_PERIPHERAL_BASE_PHYS,
> +	.io_pg_offst	= ((IXP4XX_PERIPHERAL_BASE_VIRT) >> 18) & 0xFFFC,
> +	.boot_params	= 0x00000100,
> +	.map_io		= ixp4xx_map_io,
> +	.init_irq	= ixp4xx_init_irq, /* FIXME: all irq are off here */

What's that FIXME about?

> +        .timer          = &ixp4xx_timer,
> +	.init_machine	= nslu2_init,

Please consistently format the code.

PS, please also send ARM related stuff via the patch system.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/