On Wed, Apr 13, 2022 at 12:50:50PM +0200, Javier Martinez Canillas wrote:
> On 4/13/22 11:24, Thomas Zimmermann wrote:
> > A workaround makes fbdev hot-unplugging work for framebuffers without
> > device. The only user for this feature was offb. As each OF framebuffer
> > now has an associated platform device, the workaround is no longer
> > needed. Remove it. Effectively reverts commit 0f525289ff0d ("fbdev: Fix
> > unregistering of framebuffers without device").
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > ---
> > drivers/video/fbdev/core/fbmem.c | 9 +--------
> > 1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > index bc6ed750e915..bdd00d381bbc 100644
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -1579,14 +1579,7 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
> > * If it's not a platform device, at least print a warning. A
> > * fix would add code to remove the device from the system.
> > */
> > - if (!device) {
> > - /* TODO: Represent each OF framebuffer as its own
> > - * device in the device hierarchy. For now, offb
> > - * doesn't have such a device, so unregister the
> > - * framebuffer as before without warning.
> > - */
> > - do_unregister_framebuffer(registered_fb[i]);
>
> Maybe we could still keep this for a couple of releases but with a big
> warning that's not supported in case there are out-of-tree drivers out
> there that still do this ?
>
> Or at least a warning if the do_unregister_framebuffer() call is removed.
Yeah dying while holding console_lock isn't fun, and not having a WARN_ON
+ bail-out code pretty much forces bug reporters to do a bisect here to
give us something more than "machine dies at boot with no messages".
I'd just outright keep the WARN_ON here for 1-2 years even to really make
sure we got all the bug reports, since often these older machines only
update onto LTS releases.
And it needs to be a WARN_ON + bail out since BUG_ON is as bad as just
oopsing.
-Daniel
>
> Regardless of what you chose to do, the patch looks good to me.
>
> Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
>
> --
> Best regards,
>
> Javier Martinez Canillas
> Linux Engineering
> Red Hat
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
