Re: [PATCH 1/2] fbdev: remove fb_notify support

Sam Ravnborg
2025-06-25T19:41:58Z

Hi Arnd.

I remember I stared at this code before, good to see it gone.
There is a bit more tidiying up you can do.

Also, I suggest to split it in two patches, it itches me to see the
driver specific part mixed up with the fb_notify removal.

Sam

On Wed, Jun 25, 2025 at 03:12:22PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Commit dc2139c0aa32 ("leds: backlight trigger: Replace fb events with a
> dedicated function call") removed the FB_EVENT_BLANK notifier, and now
> the only remaining user of the FB notifier is the metronomefb driver on
> the PXA/AM200EPD board.
>
> This was introduced in commit 922613436ae5 ("[ARM] 5200/1: am200epd: use
> fb notifiers and gpio api"), which converted it from an earlier version,
> but as far as I can tell this can never have worked because the notifier
> is called after the data it passes down is accessed.
>
> Commit 867187821e5e ("fbdev/metronomefb: Use struct fb_info.screen_buffer")
> broke this further, and there are likely other parts of the driver that
> no longer work.
>
> The am200epd board support itself should have also been removed long ago,
> as there are no users and it was never converted to devicetree format.
>
> Mark the board as broken to prevent build failures and remove the now
> unused notifiers.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/mach-pxa/Kconfig | 1 +
This is mixing things up a bit. I suggest
splitting the "depends on BROKEN" out in a dedicated patch.

> drivers/video/fbdev/core/Makefile | 1 -
> drivers/video/fbdev/core/fb_notify.c | 54 ----------------------------
> drivers/video/fbdev/core/fbmem.c | 15 --------
> include/linux/fb.h | 21 -----------
> 5 files changed, 1 insertion(+), 91 deletions(-)
> delete mode 100644 drivers/video/fbdev/core/fb_notify.c
>
> diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
> index 10e472f4fa43..edefc953e4f9 100644
> --- a/arch/arm/mach-pxa/Kconfig
> +++ b/arch/arm/mach-pxa/Kconfig
> @@ -69,6 +69,7 @@ choice
>
> config GUMSTIX_AM200EPD
> bool "Enable AM200EPD board support"
> + depends on BROKEN
>
> config GUMSTIX_AM300EPD
> bool "Enable AM300EPD board support"
> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index d15974759086..ac8036209501 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -1,5 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_FB_NOTIFY) += fb_notify.o
> obj-$(CONFIG_FB_CORE) += fb.o
> fb-y := fb_info.o \
> fbmem.o fbcmap.o \
> diff --git a/drivers/video/fbdev/core/fb_notify.c b/drivers/video/fbdev/core/fb_notify.c
> deleted file mode 100644
> index 10e3b9a74adc..000000000000
> --- a/drivers/video/fbdev/core/fb_notify.c
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -/*
> - * linux/drivers/video/fb_notify.c
> - *
> - * Copyright (C) 2006 Antonino Daplas <adaplas@pol.net>
> - *
> - * 2001 - Documented with DocBook
> - * - Brad Douglas <brad@neruo.com>
> - *
> - * This file is subject to the terms and conditions of the GNU General Public
> - * License. See the file COPYING in the main directory of this archive
> - * for more details.
> - */
> -#include <linux/fb.h>
> -#include <linux/notifier.h>
> -#include <linux/export.h>
> -
> -static BLOCKING_NOTIFIER_HEAD(fb_notifier_list);
> -
> -/**
> - * fb_register_client - register a client notifier
> - * @nb: notifier block to callback on events
> - *
> - * Return: 0 on success, negative error code on failure.
> - */
> -int fb_register_client(struct notifier_block *nb)
> -{
> - return blocking_notifier_chain_register(&fb_notifier_list, nb);
> -}
> -EXPORT_SYMBOL(fb_register_client);
> -
> -/**
> - * fb_unregister_client - unregister a client notifier
> - * @nb: notifier block to callback on events
> - *
> - * Return: 0 on success, negative error code on failure.
> - */
> -int fb_unregister_client(struct notifier_block *nb)
> -{
> - return blocking_notifier_chain_unregister(&fb_notifier_list, nb);
> -}
> -EXPORT_SYMBOL(fb_unregister_client);
> -
> -/**
> - * fb_notifier_call_chain - notify clients of fb_events
> - * @val: value passed to callback
> - * @v: pointer passed to callback
> - *
> - * Return: The return value of the last notifier function
> - */
> -int fb_notifier_call_chain(unsigned long val, void *v)
> -{
> - return blocking_notifier_call_chain(&fb_notifier_list, val, v);
> -}
> -EXPORT_SYMBOL_GPL(fb_notifier_call_chain);
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index dfcf5e4d1d4c..82ec7351e7da 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -498,14 +498,6 @@ static int do_register_framebuffer(struct fb_info *fb_info)
> num_registered_fb++;
> registered_fb[i] = fb_info;
>
> -#ifdef CONFIG_GUMSTIX_AM200EPD
> - {
> - struct fb_event event;
Drop the fb_event definition, it is no longer used.

> - event.info = fb_info;
> - fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
Drop the define for FB_EVENT_FB_REGISTERED

> - }
> -#endif
> -
> return fbcon_fb_registered(fb_info);
> }
>
> @@ -544,13 +536,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--;
> -#ifdef CONFIG_GUMSTIX_AM200EPD
> - {
> - struct fb_event event;
> - event.info = fb_info;
> - fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
Drop the define for FB_EVENT_FB_UNREGISTERED

> - }
> -#endif
> fbcon_fb_unregistered(fb_info);
>
> /* this may free fb info */
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 05cc251035da..520ad870b8b2 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -151,27 +151,6 @@ struct fb_blit_caps {
> u32 flags;
> };
>
> -#ifdef CONFIG_FB_NOTIFY

The Kconfig symbol FB_NOTIFY should be dropped as well.

> -extern int fb_register_client(struct notifier_block *nb);
Drop forward for notifier_block, last user in the file is gone.

> -extern int fb_unregister_client(struct notifier_block *nb);
> -extern int fb_notifier_call_chain(unsigned long val, void *v);
> -#else
> -static inline int fb_register_client(struct notifier_block *nb)
> -{
> - return 0;
> -};
> -
> -static inline int fb_unregister_client(struct notifier_block *nb)
> -{
> - return 0;
> -};
> -
> -static inline int fb_notifier_call_chain(unsigned long val, void *v)
> -{
> - return 0;
> -};
> -#endif
> -
> /*
> * Pixmap structure definition
> *
> --
> 2.39.5