Hello Jani, On 11/4/21 20:57, Jani Nikula wrote: > On Thu, 04 Nov 2021, Javier Martinez Canillas <javierm@xxxxxxxxxx> wrote: >> +/** >> + * drm_drv_enabled - Checks if a DRM driver can be enabled >> + * @driver: DRM driver to check >> + * >> + * Checks whether a DRM driver can be enabled or not. This may be the case >> + * if the "nomodeset" kernel command line parameter is used. >> + * >> + * Return: 0 on success or a negative error code on failure. >> + */ >> +int drm_drv_enabled(const struct drm_driver *driver) >> +{ >> + if (vgacon_text_force()) { >> + DRM_INFO("%s driver is disabled\n", driver->name); >> + return -ENODEV; >> + } >> + >> + return 0; >> +} >> +EXPORT_SYMBOL(drm_drv_enabled); > > The name implies a bool return, but it's not. > > if (drm_drv_enabled(...)) { > /* surprise, it's disabled! */ > } > It used to return a bool in v2 but Thomas suggested an int instead to have consistency on the errno code that was returned by the callers. I should probably name that function differently to avoid confusion. But I think you are correct and this change is caused too much churn for not that much benefit, specially since is unclear that there might be another condition to prevent a DRM driver to load besides nomodeset. I'll just drop this patch and post only #2 but making drivers to test using the drm_check_modeset() function (which doesn't have a name that implies a bool return). > > BR, > Jani. > > > Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat