All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] vme: fake: fix build for 64-bit dma_addr_t
@ 2016-09-06 12:59 Arnd Bergmann
  2016-09-06 19:39 ` Dan Carpenter
  2016-09-08 19:10 ` Martyn Welch
  0 siblings, 2 replies; 4+ messages in thread
From: Arnd Bergmann @ 2016-09-06 12:59 UTC (permalink / raw)
  To: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman
  Cc: Arnd Bergmann, devel, linux-kernel

casting between dma_addr_t and a pointer is generally tricky,
as they might not be the same size and almost never point into
the same address space. With 32-bit ARM systems and LPAE, we
get this warning for the vme_fake driver that stores a pointer
in a dma_addr_t variable:

drivers/vme/bridges/vme_fake.c: In function 'fake_slave_set':
drivers/vme/bridges/vme_fake.c:204:29: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]

To make this clearer while fixing the warning, I'm adding
a set of helper functions for the type conversion.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/vme/bridges/vme_fake.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/vme/bridges/vme_fake.c b/drivers/vme/bridges/vme_fake.c
index 7ef298b289f4..ebf35d305321 100644
--- a/drivers/vme/bridges/vme_fake.c
+++ b/drivers/vme/bridges/vme_fake.c
@@ -48,7 +48,7 @@ struct fake_slave_window {
 	int enabled;
 	unsigned long long vme_base;
 	unsigned long long size;
-	dma_addr_t buf_base;
+	void *buf_base;
 	u32 aspace;
 	u32 cycle;
 };
@@ -114,6 +114,16 @@ static void fake_irq_set(struct vme_bridge *fake_bridge, int level,
 	/* Nothing to do */
 }
 
+static void *fake_pci_to_ptr(dma_addr_t addr)
+{
+	return (void *)(uintptr_t)addr;
+}
+
+static dma_addr_t fake_ptr_to_pci(void *addr)
+{
+	return (dma_addr_t)(uintptr_t)addr;
+}
+
 /*
  * Generate a VME bus interrupt at the requested level & vector. Wait for
  * interrupt to be acked.
@@ -202,7 +212,7 @@ static int fake_slave_set(struct vme_slave_resource *image, int enabled,
 	bridge->slaves[i].enabled = enabled;
 	bridge->slaves[i].vme_base = vme_base;
 	bridge->slaves[i].size = size;
-	bridge->slaves[i].buf_base = buf_base;
+	bridge->slaves[i].buf_base = fake_pci_to_ptr(buf_base);
 	bridge->slaves[i].aspace = aspace;
 	bridge->slaves[i].cycle = cycle;
 
@@ -230,7 +240,7 @@ static int fake_slave_get(struct vme_slave_resource *image, int *enabled,
 	*enabled = bridge->slaves[i].enabled;
 	*vme_base = bridge->slaves[i].vme_base;
 	*size = bridge->slaves[i].size;
-	*buf_base = bridge->slaves[i].buf_base;
+	*buf_base = fake_ptr_to_pci(bridge->slaves[i].buf_base);
 	*aspace = bridge->slaves[i].aspace;
 	*cycle = bridge->slaves[i].cycle;
 
@@ -431,7 +441,7 @@ static u8 fake_vmeread8(struct fake_driver *bridge, unsigned long long addr,
 
 		if ((addr >= start) && (addr < end)) {
 			offset = addr - bridge->slaves[i].vme_base;
-			loc = (u8 *)((void *)bridge->slaves[i].buf_base + offset);
+			loc = (u8 *)(bridge->slaves[i].buf_base + offset);
 			retval = *loc;
 
 			break;
@@ -463,7 +473,7 @@ static u16 fake_vmeread16(struct fake_driver *bridge, unsigned long long addr,
 
 		if ((addr >= start) && ((addr + 1) < end)) {
 			offset = addr - bridge->slaves[i].vme_base;
-			loc = (u16 *)((void *)bridge->slaves[i].buf_base + offset);
+			loc = (u16 *)(bridge->slaves[i].buf_base + offset);
 			retval = *loc;
 
 			break;
@@ -495,7 +505,7 @@ static u32 fake_vmeread32(struct fake_driver *bridge, unsigned long long addr,
 
 		if ((addr >= start) && ((addr + 3) < end)) {
 			offset = addr - bridge->slaves[i].vme_base;
-			loc = (u32 *)((void *)bridge->slaves[i].buf_base + offset);
+			loc = (u32 *)(bridge->slaves[i].buf_base + offset);
 			retval = *loc;
 
 			break;
@@ -997,7 +1007,7 @@ static void *fake_alloc_consistent(struct device *parent, size_t size,
 	void *alloc = kmalloc(size, GFP_KERNEL);
 
 	if (alloc != NULL)
-		*dma = (dma_addr_t)(unsigned long)alloc;
+		*dma = fake_ptr_to_pci(alloc);
 
 	return alloc;
 }
@@ -1031,7 +1041,7 @@ static int fake_crcsr_init(struct vme_bridge *fake_bridge)
 
 	/* Allocate mem for CR/CSR image */
 	bridge->crcsr_kernel = kzalloc(VME_CRCSR_BUF_SIZE, GFP_KERNEL);
-	bridge->crcsr_bus = (dma_addr_t)bridge->crcsr_kernel;
+	bridge->crcsr_bus = fake_ptr_to_pci(bridge->crcsr_kernel);
 	if (bridge->crcsr_kernel == NULL)
 		return -ENOMEM;
 
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] vme: fake: fix build for 64-bit dma_addr_t
  2016-09-06 12:59 [PATCH] vme: fake: fix build for 64-bit dma_addr_t Arnd Bergmann
@ 2016-09-06 19:39 ` Dan Carpenter
  2016-09-06 20:21   ` Arnd Bergmann
  2016-09-08 19:10 ` Martyn Welch
  1 sibling, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2016-09-06 19:39 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, devel, linux-kernel

On Tue, Sep 06, 2016 at 02:59:41PM +0200, Arnd Bergmann wrote:
> casting between dma_addr_t and a pointer is generally tricky,
> as they might not be the same size and almost never point into
> the same address space. With 32-bit ARM systems and LPAE, we
> get this warning for the vme_fake driver that stores a pointer
> in a dma_addr_t variable:
> 
> drivers/vme/bridges/vme_fake.c: In function 'fake_slave_set':
> drivers/vme/bridges/vme_fake.c:204:29: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
> 
> To make this clearer while fixing the warning, I'm adding
> a set of helper functions for the type conversion.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/vme/bridges/vme_fake.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/vme/bridges/vme_fake.c b/drivers/vme/bridges/vme_fake.c
> index 7ef298b289f4..ebf35d305321 100644
> --- a/drivers/vme/bridges/vme_fake.c
> +++ b/drivers/vme/bridges/vme_fake.c
> @@ -48,7 +48,7 @@ struct fake_slave_window {
>  	int enabled;
>  	unsigned long long vme_base;
>  	unsigned long long size;
> -	dma_addr_t buf_base;
> +	void *buf_base;

This shouldn't be u64?  (I don't know the answer).

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] vme: fake: fix build for 64-bit dma_addr_t
  2016-09-06 19:39 ` Dan Carpenter
@ 2016-09-06 20:21   ` Arnd Bergmann
  0 siblings, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2016-09-06 20:21 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, devel, linux-kernel

On Tuesday, September 6, 2016 10:39:25 PM CEST Dan Carpenter wrote:
> On Tue, Sep 06, 2016 at 02:59:41PM +0200, Arnd Bergmann wrote:
> > casting between dma_addr_t and a pointer is generally tricky,
> > as they might not be the same size and almost never point into
> > the same address space. With 32-bit ARM systems and LPAE, we
> > get this warning for the vme_fake driver that stores a pointer
> > in a dma_addr_t variable:
> > 
> > drivers/vme/bridges/vme_fake.c: In function 'fake_slave_set':
> > drivers/vme/bridges/vme_fake.c:204:29: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
> > 
> > To make this clearer while fixing the warning, I'm adding
> > a set of helper functions for the type conversion.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/vme/bridges/vme_fake.c | 26 ++++++++++++++++++--------
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/vme/bridges/vme_fake.c b/drivers/vme/bridges/vme_fake.c
> > index 7ef298b289f4..ebf35d305321 100644
> > --- a/drivers/vme/bridges/vme_fake.c
> > +++ b/drivers/vme/bridges/vme_fake.c
> > @@ -48,7 +48,7 @@ struct fake_slave_window {
> >       int enabled;
> >       unsigned long long vme_base;
> >       unsigned long long size;
> > -     dma_addr_t buf_base;
> > +     void *buf_base;
> 
> This shouldn't be u64?  (I don't know the answer).

I changed it to 'void *' because it gets accessed as a pointer in kernel
space. It gets passed through the vme core code in a dma_addr_t, but
it seemed easier to do the conversion at the time it gets passed in and
out rather than the time it gets accessed.

An alternative that would work equally well is to make it uintptr_t.

	Arnd

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] vme: fake: fix build for 64-bit dma_addr_t
  2016-09-06 12:59 [PATCH] vme: fake: fix build for 64-bit dma_addr_t Arnd Bergmann
  2016-09-06 19:39 ` Dan Carpenter
@ 2016-09-08 19:10 ` Martyn Welch
  1 sibling, 0 replies; 4+ messages in thread
From: Martyn Welch @ 2016-09-08 19:10 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Manohar Vanga, Greg Kroah-Hartman, devel, linux-kernel

On Tue, Sep 06, 2016 at 02:59:41PM +0200, Arnd Bergmann wrote:
> casting between dma_addr_t and a pointer is generally tricky,
> as they might not be the same size and almost never point into
> the same address space. With 32-bit ARM systems and LPAE, we
> get this warning for the vme_fake driver that stores a pointer
> in a dma_addr_t variable:
> 
> drivers/vme/bridges/vme_fake.c: In function 'fake_slave_set':
> drivers/vme/bridges/vme_fake.c:204:29: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
> 
> To make this clearer while fixing the warning, I'm adding
> a set of helper functions for the type conversion.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Martyn Welch <martyn@welchs.me.uk>

> ---
>  drivers/vme/bridges/vme_fake.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/vme/bridges/vme_fake.c b/drivers/vme/bridges/vme_fake.c
> index 7ef298b289f4..ebf35d305321 100644
> --- a/drivers/vme/bridges/vme_fake.c
> +++ b/drivers/vme/bridges/vme_fake.c
> @@ -48,7 +48,7 @@ struct fake_slave_window {
>  	int enabled;
>  	unsigned long long vme_base;
>  	unsigned long long size;
> -	dma_addr_t buf_base;
> +	void *buf_base;
>  	u32 aspace;
>  	u32 cycle;
>  };
> @@ -114,6 +114,16 @@ static void fake_irq_set(struct vme_bridge *fake_bridge, int level,
>  	/* Nothing to do */
>  }
>  
> +static void *fake_pci_to_ptr(dma_addr_t addr)
> +{
> +	return (void *)(uintptr_t)addr;
> +}
> +
> +static dma_addr_t fake_ptr_to_pci(void *addr)
> +{
> +	return (dma_addr_t)(uintptr_t)addr;
> +}
> +
>  /*
>   * Generate a VME bus interrupt at the requested level & vector. Wait for
>   * interrupt to be acked.
> @@ -202,7 +212,7 @@ static int fake_slave_set(struct vme_slave_resource *image, int enabled,
>  	bridge->slaves[i].enabled = enabled;
>  	bridge->slaves[i].vme_base = vme_base;
>  	bridge->slaves[i].size = size;
> -	bridge->slaves[i].buf_base = buf_base;
> +	bridge->slaves[i].buf_base = fake_pci_to_ptr(buf_base);
>  	bridge->slaves[i].aspace = aspace;
>  	bridge->slaves[i].cycle = cycle;
>  
> @@ -230,7 +240,7 @@ static int fake_slave_get(struct vme_slave_resource *image, int *enabled,
>  	*enabled = bridge->slaves[i].enabled;
>  	*vme_base = bridge->slaves[i].vme_base;
>  	*size = bridge->slaves[i].size;
> -	*buf_base = bridge->slaves[i].buf_base;
> +	*buf_base = fake_ptr_to_pci(bridge->slaves[i].buf_base);
>  	*aspace = bridge->slaves[i].aspace;
>  	*cycle = bridge->slaves[i].cycle;
>  
> @@ -431,7 +441,7 @@ static u8 fake_vmeread8(struct fake_driver *bridge, unsigned long long addr,
>  
>  		if ((addr >= start) && (addr < end)) {
>  			offset = addr - bridge->slaves[i].vme_base;
> -			loc = (u8 *)((void *)bridge->slaves[i].buf_base + offset);
> +			loc = (u8 *)(bridge->slaves[i].buf_base + offset);
>  			retval = *loc;
>  
>  			break;
> @@ -463,7 +473,7 @@ static u16 fake_vmeread16(struct fake_driver *bridge, unsigned long long addr,
>  
>  		if ((addr >= start) && ((addr + 1) < end)) {
>  			offset = addr - bridge->slaves[i].vme_base;
> -			loc = (u16 *)((void *)bridge->slaves[i].buf_base + offset);
> +			loc = (u16 *)(bridge->slaves[i].buf_base + offset);
>  			retval = *loc;
>  
>  			break;
> @@ -495,7 +505,7 @@ static u32 fake_vmeread32(struct fake_driver *bridge, unsigned long long addr,
>  
>  		if ((addr >= start) && ((addr + 3) < end)) {
>  			offset = addr - bridge->slaves[i].vme_base;
> -			loc = (u32 *)((void *)bridge->slaves[i].buf_base + offset);
> +			loc = (u32 *)(bridge->slaves[i].buf_base + offset);
>  			retval = *loc;
>  
>  			break;
> @@ -997,7 +1007,7 @@ static void *fake_alloc_consistent(struct device *parent, size_t size,
>  	void *alloc = kmalloc(size, GFP_KERNEL);
>  
>  	if (alloc != NULL)
> -		*dma = (dma_addr_t)(unsigned long)alloc;
> +		*dma = fake_ptr_to_pci(alloc);
>  
>  	return alloc;
>  }
> @@ -1031,7 +1041,7 @@ static int fake_crcsr_init(struct vme_bridge *fake_bridge)
>  
>  	/* Allocate mem for CR/CSR image */
>  	bridge->crcsr_kernel = kzalloc(VME_CRCSR_BUF_SIZE, GFP_KERNEL);
> -	bridge->crcsr_bus = (dma_addr_t)bridge->crcsr_kernel;
> +	bridge->crcsr_bus = fake_ptr_to_pci(bridge->crcsr_kernel);
>  	if (bridge->crcsr_kernel == NULL)
>  		return -ENOMEM;
>  
> -- 
> 2.9.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-08 19:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-06 12:59 [PATCH] vme: fake: fix build for 64-bit dma_addr_t Arnd Bergmann
2016-09-06 19:39 ` Dan Carpenter
2016-09-06 20:21   ` Arnd Bergmann
2016-09-08 19:10 ` Martyn Welch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.