Re: [PATCH 1/3] libata: implement dev->acpi_init_gtm

[Jeff Garzik <jeff@garzik.org>]

Tejun Heo wrote:
> Tejun Heo wrote:
>> Jeff Garzik wrote:
>>> Tejun Heo wrote:
>>>> Add dev->acpi_init_gtm and store initial GTM values on host
>>>> initialization.  If the field is valid, ATA_PFLAG_INIT_GTM_VALID flag
>>>> is set.  This is to remember BIOS/firmware programmed initial timing
>>>> for later use before reset and mode configuration modify it.
>>>>
>>>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>>> It sounds like pata_via and pata_amd need a foo_save_initial_config()
>>> much like AHCI, during which they would fill ppriv->init_gtm rather than
>>> ap->init_gtm.  Thoughts?  Both drivers are calling ata_acpi_cbl()
>>> themselves, permitting the possibility of ppriv->init_gtm.
>>>
>>> This avoids forcing everyone else to bear the memory cost in ata_port
>>> for just these two drivers.
>> The memory overhead is pretty small and more importantly there isn't a
>> good place to load initial GTM.  ACPI is associated with the host during
>> host registration after all private host initialization is complete.
>> When the EH gets invoked, the first thing which is done is forcing PIO0
>> before initial reset.
>>
>> Unless we add another hook, the only place to put this is in private
>> ->error_handler().  A driver can check whether it's being called for the
>> first time and record it in private structure, which isn't too pretty,
>> so I thought doing it this way was fair tradeoff.
> 
> Jeff, do you agree or still think it's better done in LLDs?  I'm okay
> either way.

I trust your judgement...  My reply was more of a "gut feeling" sort of 
thing.

In general, I want to keep libata as close as possible to the "LLD 
call-out model" as possible:  LLD registers itself with system services 
[perhaps using generic helpers to do so].  LLD fills in its own scsi 
host template and ata_port_operations [perhaps using generic helpers to 
do so].  LLD implements specific helpers that control LLD-specific 
behavior, calling out to libata helpers as need arises.

So I tend to prefer adding direct LLD control points to ATA_FLAG_xxx 
control points or other solutions, if the situation permits me to choose.

If you have to ask...  I will always prefer "it" (for some value of...) 
to be done in the LLD.  ;-)

	Jeff