LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
@ 2008-02-04 22:36 Roel Kluin
  2008-02-05  8:09 ` Ballabio_Dario
  2008-02-06 17:45 ` James Bottomley
  0 siblings, 2 replies; 5+ messages in thread
From: Roel Kluin @ 2008-02-04 22:36 UTC (permalink / raw)
  To: ballabio_dario; +Cc: linux-scsi, lkml

It should be like this I guess? this patch was not yet tested, please
confirm.
--
Note the duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'

from Documentation/DMA-API.txt:
DMA_TO_DEVICE         = PCI_DMA_TODEVICE      data is going from the
                                              memory to the device
DMA_FROM_DEVICE       = PCI_DMA_FROMDEVICE    data is coming from
                                              the device to the

Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
index 662c004..1e704f9 100644
--- a/drivers/scsi/u14-34f.c
+++ b/drivers/scsi/u14-34f.c
@@ -1208,15 +1208,15 @@ static void scsi_to_dev_dir(unsigned int i, unsigned int j) {
       };
 
    struct mscp *cpp;
    struct scsi_cmnd *SCpnt;
 
    cpp = &HD(j)->cp[i]; SCpnt = cpp->SCpnt;
 
-   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) {
       cpp->xdir = DTD_OUT;
       return;
       }


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
  2008-02-04 22:36 [PATCH][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE' Roel Kluin
@ 2008-02-05  8:09 ` Ballabio_Dario
  2008-02-05  8:29   ` Roel Kluin
  2008-02-06 17:45 ` James Bottomley
  1 sibling, 1 reply; 5+ messages in thread
From: Ballabio_Dario @ 2008-02-05  8:09 UTC (permalink / raw)
  To: 12o3l; +Cc: linux-scsi, linux-kernel

 Good to know that somebody still uses the Ultrastor 14f board :).....
Yes, this typo was introduced by somebody doing massive editing to all
scsi drivers long ago.
Cheers,
--db
 

-----Original Message-----
From: Roel Kluin [mailto:12o3l@tiscali.nl] 
Sent: Monday, February 04, 2008 11:37 PM
To: Ballabio, Dario
Cc: linux-scsi@vger.kernel.org; lkml
Subject: [PATCH][drivers/scsi/u14-34f.c] duplicate test
'SCpnt->sc_data_direction == DMA_FROM_DEVICE'

It should be like this I guess? this patch was not yet tested, please
confirm.
--
Note the duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'

from Documentation/DMA-API.txt:
DMA_TO_DEVICE         = PCI_DMA_TODEVICE      data is going from the
                                              memory to the device
DMA_FROM_DEVICE       = PCI_DMA_FROMDEVICE    data is coming from
                                              the device to the

Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
index 662c004..1e704f9 100644
--- a/drivers/scsi/u14-34f.c
+++ b/drivers/scsi/u14-34f.c
@@ -1208,15 +1208,15 @@ static void scsi_to_dev_dir(unsigned int i,
unsigned int j) {
       };
 
    struct mscp *cpp;
    struct scsi_cmnd *SCpnt;
 
    cpp = &HD(j)->cp[i]; SCpnt = cpp->SCpnt;
 
-   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) {
       cpp->xdir = DTD_OUT;
       return;
       }



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
  2008-02-05  8:09 ` Ballabio_Dario
@ 2008-02-05  8:29   ` Roel Kluin
  0 siblings, 0 replies; 5+ messages in thread
From: Roel Kluin @ 2008-02-05  8:29 UTC (permalink / raw)
  To: Ballabio_Dario; +Cc: linux-scsi, linux-kernel

Ballabio_Dario@emc.com wrote:
>  Good to know that somebody still uses the Ultrastor 14f board :).....
> Yes, this typo was introduced by somebody doing massive editing to all
> scsi drivers long ago.
> Cheers,
> --db
Actually, I do not own a Ultrastor 14f board. I found this by searching for
if (test)
...
else if (exactly same test)
...
Thanks,
Roel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
  2008-02-04 22:36 [PATCH][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE' Roel Kluin
  2008-02-05  8:09 ` Ballabio_Dario
@ 2008-02-06 17:45 ` James Bottomley
  2008-02-06 18:21   ` Roel Kluin
  1 sibling, 1 reply; 5+ messages in thread
From: James Bottomley @ 2008-02-06 17:45 UTC (permalink / raw)
  To: Roel Kluin; +Cc: ballabio_dario, linux-scsi, lkml


On Mon, 2008-02-04 at 23:36 +0100, Roel Kluin wrote:
> It should be like this I guess? this patch was not yet tested, please
> confirm.
> --
> Note the duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
> 
> from Documentation/DMA-API.txt:
> DMA_TO_DEVICE         = PCI_DMA_TODEVICE      data is going from the
>                                               memory to the device
> DMA_FROM_DEVICE       = PCI_DMA_FROMDEVICE    data is coming from
>                                               the device to the
> 
> Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
> ---
> diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
> index 662c004..1e704f9 100644
> --- a/drivers/scsi/u14-34f.c
> +++ b/drivers/scsi/u14-34f.c
> @@ -1208,15 +1208,15 @@ static void scsi_to_dev_dir(unsigned int i, unsigned int j) {
>        };
>  
>     struct mscp *cpp;
>     struct scsi_cmnd *SCpnt;
>  
>     cpp = &HD(j)->cp[i]; SCpnt = cpp->SCpnt;
>  
> -   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) {
>        cpp->xdir = DTD_OUT;
>        return;
>        }

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,

James



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'
  2008-02-06 17:45 ` James Bottomley
@ 2008-02-06 18:21   ` Roel Kluin
  0 siblings, 0 replies; 5+ messages in thread
From: Roel Kluin @ 2008-02-06 18:21 UTC (permalink / raw)
  To: James Bottomley; +Cc: ballabio_dario, linux-scsi, lkml

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@tiscali.nl>
---
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) {


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-02-06 18:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-04 22:36 [PATCH][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE' Roel Kluin
2008-02-05  8:09 ` Ballabio_Dario
2008-02-05  8:29   ` Roel Kluin
2008-02-06 17:45 ` James Bottomley
2008-02-06 18:21   ` Roel Kluin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).