[0/4] ppc, fbdev: Clean up fbdev mmap helper

Message

Thomas Zimmermann Sept. 1, 2023, 2:16 p.m. UTC

Refactor fb_pgprotect() in PowerPC to work without struct file. Then
clean up and rename fb_pgprotect(). This change has been discussed at
[1] in the context of refactoring fbdev's mmap code.

The first three patches adapt PowerPC's internal interfaces to
provide a phys_mem_access_prot() that works without struct file. Neither
the architecture code or fbdev helpers need the parameter.

Patch 4 replaces fbdev's fb_pgprotect() with fb_pgprot_device() on
all architectures. The new helper with its stream-lined interface
enables more refactoring within fbdev's mmap implementation.

[1] https://lore.kernel.org/linuxppc-dev/[e-mail]/

Thomas Zimmermann (4):
  arch/powerpc: Remove trailing whitespaces
  arch/powerpc: Remove file parameter from phys_mem_access_prot code
  arch/powerpc: Call internal __phys_mem_access_prot() in fbdev code
  fbdev: Replace fb_pgprotect() with fb_pgprot_device()

 arch/ia64/include/asm/fb.h                | 15 +++++++--------
 arch/m68k/include/asm/fb.h                | 19 ++++++++++---------
 arch/mips/include/asm/fb.h                | 11 +++++------
 arch/powerpc/include/asm/book3s/pgtable.h | 10 ++++++++--
 arch/powerpc/include/asm/fb.h             | 13 +++++--------
 arch/powerpc/include/asm/machdep.h        | 13 ++++++-------
 arch/powerpc/include/asm/nohash/pgtable.h | 10 ++++++++--
 arch/powerpc/include/asm/pci.h            |  4 +---
 arch/powerpc/kernel/pci-common.c          |  3 +--
 arch/powerpc/mm/mem.c                     |  8 ++++----
 arch/sparc/include/asm/fb.h               | 15 +++++++++------
 arch/x86/include/asm/fb.h                 | 10 ++++++----
 arch/x86/video/fbdev.c                    | 15 ++++++++-------
 drivers/video/fbdev/core/fb_chrdev.c      |  3 ++-
 include/asm-generic/fb.h                  | 12 ++++++------
 15 files changed, 86 insertions(+), 75 deletions(-)

Comments

Michael Ellerman Sept. 5, 2023, 2:47 a.m. UTC | #1

Thomas Zimmermann <[e-mail]> writes:
> Refactor fb_pgprotect() in PowerPC to work without struct file. Then
> clean up and rename fb_pgprotect(). This change has been discussed at
> [1] in the context of refactoring fbdev's mmap code.
>
> The first three patches adapt PowerPC's internal interfaces to
> provide a phys_mem_access_prot() that works without struct file. Neither
> the architecture code or fbdev helpers need the parameter.
>
> Patch 4 replaces fbdev's fb_pgprotect() with fb_pgprot_device() on
> all architectures. The new helper with its stream-lined interface
> enables more refactoring within fbdev's mmap implementation.

The content of this series is OK, but the way it's structured makes it a
real headache to merge, because it's mostly powerpc changes and then a
dependant cross architecture patch at the end.

It would be simpler if patch 4 was first and just passed file=NULL to
the powerpc helper, with an explanation that it's unused and will be
dropped in a future cleanup.

We could then put the first patch (previously patch 4) in a topic branch
that is shared between the powerpc tree and the fbdev tree, and then the
powerpc changes could be staged on top of that through the powerpc tree.

cheers

Thomas Zimmermann Sept. 5, 2023, 12:44 p.m. UTC | #2

Hi

Am 05.09.23 um 04:47 schrieb Michael Ellerman:
> Thomas Zimmermann <[e-mail]> writes:
>> Refactor fb_pgprotect() in PowerPC to work without struct file. Then
>> clean up and rename fb_pgprotect(). This change has been discussed at
>> [1] in the context of refactoring fbdev's mmap code.
>>
>> The first three patches adapt PowerPC's internal interfaces to
>> provide a phys_mem_access_prot() that works without struct file. Neither
>> the architecture code or fbdev helpers need the parameter.
>>
>> Patch 4 replaces fbdev's fb_pgprotect() with fb_pgprot_device() on
>> all architectures. The new helper with its stream-lined interface
>> enables more refactoring within fbdev's mmap implementation.
> 
> The content of this series is OK, but the way it's structured makes it a
> real headache to merge, because it's mostly powerpc changes and then a
> dependant cross architecture patch at the end.
> 
> It would be simpler if patch 4 was first and just passed file=NULL to
> the powerpc helper, with an explanation that it's unused and will be
> dropped in a future cleanup.
> 
> We could then put the first patch (previously patch 4) in a topic branch
> that is shared between the powerpc tree and the fbdev tree, and then the
> powerpc changes could be staged on top of that through the powerpc tree.

Ok, thanks for reviewing. The fbdev patch would go through the drm-misc 
tree as base for further refactoring.

I'll update the patchset accordingly.

Best regards
Thomas

> 
> cheers