Re: [PATCH 03/11] fbdev/vga16fb: Auto-generate module init/exit code

From
Javier Martinez Canillas <javierm@redhat.com>
Date
2022-07-08 13:17:04
Also in
dri-devel, linux-staging
Reply to
Thomas Zimmermann, 2022-07-07
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 ?
quoted hunk ↗ jump to hunk
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 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@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat
Thread (31 messages) 31 messages, 2 authors, last reply 2022-07-15
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help