Re: [PATCH] lis3: Add axes module parameter for custom axis-mapping

[Takashi Iwai]

At Tue, 07 Sep 2010 16:25:02 +0200,
Ãric Piel wrote:
> 
> Op 07-09-10 08:45, Takashi Iwai schreef:
> > At Mon, 23 Aug 2010 14:23:25 +0200,
> > Takashi Iwai wrote:
> >>
> >> The axis-mapping of lis3dev device on many (rather most) HP machines
> >> doesn't follow the standard.  When each new model appears, users need
> >> to adjust again.  Testing this requires the rebuild of kernel, thus
> >> it's not trivial for end-users.
> >>
> >> This patch adds a module parameter "axes" to allow a custom
> >> axis-mapping without patching and recompiling the kernel driver.
> >> User can pass the parameter such as axes=3,2,1.  Also it can be
> >> changed via sysfs.
> >>
> >> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > 
> > Can anyone review / take this?
> > Jean, should it be through hwmon tree? 
> Sorry for not answering before, but I'm very busy currently.
> 
> I like the idea, but the implementation seems weird. Maybe it's just
> that I don't understand well enough module_param_array_named() though.
>
> Each time the user changes the axis parameter, param_set_axis() is
> called three times (once per axis), right? Why always calling
> copy_to_axis_array() ? Does param_set_int(val, kp) return null only on
> the 3rd time? This function really looks like a kludge. Either you need
> to put more comments in order to make clear why things work, or rework it.

Yeah, the code is a bit tricky :)
The reason is that module parameter array calls the callback for each
array member.  But it doesn't pass which array index is being
accessed.

> Wouldn't it be much cleaner if lis3_dev.ac was an array instead of a
> struct? I don't mind that it involves changing more line if it makes
> things more understandable.

OK, then it makes things easier.  The revised patch is below.


thanks,

Takashi

===