Re: [PATCH v2 2/5] fbdev: Replace fb_pgprotect() with fb_pgprot_device()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


Hi Arnd

Am 06.09.23 um 21:53 schrieb Arnd Bergmann:
On Wed, Sep 6, 2023, at 10:35, Thomas Zimmermann wrote:
Rename the fbdev mmap helper fb_pgprotect() to fb_pgprot_device().
The helper sets VMA page-access flags for framebuffers in device I/O
memory. The new name follows pgprot_device(), which does the same for
arbitrary devices.

Also clean up the helper's parameters and return value. Instead of
the VMA instance, pass the individial parameters separately: existing
page-access flags, the VMAs start and end addresses and the offset
in the underlying device memory rsp file. Return the new page-access
flags. These changes align fb_pgprot_device() closer with pgprot_device.

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>

This makes sense as a cleanup, but I'm not sure the new naming is helpful.

The 'pgprot_device' permissions are based on Arm's memory attributes,
which have slightly different behavior for "device", "uncached" and
"writecombine" mappings. I think simply calling this one pgprot_fb()
or fb_pgprot() would be less confusing, since depending on the architecture
it appears to give either uncached or writecombine mappings but not
"device" on the architectures where this is different.

I see. Thanks for the info. I like pgprot_fb() maybe pgprot_framebuffer(). I'll update the patchset.

One thing I've been wondering is whether I should attempt to integrate the helpers in <asm/fb.h> in the regular asm headers. So the pgprot code would go into pgtable.h, the I/O functions would go into io.h. The I/O functions could then be called readb_fb(), writel_fb(), memcpy_tofb() and so on. Would you prefer that or rather not?

Best regards


Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux