Re: [PATCH V2 3/3] MIPS: lantiq: add dma/etop board support

[Sergei Shtylyov <sshtylyov@mvista.com>]

Hello.

On 06-05-2011 2:10, John Crispin wrote:

> This patch adds functions to register the etop and dma platform devices and
> calls them from the board specific setup code.

> Signed-off-by: John Crispin<blogic@openwrt.org>
> Signed-off-by: Ralph Hempel<ralph.hempel@lantiq.com>
> Cc: linux-mips@linux-mips.org
[...]

> diff --git a/arch/mips/lantiq/devices.c b/arch/mips/lantiq/devices.c
> index e758863..a10244e 100644
> --- a/arch/mips/lantiq/devices.c
> +++ b/arch/mips/lantiq/devices.c
> @@ -31,6 +31,19 @@
>   #define IRQ_RES(resname, irq) \
>   	{.name = #resname, .start = (irq), .flags = IORESOURCE_IRQ}
>
> +/* dma engine */
> +static struct resource ltq_dma_resource = {
> +	.name	= "dma",
> +	.start	= LTQ_DMA_BASE_ADDR,
> +	.end	= LTQ_DMA_BASE_ADDR + LTQ_DMA_SIZE - 1,
> +	.flags  = IORESOURCE_MEM,

    Spaces before = instead of tab here.

> diff --git a/arch/mips/lantiq/xway/devices.c b/arch/mips/lantiq/xway/devices.c
> index 7d58ae5..99ff2ef 100644
> --- a/arch/mips/lantiq/xway/devices.c
> +++ b/arch/mips/lantiq/xway/devices.c
> @@ -75,3 +75,30 @@ void __init ltq_register_gpio_stp(void)
>   {
>   	platform_device_register_simple("ltq_stp", 0,&ltq_stp_resource, 1);
>   }
> +
> +/* ethernet */
> +static struct resource ltq_ethernet_resources = {
> +	.name   = "etop",
> +	.start  = LTQ_ETOP_BASE_ADDR,
> +	.end    = LTQ_ETOP_BASE_ADDR + LTQ_ETOP_SIZE - 1,
> +	.flags  = IORESOURCE_MEM,

    It's better to use tabs, not spaces between the filed names and =, like 
above...

> +};
> +
> +static struct platform_device ltq_ethernet = {
> +	.name = "ltq_etop",
> +	.resource =&ltq_ethernet_resources,
> +	.num_resources  = 1,

    Too many spaces?

> +};
> +
> +void __init
> +ltq_register_ethernet(struct ltq_eth_data *eth)
> +{
> +	if (!eth)
> +		return;
> +	if (!is_valid_ether_addr(eth->mac.sa_data)) {
> +		pr_warn("etop: invalid MAC, using random\n");
> +		random_ether_addr(eth->mac.sa_data);
> +	}

    Why not do it in the driver?

> +	ltq_ethernet.dev.platform_data = eth;
> +	platform_device_register(&ltq_ethernet);
> +}
[...]
> diff --git a/arch/mips/lantiq/xway/mach-easy50712.c b/arch/mips/lantiq/xway/mach-easy50712.c
> index 5242a27..a001761 100644
> --- a/arch/mips/lantiq/xway/mach-easy50712.c
> +++ b/arch/mips/lantiq/xway/mach-easy50712.c
> @@ -59,13 +59,19 @@ static struct ltq_pci_data ltq_pci_data = {
>   	},
>   };
>
> +static struct ltq_eth_data ltq_eth_data = {
> +	.mii_mode = PHY_INTERFACE_MODE_MII,
> +};
> +
>   static void __init easy50712_init(void)
>   {
> +	ltq_register_dma();
>   	ltq_register_gpio();
>   	ltq_register_gpio_stp();
>   	ltq_register_nor(&easy50712_flash_data);
>   	ltq_register_wdt();
>   	ltq_register_pci(&ltq_pci_data);
> +	ltq_register_ethernet(&ltq_eth_data);
>   }
>
>   MIPS_MACHINE(LTQ_MACH_EASY50712,

    I'd put the board code in a sperate patch...

WBR, Sergei