public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Thomas Zimmermann <tzimmermann@suse•de>
To: Daniel Vetter <daniel@ffwll•ch>,
	Javier Martinez Canillas <javierm@redhat•com>
Cc: devicetree@vger•kernel.org, linux-fbdev@vger•kernel.org,
	sam@ravnborg•org, frowand.list@gmail•com, deller@gmx•de,
	dri-devel@lists•freedesktop.org, robh+dt@kernel•org,
	paulus@samba•org, linuxppc-dev@lists•ozlabs.org,
	linux@roeck-us•net
Subject: Re: [PATCH 2/2] fbdev: Remove hot-unplug workaround for framebuffers without device
Date: Wed, 13 Apr 2022 20:09:00 +0200	[thread overview]
Message-ID: <9d4599d9-e094-e7dd-5b91-282c2679aae4@suse.de> (raw)
In-Reply-To: <Ylb0316ABOhOe1Rb@phenom.ffwll.local>


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

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@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@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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

  reply	other threads:[~2022-04-13 18:09 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-13  9:24 [PATCH 0/2] of: Register platform device for each framebuffer Thomas Zimmermann
2022-04-13  9:24 ` [PATCH 1/2] of: Create platform devices for OF framebuffers Thomas Zimmermann
2022-04-13 10:45   ` Javier Martinez Canillas
2022-04-13 10:49     ` Thomas Zimmermann
2022-04-13 12:51   ` Rob Herring
2022-04-13 17:58     ` Thomas Zimmermann
2022-04-13 18:02       ` Javier Martinez Canillas
2022-04-13 18:46       ` Rob Herring
2022-04-13 18:53         ` Rob Herring
2022-04-18 18:09     ` Thomas Zimmermann
2022-04-13  9:24 ` [PATCH 2/2] fbdev: Remove hot-unplug workaround for framebuffers without device Thomas Zimmermann
2022-04-13 10:50   ` Javier Martinez Canillas
2022-04-13 16:05     ` Daniel Vetter
2022-04-13 18:09       ` Thomas Zimmermann [this message]
2022-04-19  7:22         ` Javier Martinez Canillas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9d4599d9-e094-e7dd-5b91-282c2679aae4@suse.de \
    --to=tzimmermann@suse$(echo .)de \
    --cc=daniel@ffwll$(echo .)ch \
    --cc=deller@gmx$(echo .)de \
    --cc=devicetree@vger$(echo .)kernel.org \
    --cc=dri-devel@lists$(echo .)freedesktop.org \
    --cc=frowand.list@gmail$(echo .)com \
    --cc=javierm@redhat$(echo .)com \
    --cc=linux-fbdev@vger$(echo .)kernel.org \
    --cc=linux@roeck-us$(echo .)net \
    --cc=linuxppc-dev@lists$(echo .)ozlabs.org \
    --cc=paulus@samba$(echo .)org \
    --cc=robh+dt@kernel$(echo .)org \
    --cc=sam@ravnborg$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox