On Wed, Apr 5, 2023, at 17:05, Thomas Zimmermann wrote: > Generic implementations of fb_pgprotect() and fb_is_primary_device() > have been in the source code for a long time. Prepare the header file > to make use of them. > > Improve the code by using an inline function for fb_pgprotect() and > by removing include statements. > > Symbols are protected by preprocessor guards. Architectures that > provide a symbol need to define a preprocessor token of the same > name and value. Otherwise the header file will provide a generic > implementation. This pattern has been taken from <asm/io.h>. > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> Moving this into generic code is good, but I'm not sure about the default for fb_pgprotect(): > + > +#ifndef fb_pgprotect > +#define fb_pgprotect fb_pgprotect > +static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma, > + unsigned long off) > +{ } > +#endif I think most architectures will want the version we have on arc, arm, arm64, loongarch, and sh already: static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma, unsigned long off) { vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); } so I'd suggest making that version the default, and treating the empty ones (m68knommu, sparc32) as architecture specific workarounds. I see that sparc64 and parisc use pgprot_uncached here, but as they don't define a custom pgprot_writecombine, this ends up being the same, and they can use the above definition as well. mips defines pgprot_writecombine but uses pgprot_noncached in fb_pgprotect(), which is probably a mistake and should have been updated as part of commit 4b050ba7a66c ("MIPS: pgtable.h: Implement the pgprot_writecombine function for MIPS"). Arnd