[PATCH v2 00/38] fbdev: Make userspace interfaces optional

[PATCH v2 00/38] fbdev: Make userspace interfaces optional

[Thomas Zimmermann]

Add the new config option FB_DEVICE. If enabled, fbdev provides
traditional userspace interfaces in devfs, sysfs and procfs, such
as /dev/fb0 or /proc/fb.

Modern Linux distrobutions have adopted DRM drivers for graphics
output and use fbdev only for the kernel's framebuffer console.
Userspace has also moved on, with no new fbdev code being written
and existing support being removed.

OTOH, fbdev provides userspace a way of accessing kernel or I/O
memory, which might compromise the system's security. See the recent
commit c8687694bb1f ("drm/fbdev-generic: prohibit potential
out-of-bounds access") for an example. Disabling fbdev userspace
interfaces is therefore a useful feature to limit unnecessary
exposure of fbdev code to processes of low privilegues.

Patches 1 to 31 fix various bugs and issues in fbdev-related code.
In most cases the code uses the fbdev device where it should use
the Linux hardware device or something else. Most of these patches
fix existing problems and should therefore be considered in any case.

Patches 32 to 37 refactor the fbdev core code. The patches move
support for backlights, sysfs, procfs and devfs into separate files
and hide it behind simple interfaces. These changes will allow to
easily build the userspace support conditionally.

Patch 38 introduces the config option FB_DEVICE and adapts the fbdev
core to support it. The field struct fb_info.dev is now optional,
hence the name of the config option.

Tested on simpledrm and i915, including the device handover.

Future directions: With the support for disabling fbdev userspace
interfaces in place, it will be possible to make most fbdev drivers'
file-I/O code in struct fb_ops optional as well. 

v2:
	* fix fsl-diu-fb and sh7760fb
	* split backlight patches
	* set 'default y' for FB_CONFIG
	* minor fixes and corrections

Thomas Zimmermann (38):
  backlight/bd6107: Compare against struct fb_info.device
  backlight/bd6107: Rename struct bd6107_platform_data.fbdev to 'dev'
  backlight/gpio_backlight: Compare against struct fb_info.device
  backlight/gpio_backlight: Rename field 'fbdev' to 'dev'
  backlight/lv5207lp: Compare against struct fb_info.device
  backlight/lv5207lp: Rename struct lv5207lp_platform_data.fbdev to
    'dev'
  fbdev/atyfb: Reorder backlight and framebuffer init/cleanup
  fbdev/atyfb: Use hardware device as backlight parent
  fbdev/aty128fb: Reorder backlight and framebuffer init/cleanup
  fbdev/aty128fb: Use hardware device as backlight parent
  fbdev/broadsheetfb: Call device_remove_file() with hardware device
  fbdev/ep93xx-fb: Alloc DMA memory from hardware device
  fbdev/ep93xx-fb: Output messages with fb_info() and fb_err()
  fbdev/ep93xx-fb: Do not assign to struct fb_info.dev
  fbdev/fsl-diu-fb: Output messages with fb_*() helpers
  fbdev/mb862xxfb: Output messages with fb_dbg()
  fbdev/metronomefb: Use hardware device for dev_err()
  fbdev/nvidiafb: Reorder backlight and framebuffer init/cleanup
  fbdev/nvidiafb: Use hardware device as backlight parent
  fbdev/pxa168fb: Do not assign to struct fb_info.dev
  fbdev/radeonfb: Reorder backlight and framebuffer cleanup
  fbdev/radeonfb: Use hardware device as backlight parent
  fbdev/rivafb: Reorder backlight and framebuffer init/cleanup
  fbdev/rivafb: Use hardware device as backlight parent
  fbdev/sh7760fb: Use fb_dbg() in sh7760fb_get_color_info()
  fbdev/sh7760fb: Output messages with fb_dbg()
  fbdev/sh7760fb: Alloc DMA memory from hardware device
  fbdev/sh7760fb: Use hardware device with dev_() output during probe
  fbdev/sm501fb: Output message with fb_err()
  fbdev/smscufx: Detect registered fb_info from refcount
  fbdev/tdfxfb: Set i2c adapter parent to hardware device
  fbdev/core: Pass Linux device to pm_vt_switch_*() functions
  fbdev/core: Move framebuffer and backlight helpers into separate files
  fbdev/core: Add fb_device_{create,destroy}()
  fbdev/core: Move procfs code to separate file
  fbdev/core: Move file-I/O code into separate file
  fbdev/core: Rework fb init code
  fbdev: Make support for userspace interfaces configurable

 Documentation/gpu/todo.rst                   |  13 +
 arch/sh/boards/mach-ecovec24/setup.c         |   2 +-
 arch/sh/boards/mach-kfr2r09/setup.c          |   2 +-
 drivers/staging/fbtft/Kconfig                |   1 +
 drivers/video/backlight/bd6107.c             |   2 +-
 drivers/video/backlight/gpio_backlight.c     |   6 +-
 drivers/video/backlight/lv5207lp.c           |   2 +-
 drivers/video/fbdev/Kconfig                  |  13 +
 drivers/video/fbdev/aty/aty128fb.c           |  12 +-
 drivers/video/fbdev/aty/atyfb_base.c         |  18 +-
 drivers/video/fbdev/aty/radeon_backlight.c   |   2 +-
 drivers/video/fbdev/aty/radeon_base.c        |   3 +-
 drivers/video/fbdev/broadsheetfb.c           |   2 +-
 drivers/video/fbdev/core/Makefile            |   7 +-
 drivers/video/fbdev/core/fb_backlight.c      |  33 ++
 drivers/video/fbdev/core/fb_info.c           |  78 +++
 drivers/video/fbdev/core/fb_internal.h       |  67 +++
 drivers/video/fbdev/core/fb_procfs.c         |  62 ++
 drivers/video/fbdev/core/fbcon.c             |   1 +
 drivers/video/fbdev/core/fbmem.c             | 592 +------------------
 drivers/video/fbdev/core/fbsysfs.c           | 134 +----
 drivers/video/fbdev/ep93xx-fb.c              |  21 +-
 drivers/video/fbdev/fsl-diu-fb.c             |  26 +-
 drivers/video/fbdev/mb862xx/mb862xxfbdrv.c   |   9 +-
 drivers/video/fbdev/metronomefb.c            |   2 +-
 drivers/video/fbdev/nvidia/nv_backlight.c    |   2 +-
 drivers/video/fbdev/nvidia/nvidia.c          |   8 +-
 drivers/video/fbdev/omap2/omapfb/Kconfig     |   2 +-
 drivers/video/fbdev/pxa168fb.c               |   2 +-
 drivers/video/fbdev/riva/fbdev.c             |  10 +-
 drivers/video/fbdev/sh7760fb.c               |  50 +-
 drivers/video/fbdev/sm501fb.c                |   2 +-
 drivers/video/fbdev/smscufx.c                |   4 +-
 drivers/video/fbdev/tdfxfb.c                 |   4 +-
 include/linux/fb.h                           |   6 +-
 include/linux/platform_data/bd6107.h         |   2 +-
 include/linux/platform_data/gpio_backlight.h |   2 +-
 include/linux/platform_data/lv5207lp.h       |   2 +-
 38 files changed, 437 insertions(+), 769 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_backlight.c
 create mode 100644 drivers/video/fbdev/core/fb_info.c
 create mode 100644 drivers/video/fbdev/core/fb_internal.h
 create mode 100644 drivers/video/fbdev/core/fb_procfs.c


base-commit: 63a468ec7c7652afa80e3fa6ad203f9e64d04e83
prerequisite-patch-id: 0aa359f6144c4015c140c8a6750be19099c676fb
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: cbc453ee02fae02af22fbfdce56ab732c7a88c36
prerequisite-patch-id: 7c401614cf55c033f742bced317575b9f5b77bb1
prerequisite-patch-id: d3145eae4b35a1290199af6ff6cd5abfebc82033
prerequisite-patch-id: 242b6bc45675f1f1a62572542d75c89d4864f15a
-- 
2.41.0

[PATCH v2 01/38] backlight/bd6107: Compare against struct fb_info.device

[Thomas Zimmermann]

Struct bd6107_platform_data refers to a platform device within
the Linux device hierarchy. The test in bd6107_backlight_check_fb()
compares it against the fbdev device in struct fb_info.dev, which
is different. Fix the test by comparing to struct fb_info.device.

Fixes a bug in the backlight driver and prepares fbdev for making
struct fb_info.dev optional.

v2:
	* move renames into separate patch (Javier, Sam, Michael)

Fixes: 67b43e590415 ("backlight: Add ROHM BD6107 backlight driver")
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Lee Jones <lee@kernel.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v3.12+
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/backlight/bd6107.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/backlight/bd6107.c b/drivers/video/backlight/bd6107.c
index f4db6c064635b..e3410444ea235 100644
--- a/drivers/video/backlight/bd6107.c
+++ b/drivers/video/backlight/bd6107.c
@@ -104,7 +104,7 @@ static int bd6107_backlight_check_fb(struct backlight_device *backlight,
 {
 	struct bd6107 *bd = bl_get_data(backlight);
 
-	return bd->pdata->fbdev == NULL || bd->pdata->fbdev == info->dev;
+	return bd->pdata->fbdev == NULL || bd->pdata->fbdev == info->device;
 }
 
 static const struct backlight_ops bd6107_backlight_ops = {
-- 
2.41.0

Re: [PATCH v2 01/38] backlight/bd6107: Compare against struct fb_info.device

[Dan Carpenter]

On Mon, Jun 12, 2023 at 04:07:39PM +0200, Thomas Zimmermann wrote:
> Struct bd6107_platform_data refers to a platform device within
> the Linux device hierarchy. The test in bd6107_backlight_check_fb()
> compares it against the fbdev device in struct fb_info.dev, which
> is different. Fix the test by comparing to struct fb_info.device.
> 
> Fixes a bug in the backlight driver and prepares fbdev for making
> struct fb_info.dev optional.
> 
> v2:
> 	* move renames into separate patch (Javier, Sam, Michael)
> 

Thanks.  This helps a lot.  I stared at this for a long time without
seeing the fix.  Then I misunderstood Sam's review comments (my fault,
they were clear in retrospect) so I got completely lost.

regards,
dan carpenter

RE: [PATCH v2 01/38] backlight/bd6107: Compare against struct fb_info.device

[Ruhl, Michael J]

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Thomas Zimmermann
>Sent: Monday, June 12, 2023 10:08 AM
>To: daniel@ffwll.ch; javierm@redhat.com; sam@ravnborg.org;
>deller@gmx.de; geert+renesas@glider.be; lee@kernel.org;
>daniel.thompson@linaro.org; jingoohan1@gmail.com;
>dan.carpenter@linaro.org; Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: linux-fbdev@vger.kernel.org; Laurent Pinchart
><laurent.pinchart+renesas@ideasonboard.com>; linux-sh@vger.kernel.org;
>linux-staging@lists.linux.dev; linux-kernel@vger.kernel.org; dri-
>devel@lists.freedesktop.org; stable@vger.kernel.org; Thomas Zimmermann
><tzimmermann@suse.de>; linux-omap@vger.kernel.org
>Subject: [PATCH v2 01/38] backlight/bd6107: Compare against struct
>fb_info.device
>
>Struct bd6107_platform_data refers to a platform device within
>the Linux device hierarchy. The test in bd6107_backlight_check_fb()
>compares it against the fbdev device in struct fb_info.dev, which
>is different. Fix the test by comparing to struct fb_info.device.
>
>Fixes a bug in the backlight driver and prepares fbdev for making
>struct fb_info.dev optional.
>
>v2:
>	* move renames into separate patch (Javier, Sam, Michael)
>
>Fixes: 67b43e590415 ("backlight: Add ROHM BD6107 backlight driver")
>Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>Cc: Lee Jones <lee@kernel.org>
>Cc: Daniel Thompson <daniel.thompson@linaro.org>
>Cc: Jingoo Han <jingoohan1@gmail.com>
>Cc: dri-devel@lists.freedesktop.org
>Cc: <stable@vger.kernel.org> # v3.12+
>Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>---
> drivers/video/backlight/bd6107.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/video/backlight/bd6107.c
>b/drivers/video/backlight/bd6107.c
>index f4db6c064635b..e3410444ea235 100644
>--- a/drivers/video/backlight/bd6107.c
>+++ b/drivers/video/backlight/bd6107.c
>@@ -104,7 +104,7 @@ static int bd6107_backlight_check_fb(struct
>backlight_device *backlight,
> {
> 	struct bd6107 *bd = bl_get_data(backlight);
>
>-	return bd->pdata->fbdev == NULL || bd->pdata->fbdev == info->dev;
>+	return bd->pdata->fbdev == NULL || bd->pdata->fbdev == info->device;

Thomas,

Looking at the fb.h file I see:

	struct device *device;		/* This is the parent */
	struct device *dev;		/* This is this fb device */

Is this documentation "correct"?  If so, how does that match what you are doing here?

Thanks,

M

> }
>
> static const struct backlight_ops bd6107_backlight_ops = {
>--
>2.41.0

Re: [PATCH v2 01/38] backlight/bd6107: Compare against struct fb_info.device

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2303 bytes --]

Hi

Am 12.06.23 um 17:17 schrieb Ruhl, Michael J:
[...]
> 
> Thomas,
> 
> Looking at the fb.h file I see:
> 
> 	struct device *device;		/* This is the parent */
> 	struct device *dev;		/* This is this fb device */
> 
> Is this documentation "correct"?  If so, how does that match what you are doing here?

The comments are correct. Let's go through what's happening here.

The field 'device' is the Linux device (platform_device, pci_dev, etc.) 
and 'dev' is the fbdev character device that is /dev/fb*.

We set 'device' where we allocate the fb_info in framebuffer_alloc()

https://elixir.bootlin.com/linux/v6.3/source/drivers/video/fbdev/core/fbsysfs.c#L57

and we set 'dev' when we register the chrdev within register_framebuffer().

https://elixir.bootlin.com/linux/v6.3/source/drivers/video/fbdev/core/fbmem.c#L1555

(And the point of this patch series is to make the chrdev optional.)

The problem with bd6107 is that is misses the part where it registers 
the platform device. The driver appears to be unused.

But gpio_backlight from patches 3 and 4 works. The architecture code 
sets the 'fbdev' field from a platform-device structure at

https://elixir.bootlin.com/linux/v6.3/source/arch/sh/boards/mach-ecovec24/setup.c#L389

and later creates the platform device as part of

https://elixir.bootlin.com/linux/v6.3/source/arch/sh/boards/mach-ecovec24/setup.c#L1483

It will be used with the sh-mobile fbdev driver and become the 'device' 
field there.

In the backlight code, the gpio_backlight driver copies the fbdev field at

https://elixir.bootlin.com/linux/v6.3/source/drivers/video/backlight/gpio_backlight.c#L62

to later use it incorrectly in .check_fb. Hence, the helper has to 
compare the platform device to the 'device' field, not the 'dev' field; 
which is being fixed by this patchset.

The two other drivers, bd6107 and lv5207lp, have the same bug.

Best regards
Thomas


> 
> Thanks,
> 
> M
> 
>> }
>>
>> static const struct backlight_ops bd6107_backlight_ops = {
>> --
>> 2.41.0
> 

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2 01/38] backlight/bd6107: Compare against struct fb_info.device

[Daniel Thompson]

On Mon, Jun 12, 2023 at 04:07:39PM +0200, Thomas Zimmermann wrote:
> Struct bd6107_platform_data refers to a platform device within
> the Linux device hierarchy. The test in bd6107_backlight_check_fb()
> compares it against the fbdev device in struct fb_info.dev, which
> is different. Fix the test by comparing to struct fb_info.device.
>
> Fixes a bug in the backlight driver and prepares fbdev for making
> struct fb_info.dev optional.
>
> v2:
> 	* move renames into separate patch (Javier, Sam, Michael)
>
> Fixes: 67b43e590415 ("backlight: Add ROHM BD6107 backlight driver")
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v3.12+
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.

PS Please don't treat this as an Acked-by, if you want to land this
   patchset via a single tree please coordinate with Lee Jones!

Re: [PATCH v2 01/38] backlight/bd6107: Compare against struct fb_info.device

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1753 bytes --]

Hi

Am 13.06.23 um 12:37 schrieb Daniel Thompson:
> On Mon, Jun 12, 2023 at 04:07:39PM +0200, Thomas Zimmermann wrote:
>> Struct bd6107_platform_data refers to a platform device within
>> the Linux device hierarchy. The test in bd6107_backlight_check_fb()
>> compares it against the fbdev device in struct fb_info.dev, which
>> is different. Fix the test by comparing to struct fb_info.device.
>>
>> Fixes a bug in the backlight driver and prepares fbdev for making
>> struct fb_info.dev optional.
>>
>> v2:
>> 	* move renames into separate patch (Javier, Sam, Michael)
>>
>> Fixes: 67b43e590415 ("backlight: Add ROHM BD6107 backlight driver")
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> Cc: Lee Jones <lee@kernel.org>
>> Cc: Daniel Thompson <daniel.thompson@linaro.org>
>> Cc: Jingoo Han <jingoohan1@gmail.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: <stable@vger.kernel.org> # v3.12+
>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>

Thanks for going through the backlight patches.

> 
> 
> Daniel.
> 
> PS Please don't treat this as an Acked-by, if you want to land this
>     patchset via a single tree please coordinate with Lee Jones!

I'd like to merge them via drm-misc-next together with the rest of the 
patchset. It's not DRM, but fbdev patches often go through that tree 
quite often.

Best regards
Thomas

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH v2 02/38] backlight/bd6107: Rename struct bd6107_platform_data.fbdev to 'dev'

[Thomas Zimmermann]

Rename struct bd6107_platform_data.fbdev to 'dev', as it stores a
pointer to the Linux platform device; not the fbdev device. Makes
the code easier to understand.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Lee Jones <lee@kernel.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/backlight/bd6107.c     | 2 +-
 include/linux/platform_data/bd6107.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/bd6107.c b/drivers/video/backlight/bd6107.c
index e3410444ea235..fa3dd45c8f9d1 100644
--- a/drivers/video/backlight/bd6107.c
+++ b/drivers/video/backlight/bd6107.c
@@ -104,7 +104,7 @@ static int bd6107_backlight_check_fb(struct backlight_device *backlight,
 {
 	struct bd6107 *bd = bl_get_data(backlight);
 
-	return bd->pdata->fbdev == NULL || bd->pdata->fbdev == info->device;
+	return !bd->pdata->dev || bd->pdata->dev == info->device;
 }
 
 static const struct backlight_ops bd6107_backlight_ops = {
diff --git a/include/linux/platform_data/bd6107.h b/include/linux/platform_data/bd6107.h
index 54a06a4d26186..596ca4f95cfa4 100644
--- a/include/linux/platform_data/bd6107.h
+++ b/include/linux/platform_data/bd6107.h
@@ -8,7 +8,7 @@
 struct device;
 
 struct bd6107_platform_data {
-	struct device *fbdev;
+	struct device *dev;
 	unsigned int def_value;
 };
 
-- 
2.41.0

Re: [PATCH v2 02/38] backlight/bd6107: Rename struct bd6107_platform_data.fbdev to 'dev'

[Daniel Thompson]

On Mon, Jun 12, 2023 at 04:07:40PM +0200, Thomas Zimmermann wrote:
> Rename struct bd6107_platform_data.fbdev to 'dev', as it stores a
> pointer to the Linux platform device; not the fbdev device. Makes
> the code easier to understand.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.

[PATCH v2 03/38] backlight/gpio_backlight: Compare against struct fb_info.device

[Thomas Zimmermann]

Struct gpio_backlight_platform_data refers to a platform device within
the Linux device hierarchy. The test in gpio_backlight_check_fb()
compares it against the fbdev device in struct fb_info.dev, which
is different. Fix the test by comparing to struct fb_info.device.

Fixes a bug in the backlight driver and prepares fbdev for making
struct fb_info.dev optional.

v2:
	* move renames into separate patch (Javier, Sam, Michael)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 8b770e3c9824 ("backlight: Add GPIO-based backlight driver")
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Rich Felker <dalias@libc.org>
Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Lee Jones <lee@kernel.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: linux-sh@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v3.12+
---
 drivers/video/backlight/gpio_backlight.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 6f78d928f054a..5c5c99f7979e3 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -35,7 +35,7 @@ static int gpio_backlight_check_fb(struct backlight_device *bl,
 {
 	struct gpio_backlight *gbl = bl_get_data(bl);
 
-	return gbl->fbdev == NULL || gbl->fbdev == info->dev;
+	return gbl->fbdev == NULL || gbl->fbdev == info->device;
 }
 
 static const struct backlight_ops gpio_backlight_ops = {
-- 
2.41.0

Re: [PATCH v2 03/38] backlight/gpio_backlight: Compare against struct fb_info.device

[Daniel Thompson]

On Mon, Jun 12, 2023 at 04:07:41PM +0200, Thomas Zimmermann wrote:
> Struct gpio_backlight_platform_data refers to a platform device within
> the Linux device hierarchy. The test in gpio_backlight_check_fb()
> compares it against the fbdev device in struct fb_info.dev, which
> is different. Fix the test by comparing to struct fb_info.device.
>
> Fixes a bug in the backlight driver and prepares fbdev for making
> struct fb_info.dev optional.
>
> v2:
> 	* move renames into separate patch (Javier, Sam, Michael)
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 8b770e3c9824 ("backlight: Add GPIO-based backlight driver")
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Rich Felker <dalias@libc.org>
> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: linux-sh@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v3.12+

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.

[PATCH v2 04/38] backlight/gpio_backlight: Rename field 'fbdev' to 'dev'

[Thomas Zimmermann]

Rename the field 'fbdev' in struct gpio_backlight_platform_data and
struct gpio_backlight to 'dev', as they store pointers to the Linux
platform device; not the fbdev device. Makes the code easier to
understand.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Rich Felker <dalias@libc.org>
Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Lee Jones <lee@kernel.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: linux-sh@vger.kernel.org
---
 arch/sh/boards/mach-ecovec24/setup.c         | 2 +-
 drivers/video/backlight/gpio_backlight.c     | 6 +++---
 include/linux/platform_data/gpio_backlight.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 674da7ebd8b7f..310513646c9b3 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -386,7 +386,7 @@ static struct property_entry gpio_backlight_props[] = {
 };
 
 static struct gpio_backlight_platform_data gpio_backlight_data = {
-	.fbdev = &lcdc_device.dev,
+	.dev = &lcdc_device.dev,
 };
 
 static const struct platform_device_info gpio_backlight_device_info = {
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 5c5c99f7979e3..d3bea42407f15 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -17,7 +17,7 @@
 #include <linux/slab.h>
 
 struct gpio_backlight {
-	struct device *fbdev;
+	struct device *dev;
 	struct gpio_desc *gpiod;
 };
 
@@ -35,7 +35,7 @@ static int gpio_backlight_check_fb(struct backlight_device *bl,
 {
 	struct gpio_backlight *gbl = bl_get_data(bl);
 
-	return gbl->fbdev == NULL || gbl->fbdev == info->device;
+	return !gbl->dev || gbl->dev == info->device;
 }
 
 static const struct backlight_ops gpio_backlight_ops = {
@@ -59,7 +59,7 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	if (pdata)
-		gbl->fbdev = pdata->fbdev;
+		gbl->dev = pdata->dev;
 
 	def_value = device_property_read_bool(dev, "default-on");
 
diff --git a/include/linux/platform_data/gpio_backlight.h b/include/linux/platform_data/gpio_backlight.h
index 1a8b5b1946fe4..323fbf5f76139 100644
--- a/include/linux/platform_data/gpio_backlight.h
+++ b/include/linux/platform_data/gpio_backlight.h
@@ -8,7 +8,7 @@
 struct device;
 
 struct gpio_backlight_platform_data {
-	struct device *fbdev;
+	struct device *dev;
 };
 
 #endif
-- 
2.41.0

Re: [PATCH v2 04/38] backlight/gpio_backlight: Rename field 'fbdev' to 'dev'

[Daniel Thompson]

On Mon, Jun 12, 2023 at 04:07:42PM +0200, Thomas Zimmermann wrote:
> Rename the field 'fbdev' in struct gpio_backlight_platform_data and
> struct gpio_backlight to 'dev', as they store pointers to the Linux
> platform device; not the fbdev device. Makes the code easier to
> understand.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Rich Felker <dalias@libc.org>
> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: linux-sh@vger.kernel.org

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.

[PATCH v2 05/38] backlight/lv5207lp: Compare against struct fb_info.device

[Thomas Zimmermann]

Struct lv5207lp_platform_data refers to a platform device within
the Linux device hierarchy. The test in lv5207lp_backlight_check_fb()
compares it against the fbdev device in struct fb_info.dev, which
is different. Fix the test by comparing to struct fb_info.device.

Fixes a bug in the backlight driver and prepares fbdev for making
struct fb_info.dev optional.

v2:
	* move renames into separate patch (Javier, Sam, Michael)

Fixes: 82e5c40d88f9 ("backlight: Add Sanyo LV5207LP backlight driver")
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Lee Jones <lee@kernel.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: linux-sh@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v3.12+
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/backlight/lv5207lp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/backlight/lv5207lp.c b/drivers/video/backlight/lv5207lp.c
index 00673c8b66ac5..99ba4bc0a500d 100644
--- a/drivers/video/backlight/lv5207lp.c
+++ b/drivers/video/backlight/lv5207lp.c
@@ -67,7 +67,7 @@ static int lv5207lp_backlight_check_fb(struct backlight_device *backlight,
 {
 	struct lv5207lp *lv = bl_get_data(backlight);
 
-	return lv->pdata->fbdev == NULL || lv->pdata->fbdev == info->dev;
+	return lv->pdata->fbdev == NULL || lv->pdata->fbdev == info->device;
 }
 
 static const struct backlight_ops lv5207lp_backlight_ops = {
-- 
2.41.0

Re: [PATCH v2 05/38] backlight/lv5207lp: Compare against struct fb_info.device

[Daniel Thompson]

On Mon, Jun 12, 2023 at 04:07:43PM +0200, Thomas Zimmermann wrote:
> Struct lv5207lp_platform_data refers to a platform device within
> the Linux device hierarchy. The test in lv5207lp_backlight_check_fb()
> compares it against the fbdev device in struct fb_info.dev, which
> is different. Fix the test by comparing to struct fb_info.device.
>
> Fixes a bug in the backlight driver and prepares fbdev for making
> struct fb_info.dev optional.
>
> v2:
> 	* move renames into separate patch (Javier, Sam, Michael)
>
> Fixes: 82e5c40d88f9 ("backlight: Add Sanyo LV5207LP backlight driver")
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: linux-sh@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v3.12+
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.

[PATCH v2 06/38] backlight/lv5207lp: Rename struct lv5207lp_platform_data.fbdev to 'dev'

[Thomas Zimmermann]

Rename struct lv5207lp_platform_data.fbdev to 'dev', as it stores a
pointer to the Linux platform device; not the fbdev device. Makes
the code easier to understand.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Lee Jones <lee@kernel.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: linux-sh@vger.kernel.org
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 arch/sh/boards/mach-kfr2r09/setup.c    | 2 +-
 drivers/video/backlight/lv5207lp.c     | 2 +-
 include/linux/platform_data/lv5207lp.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/sh/boards/mach-kfr2r09/setup.c b/arch/sh/boards/mach-kfr2r09/setup.c
index 20f4db778ed6a..a18e80394aedc 100644
--- a/arch/sh/boards/mach-kfr2r09/setup.c
+++ b/arch/sh/boards/mach-kfr2r09/setup.c
@@ -202,7 +202,7 @@ static struct platform_device kfr2r09_sh_lcdc_device = {
 };
 
 static struct lv5207lp_platform_data kfr2r09_backlight_data = {
-	.fbdev = &kfr2r09_sh_lcdc_device.dev,
+	.dev = &kfr2r09_sh_lcdc_device.dev,
 	.def_value = 13,
 	.max_value = 13,
 };
diff --git a/drivers/video/backlight/lv5207lp.c b/drivers/video/backlight/lv5207lp.c
index 99ba4bc0a500d..739f45cd2d381 100644
--- a/drivers/video/backlight/lv5207lp.c
+++ b/drivers/video/backlight/lv5207lp.c
@@ -67,7 +67,7 @@ static int lv5207lp_backlight_check_fb(struct backlight_device *backlight,
 {
 	struct lv5207lp *lv = bl_get_data(backlight);
 
-	return lv->pdata->fbdev == NULL || lv->pdata->fbdev == info->device;
+	return !lv->pdata->dev || lv->pdata->dev == info->device;
 }
 
 static const struct backlight_ops lv5207lp_backlight_ops = {
diff --git a/include/linux/platform_data/lv5207lp.h b/include/linux/platform_data/lv5207lp.h
index c9da8d4027504..95d85c1394bca 100644
--- a/include/linux/platform_data/lv5207lp.h
+++ b/include/linux/platform_data/lv5207lp.h
@@ -8,7 +8,7 @@
 struct device;
 
 struct lv5207lp_platform_data {
-	struct device *fbdev;
+	struct device *dev;
 	unsigned int max_value;
 	unsigned int def_value;
 };
-- 
2.41.0

Re: [PATCH v2 06/38] backlight/lv5207lp: Rename struct lv5207lp_platform_data.fbdev to 'dev'

[Daniel Thompson]

On Mon, Jun 12, 2023 at 04:07:44PM +0200, Thomas Zimmermann wrote:
> Rename struct lv5207lp_platform_data.fbdev to 'dev', as it stores a
> pointer to the Linux platform device; not the fbdev device. Makes
> the code easier to understand.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: linux-sh@vger.kernel.org
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.

[PATCH v2 07/38] fbdev/atyfb: Reorder backlight and framebuffer init/cleanup

[Thomas Zimmermann]

The driver's backlight code requires the framebuffer to be
registered. Therefore reorder the init and cleanup calls for
both data structures.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/aty/atyfb_base.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
index cba2b113b28b0..51504fe39054c 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -2654,11 +2654,6 @@ static int aty_init(struct fb_info *info)
 			   USE_F32KHZ | TRISTATE_MEM_EN, par);
 	} else
 #endif
-	if (M64_HAS(MOBIL_BUS) && backlight) {
-#ifdef CONFIG_FB_ATY_BACKLIGHT
-		aty_bl_init(par);
-#endif
-	}
 
 	memset(&var, 0, sizeof(var));
 #ifdef CONFIG_PPC
@@ -2751,6 +2746,12 @@ static int aty_init(struct fb_info *info)
 		goto aty_init_exit;
 	}
 
+	if (M64_HAS(MOBIL_BUS) && backlight) {
+#ifdef CONFIG_FB_ATY_BACKLIGHT
+		aty_bl_init(par);
+#endif
+	}
+
 	fb_list = info;
 
 	PRINTKI("fb%d: %s frame buffer device on %s\n",
@@ -3716,12 +3717,13 @@ static void atyfb_remove(struct fb_info *info)
 	aty_set_crtc(par, &par->saved_crtc);
 	par->pll_ops->set_pll(info, &par->saved_pll);
 
-	unregister_framebuffer(info);
-
 #ifdef CONFIG_FB_ATY_BACKLIGHT
 	if (M64_HAS(MOBIL_BUS))
 		aty_bl_exit(info->bl_dev);
 #endif
+
+	unregister_framebuffer(info);
+
 	arch_phys_wc_del(par->wc_cookie);
 
 #ifndef __sparc__
-- 
2.41.0

[PATCH v2 08/38] fbdev/atyfb: Use hardware device as backlight parent

[Thomas Zimmermann]

Use the hardware device in struct fb_info.device as parent of the
backlight device. Aligns the driver with the rest of the codebase
and prepares fbdev for making struct fb_info.dev optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/aty/atyfb_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
index 51504fe39054c..e1602e3fbc66b 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -2255,7 +2255,7 @@ static void aty_bl_init(struct atyfb_par *par)
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = FB_BACKLIGHT_LEVELS - 1;
-	bd = backlight_device_register(name, info->dev, par, &aty_bl_data,
+	bd = backlight_device_register(name, info->device, par, &aty_bl_data,
 				       &props);
 	if (IS_ERR(bd)) {
 		info->bl_dev = NULL;
-- 
2.41.0

[PATCH v2 09/38] fbdev/aty128fb: Reorder backlight and framebuffer init/cleanup

[Thomas Zimmermann]

The driver's backlight code requires the framebuffer to be
registered. Therefore reorder the init and cleanup calls for
both data structures.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/aty/aty128fb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/aty/aty128fb.c b/drivers/video/fbdev/aty/aty128fb.c
index 36a9ac05a340f..b4a49068a522f 100644
--- a/drivers/video/fbdev/aty/aty128fb.c
+++ b/drivers/video/fbdev/aty/aty128fb.c
@@ -2028,14 +2028,14 @@ static int aty128_init(struct pci_dev *pdev, const struct pci_device_id *ent)
 	par->asleep = 0;
 	par->lock_blank = 0;
 
+	if (register_framebuffer(info) < 0)
+		return 0;
+
 #ifdef CONFIG_FB_ATY128_BACKLIGHT
 	if (backlight)
 		aty128_bl_init(par);
 #endif
 
-	if (register_framebuffer(info) < 0)
-		return 0;
-
 	fb_info(info, "%s frame buffer device on %s\n",
 		info->fix.id, video_card);
 
@@ -2167,12 +2167,12 @@ static void aty128_remove(struct pci_dev *pdev)
 
 	par = info->par;
 
-	unregister_framebuffer(info);
-
 #ifdef CONFIG_FB_ATY128_BACKLIGHT
 	aty128_bl_exit(info->bl_dev);
 #endif
 
+	unregister_framebuffer(info);
+
 	arch_phys_wc_del(par->wc_cookie);
 	iounmap(par->regbase);
 	iounmap(info->screen_base);
-- 
2.41.0

[PATCH v2 10/38] fbdev/aty128fb: Use hardware device as backlight parent

[Thomas Zimmermann]

Use the hardware device in struct fb_info.device as parent of the
backlight device. Aligns the driver with the rest of the codebase
and prepares fbdev for making struct fb_info.dev optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/aty/aty128fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/aty/aty128fb.c b/drivers/video/fbdev/aty/aty128fb.c
index b4a49068a522f..2d9320a52e519 100644
--- a/drivers/video/fbdev/aty/aty128fb.c
+++ b/drivers/video/fbdev/aty/aty128fb.c
@@ -1846,7 +1846,7 @@ static void aty128_bl_init(struct aty128fb_par *par)
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = FB_BACKLIGHT_LEVELS - 1;
-	bd = backlight_device_register(name, info->dev, par, &aty128_bl_data,
+	bd = backlight_device_register(name, info->device, par, &aty128_bl_data,
 				       &props);
 	if (IS_ERR(bd)) {
 		info->bl_dev = NULL;
-- 
2.41.0

[PATCH v2 11/38] fbdev/broadsheetfb: Call device_remove_file() with hardware device

[Thomas Zimmermann]

Call device_remove_file() with the same device that has been used
for device_create_file(), which is the hardware device stored in
struct fb_info.device. Prepares fbdev for making struct fb_info.dev
optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/broadsheetfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/broadsheetfb.c b/drivers/video/fbdev/broadsheetfb.c
index 5f18af88e7408..5a5fe4bbc10b0 100644
--- a/drivers/video/fbdev/broadsheetfb.c
+++ b/drivers/video/fbdev/broadsheetfb.c
@@ -1200,7 +1200,7 @@ static void broadsheetfb_remove(struct platform_device *dev)
 	if (info) {
 		struct broadsheetfb_par *par = info->par;
 
-		device_remove_file(info->dev, &dev_attr_loadstore_waveform);
+		device_remove_file(info->device, &dev_attr_loadstore_waveform);
 		unregister_framebuffer(info);
 		fb_deferred_io_cleanup(info);
 		par->board->cleanup(par);
-- 
2.41.0

[PATCH v2 12/38] fbdev/ep93xx-fb: Alloc DMA memory from hardware device

[Thomas Zimmermann]

Pass the hardware device to the DMA helpers dma_alloc_wc(), dma_mmap_wc()
and dma_free_coherent(). The fbdev device that is currently being used is
a software device and does not provide DMA memory.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/ep93xx-fb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/ep93xx-fb.c b/drivers/video/fbdev/ep93xx-fb.c
index 94fe52928be25..376ee59e925c2 100644
--- a/drivers/video/fbdev/ep93xx-fb.c
+++ b/drivers/video/fbdev/ep93xx-fb.c
@@ -312,7 +312,7 @@ static int ep93xxfb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	unsigned int offset = vma->vm_pgoff << PAGE_SHIFT;
 
 	if (offset < info->fix.smem_len) {
-		return dma_mmap_wc(info->dev, vma, info->screen_base,
+		return dma_mmap_wc(info->device, vma, info->screen_base,
 				   info->fix.smem_start, info->fix.smem_len);
 	}
 
@@ -423,7 +423,7 @@ static int ep93xxfb_alloc_videomem(struct fb_info *info)
 	/* Maximum 16bpp -> used memory is maximum x*y*2 bytes */
 	fb_size = EP93XXFB_MAX_XRES * EP93XXFB_MAX_YRES * 2;
 
-	virt_addr = dma_alloc_wc(info->dev, fb_size, &phys_addr, GFP_KERNEL);
+	virt_addr = dma_alloc_wc(info->device, fb_size, &phys_addr, GFP_KERNEL);
 	if (!virt_addr)
 		return -ENOMEM;
 
@@ -440,7 +440,7 @@ static int ep93xxfb_alloc_videomem(struct fb_info *info)
 			"has bit 27 set: cannot init framebuffer\n",
 			phys_addr);
 
-		dma_free_coherent(info->dev, fb_size, virt_addr, phys_addr);
+		dma_free_coherent(info->device, fb_size, virt_addr, phys_addr);
 		return -ENOMEM;
 	}
 
@@ -454,7 +454,7 @@ static int ep93xxfb_alloc_videomem(struct fb_info *info)
 static void ep93xxfb_dealloc_videomem(struct fb_info *info)
 {
 	if (info->screen_base)
-		dma_free_coherent(info->dev, info->fix.smem_len,
+		dma_free_coherent(info->device, info->fix.smem_len,
 				  info->screen_base, info->fix.smem_start);
 }
 
-- 
2.41.0

[PATCH v2 13/38] fbdev/ep93xx-fb: Output messages with fb_info() and fb_err()

[Thomas Zimmermann]

Fix cases were output helpers are called with struct fb_info.dev.
Use fb_info() and fb_err() instead.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/ep93xx-fb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/ep93xx-fb.c b/drivers/video/fbdev/ep93xx-fb.c
index 376ee59e925c2..f6cd200fe50ff 100644
--- a/drivers/video/fbdev/ep93xx-fb.c
+++ b/drivers/video/fbdev/ep93xx-fb.c
@@ -436,9 +436,9 @@ static int ep93xxfb_alloc_videomem(struct fb_info *info)
 	 * least.
 	 */
 	if (check_screenpage_bug && phys_addr & (1 << 27)) {
-		dev_err(info->dev, "ep93xx framebuffer bug. phys addr (0x%x) "
-			"has bit 27 set: cannot init framebuffer\n",
-			phys_addr);
+		fb_err(info, "ep93xx framebuffer bug. phys addr (0x%x) "
+		       "has bit 27 set: cannot init framebuffer\n",
+		       phys_addr);
 
 		dma_free_coherent(info->device, fb_size, virt_addr, phys_addr);
 		return -ENOMEM;
@@ -525,7 +525,7 @@ static int ep93xxfb_probe(struct platform_device *pdev)
 	err = fb_find_mode(&info->var, info, video_mode,
 			   NULL, 0, NULL, 16);
 	if (err == 0) {
-		dev_err(info->dev, "No suitable video mode found\n");
+		fb_err(info, "No suitable video mode found\n");
 		err = -EINVAL;
 		goto failed_resource;
 	}
@@ -554,8 +554,8 @@ static int ep93xxfb_probe(struct platform_device *pdev)
 	if (err)
 		goto failed_framebuffer;
 
-	dev_info(info->dev, "registered. Mode = %dx%d-%d\n",
-		 info->var.xres, info->var.yres, info->var.bits_per_pixel);
+	fb_info(info, "registered. Mode = %dx%d-%d\n",
+		info->var.xres, info->var.yres, info->var.bits_per_pixel);
 	return 0;
 
 failed_framebuffer:
-- 
2.41.0

[PATCH v2 14/38] fbdev/ep93xx-fb: Do not assign to struct fb_info.dev

[Thomas Zimmermann]

Do not assing the Linux device to struct fb_info.dev. The call to
register_framebuffer() initializes the field to the fbdev device.
Drivers should not override its value.

Fixes a bug where the driver incorrectly decreases the hardware
device's reference counter and leaks the fbdev device.

v2:
	* add Fixes tag (Dan)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 88017bda96a5 ("ep93xx video driver")
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Cc: <stable@vger.kernel.org> # v2.6.32+
---
 drivers/video/fbdev/ep93xx-fb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/fbdev/ep93xx-fb.c b/drivers/video/fbdev/ep93xx-fb.c
index f6cd200fe50ff..37309f9dbe828 100644
--- a/drivers/video/fbdev/ep93xx-fb.c
+++ b/drivers/video/fbdev/ep93xx-fb.c
@@ -474,7 +474,6 @@ static int ep93xxfb_probe(struct platform_device *pdev)
 	if (!info)
 		return -ENOMEM;
 
-	info->dev = &pdev->dev;
 	platform_set_drvdata(pdev, info);
 	fbi = info->par;
 	fbi->mach_info = mach_info;
-- 
2.41.0

[PATCH v2 15/38] fbdev/fsl-diu-fb: Output messages with fb_*() helpers

[Thomas Zimmermann]

Fix cases were output helpers are called with struct fb_info.dev.
Use fb_*() helpers instead. Prepares fbdev for making struct
fb_info.dev optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Timur Tabi <timur@kernel.org>
---
 drivers/video/fbdev/fsl-diu-fb.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/video/fbdev/fsl-diu-fb.c b/drivers/video/fbdev/fsl-diu-fb.c
index 730a07d23fa92..785eb8a06943f 100644
--- a/drivers/video/fbdev/fsl-diu-fb.c
+++ b/drivers/video/fbdev/fsl-diu-fb.c
@@ -872,7 +872,7 @@ static int map_video_memory(struct fb_info *info)
 
 	p = alloc_pages_exact(smem_len, GFP_DMA | __GFP_ZERO);
 	if (!p) {
-		dev_err(info->dev, "unable to allocate fb memory\n");
+		fb_err(info, "unable to allocate fb memory\n");
 		return -ENOMEM;
 	}
 	mutex_lock(&info->mm_lock);
@@ -1145,7 +1145,7 @@ static int fsl_diu_set_par(struct fb_info *info)
 
 		/* Memory allocation for framebuffer */
 		if (map_video_memory(info)) {
-			dev_err(info->dev, "unable to allocate fb memory 1\n");
+			fb_err(info, "unable to allocate fb memory 1\n");
 			return -ENOMEM;
 		}
 	}
@@ -1277,16 +1277,16 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 	if (!arg)
 		return -EINVAL;
 
-	dev_dbg(info->dev, "ioctl %08x (dir=%s%s type=%u nr=%u size=%u)\n", cmd,
+	fb_dbg(info, "ioctl %08x (dir=%s%s type=%u nr=%u size=%u)\n", cmd,
 		_IOC_DIR(cmd) & _IOC_READ ? "R" : "",
 		_IOC_DIR(cmd) & _IOC_WRITE ? "W" : "",
 		_IOC_TYPE(cmd), _IOC_NR(cmd), _IOC_SIZE(cmd));
 
 	switch (cmd) {
 	case MFB_SET_PIXFMT_OLD:
-		dev_warn(info->dev,
-			 "MFB_SET_PIXFMT value of 0x%08x is deprecated.\n",
-			 MFB_SET_PIXFMT_OLD);
+		fb_warn(info,
+			"MFB_SET_PIXFMT value of 0x%08x is deprecated.\n",
+			MFB_SET_PIXFMT_OLD);
 		fallthrough;
 	case MFB_SET_PIXFMT:
 		if (copy_from_user(&pix_fmt, buf, sizeof(pix_fmt)))
@@ -1294,9 +1294,9 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		ad->pix_fmt = pix_fmt;
 		break;
 	case MFB_GET_PIXFMT_OLD:
-		dev_warn(info->dev,
-			 "MFB_GET_PIXFMT value of 0x%08x is deprecated.\n",
-			 MFB_GET_PIXFMT_OLD);
+		fb_warn(info,
+			"MFB_GET_PIXFMT value of 0x%08x is deprecated.\n",
+			MFB_GET_PIXFMT_OLD);
 		fallthrough;
 	case MFB_GET_PIXFMT:
 		pix_fmt = ad->pix_fmt;
@@ -1375,7 +1375,7 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 	}
 #endif
 	default:
-		dev_err(info->dev, "unknown ioctl command (0x%08X)\n", cmd);
+		fb_err(info, "unknown ioctl command (0x%08X)\n", cmd);
 		return -ENOIOCTLCMD;
 	}
 
@@ -1543,21 +1543,21 @@ static int install_fb(struct fb_info *info)
 	}
 
 	if (fsl_diu_check_var(&info->var, info)) {
-		dev_err(info->dev, "fsl_diu_check_var failed\n");
+		fb_err(info, "fsl_diu_check_var failed\n");
 		unmap_video_memory(info);
 		fb_dealloc_cmap(&info->cmap);
 		return -EINVAL;
 	}
 
 	if (register_framebuffer(info) < 0) {
-		dev_err(info->dev, "register_framebuffer failed\n");
+		fb_err(info, "register_framebuffer failed\n");
 		unmap_video_memory(info);
 		fb_dealloc_cmap(&info->cmap);
 		return -EINVAL;
 	}
 
 	mfbi->registered = 1;
-	dev_info(info->dev, "%s registered successfully\n", mfbi->id);
+	fb_info(info, "%s registered successfully\n", mfbi->id);
 
 	return 0;
 }
-- 
2.41.0

[PATCH v2 16/38] fbdev/mb862xxfb: Output messages with fb_dbg()

[Thomas Zimmermann]

Fix cases were output helpers are called with struct fb_info.dev.
Use fb_dbg() instead. Prepares fbdev for making struct fb_info.dev
optional.

v2:
	* fix another reference to struct fb_info.dev (kernel test reobot)
	* remove fb_err() from commit message

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index b5c8fcab9940d..119c2a582ecbd 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -112,8 +112,7 @@ static int mb862xxfb_check_var(struct fb_var_screeninfo *var,
 {
 	unsigned long tmp;
 
-	if (fbi->dev)
-		dev_dbg(fbi->dev, "%s\n", __func__);
+	fb_dbg(fbi, "%s\n", __func__);
 
 	/* check if these values fit into the registers */
 	if (var->hsync_len > 255 || var->vsync_len > 255)
@@ -290,7 +289,7 @@ static int mb862xxfb_blank(int mode, struct fb_info *fbi)
 	struct mb862xxfb_par  *par = fbi->par;
 	unsigned long reg;
 
-	dev_dbg(fbi->dev, "blank mode=%d\n", mode);
+	fb_dbg(fbi, "blank mode=%d\n", mode);
 
 	switch (mode) {
 	case FB_BLANK_POWERDOWN:
@@ -791,7 +790,7 @@ static void of_platform_mb862xx_remove(struct platform_device *ofdev)
 	resource_size_t res_size = resource_size(par->res);
 	unsigned long reg;
 
-	dev_dbg(fbi->dev, "%s release\n", fbi->fix.id);
+	fb_dbg(fbi, "%s release\n", fbi->fix.id);
 
 	/* display off */
 	reg = inreg(disp, GC_DCM1);
@@ -1138,7 +1137,7 @@ static void mb862xx_pci_remove(struct pci_dev *pdev)
 	struct mb862xxfb_par *par = fbi->par;
 	unsigned long reg;
 
-	dev_dbg(fbi->dev, "%s release\n", fbi->fix.id);
+	fb_dbg(fbi, "%s release\n", fbi->fix.id);
 
 	/* display off */
 	reg = inreg(disp, GC_DCM1);
-- 
2.41.0

[PATCH v2 17/38] fbdev/metronomefb: Use hardware device for dev_err()

[Thomas Zimmermann]

Replace the use of the fbdev software device, stored in struct
fb_info.dev, with the hardware device from struct fb_info.device
in load_waveform(). The device is only used for printing errors
with dev_err().

This change aligns load_waveform() with the rest of the driver and
prepares fbdev for making struct fb_info.dev optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/metronomefb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/metronomefb.c b/drivers/video/fbdev/metronomefb.c
index bac255c749e78..3e1daca76e114 100644
--- a/drivers/video/fbdev/metronomefb.c
+++ b/drivers/video/fbdev/metronomefb.c
@@ -181,7 +181,7 @@ static int load_waveform(u8 *mem, size_t size, int m, int t,
 	int mem_idx = 0;
 	struct waveform_hdr *wfm_hdr;
 	u8 *metromem = par->metromem_wfm;
-	struct device *dev = par->info->dev;
+	struct device *dev = par->info->device;
 
 	if (user_wfm_size)
 		epd_frame_table[par->dt].wfm_size = user_wfm_size;
-- 
2.41.0

[PATCH v2 18/38] fbdev/nvidiafb: Reorder backlight and framebuffer init/cleanup

[Thomas Zimmermann]

The driver's backlight code requires the framebuffer to be
registered. Therefore reorder the init and cleanup calls for
both data structures.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Antonino Daplas <adaplas@gmail.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/nvidia/nvidia.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/nvidia/nvidia.c b/drivers/video/fbdev/nvidia/nvidia.c
index ea4ba3dfb96bb..039e886346fa6 100644
--- a/drivers/video/fbdev/nvidia/nvidia.c
+++ b/drivers/video/fbdev/nvidia/nvidia.c
@@ -1400,14 +1400,14 @@ static int nvidiafb_probe(struct pci_dev *pd, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pd, info);
 
-	if (backlight)
-		nvidia_bl_init(par);
-
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR PFX "error registering nVidia framebuffer\n");
 		goto err_out_iounmap_fb;
 	}
 
+	if (backlight)
+		nvidia_bl_init(par);
+
 	printk(KERN_INFO PFX
 	       "PCI nVidia %s framebuffer (%dMB @ 0x%lX)\n",
 	       info->fix.id,
@@ -1439,9 +1439,9 @@ static void nvidiafb_remove(struct pci_dev *pd)
 
 	NVTRACE_ENTER();
 
+	nvidia_bl_exit(par);
 	unregister_framebuffer(info);
 
-	nvidia_bl_exit(par);
 	arch_phys_wc_del(par->wc_cookie);
 	iounmap(info->screen_base);
 	fb_destroy_modedb(info->monspecs.modedb);
-- 
2.41.0

[PATCH v2 19/38] fbdev/nvidiafb: Use hardware device as backlight parent

[Thomas Zimmermann]

Use the hardware device in struct fb_info.device as parent of the
backlight device. Aligns the driver with the rest of the codebase
and prepares fbdev for making struct fb_info.dev optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Antonino Daplas <adaplas@gmail.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/nvidia/nv_backlight.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/nvidia/nv_backlight.c b/drivers/video/fbdev/nvidia/nv_backlight.c
index 503a7a683855a..160da9c50a52c 100644
--- a/drivers/video/fbdev/nvidia/nv_backlight.c
+++ b/drivers/video/fbdev/nvidia/nv_backlight.c
@@ -98,7 +98,7 @@ void nvidia_bl_init(struct nvidia_par *par)
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = FB_BACKLIGHT_LEVELS - 1;
-	bd = backlight_device_register(name, info->dev, par, &nvidia_bl_ops,
+	bd = backlight_device_register(name, info->device, par, &nvidia_bl_ops,
 				       &props);
 	if (IS_ERR(bd)) {
 		info->bl_dev = NULL;
-- 
2.41.0

[PATCH v2 20/38] fbdev/pxa168fb: Do not assign to struct fb_info.dev

[Thomas Zimmermann]

Do not assign the hardware device to struct fb_info.dev. The
field references the fbdev software device, which is unrelated.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/pxa168fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/pxa168fb.c b/drivers/video/fbdev/pxa168fb.c
index 79f3384630926..82cb9ffe52908 100644
--- a/drivers/video/fbdev/pxa168fb.c
+++ b/drivers/video/fbdev/pxa168fb.c
@@ -629,7 +629,7 @@ static int pxa168fb_probe(struct platform_device *pdev)
 	fbi = info->par;
 	fbi->info = info;
 	fbi->clk = clk;
-	fbi->dev = info->dev = &pdev->dev;
+	fbi->dev = &pdev->dev;
 	fbi->panel_rbswap = mi->panel_rbswap;
 	fbi->is_blanked = 0;
 	fbi->active = mi->active;
-- 
2.41.0

[PATCH v2 21/38] fbdev/radeonfb: Reorder backlight and framebuffer cleanup

[Thomas Zimmermann]

The driver's backlight code requires the framebuffer to be
registered. Therefore reorder the cleanup calls for both data
structures. The init calls are already in the correct order.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/aty/radeon_base.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index 972c4bbedfa36..8f2a527c26ebf 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2517,9 +2517,8 @@ static void radeonfb_pci_unregister(struct pci_dev *pdev)
 
 	del_timer_sync(&rinfo->lvds_timer);
 	arch_phys_wc_del(rinfo->wc_cookie);
-        unregister_framebuffer(info);
-
         radeonfb_bl_exit(rinfo);
+	unregister_framebuffer(info);
 
         iounmap(rinfo->mmio_base);
         iounmap(rinfo->fb_base);
-- 
2.41.0

Re: [PATCH v2 21/38] fbdev/radeonfb: Reorder backlight and framebuffer cleanup

[Sam Ravnborg]

Hi Thomas,

On Mon, Jun 12, 2023 at 04:07:59PM +0200, Thomas Zimmermann wrote:
> The driver's backlight code requires the framebuffer to be
> registered. Therefore reorder the cleanup calls for both data
> structures. The init calls are already in the correct order.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>  drivers/video/fbdev/aty/radeon_base.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
> index 972c4bbedfa36..8f2a527c26ebf 100644
> --- a/drivers/video/fbdev/aty/radeon_base.c
> +++ b/drivers/video/fbdev/aty/radeon_base.c
> @@ -2517,9 +2517,8 @@ static void radeonfb_pci_unregister(struct pci_dev *pdev)
>  
>  	del_timer_sync(&rinfo->lvds_timer);
>  	arch_phys_wc_del(rinfo->wc_cookie);
> -        unregister_framebuffer(info);
> -
>          radeonfb_bl_exit(rinfo);
> +	unregister_framebuffer(info);
>  
>          iounmap(rinfo->mmio_base);
>          iounmap(rinfo->fb_base);
The mix of spaces and tabs for indent looks strange in the diff.
Consider to use the style of the surrounding code if you are going to
refresh the patch-set.

	Sam

[PATCH v2 22/38] fbdev/radeonfb: Use hardware device as backlight parent

[Thomas Zimmermann]

Use the hardware device in struct fb_info.device as parent of the
backlight device. Aligns the driver with the rest of the codebase
and prepares fbdev for making struct fb_info.dev optional.

v2:
	* add Cc: tag (Dan)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/aty/radeon_backlight.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/aty/radeon_backlight.c b/drivers/video/fbdev/aty/radeon_backlight.c
index 427adc838f77e..23a38c3f3977e 100644
--- a/drivers/video/fbdev/aty/radeon_backlight.c
+++ b/drivers/video/fbdev/aty/radeon_backlight.c
@@ -147,7 +147,7 @@ void radeonfb_bl_init(struct radeonfb_info *rinfo)
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = FB_BACKLIGHT_LEVELS - 1;
-	bd = backlight_device_register(name, rinfo->info->dev, pdata,
+	bd = backlight_device_register(name, rinfo->info->device, pdata,
 				       &radeon_bl_data, &props);
 	if (IS_ERR(bd)) {
 		rinfo->info->bl_dev = NULL;
-- 
2.41.0

[PATCH v2 23/38] fbdev/rivafb: Reorder backlight and framebuffer init/cleanup

[Thomas Zimmermann]

The driver's backlight code requires the framebuffer to be
registered. Therefore reorder the init and cleanup calls for
both data structures.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Antonino Daplas <adaplas@gmail.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/riva/fbdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c
index 41edc6e794603..e328b2d39e2b6 100644
--- a/drivers/video/fbdev/riva/fbdev.c
+++ b/drivers/video/fbdev/riva/fbdev.c
@@ -2031,9 +2031,6 @@ static int rivafb_probe(struct pci_dev *pd, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pd, info);
 
-	if (backlight)
-		riva_bl_init(info->par);
-
 	ret = register_framebuffer(info);
 	if (ret < 0) {
 		printk(KERN_ERR PFX
@@ -2041,6 +2038,9 @@ static int rivafb_probe(struct pci_dev *pd, const struct pci_device_id *ent)
 		goto err_iounmap_screen_base;
 	}
 
+	if (backlight)
+		riva_bl_init(info->par);
+
 	printk(KERN_INFO PFX
 		"PCI nVidia %s framebuffer ver %s (%dMB @ 0x%lX)\n",
 		info->fix.id,
@@ -2084,9 +2084,9 @@ static void rivafb_remove(struct pci_dev *pd)
 	kfree(par->EDID);
 #endif
 
+	riva_bl_exit(info);
 	unregister_framebuffer(info);
 
-	riva_bl_exit(info);
 	arch_phys_wc_del(par->wc_cookie);
 	iounmap(par->ctrl_base);
 	iounmap(info->screen_base);
-- 
2.41.0

[PATCH v2 24/38] fbdev/rivafb: Use hardware device as backlight parent

[Thomas Zimmermann]

Use the hardware device in struct fb_info.device as parent of the
backlight device. Aligns the driver with the rest of the codebase
and prepares fbdev for making struct fb_info.dev optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Antonino Daplas <adaplas@gmail.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/riva/fbdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c
index e328b2d39e2b6..6ade8de5df4a0 100644
--- a/drivers/video/fbdev/riva/fbdev.c
+++ b/drivers/video/fbdev/riva/fbdev.c
@@ -333,7 +333,7 @@ static void riva_bl_init(struct riva_par *par)
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = FB_BACKLIGHT_LEVELS - 1;
-	bd = backlight_device_register(name, info->dev, par, &riva_bl_ops,
+	bd = backlight_device_register(name, info->device, par, &riva_bl_ops,
 				       &props);
 	if (IS_ERR(bd)) {
 		info->bl_dev = NULL;
-- 
2.41.0

[PATCH v2 25/38] fbdev/sh7760fb: Use fb_dbg() in sh7760fb_get_color_info()

[Thomas Zimmermann]

Give struct fb_info to sh7760fb_get_color_info() and use it in
call to fb_dbg(). Prepares fbdev for making struct fb_info.dev
optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/sh7760fb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/sh7760fb.c b/drivers/video/fbdev/sh7760fb.c
index 4c092c7935027..a2946f06d579e 100644
--- a/drivers/video/fbdev/sh7760fb.c
+++ b/drivers/video/fbdev/sh7760fb.c
@@ -118,7 +118,7 @@ static int sh7760_setcolreg (u_int regno,
 	return 0;
 }
 
-static int sh7760fb_get_color_info(struct device *dev,
+static int sh7760fb_get_color_info(struct fb_info *info,
 				   u16 lddfr, int *bpp, int *gray)
 {
 	int lbpp, lgray;
@@ -150,7 +150,7 @@ static int sh7760fb_get_color_info(struct device *dev,
 		lgray = 0;
 		break;
 	default:
-		dev_dbg(dev, "unsupported LDDFR bit depth.\n");
+		fb_dbg(info, "unsupported LDDFR bit depth.\n");
 		return -EINVAL;
 	}
 
@@ -170,7 +170,7 @@ static int sh7760fb_check_var(struct fb_var_screeninfo *var,
 	int ret, bpp;
 
 	/* get color info from register value */
-	ret = sh7760fb_get_color_info(info->dev, par->pd->lddfr, &bpp, NULL);
+	ret = sh7760fb_get_color_info(info, par->pd->lddfr, &bpp, NULL);
 	if (ret)
 		return ret;
 
@@ -222,7 +222,7 @@ static int sh7760fb_set_par(struct fb_info *info)
 	vdln = vm->yres;
 
 	/* get color info from register value */
-	ret = sh7760fb_get_color_info(info->dev, par->pd->lddfr, &bpp, &gray);
+	ret = sh7760fb_get_color_info(info, par->pd->lddfr, &bpp, &gray);
 	if (ret)
 		return ret;
 
@@ -381,7 +381,7 @@ static int sh7760fb_alloc_mem(struct fb_info *info)
 		return 0;
 
 	/* get color info from register value */
-	ret = sh7760fb_get_color_info(info->dev, par->pd->lddfr, &bpp, NULL);
+	ret = sh7760fb_get_color_info(info, par->pd->lddfr, &bpp, NULL);
 	if (ret) {
 		printk(KERN_ERR "colinfo\n");
 		return ret;
-- 
2.41.0

[PATCH v2 26/38] fbdev/sh7760fb: Output messages with fb_dbg()

[Thomas Zimmermann]

Fix cases were output helpers are called with struct fb_info.dev.
Use fb_dbg() instead. Prepares fbdev for making struct fb_info.dev
optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/sh7760fb.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/video/fbdev/sh7760fb.c b/drivers/video/fbdev/sh7760fb.c
index a2946f06d579e..65e2c71cf5b51 100644
--- a/drivers/video/fbdev/sh7760fb.c
+++ b/drivers/video/fbdev/sh7760fb.c
@@ -207,7 +207,7 @@ static int sh7760fb_set_par(struct fb_info *info)
 
 	/* rotate only works with xres <= 320 */
 	if (par->rot && (vm->xres > 320)) {
-		dev_dbg(info->dev, "rotation disabled due to display size\n");
+		fb_dbg(info, "rotation disabled due to display size\n");
 		par->rot = 0;
 	}
 
@@ -226,7 +226,7 @@ static int sh7760fb_set_par(struct fb_info *info)
 	if (ret)
 		return ret;
 
-	dev_dbg(info->dev, "%dx%d %dbpp %s (orientation %s)\n", hdcn,
+	fb_dbg(info, "%dx%d %dbpp %s (orientation %s)\n", hdcn,
 		vdln, bpp, gray ? "grayscale" : "color",
 		par->rot ? "rotated" : "normal");
 
@@ -306,7 +306,7 @@ static int sh7760fb_set_par(struct fb_info *info)
 	if (((ldmtr & 0x003f) >= LDMTR_DSTN_MONO_8) &&
 	    ((ldmtr & 0x003f) <= LDMTR_DSTN_COLOR_16)) {
 
-		dev_dbg(info->dev, " ***** DSTN untested! *****\n");
+		fb_dbg(info, " ***** DSTN untested! *****\n");
 
 		dstn_off = stride;
 		if (par->rot)
@@ -326,17 +326,17 @@ static int sh7760fb_set_par(struct fb_info *info)
 
 	sh7760fb_blank(FB_BLANK_UNBLANK, info);	/* panel on! */
 
-	dev_dbg(info->dev, "hdcn  : %6d htcn  : %6d\n", hdcn, htcn);
-	dev_dbg(info->dev, "hsynw : %6d hsynp : %6d\n", hsynw, hsynp);
-	dev_dbg(info->dev, "vdln  : %6d vtln  : %6d\n", vdln, vtln);
-	dev_dbg(info->dev, "vsynw : %6d vsynp : %6d\n", vsynw, vsynp);
-	dev_dbg(info->dev, "clksrc: %6d clkdiv: %6d\n",
+	fb_dbg(info, "hdcn  : %6d htcn  : %6d\n", hdcn, htcn);
+	fb_dbg(info, "hsynw : %6d hsynp : %6d\n", hsynw, hsynp);
+	fb_dbg(info, "vdln  : %6d vtln  : %6d\n", vdln, vtln);
+	fb_dbg(info, "vsynw : %6d vsynp : %6d\n", vsynw, vsynp);
+	fb_dbg(info, "clksrc: %6d clkdiv: %6d\n",
 		(par->pd->ldickr >> 12) & 3, par->pd->ldickr & 0x1f);
-	dev_dbg(info->dev, "ldpmmr: 0x%04x ldpspr: 0x%04x\n", par->pd->ldpmmr,
+	fb_dbg(info, "ldpmmr: 0x%04x ldpspr: 0x%04x\n", par->pd->ldpmmr,
 		par->pd->ldpspr);
-	dev_dbg(info->dev, "ldmtr : 0x%04x lddfr : 0x%04x\n", ldmtr, lddfr);
-	dev_dbg(info->dev, "ldlaor: %ld\n", stride);
-	dev_dbg(info->dev, "ldsaru: 0x%08lx ldsarl: 0x%08lx\n", sbase, ldsarl);
+	fb_dbg(info, "ldmtr : 0x%04x lddfr : 0x%04x\n", ldmtr, lddfr);
+	fb_dbg(info, "ldlaor: %ld\n", stride);
+	fb_dbg(info, "ldsaru: 0x%08lx ldsarl: 0x%08lx\n", sbase, ldsarl);
 
 	return 0;
 }
@@ -401,7 +401,7 @@ static int sh7760fb_alloc_mem(struct fb_info *info)
 	} else if (bpp > 8)
 		vram *= 2;
 	if ((vram < 1) || (vram > 1024 * 2048)) {
-		dev_dbg(info->dev, "too much VRAM required. Check settings\n");
+		fb_dbg(info, "too much VRAM required. Check settings\n");
 		return -ENODEV;
 	}
 
-- 
2.41.0

[PATCH v2 27/38] fbdev/sh7760fb: Alloc DMA memory from hardware device

[Thomas Zimmermann]

Pass the hardware device to the DMA helpers dma_alloc_coherent() and
dma_free_coherent(). The fbdev device that is currently being used is
a software device and does not provide DMA memory. Also update the
related dev_*() output statements similarly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/sh7760fb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/sh7760fb.c b/drivers/video/fbdev/sh7760fb.c
index 65e2c71cf5b51..74543a1e30314 100644
--- a/drivers/video/fbdev/sh7760fb.c
+++ b/drivers/video/fbdev/sh7760fb.c
@@ -359,7 +359,7 @@ static void sh7760fb_free_mem(struct fb_info *info)
 	if (!info->screen_base)
 		return;
 
-	dma_free_coherent(info->dev, info->screen_size,
+	dma_free_coherent(info->device, info->screen_size,
 			  info->screen_base, par->fbdma);
 
 	par->fbdma = 0;
@@ -408,14 +408,14 @@ static int sh7760fb_alloc_mem(struct fb_info *info)
 	if (vram < PAGE_SIZE)
 		vram = PAGE_SIZE;
 
-	fbmem = dma_alloc_coherent(info->dev, vram, &par->fbdma, GFP_KERNEL);
+	fbmem = dma_alloc_coherent(info->device, vram, &par->fbdma, GFP_KERNEL);
 
 	if (!fbmem)
 		return -ENOMEM;
 
 	if ((par->fbdma & SH7760FB_DMA_MASK) != SH7760FB_DMA_MASK) {
 		sh7760fb_free_mem(info);
-		dev_err(info->dev, "kernel gave me memory at 0x%08lx, which is"
+		dev_err(info->device, "kernel gave me memory at 0x%08lx, which is"
 			"unusable for the LCDC\n", (unsigned long)par->fbdma);
 		return -ENOMEM;
 	}
@@ -486,7 +486,7 @@ static int sh7760fb_probe(struct platform_device *pdev)
 
 	ret = sh7760fb_alloc_mem(info);
 	if (ret) {
-		dev_dbg(info->dev, "framebuffer memory allocation failed!\n");
+		dev_dbg(info->device, "framebuffer memory allocation failed!\n");
 		goto out_unmap;
 	}
 
-- 
2.41.0

[PATCH v2 28/38] fbdev/sh7760fb: Use hardware device with dev_() output during probe

[Thomas Zimmermann]

Call output helpers in the probe function with the hardware device.
The virtual fbdev device has not been initialized at that point. Also
prepares fbdev for making struct fb_info.dev optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/sh7760fb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/sh7760fb.c b/drivers/video/fbdev/sh7760fb.c
index 74543a1e30314..8566bcd664076 100644
--- a/drivers/video/fbdev/sh7760fb.c
+++ b/drivers/video/fbdev/sh7760fb.c
@@ -450,7 +450,7 @@ static int sh7760fb_probe(struct platform_device *pdev)
 
 	par->pd = pdev->dev.platform_data;
 	if (!par->pd) {
-		dev_dbg(info->dev, "no display setup data!\n");
+		dev_dbg(&pdev->dev, "no display setup data!\n");
 		ret = -ENODEV;
 		goto out_fb;
 	}
@@ -519,13 +519,13 @@ static int sh7760fb_probe(struct platform_device *pdev)
 
 	ret = fb_alloc_cmap(&info->cmap, 256, 0);
 	if (ret) {
-		dev_dbg(info->dev, "Unable to allocate cmap memory\n");
+		dev_dbg(&pdev->dev, "Unable to allocate cmap memory\n");
 		goto out_mem;
 	}
 
 	ret = register_framebuffer(info);
 	if (ret < 0) {
-		dev_dbg(info->dev, "cannot register fb!\n");
+		dev_dbg(&pdev->dev, "cannot register fb!\n");
 		goto out_cmap;
 	}
 	platform_set_drvdata(pdev, info);
-- 
2.41.0

[PATCH v2 29/38] fbdev/sm501fb: Output message with fb_err()

[Thomas Zimmermann]

Fix case were dev_err() is being called with struct fb_info.dev.
Use fb_err() instead.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/sm501fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c
index e0d29be1565bb..46951a0952741 100644
--- a/drivers/video/fbdev/sm501fb.c
+++ b/drivers/video/fbdev/sm501fb.c
@@ -1293,7 +1293,7 @@ static int sm501fb_sync(struct fb_info *info)
 		count--;
 
 	if (count <= 0) {
-		dev_err(info->dev, "Timeout waiting for 2d engine sync\n");
+		fb_err(info, "Timeout waiting for 2d engine sync\n");
 		return 1;
 	}
 	return 0;
-- 
2.41.0

[PATCH v2 30/38] fbdev/smscufx: Detect registered fb_info from refcount

[Thomas Zimmermann]

Detect registered instances of fb_info by reading the reference
counter from struct fb_info.read. Avoids looking at the dev field
and prepares fbdev for making struct fb_info.dev optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Steve Glendinning <steve.glendinning@shawell.net>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/smscufx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 17cec62cc65db..adb2b1fe8383c 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -1496,7 +1496,7 @@ static int ufx_setup_modes(struct ufx_data *dev, struct fb_info *info,
 	u8 *edid;
 	int i, result = 0, tries = 3;
 
-	if (info->dev) /* only use mutex if info has been registered */
+	if (refcount_read(&info->count)) /* only use mutex if info has been registered */
 		mutex_lock(&info->lock);
 
 	edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
@@ -1610,7 +1610,7 @@ static int ufx_setup_modes(struct ufx_data *dev, struct fb_info *info,
 	if (edid && (dev->edid != edid))
 		kfree(edid);
 
-	if (info->dev)
+	if (refcount_read(&info->count))
 		mutex_unlock(&info->lock);
 
 	return result;
-- 
2.41.0

[PATCH v2 31/38] fbdev/tdfxfb: Set i2c adapter parent to hardware device

[Thomas Zimmermann]

Use the 3dfx hardware device from the Linux device hierarchy as
parent device of the i2c adapter. Aligns the driver with the rest
of the codebase and prepares fbdev for making struct fb_info.dev
optional.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/tdfxfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
index cdf8e9fe99487..dd0fa42eceb9b 100644
--- a/drivers/video/fbdev/tdfxfb.c
+++ b/drivers/video/fbdev/tdfxfb.c
@@ -1327,8 +1327,8 @@ static void tdfxfb_create_i2c_busses(struct fb_info *info)
 	par->chan[0].par = par;
 	par->chan[1].par = par;
 
-	tdfxfb_setup_ddc_bus(&par->chan[0], "Voodoo3-DDC", info->dev);
-	tdfxfb_setup_i2c_bus(&par->chan[1], "Voodoo3-I2C", info->dev);
+	tdfxfb_setup_ddc_bus(&par->chan[0], "Voodoo3-DDC", info->device);
+	tdfxfb_setup_i2c_bus(&par->chan[1], "Voodoo3-I2C", info->device);
 }
 
 static void tdfxfb_delete_i2c_busses(struct tdfx_par *par)
-- 
2.41.0

[PATCH v2 32/38] fbdev/core: Pass Linux device to pm_vt_switch_*() functions

[Thomas Zimmermann]

Pass the Linux device to pm_vt_switch_*() instead of the virtual
fbdev device. Prepares fbdev for making struct fb_info.dev optional.

The type of device that is passed to the PM functions does not matter
much. It is only a token within the internal list of known devices.
The PM functions do not refer to any of the device's properties or its
type.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: linux-pm@vger.kernel.org
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/video/fbdev/core/fbmem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 329d16e49a900..f91ae7d4c94d6 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1478,9 +1478,9 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 		INIT_LIST_HEAD(&fb_info->modelist);
 
 	if (fb_info->skip_vt_switch)
-		pm_vt_switch_required(fb_info->dev, false);
+		pm_vt_switch_required(fb_info->device, false);
 	else
-		pm_vt_switch_required(fb_info->dev, true);
+		pm_vt_switch_required(fb_info->device, true);
 
 	fb_var_to_videomode(&mode, &fb_info->var);
 	fb_add_videomode(&mode, &fb_info->modelist);
@@ -1520,7 +1520,7 @@ static void unlink_framebuffer(struct fb_info *fb_info)
 
 	device_destroy(fb_class, MKDEV(FB_MAJOR, i));
 
-	pm_vt_switch_unregister(fb_info->dev);
+	pm_vt_switch_unregister(fb_info->device);
 
 	unbind_console(fb_info);
 
-- 
2.41.0

[PATCH v2 33/38] fbdev/core: Move framebuffer and backlight helpers into separate files

[Thomas Zimmermann]

Move framebuffer and backlight helpers into separate files. Leave
fbsysfs.c to sysfs-related code. No functional changes.

The framebuffer helpers are not in fbmem.c because they are under
GPL-2.0-or-later copyright, while fbmem.c is GPL-2.0.

v2:
	* include <linux/mutex.h> (Sam)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/video/fbdev/core/Makefile       |   4 +-
 drivers/video/fbdev/core/fb_backlight.c |  33 +++++++
 drivers/video/fbdev/core/fb_info.c      |  78 +++++++++++++++++
 drivers/video/fbdev/core/fbsysfs.c      | 110 +-----------------------
 4 files changed, 115 insertions(+), 110 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_backlight.c
 create mode 100644 drivers/video/fbdev/core/fb_info.c

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index 8f0060160ffb7..eee3295bc2252 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,7 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
 obj-$(CONFIG_FB)                  += fb.o
-fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
+fb-y                              := fb_backlight.o \
+                                     fb_info.o \
+                                     fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o fb_cmdline.o fb_io_fops.o
 fb-$(CONFIG_FB_DEFERRED_IO)       += fb_defio.o
 
diff --git a/drivers/video/fbdev/core/fb_backlight.c b/drivers/video/fbdev/core/fb_backlight.c
new file mode 100644
index 0000000000000..e2d3b3adc870f
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_backlight.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/export.h>
+#include <linux/fb.h>
+#include <linux/mutex.h>
+
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
+/*
+ * This function generates a linear backlight curve
+ *
+ *     0: off
+ *   1-7: min
+ * 8-127: linear from min to max
+ */
+void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 max)
+{
+	unsigned int i, flat, count, range = (max - min);
+
+	mutex_lock(&fb_info->bl_curve_mutex);
+
+	fb_info->bl_curve[0] = off;
+
+	for (flat = 1; flat < (FB_BACKLIGHT_LEVELS / 16); ++flat)
+		fb_info->bl_curve[flat] = min;
+
+	count = FB_BACKLIGHT_LEVELS * 15 / 16;
+	for (i = 0; i < count; ++i)
+		fb_info->bl_curve[flat + i] = min + (range * (i + 1) / count);
+
+	mutex_unlock(&fb_info->bl_curve_mutex);
+}
+EXPORT_SYMBOL_GPL(fb_bl_default_curve);
+#endif
diff --git a/drivers/video/fbdev/core/fb_info.c b/drivers/video/fbdev/core/fb_info.c
new file mode 100644
index 0000000000000..8bdbefdd4b701
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_info.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/export.h>
+#include <linux/fb.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+
+/**
+ * framebuffer_alloc - creates a new frame buffer info structure
+ *
+ * @size: size of driver private data, can be zero
+ * @dev: pointer to the device for this fb, this can be NULL
+ *
+ * Creates a new frame buffer info structure. Also reserves @size bytes
+ * for driver private data (info->par). info->par (if any) will be
+ * aligned to sizeof(long).
+ *
+ * Returns the new structure, or NULL if an error occurred.
+ *
+ */
+struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
+{
+#define BYTES_PER_LONG (BITS_PER_LONG/8)
+#define PADDING (BYTES_PER_LONG - (sizeof(struct fb_info) % BYTES_PER_LONG))
+	int fb_info_size = sizeof(struct fb_info);
+	struct fb_info *info;
+	char *p;
+
+	if (size)
+		fb_info_size += PADDING;
+
+	p = kzalloc(fb_info_size + size, GFP_KERNEL);
+
+	if (!p)
+		return NULL;
+
+	info = (struct fb_info *) p;
+
+	if (size)
+		info->par = p + fb_info_size;
+
+	info->device = dev;
+	info->fbcon_rotate_hint = -1;
+
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
+	mutex_init(&info->bl_curve_mutex);
+#endif
+
+	return info;
+#undef PADDING
+#undef BYTES_PER_LONG
+}
+EXPORT_SYMBOL(framebuffer_alloc);
+
+/**
+ * framebuffer_release - marks the structure available for freeing
+ *
+ * @info: frame buffer info structure
+ *
+ * Drop the reference count of the device embedded in the
+ * framebuffer info structure.
+ *
+ */
+void framebuffer_release(struct fb_info *info)
+{
+	if (!info)
+		return;
+
+	if (WARN_ON(refcount_read(&info->count)))
+		return;
+
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
+	mutex_destroy(&info->bl_curve_mutex);
+#endif
+
+	kfree(info);
+}
+EXPORT_SYMBOL(framebuffer_release);
diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
index 0c33c4adcd798..849073f1ca067 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -5,93 +5,12 @@
  * Copyright (c) 2004 James Simmons <jsimmons@infradead.org>
  */
 
-/*
- * Note:  currently there's only stubs for framebuffer_alloc and
- * framebuffer_release here.  The reson for that is that until all drivers
- * are converted to use it a sysfsification will open OOPSable races.
- */
-
-#include <linux/kernel.h>
-#include <linux/slab.h>
+#include <linux/console.h>
 #include <linux/fb.h>
 #include <linux/fbcon.h>
-#include <linux/console.h>
-#include <linux/module.h>
 
 #define FB_SYSFS_FLAG_ATTR 1
 
-/**
- * framebuffer_alloc - creates a new frame buffer info structure
- *
- * @size: size of driver private data, can be zero
- * @dev: pointer to the device for this fb, this can be NULL
- *
- * Creates a new frame buffer info structure. Also reserves @size bytes
- * for driver private data (info->par). info->par (if any) will be
- * aligned to sizeof(long).
- *
- * Returns the new structure, or NULL if an error occurred.
- *
- */
-struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
-{
-#define BYTES_PER_LONG (BITS_PER_LONG/8)
-#define PADDING (BYTES_PER_LONG - (sizeof(struct fb_info) % BYTES_PER_LONG))
-	int fb_info_size = sizeof(struct fb_info);
-	struct fb_info *info;
-	char *p;
-
-	if (size)
-		fb_info_size += PADDING;
-
-	p = kzalloc(fb_info_size + size, GFP_KERNEL);
-
-	if (!p)
-		return NULL;
-
-	info = (struct fb_info *) p;
-
-	if (size)
-		info->par = p + fb_info_size;
-
-	info->device = dev;
-	info->fbcon_rotate_hint = -1;
-
-#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
-	mutex_init(&info->bl_curve_mutex);
-#endif
-
-	return info;
-#undef PADDING
-#undef BYTES_PER_LONG
-}
-EXPORT_SYMBOL(framebuffer_alloc);
-
-/**
- * framebuffer_release - marks the structure available for freeing
- *
- * @info: frame buffer info structure
- *
- * Drop the reference count of the device embedded in the
- * framebuffer info structure.
- *
- */
-void framebuffer_release(struct fb_info *info)
-{
-	if (!info)
-		return;
-
-	if (WARN_ON(refcount_read(&info->count)))
-		return;
-
-#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
-	mutex_destroy(&info->bl_curve_mutex);
-#endif
-
-	kfree(info);
-}
-EXPORT_SYMBOL(framebuffer_release);
-
 static int activate(struct fb_info *fb_info, struct fb_var_screeninfo *var)
 {
 	int err;
@@ -551,30 +470,3 @@ void fb_cleanup_device(struct fb_info *fb_info)
 		fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
 	}
 }
-
-#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
-/* This function generates a linear backlight curve
- *
- *     0: off
- *   1-7: min
- * 8-127: linear from min to max
- */
-void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 max)
-{
-	unsigned int i, flat, count, range = (max - min);
-
-	mutex_lock(&fb_info->bl_curve_mutex);
-
-	fb_info->bl_curve[0] = off;
-
-	for (flat = 1; flat < (FB_BACKLIGHT_LEVELS / 16); ++flat)
-		fb_info->bl_curve[flat] = min;
-
-	count = FB_BACKLIGHT_LEVELS * 15 / 16;
-	for (i = 0; i < count; ++i)
-		fb_info->bl_curve[flat + i] = min + (range * (i + 1) / count);
-
-	mutex_unlock(&fb_info->bl_curve_mutex);
-}
-EXPORT_SYMBOL_GPL(fb_bl_default_curve);
-#endif
-- 
2.41.0

[PATCH v2 34/38] fbdev/core: Add fb_device_{create,destroy}()

[Thomas Zimmermann]

Move the logic to create and destroy fbdev devices into the new
helpers fb_device_create() and fb_device_destroy().

There was a call to fb_cleanup_device() in do_unregister_framebuffer()
that was too late. The device had already been removed at this point.
Move the call into fb_device_destroy().

Declare the helpers in the new internal header file  fb_internal.h, as
they are only used within the fbdev core module.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/core/fb_internal.h | 12 ++++++++
 drivers/video/fbdev/core/fbmem.c       | 21 +++-----------
 drivers/video/fbdev/core/fbsysfs.c     | 38 ++++++++++++++++++++++++--
 include/linux/fb.h                     |  3 --
 4 files changed, 52 insertions(+), 22 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_internal.h

diff --git a/drivers/video/fbdev/core/fb_internal.h b/drivers/video/fbdev/core/fb_internal.h
new file mode 100644
index 0000000000000..0b9640ae7a3d2
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_internal.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _FB_INTERNAL_H
+#define _FB_INTERNAL_H
+
+struct fb_info;
+
+/* fbsysfs.c */
+int fb_device_create(struct fb_info *fb_info);
+void fb_device_destroy(struct fb_info *fb_info);
+
+#endif
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index f91ae7d4c94d6..66532774d351e 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -40,6 +40,8 @@
 #include <video/nomodeset.h>
 #include <video/vga.h>
 
+#include "fb_internal.h"
+
     /*
      *  Frame buffer device initialization and setup routines
      */
@@ -1447,14 +1449,7 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	mutex_init(&fb_info->lock);
 	mutex_init(&fb_info->mm_lock);
 
-	fb_info->dev = device_create(fb_class, fb_info->device,
-				     MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
-	if (IS_ERR(fb_info->dev)) {
-		/* Not fatal */
-		printk(KERN_WARNING "Unable to create device for framebuffer %d; errno = %ld\n", i, PTR_ERR(fb_info->dev));
-		fb_info->dev = NULL;
-	} else
-		fb_init_device(fb_info);
+	fb_device_create(fb_info);
 
 	if (fb_info->pixmap.addr == NULL) {
 		fb_info->pixmap.addr = kmalloc(FBPIXMAPSIZE, GFP_KERNEL);
@@ -1515,16 +1510,9 @@ static void unlink_framebuffer(struct fb_info *fb_info)
 	if (WARN_ON(i < 0 || i >= FB_MAX || registered_fb[i] != fb_info))
 		return;
 
-	if (!fb_info->dev)
-		return;
-
-	device_destroy(fb_class, MKDEV(FB_MAJOR, i));
-
+	fb_device_destroy(fb_info);
 	pm_vt_switch_unregister(fb_info->device);
-
 	unbind_console(fb_info);
-
-	fb_info->dev = NULL;
 }
 
 static void do_unregister_framebuffer(struct fb_info *fb_info)
@@ -1539,7 +1527,6 @@ static void do_unregister_framebuffer(struct fb_info *fb_info)
 	fb_destroy_modelist(&fb_info->modelist);
 	registered_fb[fb_info->node] = NULL;
 	num_registered_fb--;
-	fb_cleanup_device(fb_info);
 #ifdef CONFIG_GUMSTIX_AM200EPD
 	{
 		struct fb_event event;
diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
index 849073f1ca067..fafe574398b01 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -8,6 +8,9 @@
 #include <linux/console.h>
 #include <linux/fb.h>
 #include <linux/fbcon.h>
+#include <linux/major.h>
+
+#include "fb_internal.h"
 
 #define FB_SYSFS_FLAG_ATTR 1
 
@@ -435,7 +438,7 @@ static struct device_attribute device_attrs[] = {
 #endif
 };
 
-int fb_init_device(struct fb_info *fb_info)
+static int fb_init_device(struct fb_info *fb_info)
 {
 	int i, error = 0;
 
@@ -459,7 +462,7 @@ int fb_init_device(struct fb_info *fb_info)
 	return 0;
 }
 
-void fb_cleanup_device(struct fb_info *fb_info)
+static void fb_cleanup_device(struct fb_info *fb_info)
 {
 	unsigned int i;
 
@@ -470,3 +473,34 @@ void fb_cleanup_device(struct fb_info *fb_info)
 		fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
 	}
 }
+
+int fb_device_create(struct fb_info *fb_info)
+{
+	int node = fb_info->node;
+	dev_t devt = MKDEV(FB_MAJOR, node);
+	int ret;
+
+	fb_info->dev = device_create(fb_class, fb_info->device, devt, NULL, "fb%d", node);
+	if (IS_ERR(fb_info->dev)) {
+		/* Not fatal */
+		ret = PTR_ERR(fb_info->dev);
+		pr_warn("Unable to create device for framebuffer %d; error %d\n", node, ret);
+		fb_info->dev = NULL;
+	} else {
+		fb_init_device(fb_info);
+	}
+
+	return 0;
+}
+
+void fb_device_destroy(struct fb_info *fb_info)
+{
+	dev_t devt = MKDEV(FB_MAJOR, fb_info->node);
+
+	if (!fb_info->dev)
+		return;
+
+	fb_cleanup_device(fb_info);
+	device_destroy(fb_class, devt);
+	fb_info->dev = NULL;
+}
diff --git a/include/linux/fb.h b/include/linux/fb.h
index ce6823e157e6b..1988d11f78bcb 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -735,11 +735,8 @@ static inline bool fb_be_math(struct fb_info *info)
 #endif /* CONFIG_FB_FOREIGN_ENDIAN */
 }
 
-/* drivers/video/fbsysfs.c */
 extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
 extern void framebuffer_release(struct fb_info *info);
-extern int fb_init_device(struct fb_info *fb_info);
-extern void fb_cleanup_device(struct fb_info *head);
 extern void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 max);
 
 /* drivers/video/fbmon.c */
-- 
2.41.0

Re: [PATCH v2 34/38] fbdev/core: Add fb_device_{create,destroy}()

[Sam Ravnborg]

On Mon, Jun 12, 2023 at 04:08:12PM +0200, Thomas Zimmermann wrote:
> Move the logic to create and destroy fbdev devices into the new
> helpers fb_device_create() and fb_device_destroy().
> 
> There was a call to fb_cleanup_device() in do_unregister_framebuffer()
> that was too late. The device had already been removed at this point.
> Move the call into fb_device_destroy().
> 
> Declare the helpers in the new internal header file  fb_internal.h, as
> they are only used within the fbdev core module.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

[PATCH v2 35/38] fbdev/core: Move procfs code to separate file

[Thomas Zimmermann]

Move fbdev's procfs code into a separate file and contain it in
init and cleanup helpers. For the cleanup, replace remove_proc_entry()
with proc_remove(). It is equivalent in functionality, but looks
more like an inverse of proc_create_seq().

v2:
	* document proc_remove() usage (Sam)
	* revert unrelated removal of for_each_registered_fb()

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/video/fbdev/core/Makefile      |  1 +
 drivers/video/fbdev/core/fb_internal.h | 12 ++++-
 drivers/video/fbdev/core/fb_procfs.c   | 62 ++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c       | 48 +++-----------------
 4 files changed, 80 insertions(+), 43 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_procfs.c

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index eee3295bc2252..665320160f70a 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fb_backlight.o \
                                      fb_info.o \
+                                     fb_procfs.o \
                                      fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o fb_cmdline.o fb_io_fops.o
 fb-$(CONFIG_FB_DEFERRED_IO)       += fb_defio.o
diff --git a/drivers/video/fbdev/core/fb_internal.h b/drivers/video/fbdev/core/fb_internal.h
index 0b9640ae7a3d2..51f7c9f04e45a 100644
--- a/drivers/video/fbdev/core/fb_internal.h
+++ b/drivers/video/fbdev/core/fb_internal.h
@@ -3,7 +3,17 @@
 #ifndef _FB_INTERNAL_H
 #define _FB_INTERNAL_H
 
-struct fb_info;
+#include <linux/fb.h>
+#include <linux/mutex.h>
+
+/* fbmem.c */
+extern struct mutex registration_lock;
+extern struct fb_info *registered_fb[FB_MAX];
+extern int num_registered_fb;
+
+/* fb_procfs.c */
+int fb_init_procfs(void);
+void fb_cleanup_procfs(void);
 
 /* fbsysfs.c */
 int fb_device_create(struct fb_info *fb_info);
diff --git a/drivers/video/fbdev/core/fb_procfs.c b/drivers/video/fbdev/core/fb_procfs.c
new file mode 100644
index 0000000000000..59641142f8aad
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_procfs.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/proc_fs.h>
+
+#include "fb_internal.h"
+
+static struct proc_dir_entry *fb_proc_dir_entry;
+
+static void *fb_seq_start(struct seq_file *m, loff_t *pos)
+{
+	mutex_lock(&registration_lock);
+
+	return (*pos < FB_MAX) ? pos : NULL;
+}
+
+static void fb_seq_stop(struct seq_file *m, void *v)
+{
+	mutex_unlock(&registration_lock);
+}
+
+static void *fb_seq_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	(*pos)++;
+
+	return (*pos < FB_MAX) ? pos : NULL;
+}
+
+static int fb_seq_show(struct seq_file *m, void *v)
+{
+	int i = *(loff_t *)v;
+	struct fb_info *fi = registered_fb[i];
+
+	if (fi)
+		seq_printf(m, "%d %s\n", fi->node, fi->fix.id);
+
+	return 0;
+}
+
+static const struct seq_operations __maybe_unused fb_proc_seq_ops = {
+	.start	= fb_seq_start,
+	.stop	= fb_seq_stop,
+	.next	= fb_seq_next,
+	.show	= fb_seq_show,
+};
+
+int fb_init_procfs(void)
+{
+	struct proc_dir_entry *proc;
+
+	proc = proc_create_seq("fb", 0, NULL, &fb_proc_seq_ops);
+	if (!proc)
+		return -ENOMEM;
+
+	fb_proc_dir_entry = proc;
+
+	return 0;
+}
+
+void fb_cleanup_procfs(void)
+{
+	proc_remove(fb_proc_dir_entry);
+}
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 66532774d351e..536209901e32a 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -24,9 +24,7 @@
 #include <linux/vt.h>
 #include <linux/init.h>
 #include <linux/linux_logo.h>
-#include <linux/proc_fs.h>
 #include <linux/platform_device.h>
-#include <linux/seq_file.h>
 #include <linux/console.h>
 #include <linux/kmod.h>
 #include <linux/err.h>
@@ -48,8 +46,7 @@
 
 #define FBPIXMAPSIZE	(1024 * 8)
 
-static DEFINE_MUTEX(registration_lock);
-
+DEFINE_MUTEX(registration_lock);
 struct fb_info *registered_fb[FB_MAX] __read_mostly;
 int num_registered_fb __read_mostly;
 #define for_each_registered_fb(i)		\
@@ -705,40 +702,6 @@ int fb_show_logo(struct fb_info *info, int rotate) { return 0; }
 EXPORT_SYMBOL(fb_prepare_logo);
 EXPORT_SYMBOL(fb_show_logo);
 
-static void *fb_seq_start(struct seq_file *m, loff_t *pos)
-{
-	mutex_lock(&registration_lock);
-	return (*pos < FB_MAX) ? pos : NULL;
-}
-
-static void *fb_seq_next(struct seq_file *m, void *v, loff_t *pos)
-{
-	(*pos)++;
-	return (*pos < FB_MAX) ? pos : NULL;
-}
-
-static void fb_seq_stop(struct seq_file *m, void *v)
-{
-	mutex_unlock(&registration_lock);
-}
-
-static int fb_seq_show(struct seq_file *m, void *v)
-{
-	int i = *(loff_t *)v;
-	struct fb_info *fi = registered_fb[i];
-
-	if (fi)
-		seq_printf(m, "%d %s\n", fi->node, fi->fix.id);
-	return 0;
-}
-
-static const struct seq_operations __maybe_unused proc_fb_seq_ops = {
-	.start	= fb_seq_start,
-	.next	= fb_seq_next,
-	.stop	= fb_seq_stop,
-	.show	= fb_seq_show,
-};
-
 /*
  * We hold a reference to the fb_info in file->private_data,
  * but if the current registered fb has changed, we don't
@@ -1624,8 +1587,9 @@ fbmem_init(void)
 {
 	int ret;
 
-	if (!proc_create_seq("fb", 0, NULL, &proc_fb_seq_ops))
-		return -ENOMEM;
+	ret = fb_init_procfs();
+	if (ret)
+		return ret;
 
 	ret = register_chrdev(FB_MAJOR, "fb", &fb_fops);
 	if (ret) {
@@ -1648,7 +1612,7 @@ fbmem_init(void)
 err_class:
 	unregister_chrdev(FB_MAJOR, "fb");
 err_chrdev:
-	remove_proc_entry("fb", NULL);
+	fb_cleanup_procfs();
 	return ret;
 }
 
@@ -1659,7 +1623,7 @@ fbmem_exit(void)
 {
 	fb_console_exit();
 
-	remove_proc_entry("fb", NULL);
+	fb_cleanup_procfs();
 	class_destroy(fb_class);
 	unregister_chrdev(FB_MAJOR, "fb");
 }
-- 
2.41.0

[PATCH v2 36/38] fbdev/core: Move file-I/O code into separate file

[Thomas Zimmermann]

Move fbdev's file-I/O code into a separate file and contain it in
init and cleanup helpers. No functional changes.

v2:
	* rename source file (Sam)
	* include <linux/compat.h> (Javier, kernel test robot)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/video/fbdev/core/Makefile      |   1 +
 drivers/video/fbdev/core/fb_internal.h |   6 +
 drivers/video/fbdev/core/fbmem.c       | 479 +------------------------
 3 files changed, 13 insertions(+), 473 deletions(-)

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index 665320160f70a..eea5938f74238 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fb_backlight.o \
+                                     fb_chrdev.o \
                                      fb_info.o \
                                      fb_procfs.o \
                                      fbmem.o fbmon.o fbcmap.o fbsysfs.o \
diff --git a/drivers/video/fbdev/core/fb_internal.h b/drivers/video/fbdev/core/fb_internal.h
index 51f7c9f04e45a..abe06c9da36e3 100644
--- a/drivers/video/fbdev/core/fb_internal.h
+++ b/drivers/video/fbdev/core/fb_internal.h
@@ -6,10 +6,16 @@
 #include <linux/fb.h>
 #include <linux/mutex.h>
 
+/* fb_devfs.c */
+int fb_register_chrdev(void);
+void fb_unregister_chrdev(void);
+
 /* fbmem.c */
 extern struct mutex registration_lock;
 extern struct fb_info *registered_fb[FB_MAX];
 extern int num_registered_fb;
+struct fb_info *get_fb_info(unsigned int idx);
+void put_fb_info(struct fb_info *fb_info);
 
 /* fb_procfs.c */
 int fb_init_procfs(void);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 536209901e32a..4edf70241a23c 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -13,11 +13,9 @@
 
 #include <linux/module.h>
 
-#include <linux/compat.h>
 #include <linux/types.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
-#include <linux/major.h>
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/mman.h>
@@ -57,7 +55,7 @@ bool fb_center_logo __read_mostly;
 
 int fb_logo_count __read_mostly = -1;
 
-static struct fb_info *get_fb_info(unsigned int idx)
+struct fb_info *get_fb_info(unsigned int idx)
 {
 	struct fb_info *fb_info;
 
@@ -73,7 +71,7 @@ static struct fb_info *get_fb_info(unsigned int idx)
 	return fb_info;
 }
 
-static void put_fb_info(struct fb_info *fb_info)
+void put_fb_info(struct fb_info *fb_info)
 {
 	if (!refcount_dec_and_test(&fb_info->count))
 		return;
@@ -702,59 +700,6 @@ int fb_show_logo(struct fb_info *info, int rotate) { return 0; }
 EXPORT_SYMBOL(fb_prepare_logo);
 EXPORT_SYMBOL(fb_show_logo);
 
-/*
- * We hold a reference to the fb_info in file->private_data,
- * but if the current registered fb has changed, we don't
- * actually want to use it.
- *
- * So look up the fb_info using the inode minor number,
- * and just verify it against the reference we have.
- */
-static struct fb_info *file_fb_info(struct file *file)
-{
-	struct inode *inode = file_inode(file);
-	int fbidx = iminor(inode);
-	struct fb_info *info = registered_fb[fbidx];
-
-	if (info != file->private_data)
-		info = NULL;
-	return info;
-}
-
-static ssize_t
-fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
-{
-	struct fb_info *info = file_fb_info(file);
-
-	if (!info)
-		return -ENODEV;
-
-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
-
-	if (info->fbops->fb_read)
-		return info->fbops->fb_read(info, buf, count, ppos);
-
-	return fb_io_read(info, buf, count, ppos);
-}
-
-static ssize_t
-fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
-{
-	struct fb_info *info = file_fb_info(file);
-
-	if (!info)
-		return -ENODEV;
-
-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
-
-	if (info->fbops->fb_write)
-		return info->fbops->fb_write(info, buf, count, ppos);
-
-	return fb_io_write(info, buf, count, ppos);
-}
-
 int
 fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
 {
@@ -954,416 +899,6 @@ fb_blank(struct fb_info *info, int blank)
 }
 EXPORT_SYMBOL(fb_blank);
 
-static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
-			unsigned long arg)
-{
-	const struct fb_ops *fb;
-	struct fb_var_screeninfo var;
-	struct fb_fix_screeninfo fix;
-	struct fb_cmap cmap_from;
-	struct fb_cmap_user cmap;
-	void __user *argp = (void __user *)arg;
-	long ret = 0;
-
-	switch (cmd) {
-	case FBIOGET_VSCREENINFO:
-		lock_fb_info(info);
-		var = info->var;
-		unlock_fb_info(info);
-
-		ret = copy_to_user(argp, &var, sizeof(var)) ? -EFAULT : 0;
-		break;
-	case FBIOPUT_VSCREENINFO:
-		if (copy_from_user(&var, argp, sizeof(var)))
-			return -EFAULT;
-		/* only for kernel-internal use */
-		var.activate &= ~FB_ACTIVATE_KD_TEXT;
-		console_lock();
-		lock_fb_info(info);
-		ret = fbcon_modechange_possible(info, &var);
-		if (!ret)
-			ret = fb_set_var(info, &var);
-		if (!ret)
-			fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
-		unlock_fb_info(info);
-		console_unlock();
-		if (!ret && copy_to_user(argp, &var, sizeof(var)))
-			ret = -EFAULT;
-		break;
-	case FBIOGET_FSCREENINFO:
-		lock_fb_info(info);
-		memcpy(&fix, &info->fix, sizeof(fix));
-		if (info->flags & FBINFO_HIDE_SMEM_START)
-			fix.smem_start = 0;
-		unlock_fb_info(info);
-
-		ret = copy_to_user(argp, &fix, sizeof(fix)) ? -EFAULT : 0;
-		break;
-	case FBIOPUTCMAP:
-		if (copy_from_user(&cmap, argp, sizeof(cmap)))
-			return -EFAULT;
-		ret = fb_set_user_cmap(&cmap, info);
-		break;
-	case FBIOGETCMAP:
-		if (copy_from_user(&cmap, argp, sizeof(cmap)))
-			return -EFAULT;
-		lock_fb_info(info);
-		cmap_from = info->cmap;
-		unlock_fb_info(info);
-		ret = fb_cmap_to_user(&cmap_from, &cmap);
-		break;
-	case FBIOPAN_DISPLAY:
-		if (copy_from_user(&var, argp, sizeof(var)))
-			return -EFAULT;
-		console_lock();
-		lock_fb_info(info);
-		ret = fb_pan_display(info, &var);
-		unlock_fb_info(info);
-		console_unlock();
-		if (ret == 0 && copy_to_user(argp, &var, sizeof(var)))
-			return -EFAULT;
-		break;
-	case FBIO_CURSOR:
-		ret = -EINVAL;
-		break;
-	case FBIOGET_CON2FBMAP:
-		ret = fbcon_get_con2fb_map_ioctl(argp);
-		break;
-	case FBIOPUT_CON2FBMAP:
-		ret = fbcon_set_con2fb_map_ioctl(argp);
-		break;
-	case FBIOBLANK:
-		if (arg > FB_BLANK_POWERDOWN)
-			return -EINVAL;
-		console_lock();
-		lock_fb_info(info);
-		ret = fb_blank(info, arg);
-		/* might again call into fb_blank */
-		fbcon_fb_blanked(info, arg);
-		unlock_fb_info(info);
-		console_unlock();
-		break;
-	default:
-		lock_fb_info(info);
-		fb = info->fbops;
-		if (fb->fb_ioctl)
-			ret = fb->fb_ioctl(info, cmd, arg);
-		else
-			ret = -ENOTTY;
-		unlock_fb_info(info);
-	}
-	return ret;
-}
-
-static long fb_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	struct fb_info *info = file_fb_info(file);
-
-	if (!info)
-		return -ENODEV;
-	return do_fb_ioctl(info, cmd, arg);
-}
-
-#ifdef CONFIG_COMPAT
-struct fb_fix_screeninfo32 {
-	char			id[16];
-	compat_caddr_t		smem_start;
-	u32			smem_len;
-	u32			type;
-	u32			type_aux;
-	u32			visual;
-	u16			xpanstep;
-	u16			ypanstep;
-	u16			ywrapstep;
-	u32			line_length;
-	compat_caddr_t		mmio_start;
-	u32			mmio_len;
-	u32			accel;
-	u16			reserved[3];
-};
-
-struct fb_cmap32 {
-	u32			start;
-	u32			len;
-	compat_caddr_t	red;
-	compat_caddr_t	green;
-	compat_caddr_t	blue;
-	compat_caddr_t	transp;
-};
-
-static int fb_getput_cmap(struct fb_info *info, unsigned int cmd,
-			  unsigned long arg)
-{
-	struct fb_cmap32 cmap32;
-	struct fb_cmap cmap_from;
-	struct fb_cmap_user cmap;
-
-	if (copy_from_user(&cmap32, compat_ptr(arg), sizeof(cmap32)))
-		return -EFAULT;
-
-	cmap = (struct fb_cmap_user) {
-		.start	= cmap32.start,
-		.len	= cmap32.len,
-		.red	= compat_ptr(cmap32.red),
-		.green	= compat_ptr(cmap32.green),
-		.blue	= compat_ptr(cmap32.blue),
-		.transp	= compat_ptr(cmap32.transp),
-	};
-
-	if (cmd == FBIOPUTCMAP)
-		return fb_set_user_cmap(&cmap, info);
-
-	lock_fb_info(info);
-	cmap_from = info->cmap;
-	unlock_fb_info(info);
-
-	return fb_cmap_to_user(&cmap_from, &cmap);
-}
-
-static int do_fscreeninfo_to_user(struct fb_fix_screeninfo *fix,
-				  struct fb_fix_screeninfo32 __user *fix32)
-{
-	__u32 data;
-	int err;
-
-	err = copy_to_user(&fix32->id, &fix->id, sizeof(fix32->id));
-
-	data = (__u32) (unsigned long) fix->smem_start;
-	err |= put_user(data, &fix32->smem_start);
-
-	err |= put_user(fix->smem_len, &fix32->smem_len);
-	err |= put_user(fix->type, &fix32->type);
-	err |= put_user(fix->type_aux, &fix32->type_aux);
-	err |= put_user(fix->visual, &fix32->visual);
-	err |= put_user(fix->xpanstep, &fix32->xpanstep);
-	err |= put_user(fix->ypanstep, &fix32->ypanstep);
-	err |= put_user(fix->ywrapstep, &fix32->ywrapstep);
-	err |= put_user(fix->line_length, &fix32->line_length);
-
-	data = (__u32) (unsigned long) fix->mmio_start;
-	err |= put_user(data, &fix32->mmio_start);
-
-	err |= put_user(fix->mmio_len, &fix32->mmio_len);
-	err |= put_user(fix->accel, &fix32->accel);
-	err |= copy_to_user(fix32->reserved, fix->reserved,
-			    sizeof(fix->reserved));
-
-	if (err)
-		return -EFAULT;
-	return 0;
-}
-
-static int fb_get_fscreeninfo(struct fb_info *info, unsigned int cmd,
-			      unsigned long arg)
-{
-	struct fb_fix_screeninfo fix;
-
-	lock_fb_info(info);
-	fix = info->fix;
-	if (info->flags & FBINFO_HIDE_SMEM_START)
-		fix.smem_start = 0;
-	unlock_fb_info(info);
-	return do_fscreeninfo_to_user(&fix, compat_ptr(arg));
-}
-
-static long fb_compat_ioctl(struct file *file, unsigned int cmd,
-			    unsigned long arg)
-{
-	struct fb_info *info = file_fb_info(file);
-	const struct fb_ops *fb;
-	long ret = -ENOIOCTLCMD;
-
-	if (!info)
-		return -ENODEV;
-	fb = info->fbops;
-	switch(cmd) {
-	case FBIOGET_VSCREENINFO:
-	case FBIOPUT_VSCREENINFO:
-	case FBIOPAN_DISPLAY:
-	case FBIOGET_CON2FBMAP:
-	case FBIOPUT_CON2FBMAP:
-		arg = (unsigned long) compat_ptr(arg);
-		fallthrough;
-	case FBIOBLANK:
-		ret = do_fb_ioctl(info, cmd, arg);
-		break;
-
-	case FBIOGET_FSCREENINFO:
-		ret = fb_get_fscreeninfo(info, cmd, arg);
-		break;
-
-	case FBIOGETCMAP:
-	case FBIOPUTCMAP:
-		ret = fb_getput_cmap(info, cmd, arg);
-		break;
-
-	default:
-		if (fb->fb_compat_ioctl)
-			ret = fb->fb_compat_ioctl(info, cmd, arg);
-		break;
-	}
-	return ret;
-}
-#endif
-
-static int
-fb_mmap(struct file *file, struct vm_area_struct * vma)
-{
-	struct fb_info *info = file_fb_info(file);
-	unsigned long mmio_pgoff;
-	unsigned long start;
-	u32 len;
-
-	if (!info)
-		return -ENODEV;
-	mutex_lock(&info->mm_lock);
-
-	if (info->fbops->fb_mmap) {
-		int res;
-
-		/*
-		 * The framebuffer needs to be accessed decrypted, be sure
-		 * SME protection is removed ahead of the call
-		 */
-		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
-		res = info->fbops->fb_mmap(info, vma);
-		mutex_unlock(&info->mm_lock);
-		return res;
-#if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
-	} else if (info->fbdefio) {
-		/*
-		 * FB deferred I/O wants you to handle mmap in your drivers. At a
-		 * minimum, point struct fb_ops.fb_mmap to fb_deferred_io_mmap().
-		 */
-		dev_warn_once(info->dev, "fbdev mmap not set up for deferred I/O.\n");
-		mutex_unlock(&info->mm_lock);
-		return -ENODEV;
-#endif
-	}
-
-	/*
-	 * Ugh. This can be either the frame buffer mapping, or
-	 * if pgoff points past it, the mmio mapping.
-	 */
-	start = info->fix.smem_start;
-	len = info->fix.smem_len;
-	mmio_pgoff = PAGE_ALIGN((start & ~PAGE_MASK) + len) >> PAGE_SHIFT;
-	if (vma->vm_pgoff >= mmio_pgoff) {
-		if (info->var.accel_flags) {
-			mutex_unlock(&info->mm_lock);
-			return -EINVAL;
-		}
-
-		vma->vm_pgoff -= mmio_pgoff;
-		start = info->fix.mmio_start;
-		len = info->fix.mmio_len;
-	}
-	mutex_unlock(&info->mm_lock);
-
-	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
-	fb_pgprotect(file, vma, start);
-
-	return vm_iomap_memory(vma, start, len);
-}
-
-static int
-fb_open(struct inode *inode, struct file *file)
-__acquires(&info->lock)
-__releases(&info->lock)
-{
-	int fbidx = iminor(inode);
-	struct fb_info *info;
-	int res = 0;
-
-	info = get_fb_info(fbidx);
-	if (!info) {
-		request_module("fb%d", fbidx);
-		info = get_fb_info(fbidx);
-		if (!info)
-			return -ENODEV;
-	}
-	if (IS_ERR(info))
-		return PTR_ERR(info);
-
-	lock_fb_info(info);
-	if (!try_module_get(info->fbops->owner)) {
-		res = -ENODEV;
-		goto out;
-	}
-	file->private_data = info;
-	if (info->fbops->fb_open) {
-		res = info->fbops->fb_open(info,1);
-		if (res)
-			module_put(info->fbops->owner);
-	}
-#ifdef CONFIG_FB_DEFERRED_IO
-	if (info->fbdefio)
-		fb_deferred_io_open(info, inode, file);
-#endif
-out:
-	unlock_fb_info(info);
-	if (res)
-		put_fb_info(info);
-	return res;
-}
-
-static int
-fb_release(struct inode *inode, struct file *file)
-__acquires(&info->lock)
-__releases(&info->lock)
-{
-	struct fb_info * const info = file->private_data;
-
-	lock_fb_info(info);
-#if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
-	if (info->fbdefio)
-		fb_deferred_io_release(info);
-#endif
-	if (info->fbops->fb_release)
-		info->fbops->fb_release(info,1);
-	module_put(info->fbops->owner);
-	unlock_fb_info(info);
-	put_fb_info(info);
-	return 0;
-}
-
-#if defined(CONFIG_FB_PROVIDE_GET_FB_UNMAPPED_AREA) && !defined(CONFIG_MMU)
-static unsigned long get_fb_unmapped_area(struct file *filp,
-				   unsigned long addr, unsigned long len,
-				   unsigned long pgoff, unsigned long flags)
-{
-	struct fb_info * const info = filp->private_data;
-	unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len);
-
-	if (pgoff > fb_size || len > fb_size - pgoff)
-		return -EINVAL;
-
-	return (unsigned long)info->screen_base + pgoff;
-}
-#endif
-
-static const struct file_operations fb_fops = {
-	.owner =	THIS_MODULE,
-	.read =		fb_read,
-	.write =	fb_write,
-	.unlocked_ioctl = fb_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = fb_compat_ioctl,
-#endif
-	.mmap =		fb_mmap,
-	.open =		fb_open,
-	.release =	fb_release,
-#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) || \
-	(defined(CONFIG_FB_PROVIDE_GET_FB_UNMAPPED_AREA) && \
-	 !defined(CONFIG_MMU))
-	.get_unmapped_area = get_fb_unmapped_area,
-#endif
-#ifdef CONFIG_FB_DEFERRED_IO
-	.fsync =	fb_deferred_io_fsync,
-#endif
-	.llseek =	default_llseek,
-};
-
 struct class *fb_class;
 EXPORT_SYMBOL(fb_class);
 
@@ -1591,11 +1126,9 @@ fbmem_init(void)
 	if (ret)
 		return ret;
 
-	ret = register_chrdev(FB_MAJOR, "fb", &fb_fops);
-	if (ret) {
-		printk("unable to get major %d for fb devs\n", FB_MAJOR);
+	ret = fb_register_chrdev();
+	if (ret)
 		goto err_chrdev;
-	}
 
 	fb_class = class_create("graphics");
 	if (IS_ERR(fb_class)) {
@@ -1610,7 +1143,7 @@ fbmem_init(void)
 	return 0;
 
 err_class:
-	unregister_chrdev(FB_MAJOR, "fb");
+	fb_unregister_chrdev();
 err_chrdev:
 	fb_cleanup_procfs();
 	return ret;
@@ -1625,7 +1158,7 @@ fbmem_exit(void)
 
 	fb_cleanup_procfs();
 	class_destroy(fb_class);
-	unregister_chrdev(FB_MAJOR, "fb");
+	fb_unregister_chrdev();
 }
 
 module_exit(fbmem_exit);
-- 
2.41.0

Re: [PATCH v2 36/38] fbdev/core: Move file-I/O code into separate file

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 17380 bytes --]

Am 12.06.23 um 16:08 schrieb Thomas Zimmermann:
> Move fbdev's file-I/O code into a separate file and contain it in
> init and cleanup helpers. No functional changes.
> 
> v2:
> 	* rename source file (Sam)

I just noticed that fb_chrdev.c is missing from this patch. It got lost 
during the rename. It'll be included in the next iteration.


> 	* include <linux/compat.h> (Javier, kernel test robot)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>   drivers/video/fbdev/core/Makefile      |   1 +
>   drivers/video/fbdev/core/fb_internal.h |   6 +
>   drivers/video/fbdev/core/fbmem.c       | 479 +------------------------
>   3 files changed, 13 insertions(+), 473 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index 665320160f70a..eea5938f74238 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -2,6 +2,7 @@
>   obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
>   obj-$(CONFIG_FB)                  += fb.o
>   fb-y                              := fb_backlight.o \
> +                                     fb_chrdev.o \
>                                        fb_info.o \
>                                        fb_procfs.o \
>                                        fbmem.o fbmon.o fbcmap.o fbsysfs.o \
> diff --git a/drivers/video/fbdev/core/fb_internal.h b/drivers/video/fbdev/core/fb_internal.h
> index 51f7c9f04e45a..abe06c9da36e3 100644
> --- a/drivers/video/fbdev/core/fb_internal.h
> +++ b/drivers/video/fbdev/core/fb_internal.h
> @@ -6,10 +6,16 @@
>   #include <linux/fb.h>
>   #include <linux/mutex.h>
>   
> +/* fb_devfs.c */
> +int fb_register_chrdev(void);
> +void fb_unregister_chrdev(void);
> +
>   /* fbmem.c */
>   extern struct mutex registration_lock;
>   extern struct fb_info *registered_fb[FB_MAX];
>   extern int num_registered_fb;
> +struct fb_info *get_fb_info(unsigned int idx);
> +void put_fb_info(struct fb_info *fb_info);
>   
>   /* fb_procfs.c */
>   int fb_init_procfs(void);
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 536209901e32a..4edf70241a23c 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -13,11 +13,9 @@
>   
>   #include <linux/module.h>
>   
> -#include <linux/compat.h>
>   #include <linux/types.h>
>   #include <linux/errno.h>
>   #include <linux/kernel.h>
> -#include <linux/major.h>
>   #include <linux/slab.h>
>   #include <linux/mm.h>
>   #include <linux/mman.h>
> @@ -57,7 +55,7 @@ bool fb_center_logo __read_mostly;
>   
>   int fb_logo_count __read_mostly = -1;
>   
> -static struct fb_info *get_fb_info(unsigned int idx)
> +struct fb_info *get_fb_info(unsigned int idx)
>   {
>   	struct fb_info *fb_info;
>   
> @@ -73,7 +71,7 @@ static struct fb_info *get_fb_info(unsigned int idx)
>   	return fb_info;
>   }
>   
> -static void put_fb_info(struct fb_info *fb_info)
> +void put_fb_info(struct fb_info *fb_info)
>   {
>   	if (!refcount_dec_and_test(&fb_info->count))
>   		return;
> @@ -702,59 +700,6 @@ int fb_show_logo(struct fb_info *info, int rotate) { return 0; }
>   EXPORT_SYMBOL(fb_prepare_logo);
>   EXPORT_SYMBOL(fb_show_logo);
>   
> -/*
> - * We hold a reference to the fb_info in file->private_data,
> - * but if the current registered fb has changed, we don't
> - * actually want to use it.
> - *
> - * So look up the fb_info using the inode minor number,
> - * and just verify it against the reference we have.
> - */
> -static struct fb_info *file_fb_info(struct file *file)
> -{
> -	struct inode *inode = file_inode(file);
> -	int fbidx = iminor(inode);
> -	struct fb_info *info = registered_fb[fbidx];
> -
> -	if (info != file->private_data)
> -		info = NULL;
> -	return info;
> -}
> -
> -static ssize_t
> -fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> -{
> -	struct fb_info *info = file_fb_info(file);
> -
> -	if (!info)
> -		return -ENODEV;
> -
> -	if (info->state != FBINFO_STATE_RUNNING)
> -		return -EPERM;
> -
> -	if (info->fbops->fb_read)
> -		return info->fbops->fb_read(info, buf, count, ppos);
> -
> -	return fb_io_read(info, buf, count, ppos);
> -}
> -
> -static ssize_t
> -fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
> -{
> -	struct fb_info *info = file_fb_info(file);
> -
> -	if (!info)
> -		return -ENODEV;
> -
> -	if (info->state != FBINFO_STATE_RUNNING)
> -		return -EPERM;
> -
> -	if (info->fbops->fb_write)
> -		return info->fbops->fb_write(info, buf, count, ppos);
> -
> -	return fb_io_write(info, buf, count, ppos);
> -}
> -
>   int
>   fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
>   {
> @@ -954,416 +899,6 @@ fb_blank(struct fb_info *info, int blank)
>   }
>   EXPORT_SYMBOL(fb_blank);
>   
> -static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> -			unsigned long arg)
> -{
> -	const struct fb_ops *fb;
> -	struct fb_var_screeninfo var;
> -	struct fb_fix_screeninfo fix;
> -	struct fb_cmap cmap_from;
> -	struct fb_cmap_user cmap;
> -	void __user *argp = (void __user *)arg;
> -	long ret = 0;
> -
> -	switch (cmd) {
> -	case FBIOGET_VSCREENINFO:
> -		lock_fb_info(info);
> -		var = info->var;
> -		unlock_fb_info(info);
> -
> -		ret = copy_to_user(argp, &var, sizeof(var)) ? -EFAULT : 0;
> -		break;
> -	case FBIOPUT_VSCREENINFO:
> -		if (copy_from_user(&var, argp, sizeof(var)))
> -			return -EFAULT;
> -		/* only for kernel-internal use */
> -		var.activate &= ~FB_ACTIVATE_KD_TEXT;
> -		console_lock();
> -		lock_fb_info(info);
> -		ret = fbcon_modechange_possible(info, &var);
> -		if (!ret)
> -			ret = fb_set_var(info, &var);
> -		if (!ret)
> -			fbcon_update_vcs(info, var.activate & FB_ACTIVATE_ALL);
> -		unlock_fb_info(info);
> -		console_unlock();
> -		if (!ret && copy_to_user(argp, &var, sizeof(var)))
> -			ret = -EFAULT;
> -		break;
> -	case FBIOGET_FSCREENINFO:
> -		lock_fb_info(info);
> -		memcpy(&fix, &info->fix, sizeof(fix));
> -		if (info->flags & FBINFO_HIDE_SMEM_START)
> -			fix.smem_start = 0;
> -		unlock_fb_info(info);
> -
> -		ret = copy_to_user(argp, &fix, sizeof(fix)) ? -EFAULT : 0;
> -		break;
> -	case FBIOPUTCMAP:
> -		if (copy_from_user(&cmap, argp, sizeof(cmap)))
> -			return -EFAULT;
> -		ret = fb_set_user_cmap(&cmap, info);
> -		break;
> -	case FBIOGETCMAP:
> -		if (copy_from_user(&cmap, argp, sizeof(cmap)))
> -			return -EFAULT;
> -		lock_fb_info(info);
> -		cmap_from = info->cmap;
> -		unlock_fb_info(info);
> -		ret = fb_cmap_to_user(&cmap_from, &cmap);
> -		break;
> -	case FBIOPAN_DISPLAY:
> -		if (copy_from_user(&var, argp, sizeof(var)))
> -			return -EFAULT;
> -		console_lock();
> -		lock_fb_info(info);
> -		ret = fb_pan_display(info, &var);
> -		unlock_fb_info(info);
> -		console_unlock();
> -		if (ret == 0 && copy_to_user(argp, &var, sizeof(var)))
> -			return -EFAULT;
> -		break;
> -	case FBIO_CURSOR:
> -		ret = -EINVAL;
> -		break;
> -	case FBIOGET_CON2FBMAP:
> -		ret = fbcon_get_con2fb_map_ioctl(argp);
> -		break;
> -	case FBIOPUT_CON2FBMAP:
> -		ret = fbcon_set_con2fb_map_ioctl(argp);
> -		break;
> -	case FBIOBLANK:
> -		if (arg > FB_BLANK_POWERDOWN)
> -			return -EINVAL;
> -		console_lock();
> -		lock_fb_info(info);
> -		ret = fb_blank(info, arg);
> -		/* might again call into fb_blank */
> -		fbcon_fb_blanked(info, arg);
> -		unlock_fb_info(info);
> -		console_unlock();
> -		break;
> -	default:
> -		lock_fb_info(info);
> -		fb = info->fbops;
> -		if (fb->fb_ioctl)
> -			ret = fb->fb_ioctl(info, cmd, arg);
> -		else
> -			ret = -ENOTTY;
> -		unlock_fb_info(info);
> -	}
> -	return ret;
> -}
> -
> -static long fb_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> -{
> -	struct fb_info *info = file_fb_info(file);
> -
> -	if (!info)
> -		return -ENODEV;
> -	return do_fb_ioctl(info, cmd, arg);
> -}
> -
> -#ifdef CONFIG_COMPAT
> -struct fb_fix_screeninfo32 {
> -	char			id[16];
> -	compat_caddr_t		smem_start;
> -	u32			smem_len;
> -	u32			type;
> -	u32			type_aux;
> -	u32			visual;
> -	u16			xpanstep;
> -	u16			ypanstep;
> -	u16			ywrapstep;
> -	u32			line_length;
> -	compat_caddr_t		mmio_start;
> -	u32			mmio_len;
> -	u32			accel;
> -	u16			reserved[3];
> -};
> -
> -struct fb_cmap32 {
> -	u32			start;
> -	u32			len;
> -	compat_caddr_t	red;
> -	compat_caddr_t	green;
> -	compat_caddr_t	blue;
> -	compat_caddr_t	transp;
> -};
> -
> -static int fb_getput_cmap(struct fb_info *info, unsigned int cmd,
> -			  unsigned long arg)
> -{
> -	struct fb_cmap32 cmap32;
> -	struct fb_cmap cmap_from;
> -	struct fb_cmap_user cmap;
> -
> -	if (copy_from_user(&cmap32, compat_ptr(arg), sizeof(cmap32)))
> -		return -EFAULT;
> -
> -	cmap = (struct fb_cmap_user) {
> -		.start	= cmap32.start,
> -		.len	= cmap32.len,
> -		.red	= compat_ptr(cmap32.red),
> -		.green	= compat_ptr(cmap32.green),
> -		.blue	= compat_ptr(cmap32.blue),
> -		.transp	= compat_ptr(cmap32.transp),
> -	};
> -
> -	if (cmd == FBIOPUTCMAP)
> -		return fb_set_user_cmap(&cmap, info);
> -
> -	lock_fb_info(info);
> -	cmap_from = info->cmap;
> -	unlock_fb_info(info);
> -
> -	return fb_cmap_to_user(&cmap_from, &cmap);
> -}
> -
> -static int do_fscreeninfo_to_user(struct fb_fix_screeninfo *fix,
> -				  struct fb_fix_screeninfo32 __user *fix32)
> -{
> -	__u32 data;
> -	int err;
> -
> -	err = copy_to_user(&fix32->id, &fix->id, sizeof(fix32->id));
> -
> -	data = (__u32) (unsigned long) fix->smem_start;
> -	err |= put_user(data, &fix32->smem_start);
> -
> -	err |= put_user(fix->smem_len, &fix32->smem_len);
> -	err |= put_user(fix->type, &fix32->type);
> -	err |= put_user(fix->type_aux, &fix32->type_aux);
> -	err |= put_user(fix->visual, &fix32->visual);
> -	err |= put_user(fix->xpanstep, &fix32->xpanstep);
> -	err |= put_user(fix->ypanstep, &fix32->ypanstep);
> -	err |= put_user(fix->ywrapstep, &fix32->ywrapstep);
> -	err |= put_user(fix->line_length, &fix32->line_length);
> -
> -	data = (__u32) (unsigned long) fix->mmio_start;
> -	err |= put_user(data, &fix32->mmio_start);
> -
> -	err |= put_user(fix->mmio_len, &fix32->mmio_len);
> -	err |= put_user(fix->accel, &fix32->accel);
> -	err |= copy_to_user(fix32->reserved, fix->reserved,
> -			    sizeof(fix->reserved));
> -
> -	if (err)
> -		return -EFAULT;
> -	return 0;
> -}
> -
> -static int fb_get_fscreeninfo(struct fb_info *info, unsigned int cmd,
> -			      unsigned long arg)
> -{
> -	struct fb_fix_screeninfo fix;
> -
> -	lock_fb_info(info);
> -	fix = info->fix;
> -	if (info->flags & FBINFO_HIDE_SMEM_START)
> -		fix.smem_start = 0;
> -	unlock_fb_info(info);
> -	return do_fscreeninfo_to_user(&fix, compat_ptr(arg));
> -}
> -
> -static long fb_compat_ioctl(struct file *file, unsigned int cmd,
> -			    unsigned long arg)
> -{
> -	struct fb_info *info = file_fb_info(file);
> -	const struct fb_ops *fb;
> -	long ret = -ENOIOCTLCMD;
> -
> -	if (!info)
> -		return -ENODEV;
> -	fb = info->fbops;
> -	switch(cmd) {
> -	case FBIOGET_VSCREENINFO:
> -	case FBIOPUT_VSCREENINFO:
> -	case FBIOPAN_DISPLAY:
> -	case FBIOGET_CON2FBMAP:
> -	case FBIOPUT_CON2FBMAP:
> -		arg = (unsigned long) compat_ptr(arg);
> -		fallthrough;
> -	case FBIOBLANK:
> -		ret = do_fb_ioctl(info, cmd, arg);
> -		break;
> -
> -	case FBIOGET_FSCREENINFO:
> -		ret = fb_get_fscreeninfo(info, cmd, arg);
> -		break;
> -
> -	case FBIOGETCMAP:
> -	case FBIOPUTCMAP:
> -		ret = fb_getput_cmap(info, cmd, arg);
> -		break;
> -
> -	default:
> -		if (fb->fb_compat_ioctl)
> -			ret = fb->fb_compat_ioctl(info, cmd, arg);
> -		break;
> -	}
> -	return ret;
> -}
> -#endif
> -
> -static int
> -fb_mmap(struct file *file, struct vm_area_struct * vma)
> -{
> -	struct fb_info *info = file_fb_info(file);
> -	unsigned long mmio_pgoff;
> -	unsigned long start;
> -	u32 len;
> -
> -	if (!info)
> -		return -ENODEV;
> -	mutex_lock(&info->mm_lock);
> -
> -	if (info->fbops->fb_mmap) {
> -		int res;
> -
> -		/*
> -		 * The framebuffer needs to be accessed decrypted, be sure
> -		 * SME protection is removed ahead of the call
> -		 */
> -		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
> -		res = info->fbops->fb_mmap(info, vma);
> -		mutex_unlock(&info->mm_lock);
> -		return res;
> -#if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
> -	} else if (info->fbdefio) {
> -		/*
> -		 * FB deferred I/O wants you to handle mmap in your drivers. At a
> -		 * minimum, point struct fb_ops.fb_mmap to fb_deferred_io_mmap().
> -		 */
> -		dev_warn_once(info->dev, "fbdev mmap not set up for deferred I/O.\n");
> -		mutex_unlock(&info->mm_lock);
> -		return -ENODEV;
> -#endif
> -	}
> -
> -	/*
> -	 * Ugh. This can be either the frame buffer mapping, or
> -	 * if pgoff points past it, the mmio mapping.
> -	 */
> -	start = info->fix.smem_start;
> -	len = info->fix.smem_len;
> -	mmio_pgoff = PAGE_ALIGN((start & ~PAGE_MASK) + len) >> PAGE_SHIFT;
> -	if (vma->vm_pgoff >= mmio_pgoff) {
> -		if (info->var.accel_flags) {
> -			mutex_unlock(&info->mm_lock);
> -			return -EINVAL;
> -		}
> -
> -		vma->vm_pgoff -= mmio_pgoff;
> -		start = info->fix.mmio_start;
> -		len = info->fix.mmio_len;
> -	}
> -	mutex_unlock(&info->mm_lock);
> -
> -	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> -	fb_pgprotect(file, vma, start);
> -
> -	return vm_iomap_memory(vma, start, len);
> -}
> -
> -static int
> -fb_open(struct inode *inode, struct file *file)
> -__acquires(&info->lock)
> -__releases(&info->lock)
> -{
> -	int fbidx = iminor(inode);
> -	struct fb_info *info;
> -	int res = 0;
> -
> -	info = get_fb_info(fbidx);
> -	if (!info) {
> -		request_module("fb%d", fbidx);
> -		info = get_fb_info(fbidx);
> -		if (!info)
> -			return -ENODEV;
> -	}
> -	if (IS_ERR(info))
> -		return PTR_ERR(info);
> -
> -	lock_fb_info(info);
> -	if (!try_module_get(info->fbops->owner)) {
> -		res = -ENODEV;
> -		goto out;
> -	}
> -	file->private_data = info;
> -	if (info->fbops->fb_open) {
> -		res = info->fbops->fb_open(info,1);
> -		if (res)
> -			module_put(info->fbops->owner);
> -	}
> -#ifdef CONFIG_FB_DEFERRED_IO
> -	if (info->fbdefio)
> -		fb_deferred_io_open(info, inode, file);
> -#endif
> -out:
> -	unlock_fb_info(info);
> -	if (res)
> -		put_fb_info(info);
> -	return res;
> -}
> -
> -static int
> -fb_release(struct inode *inode, struct file *file)
> -__acquires(&info->lock)
> -__releases(&info->lock)
> -{
> -	struct fb_info * const info = file->private_data;
> -
> -	lock_fb_info(info);
> -#if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
> -	if (info->fbdefio)
> -		fb_deferred_io_release(info);
> -#endif
> -	if (info->fbops->fb_release)
> -		info->fbops->fb_release(info,1);
> -	module_put(info->fbops->owner);
> -	unlock_fb_info(info);
> -	put_fb_info(info);
> -	return 0;
> -}
> -
> -#if defined(CONFIG_FB_PROVIDE_GET_FB_UNMAPPED_AREA) && !defined(CONFIG_MMU)
> -static unsigned long get_fb_unmapped_area(struct file *filp,
> -				   unsigned long addr, unsigned long len,
> -				   unsigned long pgoff, unsigned long flags)
> -{
> -	struct fb_info * const info = filp->private_data;
> -	unsigned long fb_size = PAGE_ALIGN(info->fix.smem_len);
> -
> -	if (pgoff > fb_size || len > fb_size - pgoff)
> -		return -EINVAL;
> -
> -	return (unsigned long)info->screen_base + pgoff;
> -}
> -#endif
> -
> -static const struct file_operations fb_fops = {
> -	.owner =	THIS_MODULE,
> -	.read =		fb_read,
> -	.write =	fb_write,
> -	.unlocked_ioctl = fb_ioctl,
> -#ifdef CONFIG_COMPAT
> -	.compat_ioctl = fb_compat_ioctl,
> -#endif
> -	.mmap =		fb_mmap,
> -	.open =		fb_open,
> -	.release =	fb_release,
> -#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) || \
> -	(defined(CONFIG_FB_PROVIDE_GET_FB_UNMAPPED_AREA) && \
> -	 !defined(CONFIG_MMU))
> -	.get_unmapped_area = get_fb_unmapped_area,
> -#endif
> -#ifdef CONFIG_FB_DEFERRED_IO
> -	.fsync =	fb_deferred_io_fsync,
> -#endif
> -	.llseek =	default_llseek,
> -};
> -
>   struct class *fb_class;
>   EXPORT_SYMBOL(fb_class);
>   
> @@ -1591,11 +1126,9 @@ fbmem_init(void)
>   	if (ret)
>   		return ret;
>   
> -	ret = register_chrdev(FB_MAJOR, "fb", &fb_fops);
> -	if (ret) {
> -		printk("unable to get major %d for fb devs\n", FB_MAJOR);
> +	ret = fb_register_chrdev();
> +	if (ret)
>   		goto err_chrdev;
> -	}
>   
>   	fb_class = class_create("graphics");
>   	if (IS_ERR(fb_class)) {
> @@ -1610,7 +1143,7 @@ fbmem_init(void)
>   	return 0;
>   
>   err_class:
> -	unregister_chrdev(FB_MAJOR, "fb");
> +	fb_unregister_chrdev();
>   err_chrdev:
>   	fb_cleanup_procfs();
>   	return ret;
> @@ -1625,7 +1158,7 @@ fbmem_exit(void)
>   
>   	fb_cleanup_procfs();
>   	class_destroy(fb_class);
> -	unregister_chrdev(FB_MAJOR, "fb");
> +	fb_unregister_chrdev();
>   }
>   
>   module_exit(fbmem_exit);

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH v2 37/38] fbdev/core: Rework fb init code

[Thomas Zimmermann]

Init the class "graphics" before the rest of fbdev. Later steps, such
as the sysfs code, depend on the class. Also arrange the module's exit
code in reverse order.

Unexport the global variable fb_class, which is only shared internally
within the fbdev core module.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/video/fbdev/core/fb_internal.h |  1 +
 drivers/video/fbdev/core/fbcon.c       |  1 +
 drivers/video/fbdev/core/fbmem.c       | 52 ++++++++++----------------
 include/linux/fb.h                     |  1 -
 4 files changed, 22 insertions(+), 33 deletions(-)

diff --git a/drivers/video/fbdev/core/fb_internal.h b/drivers/video/fbdev/core/fb_internal.h
index abe06c9da36e3..0b43c0cd50968 100644
--- a/drivers/video/fbdev/core/fb_internal.h
+++ b/drivers/video/fbdev/core/fb_internal.h
@@ -11,6 +11,7 @@ int fb_register_chrdev(void);
 void fb_unregister_chrdev(void);
 
 /* fbmem.c */
+extern struct class *fb_class;
 extern struct mutex registration_lock;
 extern struct fb_info *registered_fb[FB_MAX];
 extern int num_registered_fb;
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index c6c9d040bdec7..8e76bc246b387 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -78,6 +78,7 @@
 #include <asm/irq.h>
 
 #include "fbcon.h"
+#include "fb_internal.h"
 
 /*
  * FIXME: Locking
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 4edf70241a23c..ee44a46a66be1 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -44,6 +44,8 @@
 
 #define FBPIXMAPSIZE	(1024 * 8)
 
+struct class *fb_class;
+
 DEFINE_MUTEX(registration_lock);
 struct fb_info *registered_fb[FB_MAX] __read_mostly;
 int num_registered_fb __read_mostly;
@@ -899,9 +901,6 @@ fb_blank(struct fb_info *info, int blank)
 }
 EXPORT_SYMBOL(fb_blank);
 
-struct class *fb_class;
-EXPORT_SYMBOL(fb_class);
-
 static int fb_check_foreignness(struct fb_info *fi)
 {
 	const bool foreign_endian = fi->flags & FBINFO_FOREIGN_ENDIAN;
@@ -1108,59 +1107,48 @@ void fb_set_suspend(struct fb_info *info, int state)
 }
 EXPORT_SYMBOL(fb_set_suspend);
 
-/**
- *	fbmem_init - init frame buffer subsystem
- *
- *	Initialize the frame buffer subsystem.
- *
- *	NOTE: This function is _only_ to be called by drivers/char/mem.c.
- *
- */
-
-static int __init
-fbmem_init(void)
+static int __init fbmem_init(void)
 {
 	int ret;
 
+	fb_class = class_create("graphics");
+	if (IS_ERR(fb_class)) {
+		ret = PTR_ERR(fb_class);
+		pr_err("Unable to create fb class; errno = %d\n", ret);
+		goto err_fb_class;
+	}
+
 	ret = fb_init_procfs();
 	if (ret)
-		return ret;
+		goto err_class_destroy;
 
 	ret = fb_register_chrdev();
 	if (ret)
-		goto err_chrdev;
-
-	fb_class = class_create("graphics");
-	if (IS_ERR(fb_class)) {
-		ret = PTR_ERR(fb_class);
-		pr_warn("Unable to create fb class; errno = %d\n", ret);
-		fb_class = NULL;
-		goto err_class;
-	}
+		goto err_fb_cleanup_procfs;
 
 	fb_console_init();
 
 	return 0;
 
-err_class:
-	fb_unregister_chrdev();
-err_chrdev:
+err_fb_cleanup_procfs:
 	fb_cleanup_procfs();
+err_class_destroy:
+	class_destroy(fb_class);
+err_fb_class:
+	fb_class = NULL;
 	return ret;
 }
 
 #ifdef MODULE
-module_init(fbmem_init);
-static void __exit
-fbmem_exit(void)
+static void __exit fbmem_exit(void)
 {
 	fb_console_exit();
-
+	fb_unregister_chrdev();
 	fb_cleanup_procfs();
 	class_destroy(fb_class);
-	fb_unregister_chrdev();
 }
 
+module_init(fbmem_init);
 module_exit(fbmem_exit);
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Framebuffer base");
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 1988d11f78bcb..541a0e3ce21f4 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -609,7 +609,6 @@ extern int fb_new_modelist(struct fb_info *info);
 
 extern bool fb_center_logo;
 extern int fb_logo_count;
-extern struct class *fb_class;
 
 static inline void lock_fb_info(struct fb_info *info)
 {
-- 
2.41.0

[PATCH v2 38/38] fbdev: Make support for userspace interfaces configurable

[Thomas Zimmermann]

Add Kconfig option CONFIG_FB_DEVICE and make the virtual fbdev
device optional. If the new option has not been selected, fbdev
does not create files in devfs, sysfs or procfs.

Most modern Linux systems run a DRM-based graphics stack that uses
the kernel's framebuffer console, but has otherwise deprecated fbdev
support. Yet fbdev userspace interfaces are still present.

The option makes it possible to use the fbdev subsystem as console
implementation without support for userspace. This closes potential
entry points to manipulate kernel or I/O memory via framebuffers. It
also prevents the execution of driver code via ioctl or sysfs, both
of which might allow malicious software to exploit bugs in the fbdev
code.

A small number of fbdev drivers require struct fbinfo.dev to be
initialized, usually for the support of sysfs interface. Make these
drivers depend on FB_DEVICE. They can later be fixed if necessary.

v2:
	* set FB_DEVICE default to y (Geert)
	* comment on {get,put}_device() (Sam)
	* Kconfig fixes (Sam)
	* add TODO item about FB_DEVICE dependencies (Sam)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 Documentation/gpu/todo.rst               | 13 ++++++++
 drivers/staging/fbtft/Kconfig            |  1 +
 drivers/video/fbdev/Kconfig              | 13 ++++++++
 drivers/video/fbdev/core/Makefile        |  7 +++--
 drivers/video/fbdev/core/fb_internal.h   | 38 ++++++++++++++++++++++++
 drivers/video/fbdev/omap2/omapfb/Kconfig |  2 +-
 include/linux/fb.h                       |  2 ++
 7 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 68bdafa0284f5..f226f934ca5af 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -452,6 +452,19 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>
 
 Level: Starter
 
+Remove driver dependencies on FB_DEVICE
+---------------------------------------
+
+A number of fbdev drivers provide attributes via sysfs and therefore depend
+on CONFIG_FB_DEVICE to be selected. Review each driver and attempt to make
+any dependencies on CONFIG_FB_DEVICE optional. At the minimum, the respective
+code in the driver could be conditionalized via ifdef CONFIG_FB_DEVICE. Not
+all drivers might be able to drop CONFIG_FB_DEVICE.
+
+Contact: Thomas Zimmermann <tzimmermann@suse.de>
+
+Level: Starter
+
 
 Core refactorings
 =================
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 4d29e8c1014e0..5dda3c65a38e7 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -2,6 +2,7 @@
 menuconfig FB_TFT
 	tristate "Support for small TFT LCD display modules"
 	depends on FB && SPI
+	depends on FB_DEVICE
 	depends on GPIOLIB || COMPILE_TEST
 	select FB_SYS_FILLRECT
 	select FB_SYS_COPYAREA
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index f82357d4f84da..19eaca5e04283 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -57,6 +57,16 @@ config FIRMWARE_EDID
 	  combination with certain motherboards and monitors are known to
 	  suffer from this problem.
 
+config FB_DEVICE
+	bool "Provide legacy /dev/fb* device"
+	depends on FB
+	default y
+	help
+	  Say Y here if you want the legacy /dev/fb* device file and
+	  interfaces within sysfs anc procfs. It is only required if you
+	  have userspace programs that depend on fbdev for graphics output.
+	  This does not effect the framebuffer console. If unsure, say N.
+
 config FB_DDC
 	tristate
 	depends on FB
@@ -1545,6 +1555,7 @@ config FB_3DFX_I2C
 config FB_VOODOO1
 	tristate "3Dfx Voodoo Graphics (sst1) support"
 	depends on FB && PCI
+	depends on FB_DEVICE
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -1863,6 +1874,7 @@ config FB_SH_MOBILE_LCDC
 	tristate "SuperH Mobile LCDC framebuffer support"
 	depends on FB && HAVE_CLK && HAS_IOMEM
 	depends on SUPERH || ARCH_RENESAS || COMPILE_TEST
+	depends on FB_DEVICE
 	select FB_SYS_FILLRECT
 	select FB_SYS_COPYAREA
 	select FB_SYS_IMAGEBLIT
@@ -1932,6 +1944,7 @@ config FB_SMSCUFX
 config FB_UDL
 	tristate "Displaylink USB Framebuffer support"
 	depends on FB && USB
+	depends on FB_DEVICE
 	select FB_MODE_HELPERS
 	select FB_SYS_FILLRECT
 	select FB_SYS_COPYAREA
diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index eea5938f74238..9150bafd9e899 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -2,12 +2,13 @@
 obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fb_backlight.o \
-                                     fb_chrdev.o \
                                      fb_info.o \
-                                     fb_procfs.o \
-                                     fbmem.o fbmon.o fbcmap.o fbsysfs.o \
+                                     fbmem.o fbmon.o fbcmap.o \
                                      modedb.o fbcvt.o fb_cmdline.o fb_io_fops.o
 fb-$(CONFIG_FB_DEFERRED_IO)       += fb_defio.o
+fb-$(CONFIG_FB_DEVICE)            += fb_chrdev.o \
+                                     fb_procfs.o \
+                                     fbsysfs.o
 
 ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE),y)
 fb-y				  += fbcon.o bitblit.o softcursor.o
diff --git a/drivers/video/fbdev/core/fb_internal.h b/drivers/video/fbdev/core/fb_internal.h
index 0b43c0cd50968..4c8d509a00265 100644
--- a/drivers/video/fbdev/core/fb_internal.h
+++ b/drivers/video/fbdev/core/fb_internal.h
@@ -3,12 +3,22 @@
 #ifndef _FB_INTERNAL_H
 #define _FB_INTERNAL_H
 
+#include <linux/device.h>
 #include <linux/fb.h>
 #include <linux/mutex.h>
 
 /* fb_devfs.c */
+#if defined(CONFIG_FB_DEVICE)
 int fb_register_chrdev(void);
 void fb_unregister_chrdev(void);
+#else
+static inline int fb_register_chrdev(void)
+{
+	return 0;
+}
+static inline void fb_unregister_chrdev(void)
+{ }
+#endif
 
 /* fbmem.c */
 extern struct class *fb_class;
@@ -19,11 +29,39 @@ struct fb_info *get_fb_info(unsigned int idx);
 void put_fb_info(struct fb_info *fb_info);
 
 /* fb_procfs.c */
+#if defined(CONFIG_FB_DEVICE)
 int fb_init_procfs(void);
 void fb_cleanup_procfs(void);
+#else
+static inline int fb_init_procfs(void)
+{
+	return 0;
+}
+static inline void fb_cleanup_procfs(void)
+{ }
+#endif
 
 /* fbsysfs.c */
+#if defined(CONFIG_FB_DEVICE)
 int fb_device_create(struct fb_info *fb_info);
 void fb_device_destroy(struct fb_info *fb_info);
+#else
+static inline int fb_device_create(struct fb_info *fb_info)
+{
+	/*
+	 * Acquire a reference on the parent device to avoid
+	 * unplug operations behind our back. With the fbdev
+	 * device enabled, this is performed within register_device().
+	 */
+	get_device(fb_info->device);
+
+	return 0;
+}
+static inline void fb_device_destroy(struct fb_info *fb_info)
+{
+	/* Undo the get_device() from fb_device_create() */
+	put_device(fb_info->device);
+}
+#endif
 
 #endif
diff --git a/drivers/video/fbdev/omap2/omapfb/Kconfig b/drivers/video/fbdev/omap2/omapfb/Kconfig
index 69f9cb03507ef..21069fdb7cc21 100644
--- a/drivers/video/fbdev/omap2/omapfb/Kconfig
+++ b/drivers/video/fbdev/omap2/omapfb/Kconfig
@@ -5,9 +5,9 @@ config OMAP2_VRFB
 menuconfig FB_OMAP2
 	tristate "OMAP2+ frame buffer support"
 	depends on FB
+	depends on FB_DEVICE
 	depends on DRM_OMAP = n
 	depends on GPIOLIB
-
 	select FB_OMAP2_DSS
 	select OMAP2_VRFB if ARCH_OMAP2 || ARCH_OMAP3
 	select FB_CFB_FILLRECT
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 541a0e3ce21f4..40ed1028160c0 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -481,7 +481,9 @@ struct fb_info {
 
 	const struct fb_ops *fbops;
 	struct device *device;		/* This is the parent */
+#if defined(CONFIG_FB_DEVICE)
 	struct device *dev;		/* This is this fb device */
+#endif
 	int class_flag;                    /* private sysfs flags */
 #ifdef CONFIG_FB_TILEBLITTING
 	struct fb_tile_ops *tileops;    /* Tile Blitting */
-- 
2.41.0

Re: [PATCH v2 38/38] fbdev: Make support for userspace interfaces configurable

[Daniel Thompson]

On Mon, Jun 12, 2023 at 04:08:16PM +0200, Thomas Zimmermann wrote:
> Add Kconfig option CONFIG_FB_DEVICE and make the virtual fbdev
> device optional. If the new option has not been selected, fbdev
> does not create files in devfs, sysfs or procfs.
>
> Most modern Linux systems run a DRM-based graphics stack that uses
> the kernel's framebuffer console, but has otherwise deprecated fbdev
> support. Yet fbdev userspace interfaces are still present.
>
> The option makes it possible to use the fbdev subsystem as console
> implementation without support for userspace. This closes potential
> entry points to manipulate kernel or I/O memory via framebuffers. It
> also prevents the execution of driver code via ioctl or sysfs, both
> of which might allow malicious software to exploit bugs in the fbdev
> code.
>
> A small number of fbdev drivers require struct fbinfo.dev to be
> initialized, usually for the support of sysfs interface. Make these
> drivers depend on FB_DEVICE. They can later be fixed if necessary.
>
> v2:
> 	* set FB_DEVICE default to y (Geert)
> 	* comment on {get,put}_device() (Sam)
> 	* Kconfig fixes (Sam)
> 	* add TODO item about FB_DEVICE dependencies (Sam)
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  Documentation/gpu/todo.rst               | 13 ++++++++
>  drivers/staging/fbtft/Kconfig            |  1 +
>  drivers/video/fbdev/Kconfig              | 13 ++++++++
>  drivers/video/fbdev/core/Makefile        |  7 +++--
>  drivers/video/fbdev/core/fb_internal.h   | 38 ++++++++++++++++++++++++
>  drivers/video/fbdev/omap2/omapfb/Kconfig |  2 +-
>  include/linux/fb.h                       |  2 ++
>  7 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 68bdafa0284f5..f226f934ca5af 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -452,6 +452,19 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>
>
>  Level: Starter
>
> +Remove driver dependencies on FB_DEVICE
> +---------------------------------------
> +
> +A number of fbdev drivers provide attributes via sysfs and therefore depend
> +on CONFIG_FB_DEVICE to be selected. Review each driver and attempt to make
> +any dependencies on CONFIG_FB_DEVICE optional. At the minimum, the respective
> +code in the driver could be conditionalized via ifdef CONFIG_FB_DEVICE. Not
> +all drivers might be able to drop CONFIG_FB_DEVICE.
> +
> +Contact: Thomas Zimmermann <tzimmermann@suse.de>
> +
> +Level: Starter
> +
>
>  Core refactorings
>  =================
> diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
> index 4d29e8c1014e0..5dda3c65a38e7 100644
> --- a/drivers/staging/fbtft/Kconfig
> +++ b/drivers/staging/fbtft/Kconfig
> @@ -2,6 +2,7 @@
>  menuconfig FB_TFT
>  	tristate "Support for small TFT LCD display modules"
>  	depends on FB && SPI
> +	depends on FB_DEVICE
>  	depends on GPIOLIB || COMPILE_TEST
>  	select FB_SYS_FILLRECT
>  	select FB_SYS_COPYAREA
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index f82357d4f84da..19eaca5e04283 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -57,6 +57,16 @@ config FIRMWARE_EDID
>  	  combination with certain motherboards and monitors are known to
>  	  suffer from this problem.
>
> +config FB_DEVICE
> +	bool "Provide legacy /dev/fb* device"
> +	depends on FB
> +	default y
> +	help
> +	  Say Y here if you want the legacy /dev/fb* device file and
> +	  interfaces within sysfs anc procfs. It is only required if you
> +	  have userspace programs that depend on fbdev for graphics output.
> +	  This does not effect the framebuffer console. If unsure, say N.

Nitpicking but this *is* documentation so:
s/effect/affect/


Daniel.

Re: [PATCH v2 00/38] fbdev: Make userspace interfaces optional

[Sam Ravnborg]

Hi Thomas,

Nice series, quite some preparations.

On Mon, Jun 12, 2023 at 04:07:38PM +0200, Thomas Zimmermann wrote:
> Add the new config option FB_DEVICE. If enabled, fbdev provides
> traditional userspace interfaces in devfs, sysfs and procfs, such
> as /dev/fb0 or /proc/fb.
> 
> Modern Linux distrobutions have adopted DRM drivers for graphics
> output and use fbdev only for the kernel's framebuffer console.
> Userspace has also moved on, with no new fbdev code being written
> and existing support being removed.
> 
> OTOH, fbdev provides userspace a way of accessing kernel or I/O
> memory, which might compromise the system's security. See the recent
> commit c8687694bb1f ("drm/fbdev-generic: prohibit potential
> out-of-bounds access") for an example. Disabling fbdev userspace
> interfaces is therefore a useful feature to limit unnecessary
> exposure of fbdev code to processes of low privilegues.
> 
> Patches 1 to 31 fix various bugs and issues in fbdev-related code.
> In most cases the code uses the fbdev device where it should use
> the Linux hardware device or something else. Most of these patches
> fix existing problems and should therefore be considered in any case.
> 
> Patches 32 to 37 refactor the fbdev core code. The patches move
> support for backlights, sysfs, procfs and devfs into separate files
> and hide it behind simple interfaces. These changes will allow to
> easily build the userspace support conditionally.
> 
> Patch 38 introduces the config option FB_DEVICE and adapts the fbdev
> core to support it. The field struct fb_info.dev is now optional,
> hence the name of the config option.
> 
> Tested on simpledrm and i915, including the device handover.
> 
> Future directions: With the support for disabling fbdev userspace
> interfaces in place, it will be possible to make most fbdev drivers'
> file-I/O code in struct fb_ops optional as well. 
> 
> v2:
> 	* fix fsl-diu-fb and sh7760fb
> 	* split backlight patches
> 	* set 'default y' for FB_CONFIG
> 	* minor fixes and corrections
> 
> Thomas Zimmermann (38):
>   backlight/bd6107: Compare against struct fb_info.device
>   backlight/bd6107: Rename struct bd6107_platform_data.fbdev to 'dev'
>   backlight/gpio_backlight: Compare against struct fb_info.device
>   backlight/gpio_backlight: Rename field 'fbdev' to 'dev'
>   backlight/lv5207lp: Compare against struct fb_info.device
>   backlight/lv5207lp: Rename struct lv5207lp_platform_data.fbdev to
>     'dev'
>   fbdev/atyfb: Reorder backlight and framebuffer init/cleanup
>   fbdev/atyfb: Use hardware device as backlight parent
>   fbdev/aty128fb: Reorder backlight and framebuffer init/cleanup
>   fbdev/aty128fb: Use hardware device as backlight parent
>   fbdev/broadsheetfb: Call device_remove_file() with hardware device
>   fbdev/ep93xx-fb: Output messages with fb_info() and fb_err()
>   fbdev/ep93xx-fb: Do not assign to struct fb_info.dev
>   fbdev/fsl-diu-fb: Output messages with fb_*() helpers
>   fbdev/mb862xxfb: Output messages with fb_dbg()
>   fbdev/metronomefb: Use hardware device for dev_err()
>   fbdev/nvidiafb: Reorder backlight and framebuffer init/cleanup
>   fbdev/nvidiafb: Use hardware device as backlight parent
>   fbdev/pxa168fb: Do not assign to struct fb_info.dev
>   fbdev/radeonfb: Reorder backlight and framebuffer cleanup
>   fbdev/radeonfb: Use hardware device as backlight parent
>   fbdev/rivafb: Reorder backlight and framebuffer init/cleanup
>   fbdev/rivafb: Use hardware device as backlight parent
>   fbdev/sh7760fb: Use fb_dbg() in sh7760fb_get_color_info()
>   fbdev/sh7760fb: Output messages with fb_dbg()
>   fbdev/sh7760fb: Use hardware device with dev_() output during probe
>   fbdev/sm501fb: Output message with fb_err()
>   fbdev/tdfxfb: Set i2c adapter parent to hardware device
The above are all:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

>   fbdev/smscufx: Detect registered fb_info from refcount
I did not try to understand the code, so others must review.

>   fbdev/ep93xx-fb: Alloc DMA memory from hardware device
>   fbdev/sh7760fb: Alloc DMA memory from hardware device
This smells like bug-fixes, and I do not see what impact the change has.
So again, someone else needs to provide review here.

The rest are already reviewed or got a dedicated reply.

	Sam

Re: [PATCH v2 00/38] fbdev: Make userspace interfaces optional

[Javier Martinez Canillas]

Sam Ravnborg <sam@ravnborg.org> writes:

Hello Sam,

> Hi Thomas,
>
> Nice series, quite some preparations.
>
> On Mon, Jun 12, 2023 at 04:07:38PM +0200, Thomas Zimmermann wrote:

[...]

>>   fbdev/smscufx: Detect registered fb_info from refcount
> I did not try to understand the code, so others must review.
>

No worries, I already reviewed that one.

>>   fbdev/ep93xx-fb: Alloc DMA memory from hardware device
>>   fbdev/sh7760fb: Alloc DMA memory from hardware device
> This smells like bug-fixes, and I do not see what impact the change has.
> So again, someone else needs to provide review here.
>

And same for these.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 00/38] fbdev: Make userspace interfaces optional

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1075 bytes --]

Hi Sam and Javier

Am 12.06.23 um 18:04 schrieb Javier Martinez Canillas:
> Sam Ravnborg <sam@ravnborg.org> writes:
> 
> Hello Sam,
> 
>> Hi Thomas,
>>
>> Nice series, quite some preparations.
>>
>> On Mon, Jun 12, 2023 at 04:07:38PM +0200, Thomas Zimmermann wrote:
> 
> [...]
> 
>>>    fbdev/smscufx: Detect registered fb_info from refcount
>> I did not try to understand the code, so others must review.
>>
> 
> No worries, I already reviewed that one.
> 
>>>    fbdev/ep93xx-fb: Alloc DMA memory from hardware device
>>>    fbdev/sh7760fb: Alloc DMA memory from hardware device
>> This smells like bug-fixes, and I do not see what impact the change has.
>> So again, someone else needs to provide review here.
>>
> 
> And same for these.
> 

Thanks to both of you for reviewing the patches.

Best regards
Thomas

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]