[PATCH 2/7] video: fbdev: intelfb: delete double assignment

[PATCH 2/7] video: fbdev: intelfb: delete double assignment

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete successive assignments to the same location.

In the second case, = is converted to |=, which looks appropriate based on
the values involved.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression i;
@@

*i = ...;
 i = ...;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
The patches in this series do not depend on each other.

This changes the behavior of the code in the second case and is not tested.

 drivers/video/fbdev/intelfb/intelfbhw.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/intelfb/intelfbhw.c b/drivers/video/fbdev/intelfb/intelfbhw.c
index fbad61d..d31ed4e 100644
--- a/drivers/video/fbdev/intelfb/intelfbhw.c
+++ b/drivers/video/fbdev/intelfb/intelfbhw.c
@@ -1191,7 +1191,6 @@ int intelfbhw_mode_to_hw(struct intelfb_info *dinfo,
 	vsync_end = vsync_start + var->vsync_len;
 	vtotal = vsync_end + var->upper_margin;
 	vblank_start = vactive;
-	vblank_end = vtotal;
 	vblank_end = vsync_end + 1;
 
 	DBG_MSG("V: act %d, ss %d, se %d, tot %d bs %d, be %d\n",
@@ -1859,7 +1858,7 @@ void intelfbhw_cursor_init(struct intelfb_info *dinfo)
 		tmp = INREG(CURSOR_CONTROL);
 		tmp &= ~(CURSOR_FORMAT_MASK | CURSOR_GAMMA_ENABLE |
 			 CURSOR_ENABLE | CURSOR_STRIDE_MASK);
-		tmp = CURSOR_FORMAT_3C;
+		tmp |= CURSOR_FORMAT_3C;
 		OUTREG(CURSOR_CONTROL, tmp);
 		OUTREG(CURSOR_A_BASEADDR, dinfo->cursor.offset << 12);
 		tmp = (64 << CURSOR_SIZE_H_SHIFT) |

[PATCH 6/7] video: fbdev: sis: delete double assignment

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

Delete successive assignments to the same location.  The second assignment
is changed to update a different field, as done in other nearby code.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression i;
@@

*i = ...;
 i = ...;
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
The patches in this series do not depend on each other.

This changes the semantics of the code and is not tested.

 drivers/video/fbdev/sis/init301.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
index a89e3ca..295e0de 100644
--- a/drivers/video/fbdev/sis/init301.c
+++ b/drivers/video/fbdev/sis/init301.c
@@ -1714,7 +1714,7 @@ SiS_GetLCDResInfo(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned sh
 			       SiS_Pr->PanelVCLKIdx315 = VCLK81_315; /* ? */
 			    } else {
 			       SiS_Pr->PanelHT   = 1688; SiS_Pr->PanelVT   =  802;
-			       SiS_Pr->PanelHRS  =   48; SiS_Pr->PanelHRS  =  112;
+			       SiS_Pr->PanelHRS  =   48; SiS_Pr->PanelHRE  =  112;
 			       SiS_Pr->PanelVRS  =    3; SiS_Pr->PanelVRE  =    6;
 			       SiS_Pr->PanelVCLKIdx300 = VCLK81_300;
 			       SiS_Pr->PanelVCLKIdx315 = VCLK81_315;

Re: [PATCH 6/7] video: fbdev: sis: delete double assignment

[Geert Uytterhoeven]

On Sat, Aug 23, 2014 at 8:33 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> ---
> The patches in this series do not depend on each other.
>
> This changes the semantics of the code and is not tested.

Hence I think you should change the subject of the patch, so it's obvious
some bug is fixed, e.g. "video: fbdev: sis: Fix double assignment".

>  drivers/video/fbdev/sis/init301.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
> index a89e3ca..295e0de 100644
> --- a/drivers/video/fbdev/sis/init301.c
> +++ b/drivers/video/fbdev/sis/init301.c
> @@ -1714,7 +1714,7 @@ SiS_GetLCDResInfo(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned sh
>                                SiS_Pr->PanelVCLKIdx315 = VCLK81_315; /* ? */
>                             } else {
>                                SiS_Pr->PanelHT   = 1688; SiS_Pr->PanelVT   =  802;
> -                              SiS_Pr->PanelHRS  =   48; SiS_Pr->PanelHRS  =  112;
> +                              SiS_Pr->PanelHRS  =   48; SiS_Pr->PanelHRE  =  112;

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

Re: [PATCH 6/7] video: fbdev: sis: delete double assignment

[Julia Lawall]

On Sun, 24 Aug 2014, Geert Uytterhoeven wrote:

> On Sat, Aug 23, 2014 at 8:33 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> > ---
> > The patches in this series do not depend on each other.
> >
> > This changes the semantics of the code and is not tested.
> 
> Hence I think you should change the subject of the patch, so it's obvious
> some bug is fixed, e.g. "video: fbdev: sis: Fix double assignment".

Do you want me to resend this patch with the new subject, or just keep it 
in mind for the future?

thanks,
julia

> 
> >  drivers/video/fbdev/sis/init301.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
> > index a89e3ca..295e0de 100644
> > --- a/drivers/video/fbdev/sis/init301.c
> > +++ b/drivers/video/fbdev/sis/init301.c
> > @@ -1714,7 +1714,7 @@ SiS_GetLCDResInfo(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned sh
> >                                SiS_Pr->PanelVCLKIdx315 = VCLK81_315; /* ? */
> >                             } else {
> >                                SiS_Pr->PanelHT   = 1688; SiS_Pr->PanelVT   =  802;
> > -                              SiS_Pr->PanelHRS  =   48; SiS_Pr->PanelHRS  =  112;
> > +                              SiS_Pr->PanelHRS  =   48; SiS_Pr->PanelHRE  =  112;
> 
> 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
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 6/7] video: fbdev: sis: delete double assignment

[Geert Uytterhoeven]

Hi Julia,

On Sun, Aug 24, 2014 at 8:40 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> On Sat, Aug 23, 2014 at 8:33 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
>> > ---
>> > The patches in this series do not depend on each other.
>> >
>> > This changes the semantics of the code and is not tested.
>>
>> Hence I think you should change the subject of the patch, so it's obvious
>> some bug is fixed, e.g. "video: fbdev: sis: Fix double assignment".
>
> Do you want me to resend this patch with the new subject, or just keep it
> in mind for the future?

That's up to the fbdev maintainer.

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

Re: [PATCH 6/7] video: fbdev: sis: delete double assignment

[Tomi Valkeinen]

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

On 24/08/14 21:21, Geert Uytterhoeven wrote:
> On Sat, Aug 23, 2014 at 8:33 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
>> ---
>> The patches in this series do not depend on each other.
>>
>> This changes the semantics of the code and is not tested.
> 
> Hence I think you should change the subject of the patch, so it's obvious
> some bug is fixed, e.g. "video: fbdev: sis: Fix double assignment".

Well, to me "fix double assignment" sounds the same as "delete double
assignment".

Looking at the code, I think what's done here is "fix 1280x768 panel
timings".

I can apply this and do the change to subject, if nobody complains.

 Tomi



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

Re: [PATCH 2/7] video: fbdev: intelfb: delete double assignment

[Tomi Valkeinen]

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

On 23/08/14 21:33, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive assignments to the same location.
> 
> In the second case, = is converted to |=, which looks appropriate based on
> the values involved.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression i;
> @@
> 
> *i = ...;
>  i = ...;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> The patches in this series do not depend on each other.
> 
> This changes the behavior of the code in the second case and is not tested.

In this patch, as in the sis one, the behavior is changed. The subject
might perhaps be "fix cursor initialization"?

The patch does look like it could cause a regression, though. Anyone
able to test this?

 Tomi

> 
>  drivers/video/fbdev/intelfb/intelfbhw.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/video/fbdev/intelfb/intelfbhw.c b/drivers/video/fbdev/intelfb/intelfbhw.c
> index fbad61d..d31ed4e 100644
> --- a/drivers/video/fbdev/intelfb/intelfbhw.c
> +++ b/drivers/video/fbdev/intelfb/intelfbhw.c
> @@ -1191,7 +1191,6 @@ int intelfbhw_mode_to_hw(struct intelfb_info *dinfo,
>  	vsync_end = vsync_start + var->vsync_len;
>  	vtotal = vsync_end + var->upper_margin;
>  	vblank_start = vactive;
> -	vblank_end = vtotal;
>  	vblank_end = vsync_end + 1;
>  
>  	DBG_MSG("V: act %d, ss %d, se %d, tot %d bs %d, be %d\n",
> @@ -1859,7 +1858,7 @@ void intelfbhw_cursor_init(struct intelfb_info *dinfo)
>  		tmp = INREG(CURSOR_CONTROL);
>  		tmp &= ~(CURSOR_FORMAT_MASK | CURSOR_GAMMA_ENABLE |
>  			 CURSOR_ENABLE | CURSOR_STRIDE_MASK);
> -		tmp = CURSOR_FORMAT_3C;
> +		tmp |= CURSOR_FORMAT_3C;
>  		OUTREG(CURSOR_CONTROL, tmp);
>  		OUTREG(CURSOR_A_BASEADDR, dinfo->cursor.offset << 12);
>  		tmp = (64 << CURSOR_SIZE_H_SHIFT) |
> 



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