Messages in this thread | | | From | Javier Martinez Canillas <> | Date | Fri, 12 Nov 2021 10:39:56 +0100 | Subject | Re: [PATCH v4 0/6] Cleanups for the nomodeset kernel command line parameter logic |
| |
Hello Pekka,
On 11/12/21 09:56, Pekka Paalanen wrote:
[snip]
> > Hi, > > these ideas make sense to me, so FWIW, > > Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com> >
Thanks.
> There is one nitpick I'd like to ask about: > > +bool drm_get_modeset(void) > +{ > + return !drm_nomodeset; > +} > +EXPORT_SYMBOL(drm_get_modeset); > > Doesn't "get" have a special meaning in the kernel land, like "take a > strong reference on an object and return it"?
That's a very good point.
> As this is just returning bool without changing anything, the usual > word to use is "is". Since this function is also used at most once per > driver, which is rarely, it could have a long and descriptive name. > > For example: > > bool drm_is_modeset_driver_allowed(void); >
Yeah, naming is hard. Jani also mentioned that he didn't like this function name, so I don't mind to re-spin the series only for that.
> - "drm" is the namespace > - "is" implies it is a read-only boolean inspection > - "modeset" is the feature being checked > - "driver" implies it is supposed gate driver loading or > initialization, rather than modesets after drivers have loaded > - "allowed" says it is asking about general policy rather than what a > driver does >
I believe that name is more verbose than needed. But don't have a strong opinion and could use it if others agree.
> Just a bikeshed, I'll leave it to actual kernel devs to say if this > would be more appropriate or worth it. >
I think is worth it and better to do it now before the patches land, but we could wait for others to chime in.
Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
| |