[PATCH] uvesafb: abort initialization if video=uvesafb is not set

[PATCH] uvesafb: abort initialization if video=uvesafb is not set

[lxnay]

From: Fabio Erculiani <lxnay@sabayon.org>

This patch makes possible to ship kernels with both vesafb and uvesafb
in order to guarantee a smooth transition to uvesafb and cope with
potential incompatibiles introduced by uvesafb making possible to disable
it via cmdline.

In case both vesafb and uvesafb are built-in, the kernel will try to
initialize both, which makes possible to select the wanted one using
either video=vesafb:... or video=uvesafb:....
In this way, old distro installations will keep working as before while
new ones can adopt video=uvesafb.

The behaviour does not change if uvesafb is built as a module.
---
 drivers/video/uvesafb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index d428445..04c4742 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -1957,6 +1957,10 @@ static int uvesafb_init(void)
 
 	if (fb_get_options("uvesafb", &option))
 		return -ENODEV;
+	if (!option || !*option)
+		/* if vesafb is enabled, this will make possible to fallback to it */
+		return -ENODEV;
+
 	uvesafb_setup(option);
 #endif
 	err = cn_add_callback(&uvesafb_cn_id, "uvesafb", uvesafb_cn_callback);
-- 
1.8.1.5

Re: [PATCH] uvesafb: abort initialization if video=uvesafb is not set

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]

On 06/04/14 16:44, lxnay@sabayon.org wrote:
> From: Fabio Erculiani <lxnay@sabayon.org>
> 
> This patch makes possible to ship kernels with both vesafb and uvesafb
> in order to guarantee a smooth transition to uvesafb and cope with
> potential incompatibiles introduced by uvesafb making possible to disable
> it via cmdline.
> 
> In case both vesafb and uvesafb are built-in, the kernel will try to
> initialize both, which makes possible to select the wanted one using
> either video=vesafb:... or video=uvesafb:....
> In this way, old distro installations will keep working as before while
> new ones can adopt video=uvesafb.
> 
> The behaviour does not change if uvesafb is built as a module.
> ---
>  drivers/video/uvesafb.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
> index d428445..04c4742 100644
> --- a/drivers/video/uvesafb.c
> +++ b/drivers/video/uvesafb.c
> @@ -1957,6 +1957,10 @@ static int uvesafb_init(void)
>  
>  	if (fb_get_options("uvesafb", &option))
>  		return -ENODEV;
> +	if (!option || !*option)
> +		/* if vesafb is enabled, this will make possible to fallback to it */
> +		return -ENODEV;
> +
>  	uvesafb_setup(option);
>  #endif
>  	err = cn_add_callback(&uvesafb_cn_id, "uvesafb", uvesafb_cn_callback);
> 

I'm not familiar with vesa fbs, so I'd like to hear from other people if
this change is ok. Cc'd a bunch of random people from git log.
Reviewed-by, tested-by, acked-by from anyone?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] uvesafb: abort initialization if video=uvesafb is not set

[Wang YanQing]

On Fri, May 09, 2014 at 01:06:44PM +0300, Tomi Valkeinen wrote:
> On 06/04/14 16:44, lxnay@sabayon.org wrote:
> > From: Fabio Erculiani <lxnay@sabayon.org>
> > 
> > This patch makes possible to ship kernels with both vesafb and uvesafb
> > in order to guarantee a smooth transition to uvesafb and cope with
> > potential incompatibiles introduced by uvesafb making possible to disable
> > it via cmdline.
> > 
> > In case both vesafb and uvesafb are built-in, the kernel will try to
> > initialize both, which makes possible to select the wanted one using
> > either video=vesafb:... or video=uvesafb:....
> > In this way, old distro installations will keep working as before while
> > new ones can adopt video=uvesafb.
> > 
> > The behaviour does not change if uvesafb is built as a module.
> > ---
> >  drivers/video/uvesafb.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
> > index d428445..04c4742 100644
> > --- a/drivers/video/uvesafb.c
> > +++ b/drivers/video/uvesafb.c
> > @@ -1957,6 +1957,10 @@ static int uvesafb_init(void)
> >  
> >  	if (fb_get_options("uvesafb", &option))
> >  		return -ENODEV;
> > +	if (!option || !*option)
> > +		/* if vesafb is enabled, this will make possible to fallback to it */
> > +		return -ENODEV;
> > +
> >  	uvesafb_setup(option);
> >  #endif
> >  	err = cn_add_callback(&uvesafb_cn_id, "uvesafb", uvesafb_cn_callback);
> > 
I don't think it is right, this patch force uvesafb user to set uvesafb kernel cmdline
options, but what about the default setting? If user use UVESAFB_DEFAULT_MODE before, then
he or she have to add useless cmdline options to make uvesafb work for it in future kernel.

Thanks.

Re: [PATCH] uvesafb: abort initialization if video=uvesafb is not set

[David Herrmann]

Hi

On Sun, Apr 6, 2014 at 3:44 PM,  <lxnay@sabayon.org> wrote:
> From: Fabio Erculiani <lxnay@sabayon.org>
>
> This patch makes possible to ship kernels with both vesafb and uvesafb
> in order to guarantee a smooth transition to uvesafb and cope with
> potential incompatibiles introduced by uvesafb making possible to disable
> it via cmdline.
>
> In case both vesafb and uvesafb are built-in, the kernel will try to
> initialize both, which makes possible to select the wanted one using
> either video=vesafb:... or video=uvesafb:....
> In this way, old distro installations will keep working as before while
> new ones can adopt video=uvesafb.

Why would you want vesafb _and_ uvesafb built-in? Ship them as modules
and let users blacklist the modules they don't want. I mean the
problem you describe is specific to distros. Embedded devices or other
specific use-cases can explicitly disable any unwanted module. And for
distros I cannot see why we should support both as built-in modules.

_Iff_ you want this as in-kernel option, I'd recommend looking at the
sysfb stuff. uvesafb could easily claim the vesa-framebuffer and thus
unload any generic drivers like vesafb.

Thanks
David