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

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 <[email protected]>
> 
> 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

> 
>        Arnd

-- 
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)