Random libata comments...

Random libata comments...

[Jeff Garzik]

* I agree with others that using the "ata_drive_probe_reset" can lead to 
confusion on the uses of the word "drive".  Replacing that with "do" or 
something else would be nice.

* As the ata_drive_probe_reset argument list continues to grow, I lean 
more and more towards moving all those function pointers to struct 
ata_port_operations.  One of the problems with the drivers/ide layer IMO 
is that the list of all hooks used is not immediately clear upon first 
read, whereas with libata it is clear -- with the notable exception of 
ata_drive_probe_reset().

Re: Random libata comments...

[Tejun Heo]

Jeff Garzik wrote:
> 
> * I agree with others that using the "ata_drive_probe_reset" can lead to 
> confusion on the uses of the word "drive".  Replacing that with "do" or 
> something else would be nice.

Will do.

> * As the ata_drive_probe_reset argument list continues to grow, I lean 
> more and more towards moving all those function pointers to struct 
> ata_port_operations.  One of the problems with the drivers/ide layer IMO 
> is that the list of all hooks used is not immediately clear upon first 
> read, whereas with libata it is clear -- with the notable exception of 
> ata_drive_probe_reset().

->error_handler() takes over all of ata_drive_probe_reset() after 
hotplug patchset and all ->probe_reset() related stuff are killed.  The 
same applies to ->error_handler() though.  I agree with you that the 
arguments are ugly, but also there are already too many non-essential 
operations in ata_port_operations.  I was hoping something can be done 
to resolve both issues.

I'm okay with moving reset ops into ata_port_operations but we need to 
do more than that, IMHO.

-- 
tejun

Re: Random libata comments...

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>>
>> * I agree with others that using the "ata_drive_probe_reset" can lead 
>> to confusion on the uses of the word "drive".  Replacing that with 
>> "do" or something else would be nice.
> 
> Will do.
> 
>> * As the ata_drive_probe_reset argument list continues to grow, I lean 
>> more and more towards moving all those function pointers to struct 
>> ata_port_operations.  One of the problems with the drivers/ide layer 
>> IMO is that the list of all hooks used is not immediately clear upon 
>> first read, whereas with libata it is clear -- with the notable 
>> exception of ata_drive_probe_reset().
> 
> ->error_handler() takes over all of ata_drive_probe_reset() after 
> hotplug patchset and all ->probe_reset() related stuff are killed.  The 
> same applies to ->error_handler() though.  I agree with you that the 
> arguments are ugly, but also there are already too many non-essential 
> operations in ata_port_operations.  I was hoping something can be done 
> to resolve both issues.
> 
> I'm okay with moving reset ops into ata_port_operations but we need to 
> do more than that, IMHO.

For one example of that, BMDMA-specific operations need to be moved out 
of ata_port_operations, to a BMDMA-driver-specific API.  Ultimately 
libata should be a high level ->qc_issue/ata_qc_complete() style API 
exclusively.

	Jeff