Re: EP93xx PIO IDE driver proposal

["João Ramos" <joao.ramos@inov.pt>]

>
> Yes! :)
>
> There is still a room for improvement though -- it would be better to fix
> IDE core to set PIO0 before probing devices for all host controllers.
>
> Moreover it seems that doing it this way would allow us to remove ->init_hwif
> method from this driver and do all necessary setup in ep93xx_ide_probe()
> (this controller is a single port one so theoretically there shouldn't be
> a need for having per-port ->init_hwif implementation).
>   

So after all this discussion ;-) , my driver will have no 'init_hwif' 
method, and the setup code will be on 'ep93xx_ide_probe', which will 
configure entirely the IDE host controller.
Moreover, this initial configuration will setup the controller to work 
at PIO Mode 0. Later on, the 'set_pio_mode' method will be called and 
the controller will configure itself according to the PIO mode reported 
by the IDE core.

Can I proceed this way?

>   
>> There's just only one issue; normally, I would setup the specific 
>> timings (t0, t1, t2, t2i, etc) in the 'pio_set_mode' hook. However, if 
>> you look further in the driver, those timings aren't defined through a 
>> memory controller but instead manually enforced by 'ndelay' calls (arghhh).
>> This means that in my low-level procedures for reading and writing, I 
>> need to have access to the timings (or the struct ide_timing) 
>> corresponding to the PIO mode selected, in order to use the correct delays.
>>
>> My question is: which is the best way to accomplish this? Declaring a 
>> global struct ide_timing variable pointer that always holds the correct 
>> ide_timing struct to the selected PIO mode? Or should I always check (in 
>> some manner) what is the current PIO mode and then select the adequate 
>> delays?
>>     
>
> I think that the setting variable pointer in ->set_pio_mode method would
> work best.  Seems like the existing drive_data field of ide_drive_t is well
> suited for this purpose (however it may be worth to convert it to 'void *'
> type while we are it).
>   

Did you mean 'drive_data' field, or 'driver_data' field?
'drive_data' field is an unsigned int value; I guess you meant 
'driver_data' field as it is a (void *) field, so I can define it as a 
pointer to the correct 'struct ide_timing'.

Regards,
João


-- 
************************************************************************

    João Ramos        <joao.ramos@inov.pt>

    INOV INESC Inovação - ESTG Leiria
    Escola Superior de Tecnologia e Gestão de Leiria
    Edíficio C1, Campus 2
    Morro do Lena, Alto do Vieiro
    Leiria
    2411-901 Leiria
    Portugal

    Tel: +351244843424
    Fax: +351244843424

************************************************************************