[PATCH] ata: libata: Remove ata_noop_qc_prep()

[PATCH] ata: libata: Remove ata_noop_qc_prep()

[Damien Le Moal]

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>
---
 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);
-	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);
+		qc->err_mask |= ap->ops->qc_prep(qc);
+		if (unlikely(qc->err_mask))
+			goto err;
+	}
+
 	trace_ata_qc_issue(qc);
 	qc->err_mask |= ap->ops->qc_issue(qc);
 	if (unlikely(qc->err_mask))
@@ -6724,7 +6721,6 @@ static void ata_dummy_error_handler(struct ata_port *ap)
 }
 
 struct ata_port_operations ata_dummy_port_ops = {
-	.qc_prep		= ata_noop_qc_prep,
 	.qc_issue		= ata_dummy_qc_issue,
 	.error_handler		= ata_dummy_error_handler,
 	.sched_eh		= ata_std_sched_eh,
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 06868ec5b1fd..67f277e1c3bf 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -26,7 +26,6 @@ static struct workqueue_struct *ata_sff_wq;
 const struct ata_port_operations ata_sff_port_ops = {
 	.inherits		= &ata_base_port_ops,
 
-	.qc_prep		= ata_noop_qc_prep,
 	.qc_issue		= ata_sff_qc_issue,
 	.qc_fill_rtf		= ata_sff_qc_fill_rtf,
 
diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
index c84a20892f1b..a34e56a9d535 100644
--- a/drivers/ata/pata_ep93xx.c
+++ b/drivers/ata/pata_ep93xx.c
@@ -884,8 +884,6 @@ static const struct scsi_host_template ep93xx_pata_sht = {
 static struct ata_port_operations ep93xx_pata_port_ops = {
 	.inherits		= &ata_bmdma_port_ops,
 
-	.qc_prep		= ata_noop_qc_prep,
-
 	.softreset		= ep93xx_pata_softreset,
 	.hardreset		= ATA_OP_NULL,
 
diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c
index 9cfb064782c3..61d8760f09d9 100644
--- a/drivers/ata/pata_icside.c
+++ b/drivers/ata/pata_icside.c
@@ -328,8 +328,6 @@ static void pata_icside_postreset(struct ata_link *link, unsigned int *classes)
 
 static struct ata_port_operations pata_icside_port_ops = {
 	.inherits		= &ata_bmdma_port_ops,
-	/* no need to build any PRD tables for DMA */
-	.qc_prep		= ata_noop_qc_prep,
 	.sff_data_xfer		= ata_sff_data_xfer32,
 	.bmdma_setup		= pata_icside_bmdma_setup,
 	.bmdma_start		= pata_icside_bmdma_start,
diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c
index 6c317a461a1f..3f9258677915 100644
--- a/drivers/ata/pata_mpc52xx.c
+++ b/drivers/ata/pata_mpc52xx.c
@@ -620,7 +620,6 @@ static struct ata_port_operations mpc52xx_ata_port_ops = {
 	.bmdma_start		= mpc52xx_bmdma_start,
 	.bmdma_stop		= mpc52xx_bmdma_stop,
 	.bmdma_status		= mpc52xx_bmdma_status,
-	.qc_prep		= ata_noop_qc_prep,
 };
 
 static int mpc52xx_ata_init_one(struct device *dev,
diff --git a/drivers/ata/pata_octeon_cf.c b/drivers/ata/pata_octeon_cf.c
index 2884acfc4863..0bb9607e7348 100644
--- a/drivers/ata/pata_octeon_cf.c
+++ b/drivers/ata/pata_octeon_cf.c
@@ -789,7 +789,6 @@ static unsigned int octeon_cf_qc_issue(struct ata_queued_cmd *qc)
 static struct ata_port_operations octeon_cf_ops = {
 	.inherits		= &ata_sff_port_ops,
 	.check_atapi_dma	= octeon_cf_check_atapi_dma,
-	.qc_prep		= ata_noop_qc_prep,
 	.qc_issue		= octeon_cf_qc_issue,
 	.sff_dev_select		= octeon_cf_dev_select,
 	.sff_irq_on		= octeon_cf_ata_port_noaction,
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 88714b7b0dba..7b4e7a61965a 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -564,7 +564,6 @@ static struct ata_port_operations sas_sata_ops = {
 	.error_handler		= ata_std_error_handler,
 	.post_internal_cmd	= sas_ata_post_internal,
 	.qc_defer               = ata_std_qc_defer,
-	.qc_prep		= ata_noop_qc_prep,
 	.qc_issue		= sas_ata_qc_issue,
 	.qc_fill_rtf		= sas_ata_qc_fill_rtf,
 	.set_dmamode		= sas_ata_set_dmamode,
diff --git a/include/linux/libata.h b/include/linux/libata.h
index d598ef690e50..d5446e18d9df 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1168,7 +1168,6 @@ extern int ata_xfer_mode2shift(u8 xfer_mode);
 extern const char *ata_mode_string(unsigned int xfer_mask);
 extern unsigned int ata_id_xfermask(const u16 *id);
 extern int ata_std_qc_defer(struct ata_queued_cmd *qc);
-extern enum ata_completion_errors ata_noop_qc_prep(struct ata_queued_cmd *qc);
 extern void ata_sg_init(struct ata_queued_cmd *qc, struct scatterlist *sg,
 		 unsigned int n_elem);
 extern unsigned int ata_dev_classify(const struct ata_taskfile *tf);
-- 
2.45.2

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

[John Garry]

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.

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

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

[Damien Le Moal]

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

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

[Sergei Shtylyov]

Hello!

   Not sure why you keep leaving me out of CC on the patches that touch
the PATA drivers... :-/

On 8/1/24 12:01 PM, 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: Sergey Shtylyov <s.shtylyov@omp.ru>

[...]

MBR, Sergey

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

[Sergey Shtylyov]

Hello!

   Not sure why you keep leaving me out of CC on the patches that touch
the PATA drivers... :-/

On 8/1/24 12:01 PM, 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: Sergey Shtylyov <s.shtylyov@omp.ru>

[...]

MBR, Sergey