Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1477347 > unrolled thread

[PATCH] vme: fake: fix build for 64-bit dma_addr_t

Back to article view | Back to linux.kernel

Contents

  [PATCH] vme: fake: fix build for 64-bit dma_addr_t Arnd Bergmann <arnd@arndb.de> - 2016-09-06 15:10 +0200
    Re: [PATCH] vme: fake: fix build for 64-bit dma_addr_t Dan Carpenter <dan.carpenter@oracle.com> - 2016-09-06 21:50 +0200
      Re: [PATCH] vme: fake: fix build for 64-bit dma_addr_t Arnd Bergmann <arnd@arndb.de> - 2016-09-06 22:30 +0200
    Re: [PATCH] vme: fake: fix build for 64-bit dma_addr_t Martyn Welch <martyn@welchs.me.uk> - 2016-09-08 21:20 +0200

#1477347 — [PATCH] vme: fake: fix build for 64-bit dma_addr_t

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

[toc] | [next] | [standalone]

#1477790

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

[toc] | [prev] | [next] | [standalone]

#1477806

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

[toc] | [prev] | [next] | [standalone]

#1479458

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
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web