[PATCH 2/2] fbdev: Remove hot-unplug workaround for framebuffers without device

[Thomas Zimmermann]

Hi

Am 13.04.22 um 18:05 schrieb Daniel Vetter:
> 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 at suse.de>
>>> ---
>>>   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.

If that's what the consent is, I'll go with it.

I'm just not sure if we talk about the same problem. offb didn't have a 
platform device, so we recently added this workaround with 'if 
(!device)'.  All the other fbdev drivers have a platform device; and 
anything else that could fail is out-of-tree. We don't really care about 
those AFAIK.

With offb converted, we could practically remove all of the checks here 
and call platform_device_unregister() unconditionally.

Best regards
Thomas

> 
> 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 at redhat.com>
>>
>> -- 
>> Best regards,
>>
>> Javier Martinez Canillas
>> Linux Engineering
>> Red Hat
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20220413/e6a83452/attachment-0001.sig>