Re: EP93xx PIO IDE driver proposal

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

Hello, I wrote:

>>>>> I'm not sure what you're getting at with "such advices"... :)

>>>>> We need to cast somewhere anyway so we may as well cast from 'void 
>>>>> *' to
>>>>> 'unsigned int' where needed and not the other way around (or rather 
>>>>> from
>>>>> 'unsigned int' to 'struct ide_timing *') -- which would be an ugly 
>>>>> hack
>>>>> and could cause maintainability/portability problems later.

>>>>> BTW it seems like a good occasion to add ide_{get,set}_drivedata() 
>>>>> helpers
>>>>> (ala existing ide_{get,set}_hwifdata() ones) to <linux/ide.h> and 
>>>>> thus make
>>>>> internal IDE API more coherent.

>>>>> [ Yes, I know that we may get away with s/unsigned int/unsigned long/
>>>>>  and casting it to 'struct ide_timing *' for now but it always better
>>>>>  to at least consider more elegant solution first... ]

>>>    Changing a lot of drivers is more elegant than not? Doubt it.

>>>    And don't tell me this patch is "elegant"... :-/

>> Well, it is makes code more coherent and more maintainable...

>> Moreover those inline helpers serve as a part of documentation (though
>> real DocBook documentation would also be nice) for host drivers authors.

>> Either this or we should remove ide_{get,set}_hwifdata() and allow
>> host drivers to poke directly also at hwif->hwif_data...

    Besides, some drivers like siimage.c do poke hwif->hwif_data directly.

>    Seems like a good idea. I'm not sure why these fields should be special.

    While there are a number of 'ide_hwif_t' fields like 'select_data' or 
'config_data' that are being poked directly...

MBR, Sergei