lkml.org 
[lkml]   [2005]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2.6] (10/11) hwmon vs i2c, second round
Hi Alexey,

> > I see very little reason why vid_from_reg and vid_to_reg are
> > inlined. The former is not exactly short,
>
> 1)
> and their arguments aren't known at
> compile time,
>
> [Compiler can optimise them away _completely_ if both arguments are
> known at compile time or significantly of only one is known.]

Good point, I'll try to remember that. It doesn't apply here though
except for lm78 I think, and maybe lm93 when it gets ported. That's not
the majority of users though, so I still believe uninlining is the
correct decision.

> > and they are never called in speed critical areas. Uninlining them
> > should cause little performance loss if any, and saves a signficant
> > space and compilation time as well.
>
> 2) VID_FROM_REG is asking for removal from lm85.

True, I wrote a patch doing this already:
http://lists.lm-sensors.org/pipermail/lm-sensors/2005-July/013148.html

Just wait for Greg to pick it and it'll show in -mm.

> 3) vid_to_reg is used only by atxp1.

That's right. Do you suggest that it should be kept inlined then?
Similar drivers may be written in the future, causing vid_to_reg to gain
users and possibly grow larger (to support more VRM/VRD standards), then
we'll certainly want to uninline it anyway - but I agree that this ain't
the case at the moment.

I'll change that patch to only uninline vid_from_reg and not vid_to_reg
if a majority prefers me to do so.

Thanks for your comments :)
--
Jean Delvare
-
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/

\
 
 \ /
  Last update: 2005-07-31 23:07    [W:0.054 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site