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.
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
> > */
>
