Re: [PATCH] ata: libata: Remove ata_noop_qc_prep()

[Damien Le Moal <dlemoal@kernel.org>]

On 8/1/24 6:11 PM, John Garry wrote:
> On 01/08/2024 10:01, Damien Le Moal wrote:
>> The function ata_noop_qc_prep(), as its name implies, does nothing and
>> simply returns AC_ERR_OK. For drivers that do not need any special
>> preparations of queued commands, we can avoid having to define struct
>> ata_port qc_prep operation by simply testing if that operation is
>> defined or not in ata_qc_issue(). Make this change and remove
>> ata_noop_qc_prep().
>>
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> 
> Reviewed-by: John Garry <john.g.garry@oracle.com>
> 
>> ---
>>   drivers/ata/libata-core.c     | 18 +++++++-----------
>>   drivers/ata/libata-sff.c      |  1 -
>>   drivers/ata/pata_ep93xx.c     |  2 --
>>   drivers/ata/pata_icside.c     |  2 --
>>   drivers/ata/pata_mpc52xx.c    |  1 -
>>   drivers/ata/pata_octeon_cf.c  |  1 -
>>   drivers/scsi/libsas/sas_ata.c |  1 -
>>   include/linux/libata.h        |  1 -
>>   8 files changed, 7 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index fc9fcfda42b8..b4fdb78579c8 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -4696,12 +4696,6 @@ int ata_std_qc_defer(struct ata_queued_cmd *qc)
>>   }
>>   EXPORT_SYMBOL_GPL(ata_std_qc_defer);
>>   -enum ata_completion_errors ata_noop_qc_prep(struct ata_queued_cmd *qc)
>> -{
>> -    return AC_ERR_OK;
>> -}
>> -EXPORT_SYMBOL_GPL(ata_noop_qc_prep);
>> -
>>   /**
>>    *    ata_sg_init - Associate command with scatter-gather table.
>>    *    @qc: Command to be associated
>> @@ -5088,10 +5082,13 @@ void ata_qc_issue(struct ata_queued_cmd *qc)
>>           return;
>>       }
>>   -    trace_ata_qc_prep(qc);
> 
> I assume qc->err_mask must be zero here.

Yes it is, since this is a new command.

> 
>> -    qc->err_mask |= ap->ops->qc_prep(qc);
>> -    if (unlikely(qc->err_mask))
>> -        goto err;
>> +    if (ap->ops->qc_prep) {
>> +        trace_ata_qc_prep(qc);
> 

-- 
Damien Le Moal
Western Digital Research