On 08/17/13 15:39, Jonathan Cameron wrote:
> Introduce an enum to specify whether the attribute is separate or
> shared.
>
> Factor out the bitmap handling for loop into a separate function.
>
> Tidy up error handling and add a NULL assignment to squish a false
> positive warning from GCC.
Ooops. This doesn't handle the ext_info case properly as that just
passes a boolean. Will fix up and repost...
>
> Signed-off-by: Jonathan Cameron <jic23@xxxxxxxxxx>
> ---
> drivers/iio/iio_core.h | 5 +-
> drivers/iio/industrialio-buffer.c | 2 +-
> drivers/iio/industrialio-core.c | 148 ++++++++++++++++++++------------------
> 3 files changed, 84 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> index 05c1b74..75470ef 100644
> --- a/drivers/iio/iio_core.h
> +++ b/drivers/iio/iio_core.h
> @@ -20,6 +20,9 @@ struct iio_dev;
>
> extern struct device_type iio_device_type;
>
> +enum iio_shared_by { __IIO_SEPARATE,
> + __IIO_SHARED_BY_TYPE };
> +
> int __iio_add_chan_devattr(const char *postfix,
> struct iio_chan_spec const *chan,
> ssize_t (*func)(struct device *dev,
> @@ -30,7 +33,7 @@ int __iio_add_chan_devattr(const char *postfix,
> const char *buf,
> size_t len),
> u64 mask,
> - bool generic,
> + enum iio_shared_by generic,
> struct device *dev,
> struct list_head *attr_list);
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index e73033f..69e4a45 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -214,7 +214,7 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
> &iio_show_scan_index,
> NULL,
> 0,
> - 0,
> + __IIO_SEPARATE,
> &indio_dev->dev,
> &buffer->scan_el_dev_attr_list);
> if (ret)
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 97f0297..720cea1 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -516,14 +516,15 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> struct device_attribute *attr,
> const char *buf,
> size_t len),
> - bool generic)
> + enum iio_shared_by shared_by)
> {
> - int ret;
> - char *name_format, *full_postfix;
> + int ret = 0;
> + char *name_format = NULL;
> + char *full_postfix;
> sysfs_attr_init(&dev_attr->attr);
>
> /* Build up postfix of <extend_name>_<modifier>_postfix */
> - if (chan->modified && !generic) {
> + if (chan->modified && (shared_by == __IIO_SEPARATE)) {
> if (chan->extend_name)
> full_postfix = kasprintf(GFP_KERNEL, "%s_%s_%s",
> iio_modifier_names[chan
> @@ -544,53 +545,62 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> chan->extend_name,
> postfix);
> }
> - if (full_postfix == NULL) {
> - ret = -ENOMEM;
> - goto error_ret;
> - }
> + if (full_postfix == NULL)
> + return -ENOMEM;
>
> if (chan->differential) { /* Differential can not have modifier */
> - if (generic)
> + switch (shared_by) {
> + case __IIO_SHARED_BY_TYPE:
> name_format
> = kasprintf(GFP_KERNEL, "%s_%s-%s_%s",
> iio_direction[chan->output],
> iio_chan_type_name_spec[chan->type],
> iio_chan_type_name_spec[chan->type],
> full_postfix);
> - else if (chan->indexed)
> + break;
> + case __IIO_SEPARATE:
> + if (!chan->indexed) {
> + WARN_ON("Differential channels must be indexed\n");
> + ret = -EINVAL;
> + goto error_free_full_postfix;
> + }
> name_format
> - = kasprintf(GFP_KERNEL, "%s_%s%d-%s%d_%s",
> + = kasprintf(GFP_KERNEL,
> + "%s_%s%d-%s%d_%s",
> iio_direction[chan->output],
> iio_chan_type_name_spec[chan->type],
> chan->channel,
> iio_chan_type_name_spec[chan->type],
> chan->channel2,
> full_postfix);
> - else {
> - WARN_ON("Differential channels must be indexed\n");
> - ret = -EINVAL;
> - goto error_free_full_postfix;
> + break;
> }
> } else { /* Single ended */
> - if (generic)
> - name_format
> - = kasprintf(GFP_KERNEL, "%s_%s_%s",
> - iio_direction[chan->output],
> - iio_chan_type_name_spec[chan->type],
> - full_postfix);
> - else if (chan->indexed)
> - name_format
> - = kasprintf(GFP_KERNEL, "%s_%s%d_%s",
> - iio_direction[chan->output],
> - iio_chan_type_name_spec[chan->type],
> - chan->channel,
> - full_postfix);
> - else
> + switch (shared_by) {
> + case __IIO_SHARED_BY_TYPE:
> name_format
> = kasprintf(GFP_KERNEL, "%s_%s_%s",
> iio_direction[chan->output],
> iio_chan_type_name_spec[chan->type],
> full_postfix);
> + break;
> +
> + case __IIO_SEPARATE:
> + if (chan->indexed)
> + name_format
> + = kasprintf(GFP_KERNEL, "%s_%s%d_%s",
> + iio_direction[chan->output],
> + iio_chan_type_name_spec[chan->type],
> + chan->channel,
> + full_postfix);
> + else
> + name_format
> + = kasprintf(GFP_KERNEL, "%s_%s_%s",
> + iio_direction[chan->output],
> + iio_chan_type_name_spec[chan->type],
> + full_postfix);
> + break;
> + }
> }
> if (name_format == NULL) {
> ret = -ENOMEM;
> @@ -614,16 +624,11 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> dev_attr->attr.mode |= S_IWUSR;
> dev_attr->store = writefunc;
> }
> - kfree(name_format);
> - kfree(full_postfix);
> -
> - return 0;
> -
> error_free_name_format:
> kfree(name_format);
> error_free_full_postfix:
> kfree(full_postfix);
> -error_ret:
> +
> return ret;
> }
>
> @@ -642,7 +647,7 @@ int __iio_add_chan_devattr(const char *postfix,
> const char *buf,
> size_t len),
> u64 mask,
> - bool generic,
> + enum iio_shared_by shared_by,
> struct device *dev,
> struct list_head *attr_list)
> {
> @@ -656,7 +661,7 @@ int __iio_add_chan_devattr(const char *postfix,
> }
> ret = __iio_device_attr_init(&iio_attr->dev_attr,
> postfix, chan,
> - readfunc, writefunc, generic);
> + readfunc, writefunc, shared_by);
> if (ret)
> goto error_iio_dev_attr_free;
> iio_attr->c = chan;
> @@ -664,7 +669,7 @@ int __iio_add_chan_devattr(const char *postfix,
> list_for_each_entry(t, attr_list, l)
> if (strcmp(t->dev_attr.attr.name,
> iio_attr->dev_attr.attr.name) == 0) {
> - if (!generic)
> + if (shared_by == __IIO_SEPARATE)
> dev_err(dev, "tried to double register : %s\n",
> t->dev_attr.attr.name);
> ret = -EBUSY;
> @@ -682,46 +687,53 @@ error_ret:
> return ret;
> }
>
> -static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan)
> +static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + enum iio_shared_by shared_by,
> + const long *infomask)
> {
> - int ret, attrcount = 0;
> - int i;
> - const struct iio_chan_spec_ext_info *ext_info;
> + int i, ret, attrcount = 0;
>
> - if (chan->channel < 0)
> - return 0;
> - for_each_set_bit(i, &chan->info_mask_separate, sizeof(long)*8) {
> + for_each_set_bit(i, infomask, sizeof(infomask)*8) {
> ret = __iio_add_chan_devattr(iio_chan_info_postfix[i],
> chan,
> &iio_read_channel_info,
> &iio_write_channel_info,
> i,
> - 0,
> + shared_by,
> &indio_dev->dev,
> &indio_dev->channel_attr_list);
> - if (ret < 0)
> - goto error_ret;
> - attrcount++;
> - }
> - for_each_set_bit(i, &chan->info_mask_shared_by_type, sizeof(long)*8) {
> - ret = __iio_add_chan_devattr(iio_chan_info_postfix[i],
> - chan,
> - &iio_read_channel_info,
> - &iio_write_channel_info,
> - i,
> - 1,
> - &indio_dev->dev,
> - &indio_dev->channel_attr_list);
> - if (ret == -EBUSY) {
> - ret = 0;
> + if ((ret == -EBUSY) && (shared_by != __IIO_SEPARATE))
> continue;
> - } else if (ret < 0) {
> - goto error_ret;
> - }
> + else if (ret < 0)
> + return ret;
> attrcount++;
> }
>
> + return attrcount;
> +}
> +
> +static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan)
> +{
> + int ret, attrcount = 0;
> + const struct iio_chan_spec_ext_info *ext_info;
> +
> + if (chan->channel < 0)
> + return 0;
> + ret = iio_device_add_info_mask_type(indio_dev, chan,
> + __IIO_SEPARATE,
> + &chan->info_mask_separate);
> + if (ret < 0)
> + return ret;
> + attrcount += ret;
> +
> + ret = iio_device_add_info_mask_type(indio_dev, chan,
> + __IIO_SHARED_BY_TYPE,
> + &chan->info_mask_shared_by_type);
> + if (ret < 0)
> + return ret;
> + attrcount += ret;
> if (chan->ext_info) {
> unsigned int i = 0;
> for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
> @@ -740,15 +752,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> continue;
>
> if (ret)
> - goto error_ret;
> + return ret;
>
> attrcount++;
> }
> }
>
> - ret = attrcount;
> -error_ret:
> - return ret;
> + return attrcount;
> }
>
> static void iio_device_remove_and_free_read_attr(struct iio_dev *indio_dev,
>
--
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
