linux-kernel - RE: [PATCH] driver: adc: ltc2497: return directly after reading the adc conversion value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <PH0PR11MB5191C26AA8B6D2B855E19D67F13B9@PH0PR11MB5191.namprd11.prod.outlook.com>
Date:   Fri, 4 Jun 2021 02:16:39 +0000
From:   "Li, Meng" <Meng.Li@...driver.com>
To:     Jonathan Cameron <jic23@...nel.org>
CC:     "lars@...afoo.de" <lars@...afoo.de>,
        "Michael.Hennerich@...log.com" <Michael.Hennerich@...log.com>,
        "pmeerw@...erw.net" <pmeerw@...erw.net>,
        "u.kleine-koenig@...gutronix.de" <u.kleine-koenig@...gutronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>
Subject: RE: [PATCH] driver: adc: ltc2497: return directly after reading the
 adc conversion value



> -----Original Message-----
> From: Jonathan Cameron <jic23@...nel.org>
> Sent: Friday, June 4, 2021 12:20 AM
> To: Li, Meng <Meng.Li@...driver.com>
> Cc: lars@...afoo.de; Michael.Hennerich@...log.com;
> pmeerw@...erw.net; u.kleine-koenig@...gutronix.de; linux-
> kernel@...r.kernel.org; linux-iio@...r.kernel.org
> Subject: Re: [PATCH] driver: adc: ltc2497: return directly after reading the adc
> conversion value
> 
> [Please note: This e-mail is from an EXTERNAL e-mail address]
> 
> On Tue,  1 Jun 2021 17:28:05 +0800
> Meng.Li@...driver.com wrote:
> 
> > From: Meng Li <Meng.Li@...driver.com>
> >
> > When read adc conversion value with below command:
> > cat /sys/.../iio:device0/in_voltage0-voltage1_raw
> > There is an error reported as below:
> > ltc2497 0-0014: i2c transfer failed: -EREMOTEIO This i2c transfer
> > issue is introduced by commit 69548b7c2c4f ("iio:
> > adc: ltc2497: split protocol independent part in a separate module").
> > When extract the common code into ltc2497-core.c, it change the code
> > logic of function ltc2497core_read(). With wrong reading sequence, the
> > action of enable adc channel is sent to chip again during adc channel
> > is in conversion status. In this way, there is no ack from chip, and
> > then cause i2c transfer failed.
> > In order to keep the code logic is the same with original ideal, it is
> > need to return direct after reading the adc conversion value.
> >
> > v2:
> > According to ltc2497 datasheet, the max value of conversion time is
> > 149.9 ms. So, add 20% to the 150msecs so that there is enough time for
> > data conversion.
> 
> Version change logs go below the --- as we don't want to preserve them
> forever in the git history.
> 
> I may have lost track of the discussion, but I thought the idea was that
> perhaps the longer time period would remove the need for the early return?
> 

No!
I think the ret is essential.
Because when call i2c_master_recv() to receive data from chip, there is a stop bit sent to chip at the last step.
When the chip receive the stop bit, it will start the next conversion operation. 
So, if we send any command on I2C bus when the chip is in conversion status, there will be a i2c error occurred.

Thanks,
Limeng

> Thanks,
> 
> Jonathan
> >
> > Fixes: 69548b7c2c4f ("iio: adc: ltc2497: split protocol independent
> > part in a separate module ")
> > Cc: stable@...r.kernel.org
> > Signed-off-by: Meng Li <Meng.Li@...driver.com>
> > ---
> >  drivers/iio/adc/ltc2497.c | 2 ++
> >  drivers/iio/adc/ltc2497.h | 2 +-
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> > index 1adddf5a88a9..fd5a66860a47 100644
> > --- a/drivers/iio/adc/ltc2497.c
> > +++ b/drivers/iio/adc/ltc2497.c
> > @@ -41,6 +41,8 @@ static int ltc2497_result_and_measure(struct
> ltc2497core_driverdata *ddata,
> >               }
> >
> >               *val = (be32_to_cpu(st->buf) >> 14) - (1 << 17);
> > +
> > +             return ret;
> >       }
> >
> >       ret = i2c_smbus_write_byte(st->client, diff --git
> > a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h index
> > d0b42dd6b8ad..e451930837d8 100644
> > --- a/drivers/iio/adc/ltc2497.h
> > +++ b/drivers/iio/adc/ltc2497.h
> > @@ -2,7 +2,7 @@
> >
> >  #define LTC2497_ENABLE                       0xA0
> >  #define LTC2497_CONFIG_DEFAULT               LTC2497_ENABLE
> > -#define LTC2497_CONVERSION_TIME_MS   150ULL
> > +#define LTC2497_CONVERSION_TIME_MS   180ULL
> >
> >  struct ltc2497core_driverdata {
> >       struct regulator *ref;