Patchwork pata_arasan_cf: declare/use 'qc' and 'ap' variables in arasan_cf_dma_start()

login
register
mail settings
Submitter Sergei Shtylyov
Date Oct. 25, 2012, 5:08 p.m.
Message ID <201210252108.47085.sshtylyov@ru.mvista.com>
Download mbox | patch
Permalink /patch/194259/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Sergei Shtylyov - Oct. 25, 2012, 5:08 p.m.
'acdev->qc' and 'acdev->qc->ap' expressions are used multiple times in this
function, so it makes sense to use the local variables for them.

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

---
The patch is atop of the 'upstream' branch of libata-dev.git...

 drivers/ata/pata_arasan_cf.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
viresh kumar - Oct. 26, 2012, 3:31 a.m.
On Thu, Oct 25, 2012 at 10:38 PM, Sergei Shtylyov
<sshtylyov@ru.mvista.com> wrote:
> 'acdev->qc' and 'acdev->qc->ap' expressions are used multiple times in this
> function, so it makes sense to use the local variables for them.
>
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

> ---
> The patch is atop of the 'upstream' branch of libata-dev.git...
>
>  drivers/ata/pata_arasan_cf.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> Index: libata-dev/drivers/ata/pata_arasan_cf.c
> ===================================================================
> --- libata-dev.orig/drivers/ata/pata_arasan_cf.c
> +++ libata-dev/drivers/ata/pata_arasan_cf.c
> @@ -668,13 +668,15 @@ void arasan_cf_error_handler(struct ata_
>
>  static void arasan_cf_dma_start(struct arasan_cf_dev *acdev)
>  {
> +       struct ata_queued_cmd *qc = acdev->qc;
> +       struct ata_port *ap = qc->ap;
>         u32 xfer_ctr = readl(acdev->vbase + XFER_CTR) & ~XFER_DIR_MASK;
> -       u32 write = acdev->qc->tf.flags & ATA_TFLAG_WRITE;
> +       u32 write = qc->tf.flags & ATA_TFLAG_WRITE;
>
>         xfer_ctr |= write ? XFER_WRITE : XFER_READ;
>         writel(xfer_ctr, acdev->vbase + XFER_CTR);
>
> -       acdev->qc->ap->ops->sff_exec_command(acdev->qc->ap, &acdev->qc->tf);
> +       ap->ops->sff_exec_command(ap, &qc->tf);
>         ata_sff_queue_work(&acdev->work);
>  }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Oct. 26, 2012, 6:17 p.m.
Hello.

On 10/26/2012 07:31 AM, viresh kumar wrote:

>> 'acdev->qc' and 'acdev->qc->ap' expressions are used multiple times in this
>> function, so it makes sense to use the local variables for them.

>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

  Another idea came to me afterwards...

>> ---
>> The patch is atop of the 'upstream' branch of libata-dev.git...
>>
>>  drivers/ata/pata_arasan_cf.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)

>> Index: libata-dev/drivers/ata/pata_arasan_cf.c
>> ===================================================================
>> --- libata-dev.orig/drivers/ata/pata_arasan_cf.c
>> +++ libata-dev/drivers/ata/pata_arasan_cf.c
>> @@ -668,13 +668,15 @@ void arasan_cf_error_handler(struct ata_
>>
>>  static void arasan_cf_dma_start(struct arasan_cf_dev *acdev)
>>  {
>> +       struct ata_queued_cmd *qc = acdev->qc;
>> +       struct ata_port *ap = qc->ap;
>>         u32 xfer_ctr = readl(acdev->vbase + XFER_CTR) & ~XFER_DIR_MASK;
>> -       u32 write = acdev->qc->tf.flags & ATA_TFLAG_WRITE;
>> +       u32 write = qc->tf.flags & ATA_TFLAG_WRITE;
>>
>>         xfer_ctr |= write ? XFER_WRITE : XFER_READ;
>>         writel(xfer_ctr, acdev->vbase + XFER_CTR);
>>
>> -       acdev->qc->ap->ops->sff_exec_command(acdev->qc->ap, &acdev->qc->tf);
>> +       ap->ops->sff_exec_command(ap, &qc->tf);


   Perhaps it was also worth declaring:

	struct ata_taskfile *tf = &qc->tf;

   I'll submit the next version of patch next week, probably...

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
viresh kumar - Oct. 26, 2012, 6:20 p.m.
On 26 October 2012 23:47, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
>    Perhaps it was also worth declaring:
>
>         struct ata_taskfile *tf = &qc->tf;
>
>    I'll submit the next version of patch next week, probably...

Looks fine. Add my acked-by in advance :)
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Index: libata-dev/drivers/ata/pata_arasan_cf.c
===================================================================
--- libata-dev.orig/drivers/ata/pata_arasan_cf.c
+++ libata-dev/drivers/ata/pata_arasan_cf.c
@@ -668,13 +668,15 @@  void arasan_cf_error_handler(struct ata_
 
 static void arasan_cf_dma_start(struct arasan_cf_dev *acdev)
 {
+	struct ata_queued_cmd *qc = acdev->qc;
+	struct ata_port *ap = qc->ap;
 	u32 xfer_ctr = readl(acdev->vbase + XFER_CTR) & ~XFER_DIR_MASK;
-	u32 write = acdev->qc->tf.flags & ATA_TFLAG_WRITE;
+	u32 write = qc->tf.flags & ATA_TFLAG_WRITE;
 
 	xfer_ctr |= write ? XFER_WRITE : XFER_READ;
 	writel(xfer_ctr, acdev->vbase + XFER_CTR);
 
-	acdev->qc->ap->ops->sff_exec_command(acdev->qc->ap, &acdev->qc->tf);
+	ap->ops->sff_exec_command(ap, &qc->tf);
 	ata_sff_queue_work(&acdev->work);
 }