> -----Original Message-----
> From: Felix Knopf <knopf@xxxxxxx>
> Sent: Tuesday, May 25, 2021 4:23 PM
> To: Li, Meng <Meng.Li@xxxxxxxxxxxxx>; Uwe Kleine-König <u.kleine-
> koenig@xxxxxxxxxxxxxx>
> Cc: lars@xxxxxxxxxx; Michael.Hennerich@xxxxxxxxxx; jic23@xxxxxxxxxx;
> pmeerw@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> iio@xxxxxxxxxxxxxxx
> 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 Wed, May 12, 2021 at 12:57:25PM +0800, Meng.Li@xxxxxxxxxxxxx
> wrote:
> >>> 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.
>
> Hi,
>
> I came across the same or a very similar issue with the ltc2497 but took a
> different approach to solve it. I suspect this issue is caused by a suboptimal
> I2C access pattern.
>
> The ltc2497 triggers a new conversion on the stop condition of transactions
> addressed to it. As the chip cannot communicate during a conversion, it will
> not ACK until it is finished. The current driver produces the following
> sequence to read from an arbitrary channel:
>
> ltc2497_result_and_measure(…, NULL);
> 1) S <ADDR> W A | <CONF> A | P (select channel)
>
> 2) [sleep 150ms] (wait for conversion)
>
> ltc2497_result_and_measure(…, val);
> 3) S <ADDR> R A | <data> … | P (read data)
> 4) S <ADDR> W N | P (chip is busy, error)
>
> Transaction 3 triggers a new conversion on the previously selected channel
> and causes the following channel select (4) to fail. The examples in the
> datasheet [1] make use of repeated start conditions to prevent unintended
> triggers. In our case, 3 and 4 should be combined into one transaction.
>
> Limeng's patch sikps 4 which solves the problem but causes issues at high
> sample rates, were 1 is skipped by the core.
>
> I attached my ad-hoc solution below.
Hi Felix,
Thanks for your new ideal firstly. I will test your patch in later.
I had a look your patch, and I found out there is no essential difference from my patch.
You put the step 4 in the else branch, I think it is the same effective with my return solution.
In additional, about the high sample rates, you pointed out that my patch will skip the step1
But I check the ltc2497 datasheet
https://www.analog.com/media/en/technical-documentation/data-sheets/2497fb.pdf
in page 18, there is a "Consecutive Reading with the Same Input/Configuration" mode, I think it is able to read data continuously without setting channel address every time.
Thanks,
Limeng
> @Limeng: Could you test this with your hardware?
>
> If there is interest, I will prepare a proper patch.
> (Should that go into a new thread then?)
>
> Regards, Felix
>
> [1] https://www.analog.com/media/en/technical-documentation/data-
> sheets/2497fb.pdf#page=18
>
> --
> Felix Knopf
> von Hoerner & Sulger GmbH
> https://vh-s.de
>
>
> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c index
> 1adddf5a88a9..8968bf70859b 100644
> --- a/drivers/iio/adc/ltc2497.c
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -34,20 +34,23 @@ static int ltc2497_result_and_measure(struct
> ltc2497core_driverdata *ddata,
> int ret;
>
> if (val) {
> - ret = i2c_master_recv(st->client, (char *)&st->buf, 3);
> + ret = i2c_smbus_read_i2c_block_data(st->client,
> + LTC2497_ENABLE | address, 3,
> + (char *)&st->buf);
> if (ret < 0) {
> - dev_err(&st->client->dev, "i2c_master_recv failed\n");
> + dev_err(&st->client->dev, "i2c transfer
> + failed\n");
> return ret;
> }
>
> *val = (be32_to_cpu(st->buf) >> 14) - (1 << 17);
> + } else {
> + ret = i2c_smbus_write_byte(st->client,
> + LTC2497_ENABLE | address);
> + if (ret)
> + dev_err(&st->client->dev, "i2c write failed: %pe\n",
> + ERR_PTR(ret));
> }
>
> - ret = i2c_smbus_write_byte(st->client,
> - LTC2497_ENABLE | address);
> - if (ret)
> - dev_err(&st->client->dev, "i2c transfer failed: %pe\n",
> - ERR_PTR(ret));
> return ret;
> }
