linux-kernel - Re: [PATCH] arc: kernel: add default extern variable 'screen

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 24 Oct 2013 10:56:02 +0800
From:	Chen Gang <gang.chen@...anux.com>
To:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
CC:	Arnd Bergmann <arnd@...db.de>,
	"sachin.kamat@...aro.org" <sachin.kamat@...aro.org>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	James Hogan <james.hogan@...tec.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Will Deacon <will.deacon@....com>,
	Catalin Marinas <Catalin.Marinas@....com>
Subject: Re: [PATCH] arc: kernel: add default extern variable 'screen_info'
 in "setup.c"

On 10/23/2013 09:47 PM, Vineet Gupta wrote:
> Apologies for top posting !
> 

Not at all.

> NAK.
> 

OK, thanks. At least this patch is incorrect.


> ARC will never have VGA console. You need to add !ARC to relevant Kconfig. However that approach is frowned upon in general. The current way to doing such things is to define a new Kconfig item which relevant arches can select.

Hmm... it seems necessary to discuss about the 2 fixing ways (which
already had a long discussion in ARM64):

 - add !ARC in related place, just like another (almost 30-40%) architectures done.

 - add a new Kconfig item (e.g. HAVE_VGA_CONSOLE), and let the left (almost 50%) architectures which use VGA_CONSOLE to set it.

Catalin, Will, Geert (it seems also include you) prefer 2nd way, but I
prefer 1st, my reasons are below, please help check:

 - 1st way need add some (10-20%) of architectures in one place, which belongs to local wide.
   2nd way will let the left (almost 50%) architectures need add HAVE_VGA_CONSOLE in various place, which belongs to arch global wide.

 - 1st way will let most (80-90%) of architectures don't care about VGA_CONSOLE (50% need it, 30-40% already mark it in the same place).
   2nd way will let 50% architectures care about VGA_CONSOLE (although can let the left 10-20% architectures do nothing -- not care about it).

 - 1st way is still easy, sustainable, and clear enough in local wide fixing (although maybe it is not the best, or clearest way).
   2nd way is also easy, sustainable and clear enough (maybe the best, or clearest for 10-20% architectures), but it will let the fix in global wide

All together, if we can bear and sustainable, I always prefer to fix it
in local wide, not lead to arch global (especially 80-90% architectures
already followed 1st way).


BTW: for me, if less than 20% architectures need XXX, we can trigger
defining a new Kconfig item (e.g. HAVE_XXX), or just follow original
implementation.


Thanks.

> 
> -Vineet
> ________________________________________
> From: Chen Gang [gang.chen@...anux.com]
> Sent: Wednesday, October 23, 2013 4:39 PM
> To: vgupta@...opsys.com; Arnd Bergmann; sachin.kamat@...aro.org; Paul Gortmaker; James Hogan
> Cc: linux-kernel@...r.kernel.org
> Subject: [PATCH] arc: kernel: add default extern variable 'screen_info' in "setup.c"
> 
> Add default 'screen_info' just like some of other architectures (e.g.
> cris, score, sh, tile), or can not pass compiling.
> 
> The related error (with allmodconfig):
> 
>   drivers/built-in.o: In function `vgacon_save_screen':
>   drivers/video/console/vgacon.c:1347: undefined reference to `screen_info'
>   drivers/video/console/vgacon.c:1348: undefined reference to `screen_info'
>   drivers/built-in.o: In function `vgacon_resize':
>   drivers/video/console/vgacon.c:1314: undefined reference to `screen_info'
>   drivers/video/console/vgacon.c:1315: undefined reference to `screen_info'
>   drivers/built-in.o: In function `vgacon_switch':
>   drivers/video/console/vgacon.c:820: undefined reference to `screen_info'
>   drivers/built-in.o:drivers/video/console/vgacon.c:840: more undefined references to `screen_info' follow
> 
> 
> Signed-off-by: Chen Gang <gang.chen@...anux.com>
> ---
>  arch/arc/kernel/setup.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
> index 2c68bc7e..07130f3 100644
> --- a/arch/arc/kernel/setup.c
> +++ b/arch/arc/kernel/setup.c
> @@ -15,6 +15,7 @@
>  #include <linux/cpu.h>
>  #include <linux/of_fdt.h>
>  #include <linux/cache.h>
> +#include <linux/screen_info.h>
>  #include <asm/sections.h>
>  #include <asm/arcregs.h>
>  #include <asm/tlb.h>
> @@ -37,6 +38,8 @@ struct task_struct *_current_task[NR_CPUS];   /* For stack switching */
> 
>  struct cpuinfo_arc cpuinfo_arc700[NR_CPUS];
> 
> +struct screen_info screen_info;
> +
> 
>  void read_arc_build_cfg_regs(void)
>  {
> --
> 1.7.7.6
> 
> 


-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/