On 1/24/22 15:19, Thomas Zimmermann wrote:
[snip]
if (dev_is_platform(dev)) {
In do_register_framebuffer() creating the fb%d is not a fatal error. It would be safer to do if (!IS_ERR_OR_NULL(dev) && dev_is_platform(dev)) instead here.
https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fbme...
'dev' here refers to 'fb_info->device', which is the underlying device created by the sysfb code. fb_info->dev is something different.
oh, indeed. I conflated the two.
Maybe the local variable could be renamed to 'device' just to avoid confusion ?
[snip]
I'm not sure to follow the logic here. The forced_out bool is set when the platform device is unregistered in do_remove_conflicting_framebuffers(), but shouldn't the struct platform_driver .remove callback be executed even in this case ?
That is, the platform_device_unregister() will trigger the call to the .remove callback that in turn will call unregister_framebuffer().
Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ?
Doing the hot-unplug will end up in unregister_framebuffer(), but we already hold the lock from the do_remove_conflicting_framebuffer() code.
Yes, I realized that just after sending the first email. Sorry for the noise.
Best regards,