On 7/7/22 17:39, Thomas Zimmermann wrote:
> Move vgag16fb's option parsing into the driver's probe function and
> generate the rest of the module's init/exit functions from macros.
> Keep the options code, although there are no options defined.
>
Ah, I see now why you wanted to move the check to the probe function. If
is to allow this cleanup then discard that comment from previous patch
and I'm OK with the move.
Maybe you could comment in patch 02/11 commit message that the check is
moved to the probe handler to allow this cleanup as a follow-up patch ?
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
> drivers/video/fbdev/vga16fb.c | 35 ++++++++++-------------------------
> 1 file changed, 10 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
> index f7c1bb018843..e7767ed50c5b 100644
> --- a/drivers/video/fbdev/vga16fb.c
> +++ b/drivers/video/fbdev/vga16fb.c
> @@ -1321,12 +1321,21 @@ static int __init vga16fb_setup(char *options)
>
> static int vga16fb_probe(struct platform_device *dev)
> {
> +#ifndef MODULE
> + char *option = NULL;
> +#endif
> struct screen_info *si;
> struct fb_info *info;
> struct vga16fb_par *par;
> int i;
> int ret = 0;
>
> +#ifndef MODULE
> + if (fb_get_options("vga16fb", &option))
> + return -ENODEV;
> + vga16fb_setup(option);
> +#endif
> +
I would just drop these ifdefery and have the option unconditionally.
It seems that's what most fbdev drivers do AFAICT.
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
