diff mbox

qlogicpti: Fix compiler warnings

Message ID 1479936597-29061-1-git-send-email-tushar.n.dave@oracle.com
State Superseded
Delegated to: David Miller
Headers show

Commit Message

Tushar Dave Nov. 23, 2016, 9:29 p.m. UTC 
qlogicpti uses '__u32' for dma handle while invoking kernel DMA APIs,
instead of using dma_addr_t. This hasn't caused any 'incompatible
pointer type' warning on SPARC because until now dma_addr_t is of
type u32. However, recent changes in SPARC ATU (iommu) enabled 64bit
DMA and therefore dma_addr_t became of type u64. This makes
'incompatible pointer type' warnings inevitable.

e.g.
drivers/scsi/qlogicpti.c: In function ‘qpti_map_queues’:
drivers/scsi/qlogicpti.c:813: warning: passing argument 3 of ‘dma_alloc_coherent’ from incompatible pointer type
./include/linux/dma-mapping.h:445: note: expected ‘dma_addr_t *’ but argument is of type ‘__u32 *’
drivers/scsi/qlogicpti.c:822: warning: passing argument 3 of ‘dma_alloc_coherent’ from incompatible pointer type
./include/linux/dma-mapping.h:445: note: expected ‘dma_addr_t *’ but argument is of type ‘__u32 *’

This patch resolves above compiler warnings.

Signed-off-by: Tushar Dave <tushar.n.dave@oracle.com>
Reviewed-by: thomas tai <thomas.tai@oracle.com>
---
 drivers/scsi/qlogicpti.c | 10 ++++++----
 drivers/scsi/qlogicpti.h |  4 ++--
 2 files changed, 8 insertions(+), 6 deletions(-)

Comments

James Bottomley Nov. 23, 2016, 10:57 p.m. UTC | #1 
On Wed, 2016-11-23 at 13:29 -0800, Tushar Dave wrote:
> qlogicpti uses '__u32' for dma handle while invoking kernel DMA APIs,
> instead of using dma_addr_t. This hasn't caused any 'incompatible
> pointer type' warning on SPARC because until now dma_addr_t is of
> type u32. However, recent changes in SPARC ATU (iommu) enabled 64bit
> DMA and therefore dma_addr_t became of type u64. This makes
> 'incompatible pointer type' warnings inevitable.
> 
> e.g.
> drivers/scsi/qlogicpti.c: In function ‘qpti_map_queues’:
> drivers/scsi/qlogicpti.c:813: warning: passing argument 3 of
> ‘dma_alloc_coherent’ from incompatible pointer type
> ./include/linux/dma-mapping.h:445: note: expected ‘dma_addr_t *’ but
> argument is of type ‘__u32 *’
> drivers/scsi/qlogicpti.c:822: warning: passing argument 3 of
> ‘dma_alloc_coherent’ from incompatible pointer type
> ./include/linux/dma-mapping.h:445: note: expected ‘dma_addr_t *’ but
> argument is of type ‘__u32 *’
> 
> This patch resolves above compiler warnings.

There appears to be no point to the first three hunks of this diff:

(ushort)(x << 16) 
(ushort)(x & 0xffff)

return the same thing whether the type of x is u32 or u64, so there was
no need to alter the original code.

What's the guarantee, since the device descriptors only cope with 32
bits of physical address, that this driver never gets any dma address
beyond its addressable range?  Is it that the sbus can never be
attached to this ATU type IOMMU?  If so, saying that in the log would
be useful.

James

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tushar Dave Nov. 24, 2016, 1:08 a.m. UTC | #2 
On 11/23/2016 02:57 PM, James Bottomley wrote:
> On Wed, 2016-11-23 at 13:29 -0800, Tushar Dave wrote:
>> qlogicpti uses '__u32' for dma handle while invoking kernel DMA APIs,
>> instead of using dma_addr_t. This hasn't caused any 'incompatible
>> pointer type' warning on SPARC because until now dma_addr_t is of
>> type u32. However, recent changes in SPARC ATU (iommu) enabled 64bit
>> DMA and therefore dma_addr_t became of type u64. This makes
>> 'incompatible pointer type' warnings inevitable.
>>
>> e.g.
>> drivers/scsi/qlogicpti.c: In function ‘qpti_map_queues’:
>> drivers/scsi/qlogicpti.c:813: warning: passing argument 3 of
>> ‘dma_alloc_coherent’ from incompatible pointer type
>> ./include/linux/dma-mapping.h:445: note: expected ‘dma_addr_t *’ but
>> argument is of type ‘__u32 *’
>> drivers/scsi/qlogicpti.c:822: warning: passing argument 3 of
>> ‘dma_alloc_coherent’ from incompatible pointer type
>> ./include/linux/dma-mapping.h:445: note: expected ‘dma_addr_t *’ but
>> argument is of type ‘__u32 *’
>>
>> This patch resolves above compiler warnings.
>
> There appears to be no point to the first three hunks of this diff:
>
> (ushort)(x << 16)
> (ushort)(x & 0xffff)
>
> return the same thing whether the type of x is u32 or u64, so there was
> no need to alter the original code.
Agree, will make the change.
>
> What's the guarantee, since the device descriptors only cope with 32
> bits of physical address, that this driver never gets any dma
> address beyond its addressable range?  Is it that the sbus can never
> be attached to this ATU type IOMMU?  If so, saying that in the log
> would be useful.
Thanks for catching this.
As per my understanding, I think, all DMA map/unmap go through
ATU (iommu) in sun4v sparc. To guarantee that driver doesn't get DMA
address beyond its addressable range , driver must set dma mask before
requesting any DMA mapping!

I will investigate further and send v2.

Thanks for the review.

-Tushar


>
> James
>
>
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Nov. 24, 2016, 1:24 a.m. UTC | #3 
From: James Bottomley <James.Bottomley@HansenPartnership.com>
Date: Wed, 23 Nov 2016 14:57:39 -0800

> What's the guarantee, since the device descriptors only cope with 32
> bits of physical address, that this driver never gets any dma address
> beyond its addressable range?  Is it that the sbus can never be
> attached to this ATU type IOMMU?  If so, saying that in the log would
> be useful.

SBUS hasn't changed for 20 years and is a 32-bit bus.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Nov. 24, 2016, 1:25 a.m. UTC | #4 
From: tndave <tushar.n.dave@oracle.com>
Date: Wed, 23 Nov 2016 17:08:23 -0800

> As per my understanding, I think, all DMA map/unmap go through
> ATU (iommu) in sun4v sparc. To guarantee that driver doesn't get DMA
> address beyond its addressable range , driver must set dma mask before
> requesting any DMA mapping!
> 
> I will investigate further and send v2.
> 
> Thanks for the review.

This is an SBUS driver, it's not going to execute on any sun4v
system.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tushar Dave Nov. 24, 2016, 1:44 a.m. UTC | #5 
On 11/23/2016 05:25 PM, David Miller wrote:
> From: tndave <tushar.n.dave@oracle.com>
> Date: Wed, 23 Nov 2016 17:08:23 -0800
>
>> As per my understanding, I think, all DMA map/unmap go through
>> ATU (iommu) in sun4v sparc. To guarantee that driver doesn't get DMA
>> address beyond its addressable range , driver must set dma mask before
>> requesting any DMA mapping!
>>
>> I will investigate further and send v2.
>>
>> Thanks for the review.
>
> This is an SBUS driver, it's not going to execute on any sun4v
> system.
Thank you for the info, Dave.

In that case, the DMA address comes from legacy iommu and that address
is always in 32-bit range i.e. below 4G.

-Tushar

> --
> To unsubscribe from this list: send the line "unsubscribe sparclinux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/qlogicpti.c b/drivers/scsi/qlogicpti.c
index 69bfc0a..e25ad8c 100644
--- a/drivers/scsi/qlogicpti.c
+++ b/drivers/scsi/qlogicpti.c
@@ -315,6 +315,8 @@  static inline void qlogicpti_set_hostdev_defaults(struct qlogicpti *qpti)
 static int qlogicpti_reset_hardware(struct Scsi_Host *host)
 {
 	struct qlogicpti *qpti = (struct qlogicpti *) host->hostdata;
+	__u32 qres_dvma = (__u32)qpti->res_dvma;
+	__u32 qreq_dvma = (__u32)qpti->req_dvma;
 	u_short param[6];
 	unsigned short risc_code_addr;
 	int loop_count, i;
@@ -391,8 +393,8 @@  static int qlogicpti_reset_hardware(struct Scsi_Host *host)
 
 	param[0] = MBOX_INIT_RES_QUEUE;
 	param[1] = RES_QUEUE_LEN + 1;
-	param[2] = (u_short) (qpti->res_dvma >> 16);
-	param[3] = (u_short) (qpti->res_dvma & 0xffff);
+	param[2] = (u_short)(qres_dvma >> 16);
+	param[3] = (u_short)(qres_dvma & 0xffff);
 	param[4] = param[5] = 0;
 	if (qlogicpti_mbox_command(qpti, param, 1)) {
 		printk(KERN_EMERG "qlogicpti%d: Cannot init response queue.\n",
@@ -403,8 +405,8 @@  static int qlogicpti_reset_hardware(struct Scsi_Host *host)
 
 	param[0] = MBOX_INIT_REQ_QUEUE;
 	param[1] = QLOGICPTI_REQ_QUEUE_LEN + 1;
-	param[2] = (u_short) (qpti->req_dvma >> 16);
-	param[3] = (u_short) (qpti->req_dvma & 0xffff);
+	param[2] = (u_short)(qreq_dvma >> 16);
+	param[3] = (u_short)(qreq_dvma & 0xffff);
 	param[4] = param[5] = 0;
 	if (qlogicpti_mbox_command(qpti, param, 1)) {
 		printk(KERN_EMERG "qlogicpti%d: Cannot init request queue.\n",
diff --git a/drivers/scsi/qlogicpti.h b/drivers/scsi/qlogicpti.h
index 4377e87..892a0b0 100644
--- a/drivers/scsi/qlogicpti.h
+++ b/drivers/scsi/qlogicpti.h
@@ -356,8 +356,8 @@  struct qlogicpti {
 
 	/* The rest of the elements are unimportant for performance. */
 	struct qlogicpti         *next;
-	__u32                     res_dvma;             /* Ptr to RESPONSE bufs (DVMA)*/
-	__u32                     req_dvma;             /* Ptr to REQUEST bufs (DVMA) */
+	dma_addr_t                res_dvma;             /* Ptr to RESPONSE bufs (DVMA)*/
+	dma_addr_t                req_dvma;             /* Ptr to REQUEST bufs (DVMA) */
 	u_char	                  fware_majrev, fware_minrev, fware_micrev;
 	struct Scsi_Host         *qhost;
 	int                       qpti_id;