Messages in this thread | | | Date | Thu, 17 Nov 2022 16:30:40 +0000 | From | Jonathan Cameron <> | Subject | Re: [PATCH v10] iio: temperature: Add driver support for Maxim MAX30208 |
| |
On Fri, 18 Nov 2022 21:07:29 +0530 Rajat Khandelwal <rajat.khandelwal@linux.intel.com> wrote:
> Maxim MAX30208 is a digital temperature sensor with 0.1°C accuracy. > > Add support for max30208 driver in iio subsystem.
Blank line here.
> Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX30208.pdf > Datasheet part of the tags block, so no blank line between that and the SoB. That makes life easy for tools parsing git messages.
> Signed-off-by: Rajat Khandelwal <rajat.khandelwal@linux.intel.com>
One query inline. Basically boils down to what we do after overflow occurs. I assume you are right and the first reading is the most recent, but I think we still want to flush the whole fifo in that case to get back to a sane state for future reads.
Jonathan
> +/** > + * max30208_request() - Request a reading > + * @data: Struct comprising member elements of the device > + * > + * Requests a reading from the device and waits until the conversion is ready. > + */ > +static int max30208_request(struct max30208_data *data) > +{ > + /* > + * Sensor can take up to 500 ms to respond so execute a total of > + * 10 retries to give the device sufficient time. > + */ > + int retries = 10; > + u8 regval; > + int ret; > + > + ret = i2c_smbus_read_byte_data(data->client, MAX30208_TEMP_SENSOR_SETUP); > + if (ret < 0) > + return ret; > + > + regval = ret | MAX30208_TEMP_SENSOR_SETUP_CONV; > + > + ret = i2c_smbus_write_byte_data(data->client, MAX30208_TEMP_SENSOR_SETUP, regval); > + if (ret) > + return ret; > + > + while (retries--) { > + ret = i2c_smbus_read_byte_data(data->client, MAX30208_STATUS); > + if (ret < 0) > + return ret; > + > + if (ret & MAX30208_STATUS_TEMP_RDY) > + return 0; > + > + msleep(50); > + } > + dev_err(&data->client->dev, "Temperature conversion failed\n"); > + > + return -ETIMEDOUT; > +} > + > +static int max30208_update_temp(struct max30208_data *data) > +{ > + u8 data_count; > + int ret; > + > + mutex_lock(&data->lock); > + > + ret = max30208_request(data); > + if (ret) > + goto unlock; > + > + ret = i2c_smbus_read_byte_data(data->client, MAX30208_FIFO_OVF_CNTR); > + if (ret < 0) > + goto unlock; > + else if (!ret) { > + ret = i2c_smbus_read_byte_data(data->client, MAX30208_FIFO_DATA_CNTR); > + if (ret < 0) > + goto unlock; > + > + data_count = ret; > + } else > + data_count = 1; > + > + while (data_count) { > + ret = i2c_smbus_read_word_swapped(data->client, MAX30208_FIFO_DATA); > + if (ret < 0) > + goto unlock; > + > + data_count--; > + } Hmm. Given you've been poking this a lot, I guess this works and the part is as just odd. Just to check one last case... Does max30208_request() guarantee we can't get...
1. Read first time, overflow set so we read latest result - leaving 31 ancient values in the fifo. 2. Read again really quickly and get those ancient values. ?
Perhaps we should flush out those unwanted values from the fifo, so after overflow we get back to a normal state rather than immediately overflowing again.
More than possible that I still don't understand how this device works though!
> + > +unlock: > + mutex_unlock(&data->lock); > + return ret; > +} > +
| |