Re: [PATCH][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction== DMA_FROM_DEVICE'

[Roel Kluin]

James Bottomley wrote:
> On Mon, 2008-02-04 at 23:36 +0100, Roel Kluin wrote:

>> Note the duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'

>> -   if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
>> +   if (SCpnt->sc_data_direction == DMA_TO_DEVICE) {
>>        cpp->xdir = DTD_IN;
>>        return;
>>        }
>>     else if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {

> 
> Well spotted, this is definitely a huge bug in the driver.
> 
> However, I think on closer examination that DTD_IN actually means
> transfer from device to host, so DMA_FROM_DEVICE is correct for that
> case.  It should be DMA_TO_DEVICE in the else if() piece.
> 
> Could you correct and resend the patch and I'll apply it.

Thanks for your review.
---
Direction of data transfer 'DMA_FROM_DEVICE' was tested twice. DTD_OUT
means  transfer from host to device. This should occur when the
direction of data transfer (sc_data_direction) is 'DMA_TO_DEVICE'.

Signed-off-by: Roel Kluin <12o3l@xxxxxxxxxx>
---
diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
index 662c004..58d7eee 100644
--- a/drivers/scsi/u14-34f.c
+++ b/drivers/scsi/u14-34f.c
@@ -1215,9 +1215,9 @@ static void scsi_to_dev_dir(unsigned int i, unsigned int j) {
    if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
       cpp->xdir = DTD_IN;
       return;
       }
-   else if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
+   else if (SCpnt->sc_data_direction == DMA_TO_DEVICE) {
       cpp->xdir = DTD_OUT;
       return;
       }
    else if (SCpnt->sc_data_direction == DMA_NONE) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/