Hello, I'm still a bit confused about this patch. On Thu, Jul 14, 2016 at 09:05:45AM +0900, Christoph Hellwig wrote: > diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c > index a723ae9..acfb865 100644 > --- a/drivers/ata/sata_fsl.c > +++ b/drivers/ata/sata_fsl.c > @@ -534,7 +534,7 @@ static void sata_fsl_qc_prep(struct ata_queued_cmd *qc) > VPRINTK("Dumping cfis : 0x%x, 0x%x, 0x%x\n", > cd->cfis[0], cd->cfis[1], cd->cfis[2]); > > - if (qc->tf.protocol == ATA_PROT_NCQ) { > + if (ata_is_fpdma(qc->tf.protocol)) { ata_is_fpdma() tests NCQ or DMA. Is this right? > --- a/drivers/ata/sata_mv.c > +++ b/drivers/ata/sata_mv.c > @@ -1173,7 +1173,7 @@ static void mv_set_irq_coalescing(struct ata_host *host, > static void mv_start_edma(struct ata_port *ap, void __iomem *port_mmio, > struct mv_port_priv *pp, u8 protocol) > { > - int want_ncq = (protocol == ATA_PROT_NCQ); > + int want_ncq = (ata_is_fpdma(protocol)); Let's get rid of the parentheses while at it. > > if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) { > int using_ncq = ((pp->pp_flags & MV_PP_FLAG_NCQ_EN) != 0); > @@ -2156,8 +2156,7 @@ static void mv_qc_prep_iie(struct ata_queued_cmd *qc) > unsigned in_index; > u32 flags = 0; > > - if ((tf->protocol != ATA_PROT_DMA) && > - (tf->protocol != ATA_PROT_NCQ)) > + if (!ata_is_fpdma(tf->protocol)) This is actually testing !DMA && !NCQ and an equivalent conversion but the rest aren't. > diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c > index 734f563..89e36aa 100644 > --- a/drivers/ata/sata_nv.c > +++ b/drivers/ata/sata_nv.c > @@ -1407,7 +1407,7 @@ static void nv_adma_qc_prep(struct ata_queued_cmd *qc) > cpb->next_cpb_idx = 0; > > /* turn on NCQ flags for NCQ commands */ > - if (qc->tf.protocol == ATA_PROT_NCQ) > + if (ata_is_fpdma(qc->tf.protocol)) > ctl_flags |= NV_CPB_CTL_QUEUE | NV_CPB_CTL_FPDMA; And I don't know why testing NCQ or DMA would be safe for places like this. > + ATA_PROT_FLAG_FPDMA = ATA_PROT_FLAG_NCQ | ATA_PROT_FLAG_DMA, ... > +static inline bool ata_is_fpdma(u8 prot) > +{ > + return ata_prot_flags(prot) & ATA_PROT_FLAG_FPDMA; > +} ??? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html