Re: [PATCH 2/3] ide: add at91_ide driver

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

Hello, I wrote:

>>>> diff --git a/arch/arm/mach-at91/include/mach/board.h 
>>>> b/arch/arm/mach-at91/include/mach/board.h
>>>> index fb51f0e..6674b9b 100644
>>>> --- a/arch/arm/mach-at91/include/mach/board.h
>>>> +++ b/arch/arm/mach-at91/include/mach/board.h
>>>> @@ -59,6 +59,18 @@ struct at91_cf_data {
>>>> };
>>>> extern void __init at91_add_device_cf(struct at91_cf_data *data);
>>>>
>>>> + /* Compact Flash True IDE mode */
>>>> +struct at91_ide_data {
>>>> +    u8    irq_pin;        /* the same meaning as for CF */

>>>  I again have to express my dislike about not passing IRQ the usual 
>>> way. Also, see my comments to the platform code.

>> Yes, I know, I don't like to argue. Only reasoning to use platform irq 
>> resource
>> seams to be: "because other drivers do". However we have exception - 
>> at91_cf
>> also use board->irq_pin, so maybe this driver could also do ?

>    Then why have the memory resource when we can calculate it from the 
> chip select? (I'm not asking you to do that, since the platfrom device 
> resources are user-visible thru /proc/iomem -- even if the driver is not 
> enabled.)

    Oh, I forgot that it's ARM with its #ifdef hell. :-D
Then they're only visible when the driver is enabled.

MBR, Sergei