[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <05a5e893-12f7-49fd-9a9a-abd387571f9b@broadcom.com>
Date: Fri, 3 May 2024 13:22:10 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org
Cc: Daniel Vetter <daniel@...ll.ch>, Helge Deller <deller@....de>,
Thomas Zimmermann <tzimmermann@...e.de>,
Javier Martinez Canillas <javierm@...hat.com>,
Sam Ravnborg <sam@...nborg.org>,
"open list:FRAMEBUFFER LAYER" <linux-fbdev@...r.kernel.org>,
"open list:FRAMEBUFFER LAYER" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate
On 5/3/24 12:45, Arnd Bergmann wrote:
> On Fri, May 3, 2024, at 21:28, Florian Fainelli wrote:
>> Android devices in recovery mode make use of a framebuffer device to
>> provide an user interface. In a GKI configuration that has CONFIG_FB=m,
>> but CONFIG_FB_NOTIFY=y, loading the fb.ko module will fail with:
>>
>> fb: Unknown symbol fb_notifier_call_chain (err -2)
>>
>> Have CONFIG_FB_NOTIFY be tristate, just like CONFIG_FB such that both
>> can be loaded as module with fb_notify.ko first, and fb.ko second.
>>
>> Signed-off-by: Florian Fainelli <florian.fainelli@...adcom.com>
>
> I see two problems here, so I don't think this is the right
> approach:
>
> 1. I don't understand your description: Since fb_notifier_call_chain()
> is an exported symbol, it should work for both FB_NOTIFY=y and
> FB_NOTIFY=m, unless something in Android drops the exported
> symbols for some reason.
The symbol is still exported in the Android tree. The issue is that the
GKI defconfig does not enable any CONFIG_FB options at all. This is left
for SoC vendors to do in their own "fragment" which would add
CONFIG_FB=m. That implies CONFIG_FB_NOTIFY=y which was not part of the
original kernel image we build/run against, and so we cannot resolve the
symbol.
This could be resolved by having the GKI kernel have CONFIG_FB_NOTIFY=y
but this is a bit wasteful (not by much since the code is very slim
anyway) and it does require making changes specifically to the Android
tree which could be beneficial upstream, hence my attempt at going
upstream first.
IMHO it makes sense for all subsystem supporting code to be completely
modular or completely built-in, or at least allowed to be.
>
> 2. This breaks if any of the four callers of fb_register_client()
> are built-in while CONFIG_FB_NOTIFY is set to =m:
Ah good point, I missed that part, thanks, adding "select FB_NOTIFY"
ought to be enough for those.
>
> $ git grep -w fb_register_client
> arch/arm/mach-pxa/am200epd.c: fb_register_client(&am200_fb_notif);
> drivers/leds/trigger/ledtrig-backlight.c: ret = fb_register_client(&n->notifier);
> drivers/video/backlight/backlight.c: return fb_register_client(&bd->fb_notif);
> drivers/video/backlight/lcd.c: return fb_register_client(&ld->fb_notif);
>
> Somewhat related but not directly addressing your patch, I wonder
> if Android itself could migrate to using FB_CORE=m FB=n FB_NOTIFY=n
> instead and use simpledrm for the console in place of the legacy
> fbdev layer.
That is beyond my reach :)
--
Florian
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)
Powered by blists - more mailing lists