Re: [PATCH v1 08/23] ata: sata_dwc_460ex: don't call ata_sff_qc_issue() on DMA commands
From: Christian Lamparter
Date: Fri Apr 22 2016 - 07:44:23 EST
On Friday, April 22, 2016 11:32:00 AM David Laight wrote:
> From: Andy Shevchenko
> > Sent: 21 April 2016 19:15
> > ata_sff_qc_issue() can't handle DMA commands and thus we have to avoid it for
> > them. Do call ata_bmdma_qc_issue() instead for this case.
> >
> > Suggested-by: Christian Lamparter <chunkeey@xxxxxxxxxxxxxx>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> > drivers/ata/sata_dwc_460ex.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> > index 038e5fb..845c35d 100644
> > --- a/drivers/ata/sata_dwc_460ex.c
> > +++ b/drivers/ata/sata_dwc_460ex.c
> > @@ -1061,10 +1061,12 @@ static unsigned int sata_dwc_qc_issue(struct ata_queued_cmd *qc)
> > __func__, tag, qc->ap->link.sactive, sactive);
> >
> > ap->ops->sff_tf_load(ap, &qc->tf);
> > - sata_dwc_exec_command_by_tag(ap, &qc->tf, qc->tag,
> > + sata_dwc_exec_command_by_tag(ap, &qc->tf, tag,
> > SATA_DWC_CMD_ISSUED_PEND);
> > + } else if (ata_is_dma(qc->tf.protocol)) {
> > + return ata_bmdma_qc_issue(qc);
> > } else {
> > - ata_sff_qc_issue(qc);
> > + return ata_sff_qc_issue(qc);
> > }
> > return 0;
> > }
>
> I'd nuke those 'else if', they make it very hard to read.
> I Think the code is:
>
> sata_dwc_exec_command_by_tag(...);
> return 0;
> }
>
> if (ata_is_dma(qc->tf.protocol))
> return ata_bmdma_qc_issue(qc);
>
> return ata_sff_qc_issue(qc);
> }
What about something like this instead? ata_bmdma_qc_issue already calls
ata_sff_qc_issue, if it's not a dma transfere anyway [0].
---
diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 6a61184..67cce54 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -1096,12 +1096,9 @@ static unsigned int sata_dwc_qc_issue(struct ata_queued_cmd *qc)
ap->ops->sff_tf_load(ap, &qc->tf);
sata_dwc_exec_command_by_tag(ap, &qc->tf, tag,
SATA_DWC_CMD_ISSUED_PEND);
- } else if (ata_is_dma(qc->tf.protocol)) {
+ return 0;
+ } else
return ata_bmdma_qc_issue(qc);
- } else {
- return ata_sff_qc_issue(qc);
- }
- return 0;
}
static void sata_dwc_error_handler(struct ata_port *ap)
---
[0] <http://lxr.free-electrons.com/source/drivers/ata/libata-sff.c#L2787>