> > + > > +static int ad4000_convert_and_acquire(struct ad4000_state *st) > > +{ > > + int ret; > > + > > + /* > > + * In 4-wire mode, the CNV line is held high for the entire > > conversion > > + * and acquisition process. In other modes st->cnv_gpio is NULL and > > is > > + * ignored (CS is wired to CNV in those cases). > > + */ > > + gpiod_set_value_cansleep(st->cnv_gpio, 1); > > Not sure it's a good practise to assume internal details as you're going for > GPIO. I would prefer to have an explicit check for st->cnv_gpio being NULL or > not. Hmm. I had it in my head that this was documented behaviour, but I can't find such in the docs, so agreed checking it makes sense. I would be very surprised if this ever changed as it's one of the things that makes optional gpios easy to work with but who knows! +CC Linus and Bartosz for feedback on this one. > > > + ret = spi_sync(st->spi, &st->msg); > > + gpiod_set_value_cansleep(st->cnv_gpio, 0); > > + > > + return ret; > > +} > > + > > +static int ad4000_config(struct ad4000_state *st) > > +{ > > + unsigned int reg_val; > > + > > + if (device_property_present(&st->spi->dev, "adi,high-z-input")) > > + reg_val |= FIELD_PREP(AD4000_CFG_HIGHZ, 1); > > + > > + /* > > + * The ADC SDI pin might be connected to controller CS line in which > > + * case the write might fail. This, however, does not prevent the > > device > > + * from functioning even though in a configuration other than the > > + * requested one. > > + */ > > This raises the question if there's any way to describe that through DT (if not > doing it already)? So that, if SDI is connected to CS we don't even call this? > Other question that comes to mind is that in case SDI is connected to CS, will > all writes fail? Because if that's the case we other writes (like scale) that > won't work and we should take care of that... Definitely needs describing and all configuration sysfs etc needs to be read only if we can't control it. > > > + return ad4000_write_reg(st, reg_val); > > +} > > + Jonathan