On 2018-03-24 15:03, Jonathan Cameron wrote:
> On Mon, 19 Mar 2018 18:02:46 +0100
> Peter Rosin <[email protected]> wrote:
>
>> If an ADC channel measures the midpoint of a voltage divider, the
>> interesting voltage is often the voltage over the full resistance.
>> E.g. if the full voltage it too big for the ADC to handle.
>> Likewise, if an ADC channel measures the voltage across a resistor,
>> the interesting value is often the current through the resistor.
>>
>> This driver solves both problems by allowing to linearly scale a channel
>> and by allowing changes to the type of the channel. Or both.
>>
>> Signed-off-by: Peter Rosin <[email protected]>
> A few comments inline, but basically the code looks fine, just questions
> around naming and bindings to answer.
>
> Thanks,
>
> Jonathan
>
*snip*
>> +static int unit_converter_write_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int val, int val2, long mask)
>> +{
>> + struct unit_converter *uc = iio_priv(indio_dev);
>> +
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>> + return iio_write_channel_raw(uc->parent, val);
> Talk me through the logic of having this... Supporting a DAC?
> Bindings don't talk about that possibility...
There's no logic at all, and I don't need it. It's just leftovers,
should I remove it?
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
*snip*
>> +static int unit_converter_configure_channel(struct device *dev,
>> + struct unit_converter *uc,
>> + enum iio_chan_type type)
>> +{
>> + struct iio_chan_spec *chan = &uc->chan;
>> + struct iio_chan_spec const *pchan = uc->parent->channel;
>> + int ret;
>> +
>> + chan->indexed = 1;
>> + chan->output = pchan->output;
>> + chan->ext_info = uc->ext_info;
>> +
>> + if (type == -1) {
>> + ret = iio_get_channel_type(uc->parent, &type);
>> + if (ret < 0) {
>> + dev_err(dev, "failed to get parent channel type\n");
>> + return ret;
>> + }
>> + }
>> + chan->type = type;
>> +
>> + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW))
>> + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW);
> if the parent doesn't support RAW, is there a lot of point in carrying on?
Nope, better to error out I suppose. But I'm not familiar with channels
without RAW, what alternatives are there anyway?
>> + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE))
>> + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE);
>> +
>> + if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW))
>> + chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
>> +
>> + chan->channel = 0;
>> +
>> + return 0;
>> +}
