[PATCH 0/8] Fix virtual and physical address types

[PATCH 0/8] Fix virtual and physical address types

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

This series fixes compiler warnings on some architectures about implicit
conversions and narrowing conversions between pointer and integer types.

Please apply these to the appropriate trees.

Ben.

Ben Hutchings (8):
  IB/cxgb4: Fix formatting of physical address
  farsync: Fix confusion about DMA address and buffer offset types
  drm: Do not include page offset in argument to virt_to_page()
  drm: Pass pointers to virt_to_page()
  [SCSI] tgt: Pass pointers to virt_to_page(), not integers
  uio: Pass pointers to virt_to_page(), not integers
  rds: Pass pointers to virt_to_page(), not integers
  pmcraid: Pass pointers to access_ok(), not integers

 drivers/gpu/drm/drm_pci.c            |    4 ++--
 drivers/gpu/drm/drm_vm.c             |    2 +-
 drivers/infiniband/hw/cxgb4/device.c |    4 ++--
 drivers/net/wan/farsync.c            |   19 +++++++------------
 drivers/scsi/pmcraid.c               |    3 ++-
 drivers/scsi/scsi_tgt_if.c           |    2 +-
 drivers/uio/uio.c                    |    6 ++++--
 net/rds/message.c                    |    2 +-
 8 files changed, 20 insertions(+), 22 deletions(-)

-- 
1.7.7.3



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 0/8] Fix virtual and physical address types

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

This series fixes compiler warnings on some architectures about implicit
conversions and narrowing conversions between pointer and integer types.

Please apply these to the appropriate trees.

Ben.

Ben Hutchings (8):
  IB/cxgb4: Fix formatting of physical address
  farsync: Fix confusion about DMA address and buffer offset types
  drm: Do not include page offset in argument to virt_to_page()
  drm: Pass pointers to virt_to_page()
  [SCSI] tgt: Pass pointers to virt_to_page(), not integers
  uio: Pass pointers to virt_to_page(), not integers
  rds: Pass pointers to virt_to_page(), not integers
  pmcraid: Pass pointers to access_ok(), not integers

 drivers/gpu/drm/drm_pci.c            |    4 ++--
 drivers/gpu/drm/drm_vm.c             |    2 +-
 drivers/infiniband/hw/cxgb4/device.c |    4 ++--
 drivers/net/wan/farsync.c            |   19 +++++++------------
 drivers/scsi/pmcraid.c               |    3 ++-
 drivers/scsi/scsi_tgt_if.c           |    2 +-
 drivers/uio/uio.c                    |    6 ++++--
 net/rds/message.c                    |    2 +-
 8 files changed, 20 insertions(+), 22 deletions(-)

-- 
1.7.7.3



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Ben Hutchings]

Physical addresses may be wider than virtual addresses (e.g. on i386
with PAE) and must not be formatted with %p.

Signed-off-by: Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org>
---
The resource could alternately be formatted with %Pr.

Ben.

 drivers/infiniband/hw/cxgb4/device.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 6d0df6e..d395623 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -318,10 +318,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
 	     rdev->lldi.vr->qp.size,
 	     rdev->lldi.vr->cq.start,
 	     rdev->lldi.vr->cq.size);
-	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
+	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
 	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
 	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
-	     (void *)pci_resource_start(rdev->lldi.pdev, 2),
+	     (u64)pci_resource_start(rdev->lldi.pdev, 2),
 	     rdev->lldi.db_reg,
 	     rdev->lldi.gts_reg,
 	     rdev->qpshift, rdev->qpmask,
-- 
1.7.7.3



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Ben Hutchings]

Physical addresses may be wider than virtual addresses (e.g. on i386
with PAE) and must not be formatted with %p.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
The resource could alternately be formatted with %Pr.

Ben.

 drivers/infiniband/hw/cxgb4/device.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 6d0df6e..d395623 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -318,10 +318,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
 	     rdev->lldi.vr->qp.size,
 	     rdev->lldi.vr->cq.start,
 	     rdev->lldi.vr->cq.size);
-	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
+	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
 	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
 	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
-	     (void *)pci_resource_start(rdev->lldi.pdev, 2),
+	     (u64)pci_resource_start(rdev->lldi.pdev, 2),
 	     rdev->lldi.db_reg,
 	     rdev->lldi.gts_reg,
 	     rdev->qpshift, rdev->qpmask,
-- 
1.7.7.3

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Joe Perches]

On Wed, 2011-12-21 at 01:32 +0000, Ben Hutchings wrote:
> Physical addresses may be wider than virtual addresses (e.g. on i386
> with PAE) and must not be formatted with %p.
[]
> The resource could alternately be formatted with %Pr.
> diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
[]
> @@ -318,10 +318,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
>  	     rdev->lldi.vr->qp.size,
>  	     rdev->lldi.vr->cq.start,
>  	     rdev->lldi.vr->cq.size);
> -	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
> +	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
>  	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
>  	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
> -	     (void *)pci_resource_start(rdev->lldi.pdev, 2),
> +	     (u64)pci_resource_start(rdev->lldi.pdev, 2),

Probably should be unsigned long long


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Joe Perches]

On Wed, 2011-12-21 at 01:32 +0000, Ben Hutchings wrote:
> Physical addresses may be wider than virtual addresses (e.g. on i386
> with PAE) and must not be formatted with %p.
[]
> The resource could alternately be formatted with %Pr.
> diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
[]
> @@ -318,10 +318,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
>  	     rdev->lldi.vr->qp.size,
>  	     rdev->lldi.vr->cq.start,
>  	     rdev->lldi.vr->cq.size);
> -	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
> +	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
>  	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
>  	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
> -	     (void *)pci_resource_start(rdev->lldi.pdev, 2),
> +	     (u64)pci_resource_start(rdev->lldi.pdev, 2),

Probably should be unsigned long long

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]

On Tue, 2011-12-20 at 18:35 -0800, Joe Perches wrote:
> On Wed, 2011-12-21 at 01:32 +0000, Ben Hutchings wrote:
> > Physical addresses may be wider than virtual addresses (e.g. on i386
> > with PAE) and must not be formatted with %p.
> []
> > The resource could alternately be formatted with %Pr.
> > diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
> []
> > @@ -318,10 +318,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
> >  	     rdev->lldi.vr->qp.size,
> >  	     rdev->lldi.vr->cq.start,
> >  	     rdev->lldi.vr->cq.size);
> > -	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
> > +	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
> >  	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
> >  	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
> > -	     (void *)pci_resource_start(rdev->lldi.pdev, 2),
> > +	     (u64)pci_resource_start(rdev->lldi.pdev, 2),
> 
> Probably should be unsigned long long

unsigned long long is so long long long to type.  And u64 is now defined
as unsigned long long on all architectures, partly because of that.

Ben.

-- 
Ben Hutchings
Humans are not rational beings; they are rationalising beings.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Valdis.Kletnieks-PjAqaU27lzQ]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

On Tue, 20 Dec 2011 18:35:50 PST, Joe Perches said:
> On Wed, 2011-12-21 at 01:32 +0000, Ben Hutchings wrote:
> > Physical addresses may be wider than virtual addresses (e.g. on i386
> > with PAE) and must not be formatted with %p.
> []
> > The resource could alternately be formatted with %Pr.

> Probably should be unsigned long long

I don't have a dog in this fight, but the %Pr sounds better to me.

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

On Tue, 20 Dec 2011 18:35:50 PST, Joe Perches said:
> On Wed, 2011-12-21 at 01:32 +0000, Ben Hutchings wrote:
> > Physical addresses may be wider than virtual addresses (e.g. on i386
> > with PAE) and must not be formatted with %p.
> []
> > The resource could alternately be formatted with %Pr.

> Probably should be unsigned long long

I don't have a dog in this fight, but the %Pr sounds better to me.

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Steve Wise]

Acked-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Steve Wise]

Acked-by: Steve Wise <swise@opengridcomputing.com>

[PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2679 bytes --]

Consistently use dma_addr_t for DMA addresses, except in
fst_{rx,tx}_dma where we truncate them to 32 bits.  (We know this is
OK because that's the default DMA mask for PCI devices.)

Consistently use u32 for buffer offsets within shared memory.

Compile-tested only.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/net/wan/farsync.c |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wan/farsync.c b/drivers/net/wan/farsync.c
index ebb9f24..98c04c6 100644
--- a/drivers/net/wan/farsync.c
+++ b/drivers/net/wan/farsync.c
@@ -886,15 +886,13 @@ fst_rx_dma_complete(struct fst_card_info *card, struct fst_port_info *port,
  *      Receive a frame through the DMA
  */
 static inline void
-fst_rx_dma(struct fst_card_info *card, dma_addr_t skb,
-	   dma_addr_t mem, int len)
+fst_rx_dma(struct fst_card_info *card, dma_addr_t skb, u32 mem, int len)
 {
 	/*
 	 * This routine will setup the DMA and start it
 	 */
 
-	dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n",
-	    (unsigned long) skb, (unsigned long) mem, len);
+	dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len);
 	if (card->dmarx_in_progress) {
 		dbg(DBG_ASS, "In fst_rx_dma while dma in progress\n");
 	}
@@ -915,20 +913,19 @@ fst_rx_dma(struct fst_card_info *card, dma_addr_t skb,
  *      Send a frame through the DMA
  */
 static inline void
-fst_tx_dma(struct fst_card_info *card, unsigned char *skb,
-	   unsigned char *mem, int len)
+fst_tx_dma(struct fst_card_info *card, dma_addr_t skb, u32 mem, int len)
 {
 	/*
 	 * This routine will setup the DMA and start it.
 	 */
 
-	dbg(DBG_TX, "In fst_tx_dma %p %p %d\n", skb, mem, len);
+	dbg(DBG_TX, "In fst_tx_dma %x %x %d\n", (u32)skb, mem, len);
 	if (card->dmatx_in_progress) {
 		dbg(DBG_ASS, "In fst_tx_dma while dma in progress\n");
 	}
 
-	outl((unsigned long) skb, card->pci_conf + DMAPADR1);	/* Copy from here */
-	outl((unsigned long) mem, card->pci_conf + DMALADR1);	/* to here */
+	outl(skb, card->pci_conf + DMAPADR1);	/* Copy from here */
+	outl(mem, card->pci_conf + DMALADR1);	/* to here */
 	outl(len, card->pci_conf + DMASIZ1);	/* for this length */
 	outl(0x000000004, card->pci_conf + DMADPR1);	/* In this direction */
 
@@ -1405,9 +1402,7 @@ do_bottom_half_tx(struct fst_card_info *card)
 					card->dma_len_tx = skb->len;
 					card->dma_txpos = port->txpos;
 					fst_tx_dma(card,
-						   (char *) card->
-						   tx_dma_handle_card,
-						   (char *)
+						   card->tx_dma_handle_card,
 						   BUF_OFFSET(txBuffer[pi]
 							      [port->txpos][0]),
 						   skb->len);
-- 
1.7.7.3




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 415 bytes --]

I accidentally left signing enabled for this one, which I had intended
to disable since you've said that patchwork doesn't deal with
signatures.  Except this looks OK on patchwork:
<http://patchwork.ozlabs.org/patch/132548/>.

If that also looks OK to you, is there any reason to avoid signing
networking patches?

Ben.

-- 
Ben Hutchings
Humans are not rational beings; they are rationalising beings.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

[David Miller]

From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 21 Dec 2011 01:42:26 +0000

> I accidentally left signing enabled for this one, which I had intended
> to disable since you've said that patchwork doesn't deal with
> signatures.  Except this looks OK on patchwork:
> <http://patchwork.ozlabs.org/patch/132548/>.
> 
> If that also looks OK to you, is there any reason to avoid signing
> networking patches?

What do you mean exactly by "signing".

Do you mean simply the "Signed-off-by: " tag, which you should always
add to your own patches, or something else?

Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 775 bytes --]

On Tue, 2011-12-20 at 23:48 -0500, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Wed, 21 Dec 2011 01:42:26 +0000
> 
> > I accidentally left signing enabled for this one, which I had intended
> > to disable since you've said that patchwork doesn't deal with
> > signatures.  Except this looks OK on patchwork:
> > <http://patchwork.ozlabs.org/patch/132548/>.
> > 
> > If that also looks OK to you, is there any reason to avoid signing
> > networking patches?
> 
> What do you mean exactly by "signing".
> 
> Do you mean simply the "Signed-off-by: " tag, which you should always
> add to your own patches, or something else?

GPG signing.

Ben.

-- 
Ben Hutchings
Humans are not rational beings; they are rationalising beings.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

[David Miller]

From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 21 Dec 2011 01:32:52 +0000

> -	dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n",
> -	    (unsigned long) skb, (unsigned long) mem, len);
> +	dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len);

This is more appropriately fixed by using "%p" instead of casting
to a 32-bit int.

Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

On Tue, 2011-12-20 at 23:46 -0500, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Wed, 21 Dec 2011 01:32:52 +0000
> 
> > -	dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n",
> > -	    (unsigned long) skb, (unsigned long) mem, len);
> > +	dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len);
> 
> This is more appropriately fixed by using "%p" instead of casting
> to a 32-bit int.

'skb' is the physical address of the data in the skb.  Whereas 'mem' is
the offset of the corresponding buffer in shared memory.

All completely clear, right? ;-)

Ben.

-- 
Ben Hutchings
Humans are not rational beings; they are rationalising beings.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

RE: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

[Kevin Curtis]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 1387 bytes --]

Hi,
    Thanks for the patches.  I will apply them to the master copy of the source code here.


Regards


Kevin Curtis
Linux Development
FarSite Communications Ltd http://www.farsite.com
Winner of The Queen's Award for Enterprise 2009
tel:  +44 1256 330461
fax:  +44 1256 854931



-----Original Message-----
From: Ben Hutchings [mailto:ben@decadent.org.uk] 
Sent: 21 December 2011 05:41
To: David Miller
Cc: Kevin Curtis; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

On Tue, 2011-12-20 at 23:46 -0500, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Wed, 21 Dec 2011 01:32:52 +0000
> 
> > -	dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n",
> > -	    (unsigned long) skb, (unsigned long) mem, len);
> > +	dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len);
> 
> This is more appropriately fixed by using "%p" instead of casting to a 
> 32-bit int.

'skb' is the physical address of the data in the skb.  Whereas 'mem' is the offset of the corresponding buffer in shared memory.

All completely clear, right? ;-)

Ben.

--
Ben Hutchings
Humans are not rational beings; they are rationalising beings.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

[Kevin Curtis]

Hi,
    Thanks for the patches.  I will apply them to the master copy of the source code here.


Regards


Kevin Curtis
Linux Development
FarSite Communications Ltd http://www.farsite.com
Winner of The Queen's Award for Enterprise 2009
tel:  +44 1256 330461
fax:  +44 1256 854931



-----Original Message-----
From: Ben Hutchings [mailto:ben@decadent.org.uk] 
Sent: 21 December 2011 05:41
To: David Miller
Cc: Kevin Curtis; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types

On Tue, 2011-12-20 at 23:46 -0500, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Wed, 21 Dec 2011 01:32:52 +0000
> 
> > -	dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n",
> > -	    (unsigned long) skb, (unsigned long) mem, len);
> > +	dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len);
> 
> This is more appropriately fixed by using "%p" instead of casting to a 
> 32-bit int.

'skb' is the physical address of the data in the skb.  Whereas 'mem' is the offset of the corresponding buffer in shared memory.

All completely clear, right? ;-)

Ben.

--
Ben Hutchings
Humans are not rational beings; they are rationalising beings.

[PATCH 3/8] drm: Do not include page offset in argument to virt_to_page()

[Ben Hutchings]

By definition, the page offset will not affect the result.

Compile-tested only.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/gpu/drm/drm_vm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 8c03eaf..98c3922 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -306,7 +306,7 @@ static int drm_do_vm_dma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 
 	offset = (unsigned long)vmf->virtual_address - vma->vm_start;	/* vm_[pg]off[set] should be 0 */
 	page_nr = offset >> PAGE_SHIFT; /* page_nr could just be vmf->pgoff */
-	page = virt_to_page((dma->pagelist[page_nr] + (offset & (~PAGE_MASK))));
+	page = virt_to_page(dma->pagelist[page_nr]);
 
 	get_page(page);
 	vmf->page = page;
-- 
1.7.7.3

[PATCH 4/8] drm: Pass pointers to virt_to_page()

[Ben Hutchings]

Most architectures define virt_to_page() as a macro that casts its
argument such that an argument of type unsigned long will be accepted
without complaint.  However, the proper type is void *, and passing
unsigned long results in a warning on MIPS.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/gpu/drm/drm_pci.c |    4 ++--
 drivers/gpu/drm/drm_vm.c  |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index d4d10b7..2e19fcc 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -82,7 +82,7 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
 	/* Reserve */
 	for (addr = (unsigned long)dmah->vaddr, sz = size;
 	     sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
-		SetPageReserved(virt_to_page(addr));
+		SetPageReserved(virt_to_page((void *)addr));
 	}
 
 	return dmah;
@@ -107,7 +107,7 @@ void __drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
 		/* Unreserve */
 		for (addr = (unsigned long)dmah->vaddr, sz = dmah->size;
 		     sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {
-			ClearPageReserved(virt_to_page(addr));
+			ClearPageReserved(virt_to_page((void *)addr));
 		}
 		dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr,
 				  dmah->busaddr);
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 98c3922..8cc1c06 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -306,7 +306,7 @@ static int drm_do_vm_dma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 
 	offset = (unsigned long)vmf->virtual_address - vma->vm_start;	/* vm_[pg]off[set] should be 0 */
 	page_nr = offset >> PAGE_SHIFT; /* page_nr could just be vmf->pgoff */
-	page = virt_to_page(dma->pagelist[page_nr]);
+	page = virt_to_page((void *)dma->pagelist[page_nr]);
 
 	get_page(page);
 	vmf->page = page;
-- 
1.7.7.3

[PATCH 5/8] [SCSI] tgt: Pass pointers to virt_to_page(), not integers

[Ben Hutchings]

Most architectures define virt_to_page() as a macro that casts its
argument such that an argument of type unsigned long will be accepted
without complaint.  However, the proper type is void *, and passing
unsigned long results in a warning on MIPS.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/scsi/scsi_tgt_if.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_tgt_if.c b/drivers/scsi/scsi_tgt_if.c
index 6209110..7199753 100644
--- a/drivers/scsi/scsi_tgt_if.c
+++ b/drivers/scsi/scsi_tgt_if.c
@@ -286,7 +286,7 @@ static int uspace_ring_map(struct vm_area_struct *vma, unsigned long addr,
 	int i, err;
 
 	for (i = 0; i < TGT_RING_PAGES; i++) {
-		struct page *page = virt_to_page(ring->tr_pages[i]);
+		struct page *page = virt_to_page((void *)ring->tr_pages[i]);
 		err = vm_insert_page(vma, addr, page);
 		if (err)
 			return err;
-- 
1.7.7.3

[PATCH 6/8] uio: Pass pointers to virt_to_page(), not integers

[Ben Hutchings]

Most architectures define virt_to_page() as a macro that casts its
argument such that an argument of type unsigned long will be accepted
without complaint.  However, the proper type is void *, and passing
unsigned long results in a warning on MIPS.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
It might be clearer to assign:

	addr = (void *)((unsigned long)idev->info->mem[mi].addr + offset);

since arithmetic on void pointers is weird.  Either way should work though.

Ben.

 drivers/uio/uio.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index a783d53..1ff7927 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -620,6 +620,7 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	struct uio_device *idev = vma->vm_private_data;
 	struct page *page;
 	unsigned long offset;
+	void *addr;
 
 	int mi = uio_find_mem_index(vma);
 	if (mi < 0)
@@ -631,10 +632,11 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	 */
 	offset = (vmf->pgoff - mi) << PAGE_SHIFT;
 
+	addr = (void *)(unsigned long)idev->info->mem[mi].addr + offset;
 	if (idev->info->mem[mi].memtype == UIO_MEM_LOGICAL)
-		page = virt_to_page(idev->info->mem[mi].addr + offset);
+		page = virt_to_page(addr);
 	else
-		page = vmalloc_to_page((void *)(unsigned long)idev->info->mem[mi].addr + offset);
+		page = vmalloc_to_page(addr);
 	get_page(page);
 	vmf->page = page;
 	return 0;
-- 
1.7.7.3

Re: [PATCH 6/8] uio: Pass pointers to virt_to_page(), not integers

[Hans J. Koch]

On Wed, Dec 21, 2011 at 01:36:33AM +0000, Ben Hutchings wrote:
> Most architectures define virt_to_page() as a macro that casts its
> argument such that an argument of type unsigned long will be accepted
> without complaint.  However, the proper type is void *, and passing
> unsigned long results in a warning on MIPS.

Not only on MIPS... This is already fixed in Greg's tree and will go
to mainline in the next merge window.

Thanks,
Hans

> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> It might be clearer to assign:
> 
> 	addr = (void *)((unsigned long)idev->info->mem[mi].addr + offset);
> 
> since arithmetic on void pointers is weird.  Either way should work though.
> 
> Ben.
> 
>  drivers/uio/uio.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index a783d53..1ff7927 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -620,6 +620,7 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	struct uio_device *idev = vma->vm_private_data;
>  	struct page *page;
>  	unsigned long offset;
> +	void *addr;
>  
>  	int mi = uio_find_mem_index(vma);
>  	if (mi < 0)
> @@ -631,10 +632,11 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	 */
>  	offset = (vmf->pgoff - mi) << PAGE_SHIFT;
>  
> +	addr = (void *)(unsigned long)idev->info->mem[mi].addr + offset;
>  	if (idev->info->mem[mi].memtype == UIO_MEM_LOGICAL)
> -		page = virt_to_page(idev->info->mem[mi].addr + offset);
> +		page = virt_to_page(addr);
>  	else
> -		page = vmalloc_to_page((void *)(unsigned long)idev->info->mem[mi].addr + offset);
> +		page = vmalloc_to_page(addr);
>  	get_page(page);
>  	vmf->page = page;
>  	return 0;
> -- 
> 1.7.7.3
> 
> 
> 
> 

[PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers

[Ben Hutchings]

Most architectures define virt_to_page() as a macro that casts its
argument such that an argument of type unsigned long will be accepted
without complaint.  However, the proper type is void *, and passing
unsigned long results in a warning on MIPS.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 net/rds/message.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/rds/message.c b/net/rds/message.c
index f0a4658..ccd3de5 100644
--- a/net/rds/message.c
+++ b/net/rds/message.c
@@ -257,7 +257,7 @@ struct rds_message *rds_message_map_pages(unsigned long *page_addrs, unsigned in
 
 	for (i = 0; i < rm->data.op_nents; ++i) {
 		sg_set_page(&rm->data.op_sg[i],
-				virt_to_page(page_addrs[i]),
+				virt_to_page((void *)page_addrs[i]),
 				PAGE_SIZE, 0);
 	}
 
-- 
1.7.7.3

[PATCH 8/8] pmcraid: Pass pointers to access_ok(), not integers

[Ben Hutchings]

Most architectures define access_ok() as a macro that casts its
argument such that an argument of type unsigned long will be accepted
without complaint.  However, the proper type is void *, and passing
unsigned long results in a warning on sparc64.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/scsi/pmcraid.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 5163edb..01b29a6 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -3807,7 +3807,8 @@ static long pmcraid_ioctl_passthrough(
 	}
 
 	if (request_size > 0) {
-		rc = access_ok(access, arg, request_offset + request_size);
+		rc = access_ok(access, (void *)arg,
+			       request_offset + request_size);
 
 		if (!rc) {
 			rc = -EFAULT;
-- 
1.7.7.3

[PATCH 0/8] Fix minor address type errors

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]

Various bits of code are mixing making assumptions about the size of
dma_addr_t or resource_size_t, or mixing up pointer and integer types.

All these fixes are based on compiler warnings and so far as I can see
the bugs are practically harmless.

Ben.

Ben Hutchings (8):
  IB/cxgb4: Fix formatting of physical address
  farsync: Fix confusion about DMA address and buffer offset types
  drm: Do not include page offset in argument to virt_to_page()
  drm: Pass pointers to virt_to_page()
  [SCSI] tgt: Pass pointers to virt_to_page(), not integers
  uio: Pass pointers to virt_to_page(), not integers
  rds: Pass pointers to virt_to_page(), not integers
  [SCSI] pmcraid: Pass pointers to access_ok(), not integers

 drivers/gpu/drm/drm_pci.c            |  4 ++--
 drivers/gpu/drm/drm_vm.c             |  2 +-
 drivers/infiniband/hw/cxgb4/device.c |  4 ++--
 drivers/net/wan/farsync.c            | 19 +++++++------------
 drivers/scsi/pmcraid.c               |  3 ++-
 drivers/scsi/scsi_tgt_if.c           |  2 +-
 drivers/uio/uio.c                    |  6 ++++--
 net/rds/message.c                    |  2 +-
 8 files changed, 20 insertions(+), 22 deletions(-)


-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

Physical addresses may be wider than virtual addresses (e.g. on i386
with PAE) and must not be formatted with %p.

Compile-tested only.

Signed-off-by: Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org>
---
The resource could also be printed using '%pR' or '%pr', but that makes
a bigger change to the output.

Ben.

 drivers/infiniband/hw/cxgb4/device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 33d2cc6..4a03385 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -602,10 +602,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
 	     rdev->lldi.vr->qp.size,
 	     rdev->lldi.vr->cq.start,
 	     rdev->lldi.vr->cq.size);
-	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
+	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
 	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
 	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
-	     (void *)(unsigned long)pci_resource_start(rdev->lldi.pdev, 2),
+	     (u64)pci_resource_start(rdev->lldi.pdev, 2),
 	     rdev->lldi.db_reg,
 	     rdev->lldi.gts_reg,
 	     rdev->qpshift, rdev->qpmask,


-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

Physical addresses may be wider than virtual addresses (e.g. on i386
with PAE) and must not be formatted with %p.

Compile-tested only.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
The resource could also be printed using '%pR' or '%pr', but that makes
a bigger change to the output.

Ben.

 drivers/infiniband/hw/cxgb4/device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 33d2cc6..4a03385 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -602,10 +602,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
 	     rdev->lldi.vr->qp.size,
 	     rdev->lldi.vr->cq.start,
 	     rdev->lldi.vr->cq.size);
-	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
+	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
 	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
 	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
-	     (void *)(unsigned long)pci_resource_start(rdev->lldi.pdev, 2),
+	     (u64)pci_resource_start(rdev->lldi.pdev, 2),
 	     rdev->lldi.db_reg,
 	     rdev->lldi.gts_reg,
 	     rdev->qpshift, rdev->qpmask,


-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Joe Perches]

On Sun, 2013-10-27 at 21:50 +0000, Ben Hutchings wrote:
> Physical addresses may be wider than virtual addresses (e.g. on i386
> with PAE) and must not be formatted with %p.

%pa works.  %pa also prefixes with 0x.

> diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
[]
> @@ -602,10 +602,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
>  	     rdev->lldi.vr->qp.size,
>  	     rdev->lldi.vr->cq.start,
>  	     rdev->lldi.vr->cq.size);
> -	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
> +	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
>  	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
>  	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
> -	     (void *)(unsigned long)pci_resource_start(rdev->lldi.pdev, 2),
> +	     (u64)pci_resource_start(rdev->lldi.pdev, 2),
>  	     rdev->lldi.db_reg,
>  	     rdev->lldi.gts_reg,
>  	     rdev->qpshift, rdev->qpmask,
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Joe Perches]

On Sun, 2013-10-27 at 21:50 +0000, Ben Hutchings wrote:
> Physical addresses may be wider than virtual addresses (e.g. on i386
> with PAE) and must not be formatted with %p.

%pa works.  %pa also prefixes with 0x.

> diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
[]
> @@ -602,10 +602,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
>  	     rdev->lldi.vr->qp.size,
>  	     rdev->lldi.vr->cq.start,
>  	     rdev->lldi.vr->cq.size);
> -	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
> +	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
>  	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
>  	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
> -	     (void *)(unsigned long)pci_resource_start(rdev->lldi.pdev, 2),
> +	     (u64)pci_resource_start(rdev->lldi.pdev, 2),
>  	     rdev->lldi.db_reg,
>  	     rdev->lldi.gts_reg,
>  	     rdev->qpshift, rdev->qpmask,
> 
> 

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

On Sun, 2013-10-27 at 14:58 -0700, Joe Perches wrote:
> On Sun, 2013-10-27 at 21:50 +0000, Ben Hutchings wrote:
> > Physical addresses may be wider than virtual addresses (e.g. on i386
> > with PAE) and must not be formatted with %p.
> 
> %pa works.  %pa also prefixes with 0x.

Only as long as pci_resource_start() happens to be an lvalue.  I'd
rather not introduce that assumption.

Ben.

> > diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
> []
> > @@ -602,10 +602,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
> >  	     rdev->lldi.vr->qp.size,
> >  	     rdev->lldi.vr->cq.start,
> >  	     rdev->lldi.vr->cq.size);
> > -	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
> > +	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
> >  	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
> >  	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
> > -	     (void *)(unsigned long)pci_resource_start(rdev->lldi.pdev, 2),
> > +	     (u64)pci_resource_start(rdev->lldi.pdev, 2),
> >  	     rdev->lldi.db_reg,
> >  	     rdev->lldi.gts_reg,
> >  	     rdev->qpshift, rdev->qpmask,
> > 
> > 
> 
> 
> 

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

On Sun, 2013-10-27 at 14:58 -0700, Joe Perches wrote:
> On Sun, 2013-10-27 at 21:50 +0000, Ben Hutchings wrote:
> > Physical addresses may be wider than virtual addresses (e.g. on i386
> > with PAE) and must not be formatted with %p.
> 
> %pa works.  %pa also prefixes with 0x.

Only as long as pci_resource_start() happens to be an lvalue.  I'd
rather not introduce that assumption.

Ben.

> > diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
> []
> > @@ -602,10 +602,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
> >  	     rdev->lldi.vr->qp.size,
> >  	     rdev->lldi.vr->cq.start,
> >  	     rdev->lldi.vr->cq.size);
> > -	PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu "
> > +	PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu "
> >  	     "qpmask 0x%x cqshift %lu cqmask 0x%x\n",
> >  	     (unsigned)pci_resource_len(rdev->lldi.pdev, 2),
> > -	     (void *)(unsigned long)pci_resource_start(rdev->lldi.pdev, 2),
> > +	     (u64)pci_resource_start(rdev->lldi.pdev, 2),
> >  	     rdev->lldi.db_reg,
> >  	     rdev->lldi.gts_reg,
> >  	     rdev->qpshift, rdev->qpmask,
> > 
> > 
> 
> 
> 

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Joe Perches]

On Sun, 2013-10-27 at 22:02 +0000, Ben Hutchings wrote:
> On Sun, 2013-10-27 at 14:58 -0700, Joe Perches wrote:
> > On Sun, 2013-10-27 at 21:50 +0000, Ben Hutchings wrote:
> > > Physical addresses may be wider than virtual addresses (e.g. on i386
> > > with PAE) and must not be formatted with %p.
> > 
> > %pa works.  %pa also prefixes with 0x.
> 
> Only as long as pci_resource_start() happens to be an lvalue.  I'd
> rather not introduce that assumption.

pci_resource_start returns a resource_size_t which is a phys_addr_t.

Changing that from an lvalue would cause a _lot_ of breakage.


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Joe Perches]

On Sun, 2013-10-27 at 22:02 +0000, Ben Hutchings wrote:
> On Sun, 2013-10-27 at 14:58 -0700, Joe Perches wrote:
> > On Sun, 2013-10-27 at 21:50 +0000, Ben Hutchings wrote:
> > > Physical addresses may be wider than virtual addresses (e.g. on i386
> > > with PAE) and must not be formatted with %p.
> > 
> > %pa works.  %pa also prefixes with 0x.
> 
> Only as long as pci_resource_start() happens to be an lvalue.  I'd
> rather not introduce that assumption.

pci_resource_start returns a resource_size_t which is a phys_addr_t.

Changing that from an lvalue would cause a _lot_ of breakage.

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Sun, 2013-10-27 at 15:14 -0700, Joe Perches wrote:
> On Sun, 2013-10-27 at 22:02 +0000, Ben Hutchings wrote:
> > On Sun, 2013-10-27 at 14:58 -0700, Joe Perches wrote:
> > > On Sun, 2013-10-27 at 21:50 +0000, Ben Hutchings wrote:
> > > > Physical addresses may be wider than virtual addresses (e.g. on i386
> > > > with PAE) and must not be formatted with %p.
> > > 
> > > %pa works.  %pa also prefixes with 0x.
> > 
> > Only as long as pci_resource_start() happens to be an lvalue.  I'd
> > rather not introduce that assumption.
> 
> pci_resource_start returns a resource_size_t which is a phys_addr_t.
> 
> Changing that from an lvalue would cause a _lot_ of breakage.

I don't think so.  This doesn't find anything:
    git grep '&[ (]*pci_resource_start'

and I was able to build drivers/{net,pci,scsi}/ successfully with
pci_resource_start() changed to an inline function.

Ben.

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Sun, 2013-10-27 at 15:14 -0700, Joe Perches wrote:
> On Sun, 2013-10-27 at 22:02 +0000, Ben Hutchings wrote:
> > On Sun, 2013-10-27 at 14:58 -0700, Joe Perches wrote:
> > > On Sun, 2013-10-27 at 21:50 +0000, Ben Hutchings wrote:
> > > > Physical addresses may be wider than virtual addresses (e.g. on i386
> > > > with PAE) and must not be formatted with %p.
> > > 
> > > %pa works.  %pa also prefixes with 0x.
> > 
> > Only as long as pci_resource_start() happens to be an lvalue.  I'd
> > rather not introduce that assumption.
> 
> pci_resource_start returns a resource_size_t which is a phys_addr_t.
> 
> Changing that from an lvalue would cause a _lot_ of breakage.

I don't think so.  This doesn't find anything:
    git grep '&[ (]*pci_resource_start'

and I was able to build drivers/{net,pci,scsi}/ successfully with
pci_resource_start() changed to an inline function.

Ben.

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address

[Joe Perches]

On Sun, 2013-10-27 at 22:26 +0000, Ben Hutchings wrote:
> I don't think so.  This doesn't find anything:
>     git grep '&[ (]*pci_resource_start'
> and I was able to build drivers/{net,pci,scsi}/ successfully with
> pci_resource_start() changed to an inline function.

Hi again Ben.  You're right.

It could be nice though to have some mechanism to get from
a pci_resource_start/end that could be emitted via %pa
without an intermediate or casting like
(unsigned long long)pci_resource_<foo>(struct resource *)

There are at least a few dozen uses of dmesg/seq_printf with
%llx or %lx and (some_cast)pci_resource_<foo> that could be
converted.