On Mon, 2024-06-03 at 09:22 +0800, Kim Seer Paller wrote: > LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC > LTC2672 5 channel, 16 bit Current Output Softspan DAC > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Closes: https://lore.kernel.org/oe-kbuild-all/202405241141.kYcxrSem-lkp@xxxxxxxxx/ The above could be dropped. This is still not merged code :) > Co-developed-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx> > Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx> > Signed-off-by: Kim Seer Paller <kimseer.paller@xxxxxxxxxx> > --- LGTM... just a couple of minor points/questions that you can maybe take on if a re- spin is needed. Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx> ... > > +static int ltc2664_scale_get(const struct ltc2664_state *st, int c) > +{ > + const struct ltc2664_chan *chan = &st->channels[c]; > + const int (*span_helper)[2] = st->chip_info->span_helper; > + int span, fs; > + > + span = chan->span; > + if (span < 0) > + return span; > + > + fs = span_helper[span][1] - span_helper[span][0]; > + > + return (fs / 2500) * st->vref; no need for () ... > > +static int ltc2664_channel_config(struct ltc2664_state *st) > +{ > + const struct ltc2664_chip_info *chip_info = st->chip_info; > + struct device *dev = &st->spi->dev; > + u32 reg, tmp[2], mspan; > + int ret, span = 0; > + > + mspan = LTC2664_MSPAN_SOFTSPAN; > + ret = device_property_read_u32(dev, "adi,manual-span-operation-config", > + &mspan); > + if (!ret) { > + if (!chip_info->manual_span_support) > + return dev_err_probe(dev, -EINVAL, > + "adi,manual-span-operation-config not > supported\n"); > + > + if (mspan > ARRAY_SIZE(ltc2664_mspan_lut)) > + return dev_err_probe(dev, -EINVAL, > + "adi,manual-span-operation-config not in range\n"); > + } > + > + st->rfsadj = 20000; > + ret = device_property_read_u32(dev, "adi,rfsadj-ohms", &st->rfsadj); > + if (!ret) { > + if (!chip_info->rfsadj_support) > + return dev_err_probe(dev, -EINVAL, > + "adi,rfsadj-ohms not supported\n"); > + > + if (st->rfsadj < 19000 || st->rfsadj > 41000) > + return dev_err_probe(dev, -EINVAL, > + "adi,rfsadj-ohms not in range\n"); > + } > + > + device_for_each_child_node_scoped(dev, child) { > + struct ltc2664_chan *chan; > + > + ret = fwnode_property_read_u32(child, "reg", ®); > + if (ret) > + return dev_err_probe(dev, ret, > + "Failed to get reg property\n"); > + > + if (reg >= chip_info->num_channels) > + return dev_err_probe(dev, -EINVAL, > + "reg bigger than: %d\n", > + chip_info->num_channels); > + > + chan = &st->channels[reg]; > + > + if (fwnode_property_read_bool(child, "adi,toggle-mode")) { > + chan->toggle_chan = true; > + /* assume sw toggle ABI */ Do we have any other option :)? For the ltc2668 driver (where this code came from), we do have another way (driven by clocks) to toggle between outputs and hence the comment. BTW, there's a fair amount of duplicated code between this and ltc2668. At some point we may see if it makes sense to add some common module. Anyways, fine for now. - Nuno Sá
