[PATCH] drm: tegra: shut up harmless warning on NOMMU

[PATCH] drm: tegra: shut up harmless warning on NOMMU

[Arnd Bergmann]

The tegra DRM driver is almost ok without an MMU, but there
is one small warning that I get:

drivers/gpu/drm/tegra/gem.c: In function 'tegra_drm_mmap':
drivers/gpu/drm/tegra/gem.c:508:12: unused variable 'prot'

This marks the variable as __maybe_unused instead.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/tegra/gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 7d853e6b5ff0..63f14b7a59a0 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -505,7 +505,7 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
 
 		vma->vm_pgoff = vm_pgoff;
 	} else {
-		pgprot_t prot = vm_get_page_prot(vma->vm_flags);
+		pgprot_t prot __maybe_unused = vm_get_page_prot(vma->vm_flags);
 
 		vma->vm_flags |= VM_MIXEDMAP;
 		vma->vm_flags &= ~VM_PFNMAP;
-- 
2.9.0

Re: [PATCH] drm: tegra: shut up harmless warning on NOMMU

[Thierry Reding]

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

On Thu, Jan 12, 2017 at 12:13:51PM +0100, Arnd Bergmann wrote:
> The tegra DRM driver is almost ok without an MMU, but there
> is one small warning that I get:
> 
> drivers/gpu/drm/tegra/gem.c: In function 'tegra_drm_mmap':
> drivers/gpu/drm/tegra/gem.c:508:12: unused variable 'prot'
> 
> This marks the variable as __maybe_unused instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpu/drm/tegra/gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> index 7d853e6b5ff0..63f14b7a59a0 100644
> --- a/drivers/gpu/drm/tegra/gem.c
> +++ b/drivers/gpu/drm/tegra/gem.c
> @@ -505,7 +505,7 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
>  
>  		vma->vm_pgoff = vm_pgoff;
>  	} else {
> -		pgprot_t prot = vm_get_page_prot(vma->vm_flags);
> +		pgprot_t prot __maybe_unused = vm_get_page_prot(vma->vm_flags);

This seems to me like a suboptimal solution. The reason why this fails
is because pgprot_writecombine(prot) for NOMMU translates to __pgprot(0)
via a macro. This also means that we need to potentially add a
__maybe_unused annotation to every local variable that stores a value
that gets passed to pgprot_writecombine().

There fortunately aren't very many of those cases, but I still think
that a better solution would be to turn pgprot_writecombine() into a
static inline function, so that the parameter would get silently
ignored. Or perhaps if it must remain a macro, then doing the following
should still avoid the need to modify every call site:

	#define pgprot_writecombine(prot) ({ (void)prot; __pgprot(0); })

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] drm: tegra: shut up harmless warning on NOMMU

[Arnd Bergmann]

On Thursday, January 19, 2017 12:00:58 PM CET Thierry Reding wrote:
> On Thu, Jan 12, 2017 at 12:13:51PM +0100, Arnd Bergmann wrote:
> > The tegra DRM driver is almost ok without an MMU, but there
> > is one small warning that I get:
> > 
> > drivers/gpu/drm/tegra/gem.c: In function 'tegra_drm_mmap':
> > drivers/gpu/drm/tegra/gem.c:508:12: unused variable 'prot'
> > 
> > This marks the variable as __maybe_unused instead.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/gpu/drm/tegra/gem.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> > index 7d853e6b5ff0..63f14b7a59a0 100644
> > --- a/drivers/gpu/drm/tegra/gem.c
> > +++ b/drivers/gpu/drm/tegra/gem.c
> > @@ -505,7 +505,7 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
> >  
> >  		vma->vm_pgoff = vm_pgoff;
> >  	} else {
> > -		pgprot_t prot = vm_get_page_prot(vma->vm_flags);
> > +		pgprot_t prot __maybe_unused = vm_get_page_prot(vma->vm_flags);
> 
> This seems to me like a suboptimal solution. The reason why this fails
> is because pgprot_writecombine(prot) for NOMMU translates to __pgprot(0)
> via a macro. This also means that we need to potentially add a
> __maybe_unused annotation to every local variable that stores a value
> that gets passed to pgprot_writecombine().
> 
> There fortunately aren't very many of those cases, but I still think
> that a better solution would be to turn pgprot_writecombine() into a
> static inline function, so that the parameter would get silently
> ignored. Or perhaps if it must remain a macro, then doing the following
> should still avoid the need to modify every call site:
> 
> 	#define pgprot_writecombine(prot) ({ (void)prot; __pgprot(0); })
> 
> Thierry
> 

Makes sense. How about this version?

	Arnd
---
>From 83af6bc74423c90be7f580a827268b89f94b5c6b Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 19 Jan 2017 16:05:29 +0100
Subject: [PATCH] ARM: improve NOMMU definition of pgprot_*()

The tegra DRM driver produces a harmless warning when built for NOMMU:

drivers/gpu/drm/tegra/gem.c: In function 'tegra_drm_mmap':
drivers/gpu/drm/tegra/gem.c:508:12: unused variable 'prot'

This is because pgprot_writecombine() on ARM returns a constant and
ignores its argument. The version in asm-generic doesn't have that
problem, so let's use that one instead. We don't actually care
about the value on NOMMU, and this is consistent with what some
other architectures do.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/arm/include/asm/pgtable-nommu.h b/arch/arm/include/asm/pgtable-nommu.h
index add094d09e3e..302240c19a5a 100644
--- a/arch/arm/include/asm/pgtable-nommu.h
+++ b/arch/arm/include/asm/pgtable-nommu.h
@@ -63,9 +63,9 @@ typedef pte_t *pte_addr_t;
 /*
  * Mark the prot value as uncacheable and unbufferable.
  */
-#define pgprot_noncached(prot)	__pgprot(0)
-#define pgprot_writecombine(prot) __pgprot(0)
-#define pgprot_dmacoherent(prot) __pgprot(0)
+#define pgprot_noncached(prot)	(prot)
+#define pgprot_writecombine(prot) (prot)
+#define pgprot_dmacoherent(prot) (prot)
 
 
 /*

Re: [PATCH] drm: tegra: shut up harmless warning on NOMMU

[Thierry Reding]

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

On Thu, Jan 19, 2017 at 04:09:47PM +0100, Arnd Bergmann wrote:
> On Thursday, January 19, 2017 12:00:58 PM CET Thierry Reding wrote:
> > On Thu, Jan 12, 2017 at 12:13:51PM +0100, Arnd Bergmann wrote:
> > > The tegra DRM driver is almost ok without an MMU, but there
> > > is one small warning that I get:
> > > 
> > > drivers/gpu/drm/tegra/gem.c: In function 'tegra_drm_mmap':
> > > drivers/gpu/drm/tegra/gem.c:508:12: unused variable 'prot'
> > > 
> > > This marks the variable as __maybe_unused instead.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > >  drivers/gpu/drm/tegra/gem.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> > > index 7d853e6b5ff0..63f14b7a59a0 100644
> > > --- a/drivers/gpu/drm/tegra/gem.c
> > > +++ b/drivers/gpu/drm/tegra/gem.c
> > > @@ -505,7 +505,7 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
> > >  
> > >  		vma->vm_pgoff = vm_pgoff;
> > >  	} else {
> > > -		pgprot_t prot = vm_get_page_prot(vma->vm_flags);
> > > +		pgprot_t prot __maybe_unused = vm_get_page_prot(vma->vm_flags);
> > 
> > This seems to me like a suboptimal solution. The reason why this fails
> > is because pgprot_writecombine(prot) for NOMMU translates to __pgprot(0)
> > via a macro. This also means that we need to potentially add a
> > __maybe_unused annotation to every local variable that stores a value
> > that gets passed to pgprot_writecombine().
> > 
> > There fortunately aren't very many of those cases, but I still think
> > that a better solution would be to turn pgprot_writecombine() into a
> > static inline function, so that the parameter would get silently
> > ignored. Or perhaps if it must remain a macro, then doing the following
> > should still avoid the need to modify every call site:
> > 
> > 	#define pgprot_writecombine(prot) ({ (void)prot; __pgprot(0); })
> > 
> > Thierry
> > 
> 
> Makes sense. How about this version?
> 
> 	Arnd
> ---
> From 83af6bc74423c90be7f580a827268b89f94b5c6b Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Thu, 19 Jan 2017 16:05:29 +0100
> Subject: [PATCH] ARM: improve NOMMU definition of pgprot_*()
> 
> The tegra DRM driver produces a harmless warning when built for NOMMU:
> 
> drivers/gpu/drm/tegra/gem.c: In function 'tegra_drm_mmap':
> drivers/gpu/drm/tegra/gem.c:508:12: unused variable 'prot'
> 
> This is because pgprot_writecombine() on ARM returns a constant and
> ignores its argument. The version in asm-generic doesn't have that
> problem, so let's use that one instead. We don't actually care
> about the value on NOMMU, and this is consistent with what some
> other architectures do.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/arch/arm/include/asm/pgtable-nommu.h b/arch/arm/include/asm/pgtable-nommu.h
> index add094d09e3e..302240c19a5a 100644
> --- a/arch/arm/include/asm/pgtable-nommu.h
> +++ b/arch/arm/include/asm/pgtable-nommu.h
> @@ -63,9 +63,9 @@ typedef pte_t *pte_addr_t;
>  /*
>   * Mark the prot value as uncacheable and unbufferable.
>   */
> -#define pgprot_noncached(prot)	__pgprot(0)
> -#define pgprot_writecombine(prot) __pgprot(0)
> -#define pgprot_dmacoherent(prot) __pgprot(0)
> +#define pgprot_noncached(prot)	(prot)
> +#define pgprot_writecombine(prot) (prot)
> +#define pgprot_dmacoherent(prot) (prot)
>  
>  
>  /*
> 

I remember writing a reply to this, but it seems like I never sent it
out. The above looks good to me:

Acked-by: Thierry Reding <treding@nvidia.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]