lkml.org 
[lkml]   [2011]   [Jan]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] [SCSI] arcmsr: &/| confusion in arcmsr_build_ccb()
On Sat, Jan 01, 2011 at 05:45:11PM +0100, roel kluin wrote:
> The WRITE_{6,10,12} are defined numbers, so the branch was always taken.

You're right, but even this will basically always set the write flag,
since some bits will be set the same, even for reads. Eg:

#define READ_10 0x28
#define WRITE_10 0x2a

It should instead be:

> + if (pcmd->cmnd[0] == WRITE_6 || pcmd->cmnd[0] == WRITE_10 ||
> + pcmd->cmnd[0] == WRITE_12 ){

That will of course miss other commands which do writes, such as UNMAP
and WRITE_32. So we should do it properly:

if (pcmd->sc_data_direction == DMA_TO_DEVICE)
arcmsr_cdb->Flags |= ARCMSR_CDB_FLAG_WRITE;
I don't have an Areca card to test with.

> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> drivers/scsi/arcmsr/arcmsr_hba.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> See include/scsi/scsi.h:58-123 for
> #define WRITE_6 0x0a
> #define WRITE_10 0x2a
> #define WRITE_12 0xaa
>
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 17e3df4..a6e5222 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -1171,7 +1171,8 @@ static int arcmsr_build_ccb(struct AdapterControlBlock *acb,
> arcmsr_cdb->msgPages = arccdbsize/0x100 + (arccdbsize % 0x100 ? 1 : 0);
> if ( arccdbsize > 256)
> arcmsr_cdb->Flags |= ARCMSR_CDB_FLAG_SGL_BSIZE;
> - if (pcmd->cmnd[0]|WRITE_6 || pcmd->cmnd[0]|WRITE_10 || pcmd->cmnd[0]|WRITE_12 ){
> + if (pcmd->cmnd[0] & WRITE_6 || pcmd->cmnd[0] & WRITE_10 ||
> + pcmd->cmnd[0] & WRITE_12 ){
> arcmsr_cdb->Flags |= ARCMSR_CDB_FLAG_WRITE;
> }
> ccb->arc_cdb_size = arccdbsize;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."


\
 
 \ /
  Last update: 2011-01-01 18:11    [from the cache]
©2003-2011 Jasper Spaans