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/fbmem.c#L1605
>
> '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,
--
Javier Martinez Canillas
Linux Engineering
Red Hat
