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

[Takashi Iwai <tiwai@suse.de>]

At Thu, 23 Sep 2010 12:23:08 -0700,
Andrew Morton wrote:
> 
> On Wed, 22 Sep 2010 13:31:19 +0200
> Takashi Iwai <tiwai@suse.de> 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.
> 
> Is the sysfs interface documented anywhere?

It's the generic module parameters sysfs interface.


> > --- a/drivers/hwmon/hp_accel.c
> > +++ b/drivers/hwmon/hp_accel.c
> > @@ -146,7 +146,8 @@ int lis3lv02d_acpi_write(struct lis3lv02d *lis3, int reg, u8 val)
> >  
> >  static int lis3lv02d_dmi_matched(const struct dmi_system_id *dmi)
> >  {
> > -	lis3_dev.ac = *((struct axis_conversion *)dmi->driver_data);
> > +	memcpy(lis3_dev.axis_map, (int *)dmi->driver_data,
> > +	       sizeof(lis3_dev.axis_map));
> 
> It's unobvious why the (nice) three-member struct was converted to a
> (nasty) three-element array?  All those typesafe struct assignments
> were turned into non-typesafe memcpys?
> 
> > +module_param_array_named(axes, lis3_dev.axis_map, axis, NULL, 0644);
> 
> Just to support module_param_array_named()?

Yes, indeed.

>  If so, could have used a union?
> 
> union axis_conversion {	/* should be called lis3_axis_conversion! */
> 	struct {
> 		int x;
> 		int y;
> 		int z;
> 	};
> 	int as_array[3];
> };

This should be possible, yes.

> or use a tyecast in the module_param_array_named() statement, perhaps?

Using non-array for module parameter would become tricky like my
first version.  I'll try to rewrite with the union expression.


thanks,

Takashi