On Wed, Apr 05, 2023 at 05:53:03PM +0200, Arnd Bergmann wrote: > 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. Yeah I was about to type the same suggestion :-) -Daniel > 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch