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:
...
> >  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
ja...@redhat.com

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to