On Friday 25 March 2011, Jonathan Cameron wrote:
> > Better make it "unsigned long" instead of int when it's driver specific.
> > That way, a driver can store a pointer here if necessary.
> Makes sense. Perhaps a union to make the option explicit would be even
> better?
You can try it, but often unions cause more trouble than they are
worth. If most drivers just need an integer, using unsigned long
should be easier than a union.
> Whilst we are talking about types, some of the masks should probably switch to
> fixed length arrays of longs though so we don't have massive changes the
> moment we run out of space in the single longs. All access will have to then
> be through the bitmap functions, but that should be fine.
Depends on whether you expect the masks to be exceeded any time soon.
I would expect 32 bits to be quite a lot, and it's easy to extend to
64 before you need to move to the bitops API.
> >> @@ -116,6 +248,31 @@ struct iio_dev {
> >> u32 *available_scan_masks;
> >> struct iio_trigger *trig;
> >> struct iio_poll_func *pollfunc;
> >> +
> >> + struct iio_chan_spec *channels;
> >> + int num_channels;
> >> + struct list_head channel_attr_list;
> >> +
> >> + char *name; /*device name - IMPLEMENT */
> >> + int (*read_raw)(struct iio_dev *indio_dev,
> >> + struct iio_chan_spec *chan,
> >> + int *val,
> >> + long mask);
> >> +
> >> + int (*read_event_config)(struct iio_dev *indio_dev,
> >> + int event_code);
> >> +
> >> + int (*write_event_config)(struct iio_dev *indio_dev,
> >> + int event_code,
> >> + struct iio_event_handler_list *listel,
> >> + int state);
> >> +
> >> + int (*read_event_value)(struct iio_dev *indio_dev,
> >> + int event_code,
> >> + int *val);
> >> + int (*write_event_value)(struct iio_dev *indio_dev,
> >> + int event_code,
> >> + int val);
> >> };
> >
> > As mentioned in the comment for 2/3, a number of the members here are
> > constant, so you could split the structure into a per-driver constant
> > part and a per-device part, like most other subsystems do.
>
> Good idea. Might have to be at a finer level than per driver though.
> I can certainly envision some drivers want for example to have a number
> of variants of read_raw to account for subtle difference in what channels
> are present on individual parts.
Right.
> However I think this refactoring should wait until after the other
> big changes are in.
Ok.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
