public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] [SCSI] arcmsr: upper 32 of dma address lost
@ 2014-02-07 13:00 Dan Carpenter
  2014-02-07 13:07 ` Dan Carpenter
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Dan Carpenter @ 2014-02-07 13:00 UTC (permalink / raw)
  To: James E.J. Bottomley, Nick Cheng
  Cc: Jingoo Han, Martin K. Petersen, Jiri Kosina, linux-scsi,
	kernel-janitors

The original code always set the upper 32 bits to zero because it was
doing a shift of the wrong variable.

Fixes: 1a4f550a09f8 ('[SCSI] arcmsr: 1.20.00.15: add SATA RAID plus other fixes')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 4f6a30b8e5f9..9cfd399c47c0 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2500,16 +2500,15 @@ static int arcmsr_polling_ccbdone(struct AdapterControlBlock *acb,
 static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
 {
 	uint32_t cdb_phyaddr, cdb_phyaddr_hi32;
-	dma_addr_t dma_coherent_handle;
+
 	/*
 	********************************************************************
 	** here we need to tell iop 331 our freeccb.HighPart
 	** if freeccb.HighPart is not zero
 	********************************************************************
 	*/
-	dma_coherent_handle = acb->dma_coherent_handle;
-	cdb_phyaddr = (uint32_t)(dma_coherent_handle);
-	cdb_phyaddr_hi32 = (uint32_t)((cdb_phyaddr >> 16) >> 16);
+	cdb_phyaddr = (uint32_t)(acb->dma_coherent_handle);
+	cdb_phyaddr_hi32 = (uint32_t)(acb->dma_coherent_handle >> 32);
 	acb->cdb_phyaddr_hi32 = cdb_phyaddr_hi32;
 	/*
 	***********************************************************************

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

* Re: [patch] [SCSI] arcmsr: upper 32 of dma address lost
  2014-02-07 13:00 [patch] [SCSI] arcmsr: upper 32 of dma address lost Dan Carpenter
@ 2014-02-07 13:07 ` Dan Carpenter
  2014-02-10 15:18 ` [patch v2] " Dan Carpenter
  2014-02-11 16:06 ` [patch v3] " Dan Carpenter
  2 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2014-02-07 13:07 UTC (permalink / raw)
  To: James E.J. Bottomley, Nick Cheng
  Cc: Jingoo Han, Martin K. Petersen, Jiri Kosina, linux-scsi,
	kernel-janitors

On Fri, Feb 07, 2014 at 04:00:28PM +0300, Dan Carpenter wrote:
> The original code always set the upper 32 bits to zero because it was
> doing a shift of the wrong variable.
> 

Actually let me redo this.  I want to add a cast to prevent a static
checker warning on 32 bit systems.  Sorry, for the noise.

regards,
dan carpenter

> Fixes: 1a4f550a09f8 ('[SCSI] arcmsr: 1.20.00.15: add SATA RAID plus other fixes')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 4f6a30b8e5f9..9cfd399c47c0 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -2500,16 +2500,15 @@ static int arcmsr_polling_ccbdone(struct AdapterControlBlock *acb,
>  static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
>  {
>  	uint32_t cdb_phyaddr, cdb_phyaddr_hi32;
> -	dma_addr_t dma_coherent_handle;
> +
>  	/*
>  	********************************************************************
>  	** here we need to tell iop 331 our freeccb.HighPart
>  	** if freeccb.HighPart is not zero
>  	********************************************************************
>  	*/
> -	dma_coherent_handle = acb->dma_coherent_handle;
> -	cdb_phyaddr = (uint32_t)(dma_coherent_handle);
> -	cdb_phyaddr_hi32 = (uint32_t)((cdb_phyaddr >> 16) >> 16);
> +	cdb_phyaddr = (uint32_t)(acb->dma_coherent_handle);
> +	cdb_phyaddr_hi32 = (uint32_t)(acb->dma_coherent_handle >> 32);
>  	acb->cdb_phyaddr_hi32 = cdb_phyaddr_hi32;
>  	/*
>  	***********************************************************************

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

* [patch v2] [SCSI] arcmsr: upper 32 of dma address lost
  2014-02-07 13:00 [patch] [SCSI] arcmsr: upper 32 of dma address lost Dan Carpenter
  2014-02-07 13:07 ` Dan Carpenter
@ 2014-02-10 15:18 ` Dan Carpenter
  2014-02-10 15:22   ` James Bottomley
  2014-02-11 16:06 ` [patch v3] " Dan Carpenter
  2 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2014-02-10 15:18 UTC (permalink / raw)
  To: James E.J. Bottomley, Nick Cheng
  Cc: Jingoo Han, Martin K. Petersen, Jiri Kosina, linux-scsi,
	kernel-janitors

The original code always set the upper 32 bits to zero because it was
doing a shift of the wrong variable.

Fixes: 1a4f550a09f8 ('[SCSI] arcmsr: 1.20.00.15: add SATA RAID plus other fixes')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Add a cast to u64 to avoid a future static checker warning

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 4f6a30b8e5f9..9cfd399c47c0 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2500,16 +2500,15 @@ static int arcmsr_polling_ccbdone(struct AdapterControlBlock *acb,
 static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
 {
 	uint32_t cdb_phyaddr, cdb_phyaddr_hi32;
-	dma_addr_t dma_coherent_handle;
+
 	/*
 	********************************************************************
 	** here we need to tell iop 331 our freeccb.HighPart
 	** if freeccb.HighPart is not zero
 	********************************************************************
 	*/
-	dma_coherent_handle = acb->dma_coherent_handle;
-	cdb_phyaddr = (uint32_t)(dma_coherent_handle);
-	cdb_phyaddr_hi32 = (uint32_t)((cdb_phyaddr >> 16) >> 16);
+	cdb_phyaddr = (uint32_t)(acb->dma_coherent_handle);
+	cdb_phyaddr_hi32 = (uint32_t)((u64)acb->dma_coherent_handle >> 32);
 	acb->cdb_phyaddr_hi32 = cdb_phyaddr_hi32;
 	/*
 	***********************************************************************


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

* Re: [patch v2] [SCSI] arcmsr: upper 32 of dma address lost
  2014-02-10 15:18 ` [patch v2] " Dan Carpenter
@ 2014-02-10 15:22   ` James Bottomley
  2014-02-10 15:29     ` Dan Carpenter
  0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2014-02-10 15:22 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Nick Cheng, Jingoo Han, Martin K. Petersen, Jiri Kosina,
	linux-scsi, kernel-janitors

On Mon, 2014-02-10 at 18:18 +0300, Dan Carpenter wrote:
> The original code always set the upper 32 bits to zero because it was
> doing a shift of the wrong variable.
> 
> Fixes: 1a4f550a09f8 ('[SCSI] arcmsr: 1.20.00.15: add SATA RAID plus other fixes')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Add a cast to u64 to avoid a future static checker warning
> 
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 4f6a30b8e5f9..9cfd399c47c0 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -2500,16 +2500,15 @@ static int arcmsr_polling_ccbdone(struct AdapterControlBlock *acb,
>  static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
>  {
>  	uint32_t cdb_phyaddr, cdb_phyaddr_hi32;
> -	dma_addr_t dma_coherent_handle;
> +
>  	/*
>  	********************************************************************
>  	** here we need to tell iop 331 our freeccb.HighPart
>  	** if freeccb.HighPart is not zero
>  	********************************************************************
>  	*/
> -	dma_coherent_handle = acb->dma_coherent_handle;
> -	cdb_phyaddr = (uint32_t)(dma_coherent_handle);
> -	cdb_phyaddr_hi32 = (uint32_t)((cdb_phyaddr >> 16) >> 16);
> +	cdb_phyaddr = (uint32_t)(acb->dma_coherent_handle);
> +	cdb_phyaddr_hi32 = (uint32_t)((u64)acb->dma_coherent_handle >> 32);

The original 16 >> 16 is better because there's no requirement to cast
to u64 and take an expensive 64 bit build out on 32 bits just to avoid
the warning.

There's actually a macro in kernel.h (upper_32_bits) that does this.

James

>  	acb->cdb_phyaddr_hi32 = cdb_phyaddr_hi32;
>  	/*
>  	***********************************************************************
> 
> --
> 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




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

* Re: [patch v2] [SCSI] arcmsr: upper 32 of dma address lost
  2014-02-10 15:22   ` James Bottomley
@ 2014-02-10 15:29     ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2014-02-10 15:29 UTC (permalink / raw)
  To: James Bottomley
  Cc: Nick Cheng, Jingoo Han, Martin K. Petersen, Jiri Kosina,
	linux-scsi, kernel-janitors

On Mon, Feb 10, 2014 at 07:22:02AM -0800, James Bottomley wrote:
> On Mon, 2014-02-10 at 18:18 +0300, Dan Carpenter wrote:
> > The original code always set the upper 32 bits to zero because it was
> > doing a shift of the wrong variable.
> > 
> > Fixes: 1a4f550a09f8 ('[SCSI] arcmsr: 1.20.00.15: add SATA RAID plus other fixes')
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: Add a cast to u64 to avoid a future static checker warning
> > 
> > diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> > index 4f6a30b8e5f9..9cfd399c47c0 100644
> > --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> > @@ -2500,16 +2500,15 @@ static int arcmsr_polling_ccbdone(struct AdapterControlBlock *acb,
> >  static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
> >  {
> >  	uint32_t cdb_phyaddr, cdb_phyaddr_hi32;
> > -	dma_addr_t dma_coherent_handle;
> > +
> >  	/*
> >  	********************************************************************
> >  	** here we need to tell iop 331 our freeccb.HighPart
> >  	** if freeccb.HighPart is not zero
> >  	********************************************************************
> >  	*/
> > -	dma_coherent_handle = acb->dma_coherent_handle;
> > -	cdb_phyaddr = (uint32_t)(dma_coherent_handle);
> > -	cdb_phyaddr_hi32 = (uint32_t)((cdb_phyaddr >> 16) >> 16);
> > +	cdb_phyaddr = (uint32_t)(acb->dma_coherent_handle);
> > +	cdb_phyaddr_hi32 = (uint32_t)((u64)acb->dma_coherent_handle >> 32);
> 
> The original 16 >> 16 is better because there's no requirement to cast
> to u64 and take an expensive 64 bit build out on 32 bits just to avoid
> the warning.
> 
> There's actually a macro in kernel.h (upper_32_bits) that does this.
> 

Ah.  I was wondering what that was.  Thanks.  I will resend.

regards,
dan carpenter


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

* [patch v3] [SCSI] arcmsr: upper 32 of dma address lost
  2014-02-07 13:00 [patch] [SCSI] arcmsr: upper 32 of dma address lost Dan Carpenter
  2014-02-07 13:07 ` Dan Carpenter
  2014-02-10 15:18 ` [patch v2] " Dan Carpenter
@ 2014-02-11 16:06 ` Dan Carpenter
  2 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2014-02-11 16:06 UTC (permalink / raw)
  To: James E.J. Bottomley, Nick Cheng
  Cc: Jingoo Han, Martin K. Petersen, Jiri Kosina, linux-scsi,
	kernel-janitors

The original code always set the upper 32 bits to zero because it was
doing a shift of the wrong variable.

Fixes: 1a4f550a09f8 ('[SCSI] arcmsr: 1.20.00.15: add SATA RAID plus other fixes')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v3: In v2 I introduced an expensive cast to solve a static checker
    warning but James noticed and pointed me to the upper_32_bits()
    macro.

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 4f6a30b8e5f9..9cfd399c47c0 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2500,16 +2500,15 @@ static int arcmsr_polling_ccbdone(struct AdapterControlBlock *acb,
 static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
 {
 	uint32_t cdb_phyaddr, cdb_phyaddr_hi32;
-	dma_addr_t dma_coherent_handle;
+
 	/*
 	********************************************************************
 	** here we need to tell iop 331 our freeccb.HighPart
 	** if freeccb.HighPart is not zero
 	********************************************************************
 	*/
-	dma_coherent_handle = acb->dma_coherent_handle;
-	cdb_phyaddr = (uint32_t)(dma_coherent_handle);
-	cdb_phyaddr_hi32 = (uint32_t)((cdb_phyaddr >> 16) >> 16);
+	cdb_phyaddr = lower_32_bits(acb->dma_coherent_handle);
+	cdb_phyaddr_hi32 = upper_32_bits(acb->dma_coherent_handle);
 	acb->cdb_phyaddr_hi32 = cdb_phyaddr_hi32;
 	/*
 	***********************************************************************


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

end of thread, other threads:[~2014-02-11 16:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-07 13:00 [patch] [SCSI] arcmsr: upper 32 of dma address lost Dan Carpenter
2014-02-07 13:07 ` Dan Carpenter
2014-02-10 15:18 ` [patch v2] " Dan Carpenter
2014-02-10 15:22   ` James Bottomley
2014-02-10 15:29     ` Dan Carpenter
2014-02-11 16:06 ` [patch v3] " Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox