On Sat, 18 May 2019 22:39:09 +0300
Alexandru Ardelean <ardeleanalex@xxxxxxxxx> wrote:
> On Sat, May 18, 2019 at 1:01 PM Jonathan Cameron
> <jic23@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Fri, 17 May 2019 17:19:38 +0300
> > Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote:
> >
> > > From: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
> > >
> > > This change adds a basic verification of the EEPROM by writing a known
> > > value to the customer version ID register, and reading it back.
> > >
> > > This validates that the EEPROM & SPI communication are functioning
> > > properly, and the device is ready to use.
> > >
> > > Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> >
> > I'm going to assume that the write cycle limitations of that eeprom
> > have been taken into account here and it won't be an issue until
> > a very large number of probe and remove cycles. There is also the
> > slightly amusing possibility of breaking a customer part if someone
> > managed to pull the power whilst you have the wrong customer ID
> > in the eeprom. However your device and I assume there is a customer
> > who really wants this sanity check so fair enough...
> >
> > Applied to the togreg branch of iio.git and pushed out as testing for
> > the autobuilders to play with it.
>
> I was also a bit unsure about this patch in this form.
> But now that you've raised these points, I am now somewhat paranoid.
>
> I guess, I'll have to go back and start a internal discussion about
> this. Maybe it makes more sense to add a device[-tree] property to
> configure this somehow, and if someone really wants this behavior,
> he/she can enable it.
> This patch was also created some time ago [before I joined Analog] so
> there is some context I may be lacking here about it.
>
> Maybe let's drop this, and I can come back with a version that would
> not allow users to shoot-them-selves-in-the-foot without a safety
> mechanism off.
>
> Sorry for the noise.
Cool. Dropped for now then.
Jonathan
> Alex
>
> >
> > Thanks,
> >
> > Jonathan
> >
> > > ---
> > > drivers/iio/frequency/ad9523.c | 28 ++++++++++++++++++++++++++++
> > > 1 file changed, 28 insertions(+)
> > >
> > > diff --git a/drivers/iio/frequency/ad9523.c b/drivers/iio/frequency/ad9523.c
> > > index 9b9eee27176c..dd159a1237f3 100644
> > > --- a/drivers/iio/frequency/ad9523.c
> > > +++ b/drivers/iio/frequency/ad9523.c
> > > @@ -749,6 +749,30 @@ static int ad9523_reg_access(struct iio_dev *indio_dev,
> > > return ret;
> > > }
> > >
> > > +static int ad9523_verify_eeprom(struct iio_dev *indio_dev)
> > > +{
> > > + int ret, id;
> > > +
> > > + id = ad9523_read(indio_dev, AD9523_EEPROM_CUSTOMER_VERSION_ID);
> > > + if (id < 0)
> > > + return id;
> > > +
> > > + ret = ad9523_write(indio_dev, AD9523_EEPROM_CUSTOMER_VERSION_ID, 0xAD95);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + ret = ad9523_read(indio_dev, AD9523_EEPROM_CUSTOMER_VERSION_ID);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + if (ret != 0xAD95) {
> > > + dev_err(&indio_dev->dev, "SPI Read Verify failed (0x%X)\n", ret);
> > > + return -EIO;
> > > + }
> > > +
> > > + return ad9523_write(indio_dev, AD9523_EEPROM_CUSTOMER_VERSION_ID, id);
> > > +}
> > > +
> > > static const struct iio_info ad9523_info = {
> > > .read_raw = &ad9523_read_raw,
> > > .write_raw = &ad9523_write_raw,
> > > @@ -780,6 +804,10 @@ static int ad9523_setup(struct iio_dev *indio_dev)
> > > if (ret < 0)
> > > return ret;
> > >
> > > + ret = ad9523_verify_eeprom(indio_dev);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > /*
> > > * PLL1 Setup
> > > */
> >
