From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [PATCH #upstream] sata_sil24: always set protocol override for non-ATAPI data commands Date: Thu, 30 Jul 2009 16:53:34 -0400 Message-ID: <4A72084E.1030508@garzik.org> References: <4A71FE71.7040002@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from srv5.dvmed.net ([207.36.208.214]:56220 "EHLO mail.dvmed.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751611AbZG3Uxg (ORCPT ); Thu, 30 Jul 2009 16:53:36 -0400 In-Reply-To: <4A71FE71.7040002@gmail.com> Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Robert Hancock Cc: ide , Tejun Heo , Mark Lord Robert Hancock wrote: > The sil24 hardware has a built-in list of commands and associated protocols > that gets used by default to decide how to handle a given command. However, > if the command is not known to the controller then it presumably assumes it to > be a non-data command which then causes protocol mismatch errors if the device > ends up requesting data transfer. The new DATA SET MANAGEMENT - Trim command > causes this issue since it's a DMA data-out command. > > Since we should always know best what protocol the command should be using, > let's just set the override flag to inform the controller what protocol to use > for all non-ATAPI commands with data transfer. > > Signed-off-by: Robert Hancock > Tested-by: Mark Lord > > diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c > index 77aa8d7..e6946fc 100644 > --- a/drivers/ata/sata_sil24.c > +++ b/drivers/ata/sata_sil24.c > @@ -846,6 +846,17 @@ static void sil24_qc_prep(struct ata_queued_cmd *qc) > if (!ata_is_atapi(qc->tf.protocol)) { > prb = &cb->ata.prb; > sge = cb->ata.sge; > + if (ata_is_data(qc->tf.protocol)) { > + u16 prot = 0; > + ctrl = PRB_CTRL_PROTOCOL; > + if (ata_is_ncq(qc->tf.protocol)) > + prot |= PRB_PROT_NCQ; > + if (qc->tf.flags & ATA_TFLAG_WRITE) > + prot |= PRB_PROT_WRITE; > + else > + prot |= PRB_PROT_READ; > + prb->prot = cpu_to_le16(prot); > + } I'm trying to remember why we did not do this originally -- Tejun, do you recall? I do not see any prohibition in the docs, so I am inclined to apply this. Jeff