On 03/30/11 13:20, mailinglists@xxxxxxxxx wrote:
> From: Jan Weitzel <j.weitzel@xxxxxxxxx>
>
> by default MAX1363_SETUP_AIN3_IS_AIN3_REF_IS_VDD is set, add VDD is
> the voltage reference, not the internal one. So in_scale is wrong.
> If a regulator is available it is ask about the voltage. If it is
> not available the old (wrong) internal reference is used.
>
Hi Jan,
Nice to know this driver has some users.
Firstly there are two completely different elements to this patch, so they
should be broken out into 2 patches.
For the fix, perhaps the easier option is just to change the default to use the
internal reference? Do you have a use that requires it to be up to VDD?
Now you have pointed out this bug, I am rather dubious about simply outputting
vref if the regulator isn't specified. We could do this via platform data I guess.
I wonder how much trouble it would cause to just remove all options for not having
the reg there? After all people can just use a fixed voltage regulator to specify
what it is.
Hmm. These range attributes are a pain. Do you have a use case actually needing
that value? I'm also tempted to suggest changing the abi to at least identify them with
a type of channel (hence in_range). Right now they aren't well specified
(what's the syntax for something covering 1-2V only for example?)
Thanks,
Jonathan
> Signed-off-by: Jan Weitzel <j.weitzel@xxxxxxxxx>
> ---
> drivers/staging/iio/adc/max1363_core.c | 29 +++++++++++++++++++++++++----
> 1 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/max1363_core.c b/drivers/staging/iio/adc/max1363_core.c
> index 905f856..cf49124 100644
> --- a/drivers/staging/iio/adc/max1363_core.c
> +++ b/drivers/staging/iio/adc/max1363_core.c
> @@ -279,22 +279,31 @@ static IIO_DEV_ATTR_IN_DIFF_RAW(7, 6, max1363_read_single_channel, d7m6);
> static IIO_DEV_ATTR_IN_DIFF_RAW(9, 8, max1363_read_single_channel, d9m8);
> static IIO_DEV_ATTR_IN_DIFF_RAW(11, 10, max1363_read_single_channel, d11m10);
>
> +static u16 max1363_get_range(struct max1363_state *st)
> +{
> + u16 range;
> + if (IS_ERR(st->reg))
> + range = st->chip_info->int_vref_mv;
> + else
> + range = regulator_get_voltage(st->reg) / 1000;
> +
> + return range;
> +}
>
> static ssize_t max1363_show_scale(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> - /* Driver currently only support internal vref */
> + /* Driver currently only support vcc vref */
> struct iio_dev *dev_info = dev_get_drvdata(dev);
> struct max1363_state *st = iio_dev_get_devdata(dev_info);
> /* Corresponds to Vref / 2^(bits) */
>
> - if ((1 << (st->chip_info->bits + 1))
> - > st->chip_info->int_vref_mv)
> + if ((1 << (st->chip_info->bits + 1)) > max1363_get_range(st))
> return sprintf(buf, "0.5\n");
> else
> return sprintf(buf, "%d\n",
> - st->chip_info->int_vref_mv >> st->chip_info->bits);
> + max1363_get_range(st) >> st->chip_info->bits);
> }
>
> static IIO_DEVICE_ATTR(in_scale, S_IRUGO, max1363_show_scale, NULL, 0);
> @@ -310,6 +319,17 @@ static ssize_t max1363_show_name(struct device *dev,
>
> static IIO_DEVICE_ATTR(name, S_IRUGO, max1363_show_name, NULL, 0);
>
> +static ssize_t max1363_show_range(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *dev_info = dev_get_drvdata(dev);
> + struct max1363_state *st = iio_dev_get_devdata(dev_info);
> + return sprintf(buf, "%d\n", max1363_get_range(st));
> +}
> +
> +static IIO_DEVICE_ATTR(range, S_IRUGO, max1363_show_range, NULL, 0);
> +
> /* Applies to max1363 */
> static const enum max1363_modes max1363_mode_list[] = {
> _s0, _s1, _s2, _s3,
> @@ -329,6 +349,7 @@ static struct attribute *max1363_device_attrs[] = {
> &iio_dev_attr_in3min2_raw.dev_attr.attr,
> &iio_dev_attr_name.dev_attr.attr,
> &iio_dev_attr_in_scale.dev_attr.attr,
> + &iio_dev_attr_range.dev_attr.attr,
> NULL
> };
>
--
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
