On Sun, Aug 01, 2010 at 10:11:01PM -0400, Andy Walls wrote:

On Sat, 2010-07-31 at 17:59 +0300, Maxim Levitsky wrote:

...

quoted

 struct ir_input_dev {

@@ -69,9 +81,10 @@ struct ir_input_dev {
 	char				*driver_name;	/* Name of the driver module */
 	struct ir_scancode_table	rc_tab;		/* scan/key table */
 	unsigned long			devno;		/* device number */
-	const struct ir_dev_props	*props;		/* Device properties */
+	struct ir_dev_props		*props;		/* Device properties */

So I don't think the struct ir_dev_props structure is the right place to
be storing current operating parameters. IMO, operating parameters
stored in the ir_dev_props are "too far" from the lower level driver,
and are essentially mirroring what the low level driver should be
tracking internally as it's own state anyway.


So in summary, I think we need to keep the opertions in struct
ir_dev_props simple using ,get_parameters() and .set_parameters() to
contol the lower level driver and to fetch, retrieve, and store
parameters.

Yeah, I neglected to consider this change the first pass through an
earlier revision. Making props modifiable on the fly does feel like we're
mixing up hardware features with hardware state, and perhaps the
on-the-fly-modifiable state bits should be stored somewhere else.

-- 
Jarod Wilson
jarod@redhat.com