lkml.org 
[lkml]   [2017]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] iio/adc/ltc2497: Driver for Linear Technology LTC2497 ADC
From
Date
On 23.03.2017 12:05, Lars-Peter Clausen wrote:

Sorry - I missed some of this review feedback ...

>> +
>> +static int ltc2497_wait_conv(struct ltc2497_st *st)
>> +{
>> + s64 time_elapsed;
>> +
>> + time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev);
>> +
>> + if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
>> + /* delay if conversion time not passed
>> + * since last read or write
>> + */
>> + msleep(LTC2497_CONVERSION_TIME_MS - time_elapsed);
>
> Considering how long this sleeps msleep_interruptible() might be the better
> choice.

Wondering what should be the outcome of this?
We can't simply replace it. Actually I've seen cases here in drivers/iio
where the delay is essential, but the return value of
msleep_interruptible() is not being checked.
Thus causing a malicious access, in case a signal is received.

We must delay here. If we switch to msleep_interruptible() the only
reason for this would be to cancel the read and return -EINTR to the user.

Also there is another msleep below which would also need this kind of
handling.

>
>> + return 0;
>> + }
>> +
>> + if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
>> + /* We're in automatic mode -
>> + * so the last reading is stil not outdated
>> + */
>> + return 0;
>> + }
>> +
>> + return -ETIMEDOUT;
>> +}
>> +
>> +static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val)
>> +{
>> + struct i2c_client *client = st->client;
>> + __be32 buf = 0;
>
> transfer buffers must not be on the stack to avoid issues if the controller
> should use DMA.
>
>> + int ret;
>> +
>> + ret = ltc2497_wait_conv(st);
>> + if (ret < 0 || st->addr_prev != address) {
>> + ret = i2c_smbus_write_byte(st->client, 0xA0 | address);
>> + if (ret < 0)
>> + return ret;
>> + st->addr_prev = address;
>> + msleep(LTC2497_CONVERSION_TIME_MS);
>> + }
>> + ret = i2c_master_recv(client, (char *)&buf, 3);
>> + if (ret < 0) {
>> + dev_err(&client->dev, "i2c_master_recv failed\n");
>> + return ret;
>> + }
>> + st->time_prev = ktime_get();
>> + *val = (be32_to_cpu(buf) >> 14) - (1 << 17);
>> +
>> + return ret;
>> +}
> [...]
>


--
Greetings,
Michael

--
Analog Devices GmbH Otl-Aicher Strasse 60-64 80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

\
 
 \ /
  Last update: 2017-03-29 10:51    [W:0.164 / U:0.432 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site