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

PATCH In review Hareesh Nagarajan 2 participants 4 messages Oct 11, 2005 – Oct 12, 2005 Framebuffer layer

Share & tools

Hareesh Nagarajan

author

2005-10-11 02:29 UTC #0 lore raw

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

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

drivers/video/tcx.c +5 -7

@@ -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);

Geert Uytterhoeven

2005-10-11 17:49 UTC #1 lore raw

Quoted 5 lines · Hareesh Nagarajan

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)?

Quoted 1 line

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.

Quoted 11 lines

(AFAICT, I am not missing anything; If I am, I'm sorry for wasting your time.)
drivers/video/tcx.c +3 -0
@@ -174,13 +173,13 @@
 			 unsigned red, unsigned green, unsigned blue,
 			 unsigned transp, struct fb_info *info)
 {
+	if (regno >= 256)
+		return 1;
+

drivers/video/tcx.c line 185

Quoted 10 lines

drivers/video/tcx.c +0 -3

 	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.

drivers/video/tcx.c line 447

Quoted 9 lines

drivers/video/tcx.c +1 -1

@@ -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?

drivers/video/tcx.c line 495

Quoted 10 lines

drivers/video/tcx.c +1 -2

@@ -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 -- [email protected]

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

Hareesh Nagarajan

author

2005-10-12 01:21 UTC #2 lore raw

Quoted 8 lines · Geert Uytterhoeven

Quoted 5 lines · Hareesh Nagarajan
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?

Quoted 4 lines

Quoted 1 line
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.

Quoted 16 lines

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

drivers/video/tcx.c +3 -0

@@ -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;

Quoted 8 lines

drivers/video/tcx.c line 185
Quoted 6 lines
drivers/video/tcx.c +0 -3
-	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.

Quoted 5 lines

drivers/video/tcx.c line 442
Quoted 4 lines
drivers/video/tcx.c +1 -1
@@ -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.

Quoted 12 lines

drivers/video/tcx.c line 495

Quoted 10 lines

drivers/video/tcx.c +1 -2

@@ -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

Geert Uytterhoeven

2005-10-12 05:54 UTC #3 lore raw

Quoted 11 lines · Hareesh Nagarajan

Quoted 10 lines · Geert Uytterhoeven
Quoted 8 lines · Hareesh Nagarajan
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.

Quoted 3 lines

Quoted 1 line
What about the other changes you made (cfr. below)?
Please pardon my ignorance, but what does 'cfr.' mean?

cfr. = confer (Latin) = like

Quoted 2 lines

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 -- [email protected]

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