> -----Original Message----- > From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > Sent: Wednesday, June 14, 2023 5:12 PM > To: Paller, Kim Seer <KimSeer.Paller@xxxxxxxxxx> > Cc: jic23@xxxxxxxxxx; lars@xxxxxxxxxx; lgirdwood@xxxxxxxxx; > broonie@xxxxxxxxxx; Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx>; > robh@xxxxxxxxxx; krzysztof.kozlowski@xxxxxxxxxx; conor+dt@xxxxxxxxxx; linux- > iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v6 2/2] iio: adc: max14001: New driver > > [External] > > On Wed, Jun 14, 2023 at 3:49 AM Kim Seer Paller > <kimseer.paller@xxxxxxxxxx> wrote: > > > > The MAX14001 is configurable, isolated 10-bit ADCs for multi-range > > binary inputs. > > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> One question > below. > > > Signed-off-by: Kim Seer Paller <kimseer.paller@xxxxxxxxxx> > > --- ... > > + /* select external voltage reference source for the ADC */ > > + ret = max14001_reg_update(st, MAX14001_CFG, > > + MAX14001_CFG_EXRF, 1); > > + > > + ret = regulator_get_voltage(vref); > > + if (ret < 0) > > + return dev_err_probe(dev, ret, > > + "Failed to get vref\n"); > > Is it important to choose the external reference source _before_ getting the > voltage of the regulator? If not, I would swap these calls, otherwise the > comment is needed to explain why the sequence is important in the way it's > written. It is not important. These calls can be swap without any issues. Best regards, Kim Seer Paller