Re: [PATCH 2/2] TXx9: Add TX4939 ATA support

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

Hello.

Atsushi Nemoto wrote:

> Add a helper routine to register tx4939ide driver and use it on
> RBTX4939 board.
>
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
>   
[...]
> diff --git a/arch/mips/txx9/generic/setup_tx4939.c b/arch/mips/txx9/generic/setup_tx4939.c
> index f14a497..ee00bde 100644
> --- a/arch/mips/txx9/generic/setup_tx4939.c
> +++ b/arch/mips/txx9/generic/setup_tx4939.c
>   
[...]
> @@ -389,6 +390,34 @@ void __init tx4939_mtd_init(int ch)
>  	txx9_physmap_flash_init(ch, start, size, &pdata);
>  }
>  
> +void __init tx4939_ata_init(void)
> +{
> +	__u64 pcfg = __raw_readq(&tx4939_ccfgptr->pcfg);
> +	if (pcfg & (TX4939_PCFG_ATA0MODE | TX4939_PCFG_ATA1MODE)) {
> +		struct resource res[2];
> +		int i;
> +		memset(res, 0, sizeof(res));
> +		for (i = 0; i < 2; i++) {
> +			if (i == 0 &&
> +			    !(pcfg & TX4939_PCFG_ATA0MODE))
> +				continue;
> +			if (i == 1 &&
> +			    (pcfg & (TX4939_PCFG_ATA1MODE |
> +				     TX4939_PCFG_ET1MODE |
> +				     TX4939_PCFG_ET0MODE)) !=
> +			    TX4939_PCFG_ATA1MODE)
> +				continue;
> +			res[0].start = TX4939_ATA_REG(i) & 0xfffffffffULL;
> +			res[0].end = res[0].start + 0x1000 - 1;
> +			res[0].flags = IORESOURCE_MEM;
> +			res[1].start = TXX9_IRQ_BASE + TX4939_IR_ATA(i);
> +			res[1].flags = IORESOURCE_IRQ;
> +			platform_device_register_simple("tx4939ide", i,
> +							res, ARRAY_SIZE(res));
>   

   Hm, why not declare both IDE platform devices statically an then 
register them depending on the TX4939_PCFG_ATA[01]MODE bits?
This loop doesn't look nice. You could at least have used an array to 
check TX4939_PCFG_ATA[01]MODE bitmasks but I think it's better to just 
declare devices statically...

MBR, Sergei