On 7/11/22 10:01, Thomas Zimmermann wrote:

Hi

Am 08.07.22 um 15:16 schrieb Javier Martinez Canillas:

quoted

On 7/7/22 17:39, Thomas Zimmermann wrote:

quoted

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 ?

Sure.

I mostly wanted to use module_platform_driver(). The options handling is 
in the way.

Yes, I got it when looked at this patch.
 

quoted

quoted

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.

Or can we kill it entirely? There are no actual options.

That sounds good to me as well.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat