Re: [PATCH 00/47] fbdev: Use I/O helpers

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

 



On 7/28/23 18:39, Thomas Zimmermann wrote:
Most fbdev drivers operate on I/O memory.

Just nitpicking here:
What is I/O memory?
Isn't it either memory, or I/O ?
I mean, I would never think of the cfb* draw functions under I/O.

And most of those use the
default implementations for file I/O and console drawing. Convert all
these low-hanging fruits to the fb_ops initializer macro and Kconfig
token for fbdev I/O helpers.

I do see the motivation for your patch, but I think the
macro names are very misleading.

You have:
#define __FB_DEFAULT_IO_OPS_RDWR \
        .fb_read        = fb_io_read, \
        .fb_write       = fb_io_write

#define __FB_DEFAULT_IO_OPS_DRAW \
        .fb_fillrect    = cfb_fillrect, \
        .fb_copyarea    = cfb_copyarea, \
        .fb_imageblit   = cfb_imageblit

#define __FB_DEFAULT_IO_OPS_MMAP \
        .fb_mmap        = NULL /* default implementation */

#define FB_DEFAULT_IO_OPS \
        __FB_DEFAULT_IO_OPS_RDWR, \
        __FB_DEFAULT_IO_OPS_DRAW, \
        __FB_DEFAULT_IO_OPS_MMAP

I think FB_DEFAULT_IO_OPS is OK for read/write/mmap.
But I would suggest to split out __FB_DEFAULT_IO_OPS_DRAW.
Something like:
#define FB_DEFAULT_IO_OPS \
        __FB_DEFAULT_IO_OPS_RDWR, \
        __FB_DEFAULT_IO_OPS_MMAP
#define FB_DEFAULT_CFB_OPS \
        .fb_fillrect    = cfb_fillrect, \
        .fb_copyarea    = cfb_copyarea, \
        .fb_imageblit   = cfb_imageblit

and then add FB_DEFAULT_IO_OPS *and* FB_DEFAULT_CFB_OPS
to the various struct fb_ops in the drivers.

Helge




[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux