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

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3746 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-r2nGTMty4D4@public.gmane.org>
> > > ---
> > >  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-r2nGTMty4D4@public.gmane.org>
> 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-r2nGTMty4D4@public.gmane.org>
> 
> 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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>


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