On 12/08/2011 07:40 PM, Greg KH wrote:
> On Mon, Dec 05, 2011 at 09:56:02PM +0000, Jonathan Cameron wrote:
>> From: Jonathan Cameron <jic23@xxxxxxxxx>
>>
>> Lifted from proposal for in kernel interface built on the out of staging
>> branch.
>>
>> Two elements here:
>> * Map as defined in "inkern.h"
>> * Matching code to actually get the iio_dev and channel
>> that we want from the global list of IIO devices.
>>
>> Signed-off-by: Jonathan Cameron <jic23@xxxxxxxxx>
>> ---
>> drivers/staging/Makefile | 2 +-
>> drivers/staging/iio/Makefile | 2 +-
>> drivers/staging/iio/iio.h | 6 +-
>> drivers/staging/iio/industrialio-core.c | 268 ++++++++++++++++++++++++++++++-
>> drivers/staging/iio/inkern.c | 21 +++
>> drivers/staging/iio/inkern.h | 86 ++++++++++
>> 6 files changed, 377 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
>> index 6e615b6..f59ab89 100644
>> --- a/drivers/staging/Makefile
>> +++ b/drivers/staging/Makefile
>> @@ -34,7 +34,7 @@ obj-$(CONFIG_VT6656) += vt6656/
>> obj-$(CONFIG_HYPERV) += hv/
>> obj-$(CONFIG_VME_BUS) += vme/
>> obj-$(CONFIG_DX_SEP) += sep/
>> -obj-$(CONFIG_IIO) += iio/
>> +obj-y += iio/
>
> Hm, not good.
>
>> obj-$(CONFIG_ZRAM) += zram/
>> obj-$(CONFIG_XVMALLOC) += zram/
>> obj-$(CONFIG_ZCACHE) += zcache/
>> diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
>> index 1340aea..04d6ad2 100644
>> --- a/drivers/staging/iio/Makefile
>> +++ b/drivers/staging/iio/Makefile
>> @@ -1,7 +1,7 @@
>> #
>> # Makefile for the industrial I/O core.
>> #
>> -
>> +obj-y = inkern.o
>
> Doubly not good. You just always build this now, even if someone
> doesn't want any staging drivers, or iio, right?
Agreed. That one is definitely wrong and needs a selectable symbol.
>
> I can't accept this, sorry.
>
>> --- /dev/null
>> +++ b/drivers/staging/iio/inkern.c
>> @@ -0,0 +1,21 @@
>> +/* The industrial I/O core in kernel channel mapping
>> + *
>> + * Copyright (c) 2011 Jonathan Cameron
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + */
>> +#include "inkern.h"
>> +#include <linux/err.h>
>> +#include <linux/export.h>
>> +
>> +LIST_HEAD(iio_map_list);
>> +EXPORT_SYMBOL_GPL(iio_map_list);
>> +void iio_map_array_register(struct iio_map *map, int nummaps)
>> +{
>> + int i;
>> + for (i = 0; i < nummaps; i++)
>> + list_add(&map[i].l, &iio_map_list);
>> +}
>> +EXPORT_SYMBOL(iio_map_array_register);
>
> No _GPL here?
>
> If you have a register function, why do you need to export the list
> symbol as well? Shouldn't you have accessor functions for the whole
> list? And what is this list for?
>
> What exactly are you trying to do here with the "inkern.c" file?
>
>> --- /dev/null
>> +++ b/drivers/staging/iio/inkern.h
>> @@ -0,0 +1,86 @@
>> +#include <linux/device.h>
>> +#include <linux/list.h>
>> +#include "types.h"
>> +
>> +#ifndef _IIO_INKERN_H_
>> +#define _IIO_INKERN_H_
>> +
>> +struct iio_dev;
>> +struct iio_chan_spec;
>> +
>> +struct iio_channel {
>> + struct iio_dev *indio_dev;
>> + const struct iio_chan_spec *channel;
>> +};
>> +
>> +extern struct list_head iio_map_list;
>> +
>> +struct iio_map {
>> + /* iio device side */
>> + struct device *adc_dev;
>> + const char *adc_dev_name;
>> + const char *adc_channel_label;
>> + int channel_number; /*naughty starting point */
>> +
>> + /* consumer side */
>> + struct device *consumer_dev;
>> + const char *consumer_dev_name;
>> + const char *consumer_channel;
>> + /* management - probably neater ways of doing this */
>> + struct list_head l;
>> +};
>> +
>> +void iio_map_array_register(struct iio_map *map, int nummaps);
>> +/**
>> + * iio_channel_get() - get an opaque reference to a specified device.
>> + */
>> +struct iio_channel *iio_st_channel_get(const struct device *dev,
>> + const char *name,
>> + const char *consumer_channel);
>> +void iio_st_channel_release(struct iio_channel *chan);
>> +
>> +/**
>> + * iio_st_channel_get_all() - get all channels associated with a client
>> + *
>> + * returns a null terminated array of pointers to iio_channel structures.
>> + */
>> +struct iio_channel **iio_st_channel_get_all(const struct device *dev,
>> + const char *name);
>> +
>> +void iio_st_channel_release_all(struct iio_channel **chan);
>> +
>> +/**
>> + * iio_st_read_channel_raw() - read from a given channel
>> + * @channel: the channel being queried.
>> + * @val: value read back.
>> + *
>> + * Note raw reads from iio channels are in adc counts and hence
>> + * scale will need to be applied if standard units required.
>> + *
>> + * Maybe want to pass the type as a sanity check.
>> + */
>> +int iio_st_read_channel_raw(struct iio_channel *chan,
>> + int *val);
>> +
>> +/**
>> + * iio_st_get_channel_type() - get the type of a channel
>> + * @channel: the channel being queried.
>> + *
>> + * returns the enum iio_chan_type of the channel
>> + */
>> +enum iio_chan_type iio_st_get_channel_type(struct iio_channel *channel);
>> +
>> +/**
>> + * iio_st_read_channel_scale() - read the scale value for a channel
>> + * @channel: the channel being queried.
>> + * @val: first part of value read back.
>> + * @val2: second part of value read back.
>> + *
>> + * Note returns a description of what is in val and val2, such
>> + * as IIO_VAL_INT_PLUS_MICRO telling us we have a value of val
>> + * + val2/1e6
>> + */
>> +int iio_st_read_channel_scale(struct iio_channel *chan, int *val,
>> + int *val2);
>> +
>> +#endif
>
> You have a bunch of functions and structures here that are not used in
> the .c file, which doesn't seem to match up.
Will rework this and get back to you.
Sorry for the waste of your time. I clearly dropped the ball here.
--
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
