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

2008-02-06 Thread 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 <[EMAIL PROTECTED]>
---
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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2008-02-06 Thread James Bottomley 

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_FROMDEVICEdata is coming from
>   the device to the
> 
> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
> ---
> 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 = (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


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

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

2008-02-06 Thread James Bottomley 

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_FROMDEVICEdata is coming from
   the device to the
 
 Signed-off-by: Roel Kluin [EMAIL PROTECTED]
 ---
 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


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

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

2008-02-06 Thread 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 [EMAIL PROTECTED]
---
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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2008-02-05 Thread Roel Kluin 
[EMAIL PROTECTED] 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2008-02-05 Thread Ballabio_Dario 
 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:[EMAIL PROTECTED] 
Sent: Monday, February 04, 2008 11:37 PM
To: Ballabio, Dario
Cc: [EMAIL PROTECTED]; 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_FROMDEVICEdata is coming from
  the device to the

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
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 = (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;
   }


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

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

2008-02-05 Thread Ballabio_Dario 
 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:[EMAIL PROTECTED] 
Sent: Monday, February 04, 2008 11:37 PM
To: Ballabio, Dario
Cc: [EMAIL PROTECTED]; 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_FROMDEVICEdata is coming from
  the device to the

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
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;
   }


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

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

2008-02-05 Thread Roel Kluin 
[EMAIL PROTECTED] 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2008-02-04 Thread Roel Kluin 
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_FROMDEVICEdata is coming from
  the device to the

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
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 = (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;
   }

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

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

2008-02-04 Thread Roel Kluin 
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_FROMDEVICEdata is coming from
  the device to the

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
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;
   }

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