[PATCH] Trivial patch to remove list member from struct tcx_par in drivers/video/tcx.c

[PATCH] Trivial patch to remove list member from struct tcx_par in drivers/video/tcx.c

[Hareesh Nagarajan]

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

This patch removes the list_head member 'list' from struct tcx_par in 
the file drivers/video/tcx.c among other trivial cleanups. The member 
'list' is never used. It turns out that other frame buffer code like 
cg14.c, leo.c, bw2.c, etc. (in drivers/video) seem to have the same 
extra list_head member in their respective *_par structures.

The patch applies to the 2.6.13.4 kernel sources.

(AFAICT, I am not missing anything; If I am, I'm sorry for wasting your 
time.)

Thanks,

Hareesh Nagarajan

[-- Attachment #2: tcx.c-cleanup.patch --]
[-- Type: text/x-patch, Size: 1206 bytes --]

--- linux-2.6.13.4/drivers/video/tcx.c	2005-10-10 13:54:29.000000000 -0500
+++ linux-2.6.13.4-edit/drivers/video/tcx.c	2005-10-10 21:05:22.110156000 -0500
@@ -123,7 +123,6 @@
 	int			lowdepth;
 
 	struct sbus_dev		*sdev;
-	struct list_head	list;
 };
 
 /* Reset control plane so that WID is 8-bit plane. */
@@ -174,13 +173,13 @@
 			 unsigned red, unsigned green, unsigned blue,
 			 unsigned transp, struct fb_info *info)
 {
+	if (regno >= 256)
+		return 1;
+
 	struct tcx_par *par = (struct tcx_par *) info->par;
 	struct bt_regs __iomem *bt = par->bt;
 	unsigned long flags;
 
-	if (regno >= 256)
-		return 1;
-
 	red >>= 8;
 	green >>= 8;
 	blue >>= 8;
@@ -442,7 +441,7 @@
 
 	tcx_reset(&all->info);
 
-	tcx_blank(0, &all->info);
+	tcx_blank(FB_BLANK_UNBLANK, &all->info);
 
 	if (fb_alloc_cmap(&all->info.cmap, 256, 0)) {
 		printk(KERN_ERR "tcx: Could not allocate color map.\n");
@@ -490,8 +489,7 @@
 	struct list_head *pos, *tmp;
 
 	list_for_each_safe(pos, tmp, &tcx_list) {
-		struct all_info *all = list_entry(pos, typeof(*all), list);
-
+		struct all_info *all = list_entry(pos, struct all_info, list);
 		unregister_framebuffer(&all->info);
 		fb_dealloc_cmap(&all->info.cmap);
 		kfree(all);

Re: [PATCH] Trivial patch to remove list member from struct tcx_par in drivers/video/tcx.c

[Geert Uytterhoeven]

On Mon, 10 Oct 2005, Hareesh Nagarajan wrote:
> This patch removes the list_head member 'list' from struct tcx_par in the file
> drivers/video/tcx.c among other trivial cleanups. The member 'list' is never
> used. It turns out that other frame buffer code like cg14.c, leo.c, bw2.c,
> etc. (in drivers/video) seem to have the same extra list_head member in their
> respective *_par structures.

What about the other changes you made (cfr. below)?

> The patch applies to the 2.6.13.4 kernel sources.

Please send patches inlined in the future. If you send them as an attachment,
we cannot easily comment on them.

> (AFAICT, I am not missing anything; If I am, I'm sorry for wasting your time.)

> --- linux-2.6.13.4/drivers/video/tcx.c	2005-10-10 13:54:29.000000000 -0500
> +++ linux-2.6.13.4-edit/drivers/video/tcx.c	2005-10-10 21:05:22.110156000 -0500
> @@ -174,13 +173,13 @@
>  			 unsigned red, unsigned green, unsigned blue,
>  			 unsigned transp, struct fb_info *info)
>  {
> +	if (regno >= 256)
> +		return 1;
> +
>  	struct tcx_par *par = (struct tcx_par *) info->par;
>  	struct bt_regs __iomem *bt = par->bt;
>  	unsigned long flags;
>  
> -	if (regno >= 256)
> -		return 1;
> -
>  	red >>= 8;
>  	green >>= 8;
>  	blue >>= 8;

Please don't change this, or it won't compile anymore with gcc 2.95.

> @@ -442,7 +441,7 @@
>  
>  	tcx_reset(&all->info);
>  
> -	tcx_blank(0, &all->info);
> +	tcx_blank(FB_BLANK_UNBLANK, &all->info);
>  
>  	if (fb_alloc_cmap(&all->info.cmap, 256, 0)) {
>  		printk(KERN_ERR "tcx: Could not allocate color map.\n");

What about this change?

> @@ -490,8 +489,7 @@
>  	struct list_head *pos, *tmp;
>  
>  	list_for_each_safe(pos, tmp, &tcx_list) {
> -		struct all_info *all = list_entry(pos, typeof(*all), list);
> -
> +		struct all_info *all = list_entry(pos, struct all_info, list);
>  		unregister_framebuffer(&all->info);
>  		fb_dealloc_cmap(&all->info.cmap);
>  		kfree(all);

And this one?

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] Trivial patch to remove list member from struct tcx_par in drivers/video/tcx.c

[Hareesh Nagarajan]

Geert Uytterhoeven wrote:
> On Mon, 10 Oct 2005, Hareesh Nagarajan wrote:
>> This patch removes the list_head member 'list' from struct tcx_par in the file
>> drivers/video/tcx.c among other trivial cleanups. The member 'list' is never
>> used. It turns out that other frame buffer code like cg14.c, leo.c, bw2.c,
>> etc. (in drivers/video) seem to have the same extra list_head member in their
>> respective *_par structures.
> 
> What about the other changes you made (cfr. below)?

Please pardon my ignorance, but what does 'cfr.' mean?

>> The patch applies to the 2.6.13.4 kernel sources.
> 
> Please send patches inlined in the future. If you send them as an attachment,
> we cannot easily comment on them.

Sure, I will do so.

> 
>> (AFAICT, I am not missing anything; If I am, I'm sorry for wasting your time.)
> 
>> --- linux-2.6.13.4/drivers/video/tcx.c	2005-10-10 13:54:29.000000000 -0500
>> +++ linux-2.6.13.4-edit/drivers/video/tcx.c	2005-10-10 21:05:22.110156000 -0500
>> @@ -174,13 +173,13 @@
>>  			 unsigned red, unsigned green, unsigned blue,
>>  			 unsigned transp, struct fb_info *info)
>>  {
>> +	if (regno >= 256)
>> +		return 1;
>> +
>>  	struct tcx_par *par = (struct tcx_par *) info->par;
>>  	struct bt_regs __iomem *bt = par->bt;
>>  	unsigned long flags;
>>  
>> -	if (regno >= 256)
>> -		return 1;
>> -
>>  	red >>= 8;
>>  	green >>= 8;
>>  	blue >>= 8;
> 
> Please don't change this, or it won't compile anymore with gcc 2.95.

Ok.

>> @@ -442,7 +441,7 @@
>>  
>> -	tcx_blank(0, &all->info);
>> +	tcx_blank(FB_BLANK_UNBLANK, &all->info);
> What about this change?

Well, this isn't really a change, it just enhances readability.

>> @@ -490,8 +489,7 @@
>>  	struct list_head *pos, *tmp;
>>  
>>  	list_for_each_safe(pos, tmp, &tcx_list) {
>> -		struct all_info *all = list_entry(pos, typeof(*all), list);
>> -
>> +		struct all_info *all = list_entry(pos, struct all_info, list);
>>  		unregister_framebuffer(&all->info);
>>  		fb_dealloc_cmap(&all->info.cmap);
>>  		kfree(all);
> 
> And this one?

Why use typeof when you can specify the type. But again, this change 
does nothing. It reduces compile time a tad perhaps? :)

Must I go ahead and create a patch which removes the extra list_head 
member in the *_par structures?

Thanks,

Hareesh

Re: [PATCH] Trivial patch to remove list member from struct tcx_par in drivers/video/tcx.c

[Geert Uytterhoeven]

On Tue, 11 Oct 2005, Hareesh Nagarajan wrote:
> Geert Uytterhoeven wrote:
> > On Mon, 10 Oct 2005, Hareesh Nagarajan wrote:
> > > This patch removes the list_head member 'list' from struct tcx_par in the
> > > file
> > > drivers/video/tcx.c among other trivial cleanups. The member 'list' is
> > > never
> > > used. It turns out that other frame buffer code like cg14.c, leo.c, bw2.c,
> > > etc. (in drivers/video) seem to have the same extra list_head member in
> > > their
> > > respective *_par structures.
> > 
> > What about the other changes you made (cfr. below)?
> 
> Please pardon my ignorance, but what does 'cfr.' mean?

cfr. = confer (Latin) = like

> Must I go ahead and create a patch which removes the extra list_head member in
> the *_par structures?

If it's unused, please go ahead.

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