Messages in this thread | | | Date | Tue, 5 Oct 2021 15:51:57 +0300 | Subject | Re: [PATCH v3 0/1] iio/scmi: Add reading "raw" attribute. | From | Vasyl Vavrychuk <> |
| |
Hi, Jyoti,
> In the code below, why is the logic of enabling and disabling the > sensor in this function? Generally the function to read the sensor > value is just used for the code to read the sensor values ? and not > enable/disable the sensor
But to read sensor value we have to enable it first. Other way to enable sensor we found is, for example:
echo 1 > /sys/bus/iio/devices/.../scan_elements/in_anglvel_x_en
But, this command is related to IIO buffers use.
Other sensors drivers enable/disable sensor in read raw too, for example, drivers/iio/accel/kxcjk-1013.c has:
case IIO_CHAN_INFO_RAW: mutex_lock(&data->mutex); if (iio_buffer_enabled(indio_dev)) ret = -EBUSY; else { ret = kxcjk1013_set_power_state(data, true); ... reading ... ret = kxcjk1013_set_power_state(data, false); } mutex_unlock(&data->mutex);
But, after looking on this code I have some questions:
1. Should we block reading raw attribute and IIO buffer enabled, for for SCMI sensor it can coexist? 2. Should we wrap reading raw attribute implementation in iio_dev->mlock mutex?
>> case IIO_CHAN_INFO_RAW: >> + sensor_config = FIELD_PREP(SCMI_SENS_CFG_SENSOR_ENABLED_MASK, >> + SCMI_SENS_CFG_SENSOR_ENABLE); >> + err = sensor->sensor_ops->config_set( >> + sensor->ph, sensor->sensor_info->id, sensor_config); >> + if (err) { >> + dev_err(&iio_dev->dev, >> + "Error in enabling sensor %s err %d", >> + sensor->sensor_info->name, err); >> + return err; >> + } >> + >> + err = sensor->sensor_ops->reading_get_timestamped( >> + sensor->ph, sensor->sensor_info->id, >> + sensor->sensor_info->num_axis, readings); >> + if (err) { >> + dev_err(&iio_dev->dev, >> + "Error in reading raw attribute for sensor %s err %d", >> + sensor->sensor_info->name, err); >> + return err; >> + } >> + >> + sensor_config = FIELD_PREP(SCMI_SENS_CFG_SENSOR_ENABLED_MASK, >> + SCMI_SENS_CFG_SENSOR_DISABLE); >> + err = sensor->sensor_ops->config_set( >> + sensor->ph, sensor->sensor_info->id, sensor_config); >> + if (err) { >> + dev_err(&iio_dev->dev, >> + "Error in enabling sensor %s err %d", >> + sensor->sensor_info->name, err); >> + return err; >> + } >> + /* Check if raw value fits 32 bits */ >> + if (readings[ch->scan_index].value < INT_MIN || >> + readings[ch->scan_index].value > INT_MAX) >> + return -ERANGE; >> + /* Use 32-bit value, since practically there is no need in 64 bits */ >> + *val = (int)readings[ch->scan_index].value; >> >> + return IIO_VAL_INT;
| |