Re: EP93xx PIO IDE driver proposal

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

On Thursday 07 May 2009 17:33:20 João Ramos wrote:
> Alan Cox escreveu:
> >> So you're saying I should support all PIO modes? If so, I would have to 
> >> make conditional code, checking perhaps a module param to sort which PIO 
> >> mode to use.
> >>     
> >
> > If you advertise PIO0-PIO4 as supported the core IDE code will do all the
> > work on figuring which modes are supported by the attached devices. You
> > just need to be able to set them.
> >
> > Alan
> >   
> 
> Ok, so I've been studying that (I was quite lost for a time, I confess, 
> I'm not that much familiar with the IDE subsystem, so please bear with 
> me ;-) ).
> 
> So I need to set up a hook for 'set_pio_mode()', so that when the IDE 
> subsystem detects a device and figures the most suitable PIO mode for 
> the device, it will call the 'set_pio_mode' routine provided by the 
> driver in order to configure the host controller for that PIO mode.
> 
> This also means that my host controller driver should always default to 
> PIO Mode 0, as the initial host controller setup that is carried out by 
> the 'init_hwif' routine, allowing devices to be detected. Afterwards, 
> the IDE subsystem detects the most suitable PIO mode and calls 
> 'set_pio_mode' to change that configuration.
> 
> Am I correct on this?

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).

> 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).

Thanks,
Bart