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

[Hareesh Nagarajan <hnagar2@gmail.com>]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/