> > > +static int ad9739a_oper_mode_set(struct iio_dev *indio_dev,
> > > + const struct iio_chan_spec *chan, u32 mode)
> > > +{
> > > + struct ad9739a_state *st = iio_priv(indio_dev);
> > > +
> > > + if (mode == AD9739A_MIXED_MODE - 1)
> > > + mode = AD9739A_MIXED_MODE;
> >
> > Why? Feels like a comment is needed. Or a more obvious conversion function.
> >
>
> To match what we want to write in the register... With just two values it's too
> simple that opt not to have any helper function or table. Would you be fine with a
> comment?
yes
>
> > > +
> > > + return regmap_update_bits(st->regmap, AD9739A_REG_DEC_CNT,
> > > + AD9739A_DAC_DEC, mode);
> > > +}
> > > +
> > > +static int ad9739a_read_raw(struct iio_dev *indio_dev,
> > > + struct iio_chan_spec const *chan,
> > > + int *val, int *val2, long mask)
> > > +{
> > > + struct ad9739a_state *st = iio_priv(indio_dev);
> > > +
> > > + switch (mask) {
> > > + case IIO_CHAN_INFO_SAMP_FREQ:
> > > + *val = st->sample_rate;
> > > + *val2 = 0;
> > > + return IIO_VAL_INT_64;
> >
> > Big numbers :)
>
> My setup is using 2.5Ghz which is big enough to overflow INT but would work on UINT.
I like big numbers so it's fine doing this. Just unusual to force
val2 to 0 so it made me look closer and appreciate just how big these were getting ;)
> > > + if (id != AD9739A_ID)
> > > + return dev_err_probe(dev, -ENODEV, "Unrecognized CHIP_ID 0x%X",
> > > + id);
> > Do we have to give up here? Could it be a compatible future part?
> > If so we should fallback on what firmware told us it was + perhaps a
> > dev_info() to say we don't recognise the ID register value.
> >
>
> I typically prefer to really give up in these cases but no strong opinion... Can turn
> this into a dev_warn()...
DT maintainers generally advise carrying on and trusting the firmware.
I used to agree with you that paranoia was good but I can see there point
and we do have cases where this happened in real parts.
Jonathan
>
> - Nuno Sá
>
