On Tue, May 08, 2007 at 03:19:22AM +0200, Krzysztof Halasa wrote:
> diff --git a/arch/arm/mach-ixp4xx/ixdp425-setup.c b/arch/arm/mach-ixp4xx/ixdp425-setup.c

quoted text
> index ec4f079..f20d39d 100644

> --- a/arch/arm/mach-ixp4xx/ixdp425-setup.c

> +++ b/arch/arm/mach-ixp4xx/ixdp425-setup.c

> @@ -101,10 +101,35 @@ static struct platform_device ixdp425_uart = {

>  	.resource		= ixdp425_uart_resources

>  };

>

> +/* Built-in 10/100 Ethernet MAC interfaces */

> +static struct mac_plat_info ixdp425_plat_mac[] = {

> +	{

> +		.phy		= 0,

> +		.rxq		= 3,

> +	}, {

> +		.phy		= 1,

> +		.rxq		= 4,

> +	}

> +};

As with Christian's driver (I'm feeling like a bit of a broken record

here :-), putting knowledge of which queue to use (which is firmware-

specific) in the _board_ support file is almost certainly wrong.
I would just put the port number in there, and let the ethernet

driver map the port number to the hardware queue number.  After all,

the ethernet driver knows which queues the firmware uses, while the

board support code doesn't.
> +#ifndef __ARMEB__

quoted text
> +#warning Little endian mode not supported

> +#endif

Yay. :-)  /me hides
> +static inline void set_regbits(u32 bits, u32 __iomem *reg)

quoted text
> +{

> +	__raw_writel(__raw_readl(reg) | bits, reg);

> +}

> +static inline void clr_regbits(u32 bits, u32 __iomem *reg)

> +{

> +	__raw_writel(__raw_readl(reg) & ~bits, reg);

> +}

I generally discourage the use of such wrappers, as it often makes

people forget that the set and clear operations are not atomic, and

it ignores the fact that some of the other bits in the register you

are modifying might have side-effects.
Didn't review the rest -- not sure whether I'm looking at the latest

version.

-

unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

the body of a message to majordomo@vger.kernel.org

More majordomo info at  http://vger.kernel.org/majordomo-info.html

Please read the FAQ at  http://www.tux.org/lkml/