diff mbox series

[13/21] fbcon: move more common code into fb_open()

Message ID 20220131210552.482606-14-daniel.vetter@ffwll.ch (mailing list archive)
State Superseded
Headers show
Series some fbcon patches, mostly locking | expand

Commit Message

Daniel Vetter Jan. 31, 2022, 9:05 p.m. UTC
No idea why con2fb_acquire_newinfo() initializes much less than
fbcon_startup(), but so be it. From a quick look most of the
un-initialized stuff should be fairly harmless, but who knows.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Claudio Suarez <cssk@net-c.es>
Cc: Du Cheng <ducheng2@gmail.com>
---
 drivers/video/fbdev/core/fbcon.c | 74 +++++++++++++-------------------
 1 file changed, 31 insertions(+), 43 deletions(-)

Comments

kernel test robot Jan. 31, 2022, 11:31 p.m. UTC | #1
Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tegra-drm/drm/tegra/for-next]
[also build test WARNING on drm/drm-next linus/master v5.17-rc2 next-20220131]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/some-fbcon-patches-mostly-locking/20220201-050907
base:   git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next
config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20220201/202202010613.HT19HztX-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7a27c0ce71acfa8b67787d298de9836376fcc323
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Vetter/some-fbcon-patches-mostly-locking/20220201-050907
        git checkout 7a27c0ce71acfa8b67787d298de9836376fcc323
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=mips SHELL=/bin/bash drivers/video/fbdev/core/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/video/fbdev/core/fbcon.c: In function 'con2fb_acquire_newinfo':
>> drivers/video/fbdev/core/fbcon.c:721:27: warning: variable 'ops' set but not used [-Wunused-but-set-variable]
     721 |         struct fbcon_ops *ops = NULL;
         |                           ^~~


vim +/ops +721 drivers/video/fbdev/core/fbcon.c

^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16  717  
^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16  718  static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
7a27c0ce71acfa8 drivers/video/fbdev/core/fbcon.c Daniel Vetter      2022-01-31  719  				  int unit)
^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16  720  {
^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16 @721  	struct fbcon_ops *ops = NULL;
ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter      2022-01-31  722  	int err;
^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16  723  
ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter      2022-01-31  724  	err = fbcon_open(info);
ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter      2022-01-31  725  	if (err)
ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter      2022-01-31  726  		return err;
^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16  727  
7a27c0ce71acfa8 drivers/video/fbdev/core/fbcon.c Daniel Vetter      2022-01-31  728  	ops = info->fbcon_par;
d1baa4ffa677bf6 drivers/video/console/fbcon.c    Antonino A. Daplas 2007-07-17  729  
d1baa4ffa677bf6 drivers/video/console/fbcon.c    Antonino A. Daplas 2007-07-17  730  	if (vc)
b73deed32d08740 drivers/video/console/fbcon.c    Antonino A. Daplas 2006-01-09  731  		set_blitting_type(vc, info);
^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16  732  
^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16  733  	return err;
^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16  734  }
^1da177e4c3f415 drivers/video/console/fbcon.c    Linus Torvalds     2005-04-16  735  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Sam Ravnborg Feb. 4, 2022, 7:35 p.m. UTC | #2
On Mon, Jan 31, 2022 at 10:05:44PM +0100, Daniel Vetter wrote:
> No idea why con2fb_acquire_newinfo() initializes much less than
> fbcon_startup(), but so be it. From a quick look most of the
> un-initialized stuff should be fairly harmless, but who knows.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Claudio Suarez <cssk@net-c.es>
> Cc: Du Cheng <ducheng2@gmail.com>
> ---
>  drivers/video/fbdev/core/fbcon.c | 74 +++++++++++++-------------------
>  1 file changed, 31 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index b83a5a77d8a8..5a3391ff038d 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -680,8 +680,18 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount)
>  
>  #endif /* CONFIG_MISC_TILEBLITTING */
>  
> +static void fbcon_release(struct fb_info *info)
> +{
> +	if (info->fbops->fb_release)
> +		info->fbops->fb_release(info, 0);
> +
> +	module_put(info->fbops->owner);
> +}
> +
>  static int fbcon_open(struct fb_info *info)
>  {
> +	struct fbcon_ops *ops;
> +
>  	if (!try_module_get(info->fbops->owner))
>  		return -ENODEV;
>  
> @@ -691,19 +701,22 @@ static int fbcon_open(struct fb_info *info)
>  		return -ENODEV;
>  	}
>  
> -	return 0;
> -}
> +	ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> +	if (!ops) {
> +		fbcon_release(info);
> +		return -ENOMEM;
> +	}
>  
> -static void fbcon_release(struct fb_info *info)
> -{
> -	if (info->fbops->fb_release)
> -		info->fbops->fb_release(info, 0);
> +	INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> +	ops->info = info;
> +	info->fbcon_par = ops;
> +	ops->cur_blink_jiffies = HZ / 5;
>  
> -	module_put(info->fbops->owner);
> +	return 0;
>  }
>  
>  static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
> -				  int unit, int oldidx)
> +				  int unit)
>  {
>  	struct fbcon_ops *ops = NULL;
>  	int err;
> @@ -712,27 +725,10 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
>  	if (err)
>  		return err;
>  
> -	if (!err) {
> -		ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> -		if (!ops)
> -			err = -ENOMEM;
> -
> -		INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> -	}
> -
> -	if (!err) {
> -		ops->cur_blink_jiffies = HZ / 5;
> -		ops->info = info;
> -		info->fbcon_par = ops;
> -
> -		if (vc)
> -			set_blitting_type(vc, info);
> -	}
> +	ops = info->fbcon_par;
>  
> -	if (err) {
> -		con2fb_map[unit] = oldidx;
> -		fbcon_release(info);
> -	}
> +	if (vc)
> +		set_blitting_type(vc, info);
>  
>  	return err;
>  }
> @@ -840,9 +836,11 @@ static int set_con2fb_map(int unit, int newidx, int user)
>  
>  	found = search_fb_in_map(newidx);
>  
> -	con2fb_map[unit] = newidx;
> -	if (!err && !found)
> -		err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
> +	if (!err && !found) {
> +		err = con2fb_acquire_newinfo(vc, info, unit);
> +		if (!err)
> +			con2fb_map[unit] = newidx;
> +	}
This looks like an unintentional change of functionality as con2fb_map[unit] is
only assigned when we do a con2fb_acquire_newinfo().

Staring at the code I could not say it is wrong, but not nice to hide
the change in this patch.

	Sam


>  
>  	/*
>  	 * If old fb is not mapped to any of the consoles,
> @@ -939,20 +937,10 @@ static const char *fbcon_startup(void)
>  	if (fbcon_open(info))
>  		return NULL;
>  
> -	ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> -	if (!ops) {
> -		fbcon_release(info);
> -		return NULL;
> -	}
> -
> -	INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> -
> +	ops = info->fbcon_par;
>  	ops->currcon = -1;
>  	ops->graphics = 1;
>  	ops->cur_rotate = -1;
> -	ops->cur_blink_jiffies = HZ / 5;
> -	ops->info = info;
> -	info->fbcon_par = ops;
>  
>  	p->con_rotate = initial_rotation;
>  	if (p->con_rotate == -1)
> @@ -1022,7 +1010,7 @@ static void fbcon_init(struct vc_data *vc, int init)
>  		return;
>  
>  	if (!info->fbcon_par)
> -		con2fb_acquire_newinfo(vc, info, vc->vc_num, -1);
> +		con2fb_acquire_newinfo(vc, info, vc->vc_num);
>  
>  	/* If we are not the first console on this
>  	   fb, copy the font from that console */
> -- 
> 2.33.0
Daniel Vetter Feb. 8, 2022, 1:53 p.m. UTC | #3
On Fri, Feb 04, 2022 at 08:35:31PM +0100, Sam Ravnborg wrote:
> On Mon, Jan 31, 2022 at 10:05:44PM +0100, Daniel Vetter wrote:
> > No idea why con2fb_acquire_newinfo() initializes much less than
> > fbcon_startup(), but so be it. From a quick look most of the
> > un-initialized stuff should be fairly harmless, but who knows.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Claudio Suarez <cssk@net-c.es>
> > Cc: Du Cheng <ducheng2@gmail.com>
> > ---
> >  drivers/video/fbdev/core/fbcon.c | 74 +++++++++++++-------------------
> >  1 file changed, 31 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> > index b83a5a77d8a8..5a3391ff038d 100644
> > --- a/drivers/video/fbdev/core/fbcon.c
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -680,8 +680,18 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount)
> >  
> >  #endif /* CONFIG_MISC_TILEBLITTING */
> >  
> > +static void fbcon_release(struct fb_info *info)
> > +{
> > +	if (info->fbops->fb_release)
> > +		info->fbops->fb_release(info, 0);
> > +
> > +	module_put(info->fbops->owner);
> > +}
> > +
> >  static int fbcon_open(struct fb_info *info)
> >  {
> > +	struct fbcon_ops *ops;
> > +
> >  	if (!try_module_get(info->fbops->owner))
> >  		return -ENODEV;
> >  
> > @@ -691,19 +701,22 @@ static int fbcon_open(struct fb_info *info)
> >  		return -ENODEV;
> >  	}
> >  
> > -	return 0;
> > -}
> > +	ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> > +	if (!ops) {
> > +		fbcon_release(info);
> > +		return -ENOMEM;
> > +	}
> >  
> > -static void fbcon_release(struct fb_info *info)
> > -{
> > -	if (info->fbops->fb_release)
> > -		info->fbops->fb_release(info, 0);
> > +	INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> > +	ops->info = info;
> > +	info->fbcon_par = ops;
> > +	ops->cur_blink_jiffies = HZ / 5;
> >  
> > -	module_put(info->fbops->owner);
> > +	return 0;
> >  }
> >  
> >  static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
> > -				  int unit, int oldidx)
> > +				  int unit)
> >  {
> >  	struct fbcon_ops *ops = NULL;
> >  	int err;
> > @@ -712,27 +725,10 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
> >  	if (err)
> >  		return err;
> >  
> > -	if (!err) {
> > -		ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> > -		if (!ops)
> > -			err = -ENOMEM;
> > -
> > -		INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> > -	}
> > -
> > -	if (!err) {
> > -		ops->cur_blink_jiffies = HZ / 5;
> > -		ops->info = info;
> > -		info->fbcon_par = ops;
> > -
> > -		if (vc)
> > -			set_blitting_type(vc, info);
> > -	}
> > +	ops = info->fbcon_par;
> >  
> > -	if (err) {
> > -		con2fb_map[unit] = oldidx;
> > -		fbcon_release(info);
> > -	}
> > +	if (vc)
> > +		set_blitting_type(vc, info);
> >  
> >  	return err;
> >  }
> > @@ -840,9 +836,11 @@ static int set_con2fb_map(int unit, int newidx, int user)
> >  
> >  	found = search_fb_in_map(newidx);
> >  
> > -	con2fb_map[unit] = newidx;
> > -	if (!err && !found)
> > -		err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
> > +	if (!err && !found) {
> > +		err = con2fb_acquire_newinfo(vc, info, unit);
> > +		if (!err)
> > +			con2fb_map[unit] = newidx;
> > +	}
> This looks like an unintentional change of functionality as con2fb_map[unit] is
> only assigned when we do a con2fb_acquire_newinfo().
> 
> Staring at the code I could not say it is wrong, but not nice to hide
> the change in this patch.

Nope, it's not an unintentional bugfix. The old con2fb_acquire_newinfo did
reset con2fb_map to oldidx upon failure, which I've found to be a most
bizarre calling convention. So this sorts this out.

The reason I smashed this into the same patch is that I had to remove the
fbcon_release call, and so the error handling in there looked even more
funny. But I indeed failed to explain this all in the commit message.

Ack with that explainer, or do you want me to split this out properly?
-Daniel

> 
> 	Sam
> 
> 
> >  
> >  	/*
> >  	 * If old fb is not mapped to any of the consoles,
> > @@ -939,20 +937,10 @@ static const char *fbcon_startup(void)
> >  	if (fbcon_open(info))
> >  		return NULL;
> >  
> > -	ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> > -	if (!ops) {
> > -		fbcon_release(info);
> > -		return NULL;
> > -	}
> > -
> > -	INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> > -
> > +	ops = info->fbcon_par;
> >  	ops->currcon = -1;
> >  	ops->graphics = 1;
> >  	ops->cur_rotate = -1;
> > -	ops->cur_blink_jiffies = HZ / 5;
> > -	ops->info = info;
> > -	info->fbcon_par = ops;
> >  
> >  	p->con_rotate = initial_rotation;
> >  	if (p->con_rotate == -1)
> > @@ -1022,7 +1010,7 @@ static void fbcon_init(struct vc_data *vc, int init)
> >  		return;
> >  
> >  	if (!info->fbcon_par)
> > -		con2fb_acquire_newinfo(vc, info, vc->vc_num, -1);
> > +		con2fb_acquire_newinfo(vc, info, vc->vc_num);
> >  
> >  	/* If we are not the first console on this
> >  	   fb, copy the font from that console */
> > -- 
> > 2.33.0
Sam Ravnborg Feb. 8, 2022, 6:25 p.m. UTC | #4
On Tue, Feb 08, 2022 at 02:53:59PM +0100, Daniel Vetter wrote:
> On Fri, Feb 04, 2022 at 08:35:31PM +0100, Sam Ravnborg wrote:
> > On Mon, Jan 31, 2022 at 10:05:44PM +0100, Daniel Vetter wrote:
> > > No idea why con2fb_acquire_newinfo() initializes much less than
> > > fbcon_startup(), but so be it. From a quick look most of the
> > > un-initialized stuff should be fairly harmless, but who knows.
> > > 
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: Claudio Suarez <cssk@net-c.es>
> > > Cc: Du Cheng <ducheng2@gmail.com>
> > > ---
> > >  drivers/video/fbdev/core/fbcon.c | 74 +++++++++++++-------------------
> > >  1 file changed, 31 insertions(+), 43 deletions(-)
> > > 
> > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> > > index b83a5a77d8a8..5a3391ff038d 100644
> > > --- a/drivers/video/fbdev/core/fbcon.c
> > > +++ b/drivers/video/fbdev/core/fbcon.c
> > > @@ -680,8 +680,18 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount)
> > >  
> > >  #endif /* CONFIG_MISC_TILEBLITTING */
> > >  
> > > +static void fbcon_release(struct fb_info *info)
> > > +{
> > > +	if (info->fbops->fb_release)
> > > +		info->fbops->fb_release(info, 0);
> > > +
> > > +	module_put(info->fbops->owner);
> > > +}
> > > +
> > >  static int fbcon_open(struct fb_info *info)
> > >  {
> > > +	struct fbcon_ops *ops;
> > > +
> > >  	if (!try_module_get(info->fbops->owner))
> > >  		return -ENODEV;
> > >  
> > > @@ -691,19 +701,22 @@ static int fbcon_open(struct fb_info *info)
> > >  		return -ENODEV;
> > >  	}
> > >  
> > > -	return 0;
> > > -}
> > > +	ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> > > +	if (!ops) {
> > > +		fbcon_release(info);
> > > +		return -ENOMEM;
> > > +	}
> > >  
> > > -static void fbcon_release(struct fb_info *info)
> > > -{
> > > -	if (info->fbops->fb_release)
> > > -		info->fbops->fb_release(info, 0);
> > > +	INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> > > +	ops->info = info;
> > > +	info->fbcon_par = ops;
> > > +	ops->cur_blink_jiffies = HZ / 5;
> > >  
> > > -	module_put(info->fbops->owner);
> > > +	return 0;
> > >  }
> > >  
> > >  static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
> > > -				  int unit, int oldidx)
> > > +				  int unit)
> > >  {
> > >  	struct fbcon_ops *ops = NULL;
> > >  	int err;
> > > @@ -712,27 +725,10 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
> > >  	if (err)
> > >  		return err;
> > >  
> > > -	if (!err) {
> > > -		ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
> > > -		if (!ops)
> > > -			err = -ENOMEM;
> > > -
> > > -		INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
> > > -	}
> > > -
> > > -	if (!err) {
> > > -		ops->cur_blink_jiffies = HZ / 5;
> > > -		ops->info = info;
> > > -		info->fbcon_par = ops;
> > > -
> > > -		if (vc)
> > > -			set_blitting_type(vc, info);
> > > -	}
> > > +	ops = info->fbcon_par;
> > >  
> > > -	if (err) {
> > > -		con2fb_map[unit] = oldidx;
> > > -		fbcon_release(info);
> > > -	}
> > > +	if (vc)
> > > +		set_blitting_type(vc, info);
> > >  
> > >  	return err;
> > >  }
> > > @@ -840,9 +836,11 @@ static int set_con2fb_map(int unit, int newidx, int user)
> > >  
> > >  	found = search_fb_in_map(newidx);
> > >  
> > > -	con2fb_map[unit] = newidx;
> > > -	if (!err && !found)
> > > -		err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
> > > +	if (!err && !found) {
> > > +		err = con2fb_acquire_newinfo(vc, info, unit);
> > > +		if (!err)
> > > +			con2fb_map[unit] = newidx;
> > > +	}
> > This looks like an unintentional change of functionality as con2fb_map[unit] is
> > only assigned when we do a con2fb_acquire_newinfo().
> > 
> > Staring at the code I could not say it is wrong, but not nice to hide
> > the change in this patch.
> 
> Nope, it's not an unintentional bugfix. The old con2fb_acquire_newinfo did
> reset con2fb_map to oldidx upon failure, which I've found to be a most
> bizarre calling convention. So this sorts this out.
> 
> The reason I smashed this into the same patch is that I had to remove the
> fbcon_release call, and so the error handling in there looked even more
> funny. But I indeed failed to explain this all in the commit message.
> 
> Ack with that explainer, or do you want me to split this out properly?

Please update the commit message, then this patch has my:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
diff mbox series

Patch

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index b83a5a77d8a8..5a3391ff038d 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -680,8 +680,18 @@  static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount)
 
 #endif /* CONFIG_MISC_TILEBLITTING */
 
+static void fbcon_release(struct fb_info *info)
+{
+	if (info->fbops->fb_release)
+		info->fbops->fb_release(info, 0);
+
+	module_put(info->fbops->owner);
+}
+
 static int fbcon_open(struct fb_info *info)
 {
+	struct fbcon_ops *ops;
+
 	if (!try_module_get(info->fbops->owner))
 		return -ENODEV;
 
@@ -691,19 +701,22 @@  static int fbcon_open(struct fb_info *info)
 		return -ENODEV;
 	}
 
-	return 0;
-}
+	ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
+	if (!ops) {
+		fbcon_release(info);
+		return -ENOMEM;
+	}
 
-static void fbcon_release(struct fb_info *info)
-{
-	if (info->fbops->fb_release)
-		info->fbops->fb_release(info, 0);
+	INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
+	ops->info = info;
+	info->fbcon_par = ops;
+	ops->cur_blink_jiffies = HZ / 5;
 
-	module_put(info->fbops->owner);
+	return 0;
 }
 
 static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
-				  int unit, int oldidx)
+				  int unit)
 {
 	struct fbcon_ops *ops = NULL;
 	int err;
@@ -712,27 +725,10 @@  static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
 	if (err)
 		return err;
 
-	if (!err) {
-		ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
-		if (!ops)
-			err = -ENOMEM;
-
-		INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
-	}
-
-	if (!err) {
-		ops->cur_blink_jiffies = HZ / 5;
-		ops->info = info;
-		info->fbcon_par = ops;
-
-		if (vc)
-			set_blitting_type(vc, info);
-	}
+	ops = info->fbcon_par;
 
-	if (err) {
-		con2fb_map[unit] = oldidx;
-		fbcon_release(info);
-	}
+	if (vc)
+		set_blitting_type(vc, info);
 
 	return err;
 }
@@ -840,9 +836,11 @@  static int set_con2fb_map(int unit, int newidx, int user)
 
 	found = search_fb_in_map(newidx);
 
-	con2fb_map[unit] = newidx;
-	if (!err && !found)
-		err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
+	if (!err && !found) {
+		err = con2fb_acquire_newinfo(vc, info, unit);
+		if (!err)
+			con2fb_map[unit] = newidx;
+	}
 
 	/*
 	 * If old fb is not mapped to any of the consoles,
@@ -939,20 +937,10 @@  static const char *fbcon_startup(void)
 	if (fbcon_open(info))
 		return NULL;
 
-	ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
-	if (!ops) {
-		fbcon_release(info);
-		return NULL;
-	}
-
-	INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
-
+	ops = info->fbcon_par;
 	ops->currcon = -1;
 	ops->graphics = 1;
 	ops->cur_rotate = -1;
-	ops->cur_blink_jiffies = HZ / 5;
-	ops->info = info;
-	info->fbcon_par = ops;
 
 	p->con_rotate = initial_rotation;
 	if (p->con_rotate == -1)
@@ -1022,7 +1010,7 @@  static void fbcon_init(struct vc_data *vc, int init)
 		return;
 
 	if (!info->fbcon_par)
-		con2fb_acquire_newinfo(vc, info, vc->vc_num, -1);
+		con2fb_acquire_newinfo(vc, info, vc->vc_num);
 
 	/* If we are not the first console on this
 	   fb, copy the font from that console */