diff mbox series

[v4,1/5] fbdev: Avoid file argument in fb_pgprotect()

Message ID 20230912135050.17155-2-tzimmermann@suse.de
State Superseded
Headers show
Series ppc, fbdev: Clean up fbdev mmap helper | expand

Commit Message

Thomas Zimmermann Sept. 12, 2023, 1:48 p.m. UTC
Only PowerPC's fb_pgprotect() needs the file argument, although
the implementation does not use it. Pass NULL to the internal
helper in preparation of further updates. A later patch will remove
the file parameter from fb_pgprotect().

While at it, replace the shift operation with PHYS_PFN().

Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 arch/powerpc/include/asm/fb.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Javier Martinez Canillas Sept. 20, 2023, 8:01 a.m. UTC | #1
Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Only PowerPC's fb_pgprotect() needs the file argument, although
> the implementation does not use it. Pass NULL to the internal

Can you please mention the function that's the implementation for
PowerPC ? If I'm looking at the code correctly, that function is
phys_mem_access_prot() defined in the arch/powerpc/mm/mem.c file:

pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
			      unsigned long size, pgprot_t vma_prot)
{
	if (ppc_md.phys_mem_access_prot)
		return ppc_md.phys_mem_access_prot(file, pfn, size, vma_prot);

	if (!page_is_ram(pfn))
		vma_prot = pgprot_noncached(vma_prot);

	return vma_prot;
}

and if set, ppc_md.phys_mem_access_prot is pci_phys_mem_access_prot()
that is defined in the arch/powerpc/kernel/pci-common.c source file:

https://elixir.bootlin.com/linux/v6.6-rc2/source/arch/powerpc/kernel/pci-common.c#L524

That function indeed doesn't use the file argument. So your patch looks
correct to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann Sept. 22, 2023, 7:40 a.m. UTC | #2
Hi Javier

Am 20.09.23 um 10:01 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
> 
> Hello Thomas,
> 
>> Only PowerPC's fb_pgprotect() needs the file argument, although
>> the implementation does not use it. Pass NULL to the internal
> 
> Can you please mention the function that's the implementation for

Sure

> PowerPC ? If I'm looking at the code correctly, that function is
> phys_mem_access_prot() defined in the arch/powerpc/mm/mem.c file:
> 
> pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
> 			      unsigned long size, pgprot_t vma_prot)
> {
> 	if (ppc_md.phys_mem_access_prot)
> 		return ppc_md.phys_mem_access_prot(file, pfn, size, vma_prot);
> 
> 	if (!page_is_ram(pfn))
> 		vma_prot = pgprot_noncached(vma_prot);
> 
> 	return vma_prot;
> }
> 
> and if set, ppc_md.phys_mem_access_prot is pci_phys_mem_access_prot()
> that is defined in the arch/powerpc/kernel/pci-common.c source file:
> 
> https://elixir.bootlin.com/linux/v6.6-rc2/source/arch/powerpc/kernel/pci-common.c#L524

Yes, that's correct. The only value for that function pointer appears to 
be pci_phys_mem_access_prot()

> 
> That function indeed doesn't use the file argument. So your patch looks
> correct to me.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Thanks

Best regards
Thomas

>
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/fb.h b/arch/powerpc/include/asm/fb.h
index 5f1a2e5f76548..61e3b8806db69 100644
--- a/arch/powerpc/include/asm/fb.h
+++ b/arch/powerpc/include/asm/fb.h
@@ -9,7 +9,12 @@ 
 static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
 				unsigned long off)
 {
-	vma->vm_page_prot = phys_mem_access_prot(file, off >> PAGE_SHIFT,
+	/*
+	 * PowerPC's implementation of phys_mem_access_prot() does
+	 * not use the file argument. Set it to NULL in preparation
+	 * of later updates to the interface.
+	 */
+	vma->vm_page_prot = phys_mem_access_prot(NULL, PHYS_PFN(off),
 						 vma->vm_end - vma->vm_start,
 						 vma->vm_page_prot);
 }