* Re: [PATCH v2] video: fbdev: sis: init.c: Cleaning up variable that is never used
From: Dan Carpenter @ 2014-07-08 5:59 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
Daniel Vetter, Mauro Carvalho Chehab, Chen, Gong, linux-fbdev,
linux-kernel
In-Reply-To: <1404764628-25192-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
On Mon, Jul 07, 2014 at 10:23:47PM +0200, Rickard Strandqvist wrote:
> Variable ar assigned a value that is never used.
> I have also removed all the code that thereby serves no purpose.
>
> This was found using a static code analysis program called cppcheck
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Thanks.
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 5/5] drm/i915: Kick out vga console
From: Ed Tomlinson @ 2014-07-08 2:53 UTC (permalink / raw)
To: Chris Wilson, Intel Graphics Development, DRI Development, LKML,
David Herrmann, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
linux-fbdev, Jani Nikula, Dave Airlie
In-Reply-To: <20140707122654.GQ5821@phenom.ffwll.local>
Hi Daniel,
The patch below also works. You can use my Tested By for it.
Thanks
Ed Tomlinson <edtoml@gmail.com>
PS. I _really_ need to get a serial console working on my i7 box.
On Monday 07 July 2014 14:26:54 Daniel Vetter wrote:
> On Mon, Jul 07, 2014 at 06:45:49AM -0400, Ed Tomlinson wrote:
> > Daniel,
> >
> > I am not quite sure I understand what you want me to test?
> > Do you want me to try it without:
> >
> > > > + if (ret = 0) {
> > > > + ret = do_unregister_con_driver(&vga_con);
>
> Below the diff of what I mean.
> -Daniel
>
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5e583a1838f8..bd8517151479 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1466,12 +1466,13 @@ static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> #else
> static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> {
> - int ret;
> + int ret = 0;
>
> DRM_INFO("Replacing VGA console driver\n");
>
> console_lock();
> - ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> + if (con_is_bound(&vga_con))
> + ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> if (ret = 0) {
> ret = do_unregister_con_driver(&vga_con);
>
>
^ permalink raw reply
* [PATCH 3.10 52/53] drivers/video/fbdev/fb-puv3.c: Add header files for function unifb_mmap
From: Greg Kroah-Hartman @ 2014-07-07 23:58 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Zhichuang Sun,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
Daniel Vetter, Joe Perches, Laurent Pinchart, linux-fbdev,
Xuetao Guan, Guenter Roeck
In-Reply-To: <20140707235829.281783607@linuxfoundation.org>
3.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: Zhichuang SUN <sunzc522@gmail.com>
commit fbc6c4a13bbfb420eedfdb26a0a859f9c07e8a7b upstream.
Function unifb_mmap calls functions which are defined in linux/mm.h
and asm/pgtable.h
The related error (for unicore32 with unicore32_defconfig):
CC drivers/video/fbdev/fb-puv3.o
drivers/video/fbdev/fb-puv3.c: In function 'unifb_mmap':
drivers/video/fbdev/fb-puv3.c:646: error: implicit declaration of
function 'vm_iomap_memory'
drivers/video/fbdev/fb-puv3.c:646: error: implicit declaration of
function 'pgprot_noncached'
Signed-off-by: Zhichuang Sun <sunzc522@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joe Perches <joe@perches.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-fbdev@vger.kernel.org
Acked-by: Xuetao Guan <gxt@mprc.pku.edu.cn>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/video/fb-puv3.c | 2 ++
1 file changed, 2 insertions(+)
--- a/drivers/video/fb-puv3.c
+++ b/drivers/video/fb-puv3.c
@@ -18,8 +18,10 @@
#include <linux/fb.h>
#include <linux/init.h>
#include <linux/console.h>
+#include <linux/mm.h>
#include <asm/sizes.h>
+#include <asm/pgtable.h>
#include <mach/hardware.h>
/* Platform_data reserved for unifb registers. */
^ permalink raw reply
* [PATCH 3.14 91/94] drivers/video/fbdev/fb-puv3.c: Add header files for function unifb_mmap
From: Greg Kroah-Hartman @ 2014-07-07 23:58 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Zhichuang Sun,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
Daniel Vetter, Joe Perches, Laurent Pinchart, linux-fbdev,
Xuetao Guan, Guenter Roeck
In-Reply-To: <20140707235756.780319003@linuxfoundation.org>
3.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Zhichuang SUN <sunzc522@gmail.com>
commit fbc6c4a13bbfb420eedfdb26a0a859f9c07e8a7b upstream.
Function unifb_mmap calls functions which are defined in linux/mm.h
and asm/pgtable.h
The related error (for unicore32 with unicore32_defconfig):
CC drivers/video/fbdev/fb-puv3.o
drivers/video/fbdev/fb-puv3.c: In function 'unifb_mmap':
drivers/video/fbdev/fb-puv3.c:646: error: implicit declaration of
function 'vm_iomap_memory'
drivers/video/fbdev/fb-puv3.c:646: error: implicit declaration of
function 'pgprot_noncached'
Signed-off-by: Zhichuang Sun <sunzc522@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joe Perches <joe@perches.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-fbdev@vger.kernel.org
Acked-by: Xuetao Guan <gxt@mprc.pku.edu.cn>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/video/fb-puv3.c | 2 ++
1 file changed, 2 insertions(+)
--- a/drivers/video/fb-puv3.c
+++ b/drivers/video/fb-puv3.c
@@ -18,8 +18,10 @@
#include <linux/fb.h>
#include <linux/init.h>
#include <linux/console.h>
+#include <linux/mm.h>
#include <asm/sizes.h>
+#include <asm/pgtable.h>
#include <mach/hardware.h>
/* Platform_data reserved for unifb registers. */
^ permalink raw reply
* [PATCH 3.15 121/122] drivers/video/fbdev/fb-puv3.c: Add header files for function unifb_mmap
From: Greg Kroah-Hartman @ 2014-07-07 23:58 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Zhichuang Sun,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
Daniel Vetter, Joe Perches, Laurent Pinchart, linux-fbdev,
Xuetao Guan, Guenter Roeck
In-Reply-To: <20140707235734.234226883@linuxfoundation.org>
3.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Zhichuang SUN <sunzc522@gmail.com>
commit fbc6c4a13bbfb420eedfdb26a0a859f9c07e8a7b upstream.
Function unifb_mmap calls functions which are defined in linux/mm.h
and asm/pgtable.h
The related error (for unicore32 with unicore32_defconfig):
CC drivers/video/fbdev/fb-puv3.o
drivers/video/fbdev/fb-puv3.c: In function 'unifb_mmap':
drivers/video/fbdev/fb-puv3.c:646: error: implicit declaration of
function 'vm_iomap_memory'
drivers/video/fbdev/fb-puv3.c:646: error: implicit declaration of
function 'pgprot_noncached'
Signed-off-by: Zhichuang Sun <sunzc522@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joe Perches <joe@perches.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-fbdev@vger.kernel.org
Acked-by: Xuetao Guan <gxt@mprc.pku.edu.cn>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/video/fbdev/fb-puv3.c | 2 ++
1 file changed, 2 insertions(+)
--- a/drivers/video/fbdev/fb-puv3.c
+++ b/drivers/video/fbdev/fb-puv3.c
@@ -18,8 +18,10 @@
#include <linux/fb.h>
#include <linux/init.h>
#include <linux/console.h>
+#include <linux/mm.h>
#include <asm/sizes.h>
+#include <asm/pgtable.h>
#include <mach/hardware.h>
/* Platform_data reserved for unifb registers. */
^ permalink raw reply
* [PATCH v2] video: fbdev: sis: init.c: Cleaning up redundant condition is always true
From: Rickard Strandqvist @ 2014-07-07 20:56 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Rickard Strandqvist, Jingoo Han, Daniel Vetter,
Mauro Carvalho Chehab, Chen, Gong, Dan Carpenter, linux-fbdev,
linux-kernel
In-Reply-To: <1404764628-25192-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
Find a redundant condition that is always true, that after
some discussions prove to be that wrong variable that was checked.
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
drivers/video/fbdev/sis/init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/sis/init.c b/drivers/video/fbdev/sis/init.c
index bd40f5e..dc9613a 100644
--- a/drivers/video/fbdev/sis/init.c
+++ b/drivers/video/fbdev/sis/init.c
@@ -355,12 +355,12 @@ SiS_GetModeID(int VGAEngine, unsigned int VBFlags, int HDisplay, int VDisplay,
}
break;
case 400:
- if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth >= 800) && (LCDwidth >= 600))) {
+ if ((!(VBFlags & CRT1_LCDA)) || ((LCDwidth >= 800) && (LCDheight >= 600))) {
if(VDisplay = 300) ModeIndex = ModeIndex_400x300[Depth];
}
break;
case 512:
- if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth >= 1024) && (LCDwidth >= 768))) {
+ if ((!(VBFlags & CRT1_LCDA)) || ((LCDwidth >= 1024) && (LCDheight >= 768))) {
if(VDisplay = 384) ModeIndex = ModeIndex_512x384[Depth];
}
break;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] video: fbdev: core: fbmon.c: Cleaning up useless assignment of function parameter
From: Rickard Strandqvist @ 2014-07-07 20:52 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
Daniel Vetter, Laurent Pinchart, Rob Clark, David Ung,
Linux Fbdev development list, linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdV++wyFLbqmnqEBTrWdgTNKpy=r9-mYFhQGCj7kUUUhKQ@mail.gmail.com>
2014-07-07 22:43 GMT+02:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> On Sun, Jul 6, 2014 at 7:25 PM, Rickard Strandqvist
> <rickard_strandqvist@spectrumdigital.se> wrote:
>> Remove assignment of function parameter, that has no effect outside the function
>>
>> This was found using a static code analysis program called cppcheck
>>
>> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
>> ---
>> drivers/video/fbdev/core/fbmon.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
>> index 5b0e313..71ec1ab 100644
>> --- a/drivers/video/fbdev/core/fbmon.c
>> +++ b/drivers/video/fbdev/core/fbmon.c
>> @@ -1481,7 +1481,6 @@ int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
>> }
>> void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
>> {
>> - specs = NULL;
>
> I guess the intention was:
>
> memset(specs, 0, sizeof(struct fb_monspecs));
Hi Geert!
Yeah, maybe. Just looks to be a little too elementary mistake..?
But do you want me to change it to that?
With a:
if(specs)
memset()
maybe..?
Kind regards
Rickard Strandqvist
^ permalink raw reply
* Re: [PATCH] video: fbdev: core: fbmon.c: Cleaning up useless assignment of function parameter
From: Geert Uytterhoeven @ 2014-07-07 20:43 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
Daniel Vetter, Laurent Pinchart, Rob Clark, David Ung,
Linux Fbdev development list, linux-kernel@vger.kernel.org
In-Reply-To: <1404667555-14675-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
On Sun, Jul 6, 2014 at 7:25 PM, Rickard Strandqvist
<rickard_strandqvist@spectrumdigital.se> wrote:
> Remove assignment of function parameter, that has no effect outside the function
>
> This was found using a static code analysis program called cppcheck
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/video/fbdev/core/fbmon.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
> index 5b0e313..71ec1ab 100644
> --- a/drivers/video/fbdev/core/fbmon.c
> +++ b/drivers/video/fbdev/core/fbmon.c
> @@ -1481,7 +1481,6 @@ int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
> }
> void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
> {
> - specs = NULL;
I guess the intention was:
memset(specs, 0, sizeof(struct fb_monspecs));
> }
> void fb_edid_add_monspecs(unsigned char *edid, struct fb_monspecs *specs)
> {
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] video: fbdev: sis: init.c: Cleaning up redundant condition is always true
From: Rickard Strandqvist @ 2014-07-07 20:36 UTC (permalink / raw)
To: Noralf Tronnes
Cc: Dan Carpenter, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Jingoo Han, Daniel Vetter, Mauro Carvalho Chehab, Chen, Gong,
Linux Fbdev development list, linux-kernel@vger.kernel.org
In-Reply-To: <53BA66F6.9020605@tronnes.org>
2014-07-07 11:23 GMT+02:00 Noralf Tronnes <notro@tronnes.org>:
> Den 07.07.2014 09:57, skrev Dan Carpenter:
>
>> On Thu, Jul 03, 2014 at 11:15:21PM +0200, Rickard Strandqvist wrote:
>>>
>>> diff --git a/drivers/video/fbdev/sis/init.c
>>> b/drivers/video/fbdev/sis/init.c
>>> index bd40f5e..9e2dd96 100644
>>> --- a/drivers/video/fbdev/sis/init.c
>>> +++ b/drivers/video/fbdev/sis/init.c
>>> @@ -355,12 +355,12 @@ SiS_GetModeID(int VGAEngine, unsigned int VBFlags,
>>> int HDisplay, int VDisplay,
>>> }
>>> break;
>>> case 400:
>>> - if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth >= 800) &&
>>> (LCDwidth >= 600))) {
>>> + if ((!(VBFlags & CRT1_LCDA)) || (LCDwidth >= 600)) {
>>
>>
>> It might be that this was supposed to be:
>>
>> if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth <= 800) &&
>> (LCDwidth >= 600))) {
>>
>> But why would people write a range from high to low? That's crazy
>> people who write backwards code...
>
> The numbers 800x600 (1024x768) indicate that LCDheight is meant to be used:
> if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth >= 800) &&
> (LCDheight >= 600))) {
>
> regards,
> Noralf Tronnes
Hi
Nice, this is what is the point of fix this type of minor errors.
Sometimes you can find something more serious :)
New patch on the way!
Kind regards
Rickard Strandqvist
^ permalink raw reply
* [PATCH v2] video: fbdev: sis: init.c: Cleaning up variable that is never used
From: Rickard Strandqvist @ 2014-07-07 20:23 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: Rickard Strandqvist, Jingoo Han, Daniel Vetter,
Mauro Carvalho Chehab, Chen, Gong, Dan Carpenter, linux-fbdev,
linux-kernel
Variable ar assigned a value that is never used.
I have also removed all the code that thereby serves no purpose.
This was found using a static code analysis program called cppcheck
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
drivers/video/fbdev/sis/init.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/video/fbdev/sis/init.c b/drivers/video/fbdev/sis/init.c
index bd40f5e..1948b623 100644
--- a/drivers/video/fbdev/sis/init.c
+++ b/drivers/video/fbdev/sis/init.c
@@ -2649,7 +2649,7 @@ static void
SiS_SetCRT1ModeRegs(struct SiS_Private *SiS_Pr, unsigned short ModeNo,
unsigned short ModeIdIndex, unsigned short RRTI)
{
- unsigned short data, infoflag = 0, modeflag, resindex;
+ unsigned short data, infoflag = 0, modeflag;
#ifdef CONFIG_FB_SIS_315
unsigned char *ROMAddr = SiS_Pr->VirtualRomBase;
unsigned short data2, data3;
@@ -2659,11 +2659,8 @@ SiS_SetCRT1ModeRegs(struct SiS_Private *SiS_Pr, unsigned short ModeNo,
if(SiS_Pr->UseCustomMode) {
infoflag = SiS_Pr->CInfoFlag;
- } else {
- resindex = SiS_GetResInfo(SiS_Pr, ModeNo, ModeIdIndex);
- if(ModeNo > 0x13) {
- infoflag = SiS_Pr->SiS_RefIndex[RRTI].Ext_InfoFlag;
- }
+ } else if (ModeNo > 0x13) {
+ infoflag = SiS_Pr->SiS_RefIndex[RRTI].Ext_InfoFlag;
}
/* Disable DPMS */
@@ -3539,16 +3536,15 @@ SiS_Generic_ConvertCRData(struct SiS_Private *SiS_Pr, unsigned char *crdata,
struct fb_var_screeninfo *var, bool writeres
)
{
- unsigned short HRE, HBE, HRS, HBS, HDE, HT;
- unsigned short VRE, VBE, VRS, VBS, VDE, VT;
+ unsigned short HRE, HBE, HRS, HDE, HT;
+ unsigned short VRE, VBE, VRS, VDE, VT;
unsigned char sr_data, cr_data, cr_data2;
- int A, B, C, D, E, F, temp;
+ int B, C, D, E, F, temp;
sr_data = crdata[14];
/* Horizontal total */
HT = crdata[0] | ((unsigned short)(sr_data & 0x03) << 8);
- A = HT + 5;
/* Horizontal display enable end */
HDE = crdata[1] | ((unsigned short)(sr_data & 0x0C) << 6);
@@ -3558,9 +3554,6 @@ SiS_Generic_ConvertCRData(struct SiS_Private *SiS_Pr, unsigned char *crdata,
HRS = crdata[4] | ((unsigned short)(sr_data & 0xC0) << 2);
F = HRS - E - 3;
- /* Horizontal blank start */
- HBS = crdata[2] | ((unsigned short)(sr_data & 0x30) << 4);
-
sr_data = crdata[15];
cr_data = crdata[5];
@@ -3594,7 +3587,6 @@ SiS_Generic_ConvertCRData(struct SiS_Private *SiS_Pr, unsigned char *crdata,
((unsigned short)(cr_data & 0x01) << 8) |
((unsigned short)(cr_data & 0x20) << 4) |
((unsigned short)(sr_data & 0x01) << 10);
- A = VT + 2;
/* Vertical display enable end */
VDE = crdata[10] |
@@ -3612,12 +3604,6 @@ SiS_Generic_ConvertCRData(struct SiS_Private *SiS_Pr, unsigned char *crdata,
cr_data2 = (crdata[16] & 0x01) << 5;
- /* Vertical blank start */
- VBS = crdata[11] |
- ((unsigned short)(cr_data & 0x08) << 5) |
- ((unsigned short)(cr_data2 & 0x20) << 4) |
- ((unsigned short)(sr_data & 0x04) << 8);
-
/* Vertical blank end */
VBE = crdata[12] | ((unsigned short)(sr_data & 0x10) << 4);
temp = VBE - ((E - 1) & 511);
--
1.7.10.4
^ permalink raw reply related
* [PATCH] video: backlight: jornada720_lcd.c: Cleaning up variable that is never used
From: Rickard Strandqvist @ 2014-07-07 20:01 UTC (permalink / raw)
To: Jingoo Han, Bryan Wu
Cc: Rickard Strandqvist, Lee Jones, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1404562325-18471-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
Variable ar assigned a value that is never used.
I have also removed all the code that thereby serves no purpose,
and made same change to clarify the similarities in function.
This was found using a static code analysis program called cppcheck
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
drivers/video/backlight/jornada720_lcd.c | 31 ++++++++++++++----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/drivers/video/backlight/jornada720_lcd.c b/drivers/video/backlight/jornada720_lcd.c
index da3876c..d16abcf 100644
--- a/drivers/video/backlight/jornada720_lcd.c
+++ b/drivers/video/backlight/jornada720_lcd.c
@@ -36,44 +36,41 @@ static int jornada_lcd_get_power(struct lcd_device *ld)
static int jornada_lcd_get_contrast(struct lcd_device *ld)
{
- int ret;
+ int ret = -ETIMEDOUT;
if (jornada_lcd_get_power(ld) != FB_BLANK_UNBLANK)
return 0;
jornada_ssp_start();
- if (jornada_ssp_byte(GETCONTRAST) != TXDUMMY) {
+ if (jornada_ssp_byte(GETCONTRAST) != TXDUMMY)
dev_err(&ld->dev, "get contrast failed\n");
- jornada_ssp_end();
- return -ETIMEDOUT;
- } else {
+ else
ret = jornada_ssp_byte(TXDUMMY);
- jornada_ssp_end();
- return ret;
- }
+
+ jornada_ssp_end();
+
+ return ret;
}
static int jornada_lcd_set_contrast(struct lcd_device *ld, int value)
{
- int ret;
+ int ret = -ETIMEDOUT;
jornada_ssp_start();
/* start by sending our set contrast cmd to mcu */
- ret = jornada_ssp_byte(SETCONTRAST);
-
+ if (jornada_ssp_byte(SETCONTRAST) != TXDUMMY)
+ dev_err(&ld->dev, "set contrast failed\n");
/* push the new value */
- if (jornada_ssp_byte(value) != TXDUMMY) {
+ else if (jornada_ssp_byte(value) != TXDUMMY)
dev_err(&ld->dev, "set contrast failed\n");
- jornada_ssp_end();
- return -ETIMEDOUT;
- }
+ else /* if we get here we can assume everything went well */
+ ret = 0;
- /* if we get here we can assume everything went well */
jornada_ssp_end();
- return 0;
+ return ret;
}
static int jornada_lcd_set_power(struct lcd_device *ld, int power)
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] video: backlight: jornada720_lcd.c: Cleaning up variable that is never used
From: Rickard Strandqvist @ 2014-07-07 20:00 UTC (permalink / raw)
To: Lee Jones
Cc: Jingoo Han, Bryan Wu, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Linux Fbdev development list,
linux-kernel@vger.kernel.org
In-Reply-To: <20140707104635.GA6407@lee--X1>
2014-07-07 12:46 GMT+02:00 Lee Jones <lee.jones@linaro.org>:
>> Variable ar assigned a value that is never used.
>> I have also removed all the code that thereby serves no purpose.
>>
>> This was found using a static code analysis program called cppcheck
>>
>> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
>> ---
>> drivers/video/backlight/jornada720_lcd.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> Solve it the other way around.
>
> Add a check and return the value if an error is returned.
Hi
Ok, I take the opportunity to make a bigger change to clarify the
similarities in function.
Hope you like it, otherwise I can make the minor change instead.
Kind regards
Rickard Strandqvist
^ permalink raw reply
* Re: [PATCH] video: fbdev: aty: atyfb_base.c: Cleaning up variable that is never used
From: Mikulas Patocka @ 2014-07-07 14:04 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
Daniel Vetter, Randy Dunlap, Dan Carpenter, Sachin Kamat,
linux-fbdev, linux-kernel
In-Reply-To: <1404671093-17590-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
On Sun, 6 Jul 2014, Rickard Strandqvist wrote:
> Variable ar assigned a value that is never used.
> I have also removed all the code that thereby serves no purpose.
>
> This was found using a static code analysis program called cppcheck
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Acked-by: Mikulas Patocka <mpatocka@redhat.com>
> ---
> drivers/video/fbdev/aty/atyfb_base.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
> index c3d0074..46188a4 100644
> --- a/drivers/video/fbdev/aty/atyfb_base.c
> +++ b/drivers/video/fbdev/aty/atyfb_base.c
> @@ -3088,7 +3088,6 @@ static int atyfb_setup_sparc(struct pci_dev *pdev, struct fb_info *info,
> if (dp = of_console_device) {
> struct fb_var_screeninfo *var = &default_var;
> unsigned int N, P, Q, M, T, R;
> - u32 v_total, h_total;
> struct crtc crtc;
> u8 pll_regs[16];
> u8 clock_cntl;
> @@ -3104,9 +3103,6 @@ static int atyfb_setup_sparc(struct pci_dev *pdev, struct fb_info *info,
> crtc.gen_cntl = aty_ld_le32(CRTC_GEN_CNTL, par);
> aty_crtc_to_var(&crtc, var);
>
> - h_total = var->xres + var->right_margin + var->hsync_len + var->left_margin;
> - v_total = var->yres + var->lower_margin + var->vsync_len + var->upper_margin;
> -
> /*
> * Read the PLL to figure actual Refresh Rate.
> */
> --
> 1.7.10.4
>
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 5/5] drm/i915: Kick out vga console
From: Daniel Vetter @ 2014-07-07 12:26 UTC (permalink / raw)
To: Ed Tomlinson
Cc: linux-fbdev, Intel Graphics Development, LKML, DRI Development,
Tomi Valkeinen, Jean-Christophe Plagniol-Villard
In-Reply-To: <1974170.pJAPFfvtjC@grover>
On Mon, Jul 07, 2014 at 06:45:49AM -0400, Ed Tomlinson wrote:
> Daniel,
>
> I am not quite sure I understand what you want me to test?
> Do you want me to try it without:
>
> > > + if (ret = 0) {
> > > + ret = do_unregister_con_driver(&vga_con);
Below the diff of what I mean.
-Daniel
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5e583a1838f8..bd8517151479 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1466,12 +1466,13 @@ static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
#else
static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
{
- int ret;
+ int ret = 0;
DRM_INFO("Replacing VGA console driver\n");
console_lock();
- ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
+ if (con_is_bound(&vga_con))
+ ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
if (ret = 0) {
ret = do_unregister_con_driver(&vga_con);
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply related
* Re: [PATCH 01/12] ARM: OMAP2+: Remove non working OMAP HDMI audio initialization
From: Tony Lindgren @ 2014-07-07 11:46 UTC (permalink / raw)
To: Jyri Sarha
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi, broonie,
liam.r.girdwood, tomi.valkeinen, detheridge, jyri.sarha
In-Reply-To: <b0764a3af8181db55db046d6139119def363e8eb.1403807781.git.jsarha@ti.com>
* Jyri Sarha <jsarha@ti.com> [140626 12:26]:
> This code is not working currently and it can be removed. There is a
> conflict in sharing resources with the actual HDMI driver and with
> the ASoC HDMI audio DAI driver.
Is this OK to queue for v3.17 or do we need this for v3.16-rc series?
Regards,
Tony
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
> arch/arm/mach-omap2/devices.c | 28 ----------------------------
> 1 file changed, 28 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 592ba0a..b6f8f34 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -297,33 +297,6 @@ static void omap_init_audio(void)
> static inline void omap_init_audio(void) {}
> #endif
>
> -#if defined(CONFIG_SND_OMAP_SOC_OMAP_HDMI) || \
> - defined(CONFIG_SND_OMAP_SOC_OMAP_HDMI_MODULE)
> -
> -static struct platform_device omap_hdmi_audio = {
> - .name = "omap-hdmi-audio",
> - .id = -1,
> -};
> -
> -static void __init omap_init_hdmi_audio(void)
> -{
> - struct omap_hwmod *oh;
> - struct platform_device *pdev;
> -
> - oh = omap_hwmod_lookup("dss_hdmi");
> - if (!oh)
> - return;
> -
> - pdev = omap_device_build("omap-hdmi-audio-dai", -1, oh, NULL, 0);
> - WARN(IS_ERR(pdev),
> - "Can't build omap_device for omap-hdmi-audio-dai.\n");
> -
> - platform_device_register(&omap_hdmi_audio);
> -}
> -#else
> -static inline void omap_init_hdmi_audio(void) {}
> -#endif
> -
> #if defined(CONFIG_SPI_OMAP24XX) || defined(CONFIG_SPI_OMAP24XX_MODULE)
>
> #include <linux/platform_data/spi-omap2-mcspi.h>
> @@ -459,7 +432,6 @@ static int __init omap2_init_devices(void)
> */
> omap_init_audio();
> omap_init_camera();
> - omap_init_hdmi_audio();
> omap_init_mbox();
> /* If dtb is there, the devices will be created dynamically */
> if (!of_have_populated_dt()) {
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 5/5] drm/i915: Kick out vga console
From: Ed Tomlinson @ 2014-07-07 10:59 UTC (permalink / raw)
To: Chris Wilson, Intel Graphics Development, DRI Development, LKML,
David Herrmann, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
linux-fbdev, Jani Nikula, Dave Airlie
In-Reply-To: <20140707084826.GH5821@phenom.ffwll.local>
Daniel,
Just to be sure. The intel card here should not be claiming the real console. It does
not have an output device and the bios set set so the radeon is the primary device.
Ed
On Monday 07 July 2014 10:48:26 Daniel Vetter wrote:
> On Mon, Jun 30, 2014 at 07:59:55AM +0100, Chris Wilson wrote:
> > On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
> > > On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
> > >
> > > Resend without html krud which causes list to bounce the message.
> > >
> > > > Hi
> > > >
> > > > This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot with 3.16-git. Reverting it lets the boot proceed.
> > > >
> > > > I have an i7 with a built-in i915 and an pcie r7 260x. The R7 is the primary console. The i915 is initialized
> > > > but does not have a physical display attached.
> > > >
> > > > With the patch applied the boot stops at the messages:
> > > >
> > > > [drm] Memory usable by graphics device = 2048M
> > > > [drm] Replacing VGA console driver
> >
> > The issue looks like that we are ripping out the radeon fb_con whilst it
> > is active and that upsets everyone. In which case, I think the
> > compromise is:
> >
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 5f44581..4915f1d 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> > #else
> > static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> > {
> > - int ret;
> > + int ret = 0;
> >
> > DRM_INFO("Replacing VGA console driver\n");
> >
> > console_lock();
> > - ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> > - if (ret = 0) {
> > - ret = do_unregister_con_driver(&vga_con);
> > -
> > - /* Ignore "already unregistered". */
> > - if (ret = -ENODEV)
> > - ret = 0;
> > + if (con_is_bound(&vga_con)) {
> > + ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> > + if (ret = 0) {
> > + ret = do_unregister_con_driver(&vga_con);
>
> Hm, we should only conditionalize the take_over_console - unregistering
> vga_con is kinda the point to make sure it's gone for real. Ed, can you
> please retest with the if (con_is_bound) check just for the
> do_take_over_console call?
>
> Still puzzled wtf is going on here since as David says this should be a
> no-op.
>
> Thanks, Daniel
> > +
> > + /* Ignore "already unregistered". */
> > + if (ret = -ENODEV)
> > + ret = 0;
> > + }
> > }
> > console_unlock();
> >
> > -Chris
> >
>
>
^ permalink raw reply
* Re: [PATCH] video: backlight: ili922x.c: Cleaning up variable that is never used
From: Lee Jones @ 2014-07-07 10:50 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Jingoo Han, Bryan Wu, Rickard Strandqvist,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel
In-Reply-To: <1404562270-18387-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
On Sat, 05 Jul 2014, Rickard Strandqvist wrote:
> From: Rickard Strandqvist <rickard.strandqvist@sonymobile.com>
>
> Variable ar assigned a value that is never used.
> I have also removed all the code that thereby serves no purpose.
>
> This was found using a static code analysis program called cppcheck
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/video/backlight/ili922x.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Is this different from the patch I just applied?
> diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c
> index ea67fe1..06936c8 100644
> --- a/drivers/video/backlight/ili922x.c
> +++ b/drivers/video/backlight/ili922x.c
> @@ -251,7 +251,7 @@ static int ili922x_write(struct spi_device *spi, u8 reg, u16 value)
> struct spi_transfer xfer_regindex, xfer_regvalue;
> unsigned char tbuf[CMD_BUFSIZE];
> unsigned char rbuf[CMD_BUFSIZE];
> - int ret, len = 0;
> + int ret;
>
> memset(&xfer_regindex, 0, sizeof(struct spi_transfer));
> memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
> @@ -273,7 +273,6 @@ static int ili922x_write(struct spi_device *spi, u8 reg, u16 value)
> ret = spi_sync(spi, &msg);
>
> spi_message_init(&msg);
> - len = 0;
> tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
> START_RW_WRITE));
> tbuf[1] = set_tx_byte((value & 0xFF00) >> 8);
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] video: backlight: jornada720_lcd.c: Cleaning up variable that is never used
From: Lee Jones @ 2014-07-07 10:50 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Jingoo Han, Bryan Wu, Rickard Strandqvist,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel
In-Reply-To: <1404562325-18471-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
On Sat, 05 Jul 2014, Rickard Strandqvist wrote:
> From: Rickard Strandqvist <rickard.strandqvist@sonymobile.com>
>
> Variable ar assigned a value that is never used.
> I have also removed all the code that thereby serves no purpose.
>
> This was found using a static code analysis program called cppcheck
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/video/backlight/jornada720_lcd.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Is this different from the patch I just NACKed?
> diff --git a/drivers/video/backlight/jornada720_lcd.c b/drivers/video/backlight/jornada720_lcd.c
> index da3876c..b304319 100644
> --- a/drivers/video/backlight/jornada720_lcd.c
> +++ b/drivers/video/backlight/jornada720_lcd.c
> @@ -56,12 +56,10 @@ static int jornada_lcd_get_contrast(struct lcd_device *ld)
>
> static int jornada_lcd_set_contrast(struct lcd_device *ld, int value)
> {
> - int ret;
> -
> jornada_ssp_start();
>
> /* start by sending our set contrast cmd to mcu */
> - ret = jornada_ssp_byte(SETCONTRAST);
> + jornada_ssp_byte(SETCONTRAST);
>
> /* push the new value */
> if (jornada_ssp_byte(value) != TXDUMMY) {
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] video: backlight: ili922x.c: Cleaning up variable that is never used
From: Lee Jones @ 2014-07-07 10:48 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Jingoo Han, Bryan Wu, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1404670847-17249-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
On Sun, 06 Jul 2014, Rickard Strandqvist wrote:
> Variable ar assigned a value that is never used.
> I have also removed all the code that thereby serves no purpose.
>
> This was found using a static code analysis program called cppcheck
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/video/backlight/ili922x.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Applied, thanks.
> diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c
> index ea67fe1..06936c8 100644
> --- a/drivers/video/backlight/ili922x.c
> +++ b/drivers/video/backlight/ili922x.c
> @@ -251,7 +251,7 @@ static int ili922x_write(struct spi_device *spi, u8 reg, u16 value)
> struct spi_transfer xfer_regindex, xfer_regvalue;
> unsigned char tbuf[CMD_BUFSIZE];
> unsigned char rbuf[CMD_BUFSIZE];
> - int ret, len = 0;
> + int ret;
>
> memset(&xfer_regindex, 0, sizeof(struct spi_transfer));
> memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
> @@ -273,7 +273,6 @@ static int ili922x_write(struct spi_device *spi, u8 reg, u16 value)
> ret = spi_sync(spi, &msg);
>
> spi_message_init(&msg);
> - len = 0;
> tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
> START_RW_WRITE));
> tbuf[1] = set_tx_byte((value & 0xFF00) >> 8);
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] video: backlight: jornada720_lcd.c: Cleaning up variable that is never used
From: Lee Jones @ 2014-07-07 10:46 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Jingoo Han, Bryan Wu, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1404670906-17335-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
> Variable ar assigned a value that is never used.
> I have also removed all the code that thereby serves no purpose.
>
> This was found using a static code analysis program called cppcheck
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/video/backlight/jornada720_lcd.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Solve it the other way around.
Add a check and return the value if an error is returned.
> diff --git a/drivers/video/backlight/jornada720_lcd.c b/drivers/video/backlight/jornada720_lcd.c
> index da3876c..b304319 100644
> --- a/drivers/video/backlight/jornada720_lcd.c
> +++ b/drivers/video/backlight/jornada720_lcd.c
> @@ -56,12 +56,10 @@ static int jornada_lcd_get_contrast(struct lcd_device *ld)
>
> static int jornada_lcd_set_contrast(struct lcd_device *ld, int value)
> {
> - int ret;
> -
> jornada_ssp_start();
>
> /* start by sending our set contrast cmd to mcu */
> - ret = jornada_ssp_byte(SETCONTRAST);
> + jornada_ssp_byte(SETCONTRAST);
>
> /* push the new value */
> if (jornada_ssp_byte(value) != TXDUMMY) {
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 5/5] drm/i915: Kick out vga console
From: Ed Tomlinson @ 2014-07-07 10:45 UTC (permalink / raw)
To: Chris Wilson, Intel Graphics Development, DRI Development, LKML,
David Herrmann, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
linux-fbdev, Jani Nikula, Dave Airlie
In-Reply-To: <20140707084826.GH5821@phenom.ffwll.local>
Daniel,
I am not quite sure I understand what you want me to test?
Do you want me to try it without:
> > + if (ret = 0) {
> > + ret = do_unregister_con_driver(&vga_con);
Thanks
Ed
On Monday 07 July 2014 10:48:26 Daniel Vetter wrote:
> On Mon, Jun 30, 2014 at 07:59:55AM +0100, Chris Wilson wrote:
> > On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
> > > On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
> > >
> > > Resend without html krud which causes list to bounce the message.
> > >
> > > > Hi
> > > >
> > > > This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot with 3.16-git. Reverting it lets the boot proceed.
> > > >
> > > > I have an i7 with a built-in i915 and an pcie r7 260x. The R7 is the primary console. The i915 is initialized
> > > > but does not have a physical display attached.
> > > >
> > > > With the patch applied the boot stops at the messages:
> > > >
> > > > [drm] Memory usable by graphics device = 2048M
> > > > [drm] Replacing VGA console driver
> >
> > The issue looks like that we are ripping out the radeon fb_con whilst it
> > is active and that upsets everyone. In which case, I think the
> > compromise is:
> >
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 5f44581..4915f1d 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> > #else
> > static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> > {
> > - int ret;
> > + int ret = 0;
> >
> > DRM_INFO("Replacing VGA console driver\n");
> >
> > console_lock();
> > - ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> > - if (ret = 0) {
> > - ret = do_unregister_con_driver(&vga_con);
> > -
> > - /* Ignore "already unregistered". */
> > - if (ret = -ENODEV)
> > - ret = 0;
> > + if (con_is_bound(&vga_con)) {
> > + ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> > + if (ret = 0) {
> > + ret = do_unregister_con_driver(&vga_con);
>
> Hm, we should only conditionalize the take_over_console - unregistering
> vga_con is kinda the point to make sure it's gone for real. Ed, can you
> please retest with the if (con_is_bound) check just for the
> do_take_over_console call?
>
> Still puzzled wtf is going on here since as David says this should be a
> no-op.
>
> Thanks, Daniel
> > +
> > + /* Ignore "already unregistered". */
> > + if (ret = -ENODEV)
> > + ret = 0;
> > + }
> > }
> > console_unlock();
> >
> > -Chris
> >
>
>
^ permalink raw reply
* Re: [PATCH] video: fbdev: xen-fbfront.c: Cleaning up useless assignment of function parameter
From: Stefano Stabellini @ 2014-07-07 10:20 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Stefano Stabellini, Konrad Rzeszutek Wilk, Jingoo Han,
Daniel Vetter, linux-fbdev, linux-kernel
In-Reply-To: <1404667640-14855-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
On Sun, 6 Jul 2014, Rickard Strandqvist wrote:
> Remove assignment of function parameter, that has no effect outside the function
>
> This was found using a static code analysis program called cppcheck
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> drivers/video/fbdev/xen-fbfront.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
> index 901014b..e1207a1 100644
> --- a/drivers/video/fbdev/xen-fbfront.c
> +++ b/drivers/video/fbdev/xen-fbfront.c
> @@ -222,7 +222,6 @@ static int xenfb_setcolreg(unsigned regno, unsigned red, unsigned green,
> red = CNVT_TOHW(red, info->var.red.length);
> green = CNVT_TOHW(green, info->var.green.length);
> blue = CNVT_TOHW(blue, info->var.blue.length);
> - transp = CNVT_TOHW(transp, info->var.transp.length);
> #undef CNVT_TOHW
>
> v = (red << info->var.red.offset) |
> --
> 1.7.10.4
>
^ permalink raw reply
* Re: [PATCH] video: fbdev: sis: init.c: Cleaning up redundant condition is always true
From: Noralf Tronnes @ 2014-07-07 9:23 UTC (permalink / raw)
To: Dan Carpenter, Rickard Strandqvist
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
Daniel Vetter, Mauro Carvalho Chehab, Chen, Gong, linux-fbdev,
linux-kernel
In-Reply-To: <20140707075709.GI25934@mwanda>
Den 07.07.2014 09:57, skrev Dan Carpenter:
> On Thu, Jul 03, 2014 at 11:15:21PM +0200, Rickard Strandqvist wrote:
>> diff --git a/drivers/video/fbdev/sis/init.c b/drivers/video/fbdev/sis/init.c
>> index bd40f5e..9e2dd96 100644
>> --- a/drivers/video/fbdev/sis/init.c
>> +++ b/drivers/video/fbdev/sis/init.c
>> @@ -355,12 +355,12 @@ SiS_GetModeID(int VGAEngine, unsigned int VBFlags, int HDisplay, int VDisplay,
>> }
>> break;
>> case 400:
>> - if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth >= 800) && (LCDwidth >= 600))) {
>> + if ((!(VBFlags & CRT1_LCDA)) || (LCDwidth >= 600)) {
>
> It might be that this was supposed to be:
>
> if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth <= 800) && (LCDwidth >= 600))) {
>
> But why would people write a range from high to low? That's crazy
> people who write backwards code...
The numbers 800x600 (1024x768) indicate that LCDheight is meant to be used:
if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth >= 800) &&
(LCDheight >= 600))) {
regards,
Noralf Tronnes
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 5/5] drm/i915: Kick out vga console
From: Daniel Vetter @ 2014-07-07 8:48 UTC (permalink / raw)
To: Chris Wilson, Ed Tomlinson, Daniel Vetter,
Intel Graphics Development, DRI Development, LKML, David Herrmann,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
Jani Nikula, Dave Airlie
In-Reply-To: <20140630065955.GD7687@nuc-i3427.alporthouse.com>
On Mon, Jun 30, 2014 at 07:59:55AM +0100, Chris Wilson wrote:
> On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
> > On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
> >
> > Resend without html krud which causes list to bounce the message.
> >
> > > Hi
> > >
> > > This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot with 3.16-git. Reverting it lets the boot proceed.
> > >
> > > I have an i7 with a built-in i915 and an pcie r7 260x. The R7 is the primary console. The i915 is initialized
> > > but does not have a physical display attached.
> > >
> > > With the patch applied the boot stops at the messages:
> > >
> > > [drm] Memory usable by graphics device = 2048M
> > > [drm] Replacing VGA console driver
>
> The issue looks like that we are ripping out the radeon fb_con whilst it
> is active and that upsets everyone. In which case, I think the
> compromise is:
>
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5f44581..4915f1d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> #else
> static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> {
> - int ret;
> + int ret = 0;
>
> DRM_INFO("Replacing VGA console driver\n");
>
> console_lock();
> - ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> - if (ret = 0) {
> - ret = do_unregister_con_driver(&vga_con);
> -
> - /* Ignore "already unregistered". */
> - if (ret = -ENODEV)
> - ret = 0;
> + if (con_is_bound(&vga_con)) {
> + ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> + if (ret = 0) {
> + ret = do_unregister_con_driver(&vga_con);
Hm, we should only conditionalize the take_over_console - unregistering
vga_con is kinda the point to make sure it's gone for real. Ed, can you
please retest with the if (con_is_bound) check just for the
do_take_over_console call?
Still puzzled wtf is going on here since as David says this should be a
no-op.
Thanks, Daniel
> +
> + /* Ignore "already unregistered". */
> + if (ret = -ENODEV)
> + ret = 0;
> + }
> }
> console_unlock();
>
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH] video: fbdev: sis: init.c: Cleaning up redundant condition is always true
From: Dan Carpenter @ 2014-07-07 7:57 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
Daniel Vetter, Mauro Carvalho Chehab, Chen, Gong, linux-fbdev,
linux-kernel
In-Reply-To: <1404422121-11298-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
On Thu, Jul 03, 2014 at 11:15:21PM +0200, Rickard Strandqvist wrote:
> diff --git a/drivers/video/fbdev/sis/init.c b/drivers/video/fbdev/sis/init.c
> index bd40f5e..9e2dd96 100644
> --- a/drivers/video/fbdev/sis/init.c
> +++ b/drivers/video/fbdev/sis/init.c
> @@ -355,12 +355,12 @@ SiS_GetModeID(int VGAEngine, unsigned int VBFlags, int HDisplay, int VDisplay,
> }
> break;
> case 400:
> - if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth >= 800) && (LCDwidth >= 600))) {
> + if ((!(VBFlags & CRT1_LCDA)) || (LCDwidth >= 600)) {
It might be that this was supposed to be:
if((!(VBFlags & CRT1_LCDA)) || ((LCDwidth <= 800) && (LCDwidth >= 600))) {
But why would people write a range from high to low? That's crazy
people who write backwards code...
The second condition was supposed to do *something* although it's
unclear what. I would normally say that we should leave the static
checker warning here instead of guessing. Perhaps add a comment like
/* fixme: huh? What's with this nonsense condition? */ But in this
case it's all dead code because LCDwidth is always zero and the code
hasn't changed since 2.6.12 so I don't care.
regards,
dan carpenter
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox