Re: [PATCH v2 7/9] iio: hid-sensors: Added Inclinometer 3D

[Jonathan Cameron <jic23@kernel.org>]

On 10/23/13 20:11, Srinivas Pandruvada wrote:
> Added usage id processing for Inclinometer 3D. This uses IIO
> interfaces for triggered buffer to present data to user
> mode.This uses HID sensor framework for registering callback
> events from the sensor hub.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Looks good.  One little comment inline... I'd probably not have mentioned that
but as there is going to be another version fo the series, might as well tidy
it up ;)

Thanks,

> +/* Channel definitions */
> +static const struct iio_chan_spec incl_3d_channels[] = {
> +	{
> +		.type = IIO_INCLI,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_X,
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> +		BIT(IIO_CHAN_INFO_SCALE) |
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +		BIT(IIO_CHAN_INFO_HYSTERESIS),
> +		.scan_index = CHANNEL_SCAN_INDEX_X,
> +	}, {
> +		.type = IIO_INCLI,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_Y,
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> +		BIT(IIO_CHAN_INFO_SCALE) |
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +		BIT(IIO_CHAN_INFO_HYSTERESIS),
> +		.scan_index = CHANNEL_SCAN_INDEX_Y,
> +	}, {
> +		.type = IIO_INCLI,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_Z,
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> +		BIT(IIO_CHAN_INFO_SCALE) |
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +		BIT(IIO_CHAN_INFO_HYSTERESIS),
> +		.scan_index = CHANNEL_SCAN_INDEX_Z,
> +	}
> +};
> +
> +/* Adjust channel real bits based on report 
This would be cleaner if it just took the channel in question?
so
static void incl_3d_adjust_channel_bit_mask(struct iio_chan_spec *chan,
					    int size)
{
	chan->scan_type.sign = 's';
	/* Real storage bits will change based on the report desc. */
	chan->scan_type.realbits = size * 8;
	/* Maximum size of a sample to capture is u32 */
	chan->scan_type.storagebits = sizeof(u32) * 8;
}

then call as

	incl_3d_adjust_channel_bit_mask(&channels[CHANNEL_SCAN_INDEX_X],
				st->incl[CHANNEL_SCAN_INDEX_X].size);

> +static void incl_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
> +						int channel, int size)
> +{
> +	channels[channel].scan_type.sign = 's';
> +	/* Real storage bits will change based on the report desc. */
> +	channels[channel].scan_type.realbits = size * 8;
> +	/* Maximum size of a sample to capture is u32 */
> +	channels[channel].scan_type.storagebits = sizeof(u32) * 8;
> +}
> +
...