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