* [PATCH 0/1] iio/scmi: Add reading "raw" attribute. @ 2021-09-22 6:52 Andriy Tryshnivskyy 2021-09-22 6:52 ` [PATCH 1/1] " Andriy Tryshnivskyy 0 siblings, 1 reply; 5+ messages in thread From: Andriy Tryshnivskyy @ 2021-09-22 6:52 UTC (permalink / raw) To: jbhayana, jic23 Cc: lars, linux-iio, linux-kernel, Vasyl.Vavrychuk, andriy.tryshnivskyy This patch implements reading "raw" attribute. The patch is based on v5.14. Any comments are very welcome. Thanks, Andriy. Andriy Tryshnivskyy (1): iio/scmi: Add reading "raw" attribute. drivers/iio/common/scmi_sensors/scmi_iio.c | 38 +++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) base-commit: 7d2a07b769330c34b4deabeed939325c77a7ec2f -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] iio/scmi: Add reading "raw" attribute. 2021-09-22 6:52 [PATCH 0/1] iio/scmi: Add reading "raw" attribute Andriy Tryshnivskyy @ 2021-09-22 6:52 ` Andriy Tryshnivskyy 2021-09-25 14:43 ` Jonathan Cameron 0 siblings, 1 reply; 5+ messages in thread From: Andriy Tryshnivskyy @ 2021-09-22 6:52 UTC (permalink / raw) To: jbhayana, jic23 Cc: lars, linux-iio, linux-kernel, Vasyl.Vavrychuk, andriy.tryshnivskyy Add IIO_CHAN_INFO_RAW to the mask and implement corresponding reading "raw" attribute in scmi_iio_read_raw. Signed-off-by: Andriy Tryshnivskyy <andriy.tryshnivskyy@opensynergy.com> --- drivers/iio/common/scmi_sensors/scmi_iio.c | 38 +++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/iio/common/scmi_sensors/scmi_iio.c b/drivers/iio/common/scmi_sensors/scmi_iio.c index 7cf2bf282cef..b88780a25796 100644 --- a/drivers/iio/common/scmi_sensors/scmi_iio.c +++ b/drivers/iio/common/scmi_sensors/scmi_iio.c @@ -286,6 +286,9 @@ static int scmi_iio_read_raw(struct iio_dev *iio_dev, struct scmi_iio_priv *sensor = iio_priv(iio_dev); s8 scale; int ret; + int err; + u32 sensor_config; + struct scmi_sensor_reading readings[SCMI_IIO_NUM_OF_AXIS]; switch (mask) { case IIO_CHAN_INFO_SCALE: @@ -300,6 +303,38 @@ static int scmi_iio_read_raw(struct iio_dev *iio_dev, case IIO_CHAN_INFO_SAMP_FREQ: ret = scmi_iio_get_odr_val(iio_dev, val, val2); return ret ? ret : IIO_VAL_INT_PLUS_MICRO; + case IIO_CHAN_INFO_RAW: + sensor_config = FIELD_PREP(SCMI_SENS_CFG_SENSOR_ENABLED_MASK, + SCMI_SENS_CFG_SENSOR_ENABLE); + err = sensor->handle->sensor_ops->config_set( + sensor->handle, 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); + + err = sensor->handle->sensor_ops->reading_get_timestamped( + sensor->handle, sensor->sensor_info->id, + sensor->sensor_info->num_axis, + (struct scmi_sensor_reading *)&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->handle->sensor_ops->config_set( + sensor->handle, 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); + /* Use 32-bit value, since practically there is no need in 64 bits */ + *val = (u32)readings[ch->scan_index].value; + return IIO_VAL_INT; default: return -EINVAL; } @@ -381,7 +416,8 @@ static void scmi_iio_set_data_channel(struct iio_chan_spec *iio_chan, iio_chan->type = type; iio_chan->modified = 1; iio_chan->channel2 = mod; - iio_chan->info_mask_separate = BIT(IIO_CHAN_INFO_SCALE); + iio_chan->info_mask_separate = + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_RAW); iio_chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ); iio_chan->info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] iio/scmi: Add reading "raw" attribute. 2021-09-22 6:52 ` [PATCH 1/1] " Andriy Tryshnivskyy @ 2021-09-25 14:43 ` Jonathan Cameron [not found] ` <18401c96-a88e-c7fa-8f20-56605945891d@opensynergy.com> 0 siblings, 1 reply; 5+ messages in thread From: Jonathan Cameron @ 2021-09-25 14:43 UTC (permalink / raw) To: Andriy Tryshnivskyy Cc: jbhayana, lars, linux-iio, linux-kernel, Vasyl.Vavrychuk On Wed, 22 Sep 2021 09:52:35 +0300 Andriy Tryshnivskyy <andriy.tryshnivskyy@opensynergy.com> wrote: > Add IIO_CHAN_INFO_RAW to the mask and implement corresponding > reading "raw" attribute in scmi_iio_read_raw. > > Signed-off-by: Andriy Tryshnivskyy <andriy.tryshnivskyy@opensynergy.com> Hi Andriy, A few comments inline. I don't haven't looked closely at whether the scmi side of things is correct though so will rely on Jyoti for that. Thanks, Jonathan > --- > drivers/iio/common/scmi_sensors/scmi_iio.c | 38 +++++++++++++++++++++- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/common/scmi_sensors/scmi_iio.c b/drivers/iio/common/scmi_sensors/scmi_iio.c > index 7cf2bf282cef..b88780a25796 100644 > --- a/drivers/iio/common/scmi_sensors/scmi_iio.c > +++ b/drivers/iio/common/scmi_sensors/scmi_iio.c > @@ -286,6 +286,9 @@ static int scmi_iio_read_raw(struct iio_dev *iio_dev, > struct scmi_iio_priv *sensor = iio_priv(iio_dev); > s8 scale; > int ret; > + int err; > + u32 sensor_config; > + struct scmi_sensor_reading readings[SCMI_IIO_NUM_OF_AXIS]; > > switch (mask) { > case IIO_CHAN_INFO_SCALE: > @@ -300,6 +303,38 @@ static int scmi_iio_read_raw(struct iio_dev *iio_dev, > case IIO_CHAN_INFO_SAMP_FREQ: > ret = scmi_iio_get_odr_val(iio_dev, val, val2); > return ret ? ret : IIO_VAL_INT_PLUS_MICRO; > + case IIO_CHAN_INFO_RAW: > + sensor_config = FIELD_PREP(SCMI_SENS_CFG_SENSOR_ENABLED_MASK, > + SCMI_SENS_CFG_SENSOR_ENABLE); > + err = sensor->handle->sensor_ops->config_set( > + sensor->handle, 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); Don't try to carry on if you got an error. Unless something very odd has gone on that should mean the sensor isn't not enabled so nothing else will work. If this returns and error just return it here. > + > + err = sensor->handle->sensor_ops->reading_get_timestamped( > + sensor->handle, sensor->sensor_info->id, > + sensor->sensor_info->num_axis, > + (struct scmi_sensor_reading *)&readings); Why do we need the cast? Also, this should probably just be readings unless I'm missing something rather than the address of readings. That will pass the address of the first element which is likely to be what you want. > + 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->handle->sensor_ops->config_set( > + sensor->handle, 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); As above, this indicates something has gone wrong and we should tell userspace that in preference to trying to get one last value out. > + /* Use 32-bit value, since practically there is no need in 64 bits */ > + *val = (u32)readings[ch->scan_index].value; We should check it fits and if doesn't return an error rather than pretending all was fine. > + return IIO_VAL_INT; > default: > return -EINVAL; > } > @@ -381,7 +416,8 @@ static void scmi_iio_set_data_channel(struct iio_chan_spec *iio_chan, > iio_chan->type = type; > iio_chan->modified = 1; > iio_chan->channel2 = mod; > - iio_chan->info_mask_separate = BIT(IIO_CHAN_INFO_SCALE); > + iio_chan->info_mask_separate = > + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_RAW); > iio_chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ); > iio_chan->info_mask_shared_by_type_available = > BIT(IIO_CHAN_INFO_SAMP_FREQ); ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <18401c96-a88e-c7fa-8f20-56605945891d@opensynergy.com>]
* Re: [PATCH 1/1] iio/scmi: Add reading "raw" attribute. [not found] ` <18401c96-a88e-c7fa-8f20-56605945891d@opensynergy.com> @ 2021-09-30 15:26 ` Jonathan Cameron 2021-10-01 6:59 ` Andriy Tryshnivskyy 0 siblings, 1 reply; 5+ messages in thread From: Jonathan Cameron @ 2021-09-30 15:26 UTC (permalink / raw) To: Andriy Tryshnivskyy Cc: jbhayana, lars, linux-iio, linux-kernel, Vasyl.Vavrychuk > >> + /* Use 32-bit value, since practically there is no need in 64 bits */ > >> + *val = (u32)readings[ch->scan_index].value; > > We should check it fits and if doesn't return an error rather than pretending all was fine. > > > What kind of error to return in the case it does not fit - ENOEXEC (Exec format error) ? That one is a bit obscure. I'd go with either -ERANGE I think as we are reflecting that the clamp to 32 bit is not representable. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] iio/scmi: Add reading "raw" attribute. 2021-09-30 15:26 ` Jonathan Cameron @ 2021-10-01 6:59 ` Andriy Tryshnivskyy 0 siblings, 0 replies; 5+ messages in thread From: Andriy Tryshnivskyy @ 2021-10-01 6:59 UTC (permalink / raw) To: Jonathan Cameron; +Cc: jbhayana, lars, linux-iio, linux-kernel, Vasyl.Vavrychuk >>>> + /* Use 32-bit value, since practically there is no need in 64 bits */ >>>> + *val = (u32)readings[ch->scan_index].value; >>> We should check it fits and if doesn't return an error rather than pretending all was fine. >>> >> What kind of error to return in the case it does not fit - ENOEXEC (Exec format error) ? > That one is a bit obscure. > > I'd go with either -ERANGE I think as we are reflecting that the clamp to 32 bit > is not representable. Thanks for hint. Later I've found -ERANGE also and used it in my next patch versions. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-01 7:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 6:52 [PATCH 0/1] iio/scmi: Add reading "raw" attribute Andriy Tryshnivskyy
2021-09-22 6:52 ` [PATCH 1/1] " Andriy Tryshnivskyy
2021-09-25 14:43 ` Jonathan Cameron
[not found] ` <18401c96-a88e-c7fa-8f20-56605945891d@opensynergy.com>
2021-09-30 15:26 ` Jonathan Cameron
2021-10-01 6:59 ` Andriy Tryshnivskyy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).