On 2018-03-24 15:03, Jonathan Cameron wrote: > On Mon, 19 Mar 2018 18:02:46 +0100 > Peter Rosin <peda@xxxxxxxxxx> 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 <peda@xxxxxxxxxx> > 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; >> +} -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html