| Message ID | 20201123144017.18311-1-alexandru.ardelean@analog.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] iio: cros_ec: do an early exit if not physical_device case | expand |
On Mon, 23 Nov 2020 16:40:16 +0200 Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > This whole code-block was put under one big if() condition/block. > This change does an early return if the 'physical_device' boolean is false, > thus unindenting the block by one level. > > No other functional change has been done. > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> @Gwendal, others This series from Alex has been outstanding for a while but may well still apply. Ideally looking for an ack. Thanks, Jonathan > --- > .../cros_ec_sensors/cros_ec_sensors_core.c | 161 +++++++++--------- > 1 file changed, 81 insertions(+), 80 deletions(-) > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > index 5c6c4e6fec9b..9470014936f2 100644 > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > @@ -287,89 +287,90 @@ int cros_ec_sensors_core_init(struct platform_device *pdev, > > indio_dev->name = pdev->name; > > - if (physical_device) { > - state->param.cmd = MOTIONSENSE_CMD_INFO; > - state->param.info.sensor_num = sensor_platform->sensor_num; > - ret = cros_ec_motion_send_host_cmd(state, 0); > - if (ret) { > - dev_warn(dev, "Can not access sensor info\n"); > + if (!physical_device) > + return 0; > + > + state->param.cmd = MOTIONSENSE_CMD_INFO; > + state->param.info.sensor_num = sensor_platform->sensor_num; > + ret = cros_ec_motion_send_host_cmd(state, 0); > + if (ret) { > + dev_warn(dev, "Can not access sensor info\n"); > + return ret; > + } > + state->type = state->resp->info.type; > + state->loc = state->resp->info.location; > + > + /* Set sign vector, only used for backward compatibility. */ > + memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); > + > + for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) > + state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; > + > + /* 0 is a correct value used to stop the device */ > + if (state->msg->version < 3) { > + get_default_min_max_freq(state->resp->info.type, > + &frequencies[1], > + &frequencies[2], > + &state->fifo_max_event_count); > + } else { > + frequencies[1] = state->resp->info_3.min_frequency; > + frequencies[2] = state->resp->info_3.max_frequency; > + state->fifo_max_event_count = > + state->resp->info_3.fifo_max_event_count; > + } > + for (i = 0; i < ARRAY_SIZE(frequencies); i++) { > + state->frequencies[2 * i] = frequencies[i] / 1000; > + state->frequencies[2 * i + 1] = > + (frequencies[i] % 1000) * 1000; > + } > + > + if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { > + /* > + * Create a software buffer, feed by the EC FIFO. > + * We can not use trigger here, as events are generated > + * as soon as sample_frequency is set. > + */ > + struct iio_buffer *buffer; > + > + buffer = devm_iio_kfifo_allocate(dev); > + if (!buffer) > + return -ENOMEM; > + > + iio_device_attach_buffer(indio_dev, buffer); > + indio_dev->modes = INDIO_BUFFER_SOFTWARE; > + > + ret = cros_ec_sensorhub_register_push_data( > + sensor_hub, sensor_platform->sensor_num, > + indio_dev, push_data); > + if (ret) > return ret; > - } > - state->type = state->resp->info.type; > - state->loc = state->resp->info.location; > > - /* Set sign vector, only used for backward compatibility. */ > - memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); > + ret = devm_add_action_or_reset( > + dev, cros_ec_sensors_core_clean, pdev); > + if (ret) > + return ret; > > - for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) > - state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; > - > - /* 0 is a correct value used to stop the device */ > - if (state->msg->version < 3) { > - get_default_min_max_freq(state->resp->info.type, > - &frequencies[1], > - &frequencies[2], > - &state->fifo_max_event_count); > - } else { > - frequencies[1] = state->resp->info_3.min_frequency; > - frequencies[2] = state->resp->info_3.max_frequency; > - state->fifo_max_event_count = > - state->resp->info_3.fifo_max_event_count; > - } > - for (i = 0; i < ARRAY_SIZE(frequencies); i++) { > - state->frequencies[2 * i] = frequencies[i] / 1000; > - state->frequencies[2 * i + 1] = > - (frequencies[i] % 1000) * 1000; > - } > - > - if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { > - /* > - * Create a software buffer, feed by the EC FIFO. > - * We can not use trigger here, as events are generated > - * as soon as sample_frequency is set. > - */ > - struct iio_buffer *buffer; > - > - buffer = devm_iio_kfifo_allocate(dev); > - if (!buffer) > - return -ENOMEM; > - > - iio_device_attach_buffer(indio_dev, buffer); > - indio_dev->modes = INDIO_BUFFER_SOFTWARE; > - > - ret = cros_ec_sensorhub_register_push_data( > - sensor_hub, sensor_platform->sensor_num, > - indio_dev, push_data); > - if (ret) > - return ret; > - > - ret = devm_add_action_or_reset( > - dev, cros_ec_sensors_core_clean, pdev); > - if (ret) > - return ret; > - > - /* Timestamp coming from FIFO are in ns since boot. */ > - ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); > - if (ret) > - return ret; > - } else { > - const struct attribute **fifo_attrs; > - > - if (has_hw_fifo) > - fifo_attrs = cros_ec_sensor_fifo_attributes; > - else > - fifo_attrs = NULL; > - > - /* > - * The only way to get samples in buffer is to set a > - * software trigger (systrig, hrtimer). > - */ > - ret = devm_iio_triggered_buffer_setup_ext( > - dev, indio_dev, NULL, trigger_capture, > - NULL, fifo_attrs); > - if (ret) > - return ret; > - } > + /* Timestamp coming from FIFO are in ns since boot. */ > + ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); > + if (ret) > + return ret; > + } else { > + const struct attribute **fifo_attrs; > + > + if (has_hw_fifo) > + fifo_attrs = cros_ec_sensor_fifo_attributes; > + else > + fifo_attrs = NULL; > + > + /* > + * The only way to get samples in buffer is to set a > + * software trigger (systrig, hrtimer). > + */ > + ret = devm_iio_triggered_buffer_setup_ext( > + dev, indio_dev, NULL, trigger_capture, > + NULL, fifo_attrs); > + if (ret) > + return ret; > } > > return 0;
Hi all, On 21/2/21 17:29, Jonathan Cameron wrote: > On Mon, 23 Nov 2020 16:40:16 +0200 > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > >> This whole code-block was put under one big if() condition/block. >> This change does an early return if the 'physical_device' boolean is false, >> thus unindenting the block by one level. >> >> No other functional change has been done. >> >> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > @Gwendal, others This series from Alex has been outstanding for a while > but may well still apply. > Ideally looking for an ack. > This looks good to me. Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Thanks, Enric > Thanks, > > Jonathan > >> --- >> .../cros_ec_sensors/cros_ec_sensors_core.c | 161 +++++++++--------- >> 1 file changed, 81 insertions(+), 80 deletions(-) >> >> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c >> index 5c6c4e6fec9b..9470014936f2 100644 >> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c >> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c >> @@ -287,89 +287,90 @@ int cros_ec_sensors_core_init(struct platform_device *pdev, >> >> indio_dev->name = pdev->name; >> >> - if (physical_device) { >> - state->param.cmd = MOTIONSENSE_CMD_INFO; >> - state->param.info.sensor_num = sensor_platform->sensor_num; >> - ret = cros_ec_motion_send_host_cmd(state, 0); >> - if (ret) { >> - dev_warn(dev, "Can not access sensor info\n"); >> + if (!physical_device) >> + return 0; >> + >> + state->param.cmd = MOTIONSENSE_CMD_INFO; >> + state->param.info.sensor_num = sensor_platform->sensor_num; >> + ret = cros_ec_motion_send_host_cmd(state, 0); >> + if (ret) { >> + dev_warn(dev, "Can not access sensor info\n"); >> + return ret; >> + } >> + state->type = state->resp->info.type; >> + state->loc = state->resp->info.location; >> + >> + /* Set sign vector, only used for backward compatibility. */ >> + memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); >> + >> + for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) >> + state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; >> + >> + /* 0 is a correct value used to stop the device */ >> + if (state->msg->version < 3) { >> + get_default_min_max_freq(state->resp->info.type, >> + &frequencies[1], >> + &frequencies[2], >> + &state->fifo_max_event_count); >> + } else { >> + frequencies[1] = state->resp->info_3.min_frequency; >> + frequencies[2] = state->resp->info_3.max_frequency; >> + state->fifo_max_event_count = >> + state->resp->info_3.fifo_max_event_count; >> + } >> + for (i = 0; i < ARRAY_SIZE(frequencies); i++) { >> + state->frequencies[2 * i] = frequencies[i] / 1000; >> + state->frequencies[2 * i + 1] = >> + (frequencies[i] % 1000) * 1000; >> + } >> + >> + if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { >> + /* >> + * Create a software buffer, feed by the EC FIFO. >> + * We can not use trigger here, as events are generated >> + * as soon as sample_frequency is set. >> + */ >> + struct iio_buffer *buffer; >> + >> + buffer = devm_iio_kfifo_allocate(dev); >> + if (!buffer) >> + return -ENOMEM; >> + >> + iio_device_attach_buffer(indio_dev, buffer); >> + indio_dev->modes = INDIO_BUFFER_SOFTWARE; >> + >> + ret = cros_ec_sensorhub_register_push_data( >> + sensor_hub, sensor_platform->sensor_num, >> + indio_dev, push_data); >> + if (ret) >> return ret; >> - } >> - state->type = state->resp->info.type; >> - state->loc = state->resp->info.location; >> >> - /* Set sign vector, only used for backward compatibility. */ >> - memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); >> + ret = devm_add_action_or_reset( >> + dev, cros_ec_sensors_core_clean, pdev); >> + if (ret) >> + return ret; >> >> - for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) >> - state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; >> - >> - /* 0 is a correct value used to stop the device */ >> - if (state->msg->version < 3) { >> - get_default_min_max_freq(state->resp->info.type, >> - &frequencies[1], >> - &frequencies[2], >> - &state->fifo_max_event_count); >> - } else { >> - frequencies[1] = state->resp->info_3.min_frequency; >> - frequencies[2] = state->resp->info_3.max_frequency; >> - state->fifo_max_event_count = >> - state->resp->info_3.fifo_max_event_count; >> - } >> - for (i = 0; i < ARRAY_SIZE(frequencies); i++) { >> - state->frequencies[2 * i] = frequencies[i] / 1000; >> - state->frequencies[2 * i + 1] = >> - (frequencies[i] % 1000) * 1000; >> - } >> - >> - if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { >> - /* >> - * Create a software buffer, feed by the EC FIFO. >> - * We can not use trigger here, as events are generated >> - * as soon as sample_frequency is set. >> - */ >> - struct iio_buffer *buffer; >> - >> - buffer = devm_iio_kfifo_allocate(dev); >> - if (!buffer) >> - return -ENOMEM; >> - >> - iio_device_attach_buffer(indio_dev, buffer); >> - indio_dev->modes = INDIO_BUFFER_SOFTWARE; >> - >> - ret = cros_ec_sensorhub_register_push_data( >> - sensor_hub, sensor_platform->sensor_num, >> - indio_dev, push_data); >> - if (ret) >> - return ret; >> - >> - ret = devm_add_action_or_reset( >> - dev, cros_ec_sensors_core_clean, pdev); >> - if (ret) >> - return ret; >> - >> - /* Timestamp coming from FIFO are in ns since boot. */ >> - ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); >> - if (ret) >> - return ret; >> - } else { >> - const struct attribute **fifo_attrs; >> - >> - if (has_hw_fifo) >> - fifo_attrs = cros_ec_sensor_fifo_attributes; >> - else >> - fifo_attrs = NULL; >> - >> - /* >> - * The only way to get samples in buffer is to set a >> - * software trigger (systrig, hrtimer). >> - */ >> - ret = devm_iio_triggered_buffer_setup_ext( >> - dev, indio_dev, NULL, trigger_capture, >> - NULL, fifo_attrs); >> - if (ret) >> - return ret; >> - } >> + /* Timestamp coming from FIFO are in ns since boot. */ >> + ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); >> + if (ret) >> + return ret; >> + } else { >> + const struct attribute **fifo_attrs; >> + >> + if (has_hw_fifo) >> + fifo_attrs = cros_ec_sensor_fifo_attributes; >> + else >> + fifo_attrs = NULL; >> + >> + /* >> + * The only way to get samples in buffer is to set a >> + * software trigger (systrig, hrtimer). >> + */ >> + ret = devm_iio_triggered_buffer_setup_ext( >> + dev, indio_dev, NULL, trigger_capture, >> + NULL, fifo_attrs); >> + if (ret) >> + return ret; >> } >> >> return 0; >
On Tue, 2 Mar 2021 11:46:06 +0100 Enric Balletbo i Serra <enric.balletbo@collabora.com> wrote: > Hi all, > > On 21/2/21 17:29, Jonathan Cameron wrote: > > On Mon, 23 Nov 2020 16:40:16 +0200 > > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > > >> This whole code-block was put under one big if() condition/block. > >> This change does an early return if the 'physical_device' boolean is false, > >> thus unindenting the block by one level. > >> > >> No other functional change has been done. > >> > >> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > @Gwendal, others This series from Alex has been outstanding for a while > > but may well still apply. > > Ideally looking for an ack. > > > > This looks good to me. > > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Hi Enric, Ack or both patches or just this one? @Alex, series needs a rebase. I'm not totally sure what's changed. If you don't get to it I'll do it at somepoint but unlikely to be terribly soon! Jonathan > > Thanks, > Enric > > > Thanks, > > > > Jonathan > > > >> --- > >> .../cros_ec_sensors/cros_ec_sensors_core.c | 161 +++++++++--------- > >> 1 file changed, 81 insertions(+), 80 deletions(-) > >> > >> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > >> index 5c6c4e6fec9b..9470014936f2 100644 > >> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > >> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > >> @@ -287,89 +287,90 @@ int cros_ec_sensors_core_init(struct platform_device *pdev, > >> > >> indio_dev->name = pdev->name; > >> > >> - if (physical_device) { > >> - state->param.cmd = MOTIONSENSE_CMD_INFO; > >> - state->param.info.sensor_num = sensor_platform->sensor_num; > >> - ret = cros_ec_motion_send_host_cmd(state, 0); > >> - if (ret) { > >> - dev_warn(dev, "Can not access sensor info\n"); > >> + if (!physical_device) > >> + return 0; > >> + > >> + state->param.cmd = MOTIONSENSE_CMD_INFO; > >> + state->param.info.sensor_num = sensor_platform->sensor_num; > >> + ret = cros_ec_motion_send_host_cmd(state, 0); > >> + if (ret) { > >> + dev_warn(dev, "Can not access sensor info\n"); > >> + return ret; > >> + } > >> + state->type = state->resp->info.type; > >> + state->loc = state->resp->info.location; > >> + > >> + /* Set sign vector, only used for backward compatibility. */ > >> + memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); > >> + > >> + for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) > >> + state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; > >> + > >> + /* 0 is a correct value used to stop the device */ > >> + if (state->msg->version < 3) { > >> + get_default_min_max_freq(state->resp->info.type, > >> + &frequencies[1], > >> + &frequencies[2], > >> + &state->fifo_max_event_count); > >> + } else { > >> + frequencies[1] = state->resp->info_3.min_frequency; > >> + frequencies[2] = state->resp->info_3.max_frequency; > >> + state->fifo_max_event_count = > >> + state->resp->info_3.fifo_max_event_count; > >> + } > >> + for (i = 0; i < ARRAY_SIZE(frequencies); i++) { > >> + state->frequencies[2 * i] = frequencies[i] / 1000; > >> + state->frequencies[2 * i + 1] = > >> + (frequencies[i] % 1000) * 1000; > >> + } > >> + > >> + if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { > >> + /* > >> + * Create a software buffer, feed by the EC FIFO. > >> + * We can not use trigger here, as events are generated > >> + * as soon as sample_frequency is set. > >> + */ > >> + struct iio_buffer *buffer; > >> + > >> + buffer = devm_iio_kfifo_allocate(dev); > >> + if (!buffer) > >> + return -ENOMEM; > >> + > >> + iio_device_attach_buffer(indio_dev, buffer); > >> + indio_dev->modes = INDIO_BUFFER_SOFTWARE; > >> + > >> + ret = cros_ec_sensorhub_register_push_data( > >> + sensor_hub, sensor_platform->sensor_num, > >> + indio_dev, push_data); > >> + if (ret) > >> return ret; > >> - } > >> - state->type = state->resp->info.type; > >> - state->loc = state->resp->info.location; > >> > >> - /* Set sign vector, only used for backward compatibility. */ > >> - memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); > >> + ret = devm_add_action_or_reset( > >> + dev, cros_ec_sensors_core_clean, pdev); > >> + if (ret) > >> + return ret; > >> > >> - for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) > >> - state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; > >> - > >> - /* 0 is a correct value used to stop the device */ > >> - if (state->msg->version < 3) { > >> - get_default_min_max_freq(state->resp->info.type, > >> - &frequencies[1], > >> - &frequencies[2], > >> - &state->fifo_max_event_count); > >> - } else { > >> - frequencies[1] = state->resp->info_3.min_frequency; > >> - frequencies[2] = state->resp->info_3.max_frequency; > >> - state->fifo_max_event_count = > >> - state->resp->info_3.fifo_max_event_count; > >> - } > >> - for (i = 0; i < ARRAY_SIZE(frequencies); i++) { > >> - state->frequencies[2 * i] = frequencies[i] / 1000; > >> - state->frequencies[2 * i + 1] = > >> - (frequencies[i] % 1000) * 1000; > >> - } > >> - > >> - if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { > >> - /* > >> - * Create a software buffer, feed by the EC FIFO. > >> - * We can not use trigger here, as events are generated > >> - * as soon as sample_frequency is set. > >> - */ > >> - struct iio_buffer *buffer; > >> - > >> - buffer = devm_iio_kfifo_allocate(dev); > >> - if (!buffer) > >> - return -ENOMEM; > >> - > >> - iio_device_attach_buffer(indio_dev, buffer); > >> - indio_dev->modes = INDIO_BUFFER_SOFTWARE; > >> - > >> - ret = cros_ec_sensorhub_register_push_data( > >> - sensor_hub, sensor_platform->sensor_num, > >> - indio_dev, push_data); > >> - if (ret) > >> - return ret; > >> - > >> - ret = devm_add_action_or_reset( > >> - dev, cros_ec_sensors_core_clean, pdev); > >> - if (ret) > >> - return ret; > >> - > >> - /* Timestamp coming from FIFO are in ns since boot. */ > >> - ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); > >> - if (ret) > >> - return ret; > >> - } else { > >> - const struct attribute **fifo_attrs; > >> - > >> - if (has_hw_fifo) > >> - fifo_attrs = cros_ec_sensor_fifo_attributes; > >> - else > >> - fifo_attrs = NULL; > >> - > >> - /* > >> - * The only way to get samples in buffer is to set a > >> - * software trigger (systrig, hrtimer). > >> - */ > >> - ret = devm_iio_triggered_buffer_setup_ext( > >> - dev, indio_dev, NULL, trigger_capture, > >> - NULL, fifo_attrs); > >> - if (ret) > >> - return ret; > >> - } > >> + /* Timestamp coming from FIFO are in ns since boot. */ > >> + ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); > >> + if (ret) > >> + return ret; > >> + } else { > >> + const struct attribute **fifo_attrs; > >> + > >> + if (has_hw_fifo) > >> + fifo_attrs = cros_ec_sensor_fifo_attributes; > >> + else > >> + fifo_attrs = NULL; > >> + > >> + /* > >> + * The only way to get samples in buffer is to set a > >> + * software trigger (systrig, hrtimer). > >> + */ > >> + ret = devm_iio_triggered_buffer_setup_ext( > >> + dev, indio_dev, NULL, trigger_capture, > >> + NULL, fifo_attrs); > >> + if (ret) > >> + return ret; > >> } > >> > >> return 0; > >
On Sun, Mar 7, 2021 at 1:59 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Tue, 2 Mar 2021 11:46:06 +0100 > Enric Balletbo i Serra <enric.balletbo@collabora.com> wrote: > > > Hi all, > > > > On 21/2/21 17:29, Jonathan Cameron wrote: > > > On Mon, 23 Nov 2020 16:40:16 +0200 > > > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > > > > >> This whole code-block was put under one big if() condition/block. > > >> This change does an early return if the 'physical_device' boolean is false, > > >> thus unindenting the block by one level. > > >> > > >> No other functional change has been done. > > >> > > >> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > @Gwendal, others This series from Alex has been outstanding for a while > > > but may well still apply. > > > Ideally looking for an ack. > > > > > > > This looks good to me. > > > > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > > Hi Enric, Ack or both patches or just this one? > > @Alex, series needs a rebase. I'm not totally sure what's changed. > If you don't get to it I'll do it at somepoint but unlikely to > be terribly soon! It's likely that it's that iio_device_attach_buffer() went away and the block got replaced by devm_iio_kfifo_buffer_setup(). I'll re-spin it. I dropped this set from my main work branch because I didn't know if it was forgotten, and rebasing it with every change on devm_iio_kfifo_buffer_setup() and devm_iio_triggered_buffer_setup_ext() became annoying. But it shouldn't be hard to re-spin. > > Jonathan > > > > > Thanks, > > Enric > > > > > Thanks, > > > > > > Jonathan > > > > > >> --- > > >> .../cros_ec_sensors/cros_ec_sensors_core.c | 161 +++++++++--------- > > >> 1 file changed, 81 insertions(+), 80 deletions(-) > > >> > > >> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > > >> index 5c6c4e6fec9b..9470014936f2 100644 > > >> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > > >> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c > > >> @@ -287,89 +287,90 @@ int cros_ec_sensors_core_init(struct platform_device *pdev, > > >> > > >> indio_dev->name = pdev->name; > > >> > > >> - if (physical_device) { > > >> - state->param.cmd = MOTIONSENSE_CMD_INFO; > > >> - state->param.info.sensor_num = sensor_platform->sensor_num; > > >> - ret = cros_ec_motion_send_host_cmd(state, 0); > > >> - if (ret) { > > >> - dev_warn(dev, "Can not access sensor info\n"); > > >> + if (!physical_device) > > >> + return 0; > > >> + > > >> + state->param.cmd = MOTIONSENSE_CMD_INFO; > > >> + state->param.info.sensor_num = sensor_platform->sensor_num; > > >> + ret = cros_ec_motion_send_host_cmd(state, 0); > > >> + if (ret) { > > >> + dev_warn(dev, "Can not access sensor info\n"); > > >> + return ret; > > >> + } > > >> + state->type = state->resp->info.type; > > >> + state->loc = state->resp->info.location; > > >> + > > >> + /* Set sign vector, only used for backward compatibility. */ > > >> + memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); > > >> + > > >> + for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) > > >> + state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; > > >> + > > >> + /* 0 is a correct value used to stop the device */ > > >> + if (state->msg->version < 3) { > > >> + get_default_min_max_freq(state->resp->info.type, > > >> + &frequencies[1], > > >> + &frequencies[2], > > >> + &state->fifo_max_event_count); > > >> + } else { > > >> + frequencies[1] = state->resp->info_3.min_frequency; > > >> + frequencies[2] = state->resp->info_3.max_frequency; > > >> + state->fifo_max_event_count = > > >> + state->resp->info_3.fifo_max_event_count; > > >> + } > > >> + for (i = 0; i < ARRAY_SIZE(frequencies); i++) { > > >> + state->frequencies[2 * i] = frequencies[i] / 1000; > > >> + state->frequencies[2 * i + 1] = > > >> + (frequencies[i] % 1000) * 1000; > > >> + } > > >> + > > >> + if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { > > >> + /* > > >> + * Create a software buffer, feed by the EC FIFO. > > >> + * We can not use trigger here, as events are generated > > >> + * as soon as sample_frequency is set. > > >> + */ > > >> + struct iio_buffer *buffer; > > >> + > > >> + buffer = devm_iio_kfifo_allocate(dev); > > >> + if (!buffer) > > >> + return -ENOMEM; > > >> + > > >> + iio_device_attach_buffer(indio_dev, buffer); > > >> + indio_dev->modes = INDIO_BUFFER_SOFTWARE; > > >> + > > >> + ret = cros_ec_sensorhub_register_push_data( > > >> + sensor_hub, sensor_platform->sensor_num, > > >> + indio_dev, push_data); > > >> + if (ret) > > >> return ret; > > >> - } > > >> - state->type = state->resp->info.type; > > >> - state->loc = state->resp->info.location; > > >> > > >> - /* Set sign vector, only used for backward compatibility. */ > > >> - memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); > > >> + ret = devm_add_action_or_reset( > > >> + dev, cros_ec_sensors_core_clean, pdev); > > >> + if (ret) > > >> + return ret; > > >> > > >> - for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) > > >> - state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; > > >> - > > >> - /* 0 is a correct value used to stop the device */ > > >> - if (state->msg->version < 3) { > > >> - get_default_min_max_freq(state->resp->info.type, > > >> - &frequencies[1], > > >> - &frequencies[2], > > >> - &state->fifo_max_event_count); > > >> - } else { > > >> - frequencies[1] = state->resp->info_3.min_frequency; > > >> - frequencies[2] = state->resp->info_3.max_frequency; > > >> - state->fifo_max_event_count = > > >> - state->resp->info_3.fifo_max_event_count; > > >> - } > > >> - for (i = 0; i < ARRAY_SIZE(frequencies); i++) { > > >> - state->frequencies[2 * i] = frequencies[i] / 1000; > > >> - state->frequencies[2 * i + 1] = > > >> - (frequencies[i] % 1000) * 1000; > > >> - } > > >> - > > >> - if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { > > >> - /* > > >> - * Create a software buffer, feed by the EC FIFO. > > >> - * We can not use trigger here, as events are generated > > >> - * as soon as sample_frequency is set. > > >> - */ > > >> - struct iio_buffer *buffer; > > >> - > > >> - buffer = devm_iio_kfifo_allocate(dev); > > >> - if (!buffer) > > >> - return -ENOMEM; > > >> - > > >> - iio_device_attach_buffer(indio_dev, buffer); > > >> - indio_dev->modes = INDIO_BUFFER_SOFTWARE; > > >> - > > >> - ret = cros_ec_sensorhub_register_push_data( > > >> - sensor_hub, sensor_platform->sensor_num, > > >> - indio_dev, push_data); > > >> - if (ret) > > >> - return ret; > > >> - > > >> - ret = devm_add_action_or_reset( > > >> - dev, cros_ec_sensors_core_clean, pdev); > > >> - if (ret) > > >> - return ret; > > >> - > > >> - /* Timestamp coming from FIFO are in ns since boot. */ > > >> - ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); > > >> - if (ret) > > >> - return ret; > > >> - } else { > > >> - const struct attribute **fifo_attrs; > > >> - > > >> - if (has_hw_fifo) > > >> - fifo_attrs = cros_ec_sensor_fifo_attributes; > > >> - else > > >> - fifo_attrs = NULL; > > >> - > > >> - /* > > >> - * The only way to get samples in buffer is to set a > > >> - * software trigger (systrig, hrtimer). > > >> - */ > > >> - ret = devm_iio_triggered_buffer_setup_ext( > > >> - dev, indio_dev, NULL, trigger_capture, > > >> - NULL, fifo_attrs); > > >> - if (ret) > > >> - return ret; > > >> - } > > >> + /* Timestamp coming from FIFO are in ns since boot. */ > > >> + ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); > > >> + if (ret) > > >> + return ret; > > >> + } else { > > >> + const struct attribute **fifo_attrs; > > >> + > > >> + if (has_hw_fifo) > > >> + fifo_attrs = cros_ec_sensor_fifo_attributes; > > >> + else > > >> + fifo_attrs = NULL; > > >> + > > >> + /* > > >> + * The only way to get samples in buffer is to set a > > >> + * software trigger (systrig, hrtimer). > > >> + */ > > >> + ret = devm_iio_triggered_buffer_setup_ext( > > >> + dev, indio_dev, NULL, trigger_capture, > > >> + NULL, fifo_attrs); > > >> + if (ret) > > >> + return ret; > > >> } > > >> > > >> return 0; > > > >
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c index 5c6c4e6fec9b..9470014936f2 100644 --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c @@ -287,89 +287,90 @@ int cros_ec_sensors_core_init(struct platform_device *pdev, indio_dev->name = pdev->name; - if (physical_device) { - state->param.cmd = MOTIONSENSE_CMD_INFO; - state->param.info.sensor_num = sensor_platform->sensor_num; - ret = cros_ec_motion_send_host_cmd(state, 0); - if (ret) { - dev_warn(dev, "Can not access sensor info\n"); + if (!physical_device) + return 0; + + state->param.cmd = MOTIONSENSE_CMD_INFO; + state->param.info.sensor_num = sensor_platform->sensor_num; + ret = cros_ec_motion_send_host_cmd(state, 0); + if (ret) { + dev_warn(dev, "Can not access sensor info\n"); + return ret; + } + state->type = state->resp->info.type; + state->loc = state->resp->info.location; + + /* Set sign vector, only used for backward compatibility. */ + memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); + + for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) + state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; + + /* 0 is a correct value used to stop the device */ + if (state->msg->version < 3) { + get_default_min_max_freq(state->resp->info.type, + &frequencies[1], + &frequencies[2], + &state->fifo_max_event_count); + } else { + frequencies[1] = state->resp->info_3.min_frequency; + frequencies[2] = state->resp->info_3.max_frequency; + state->fifo_max_event_count = + state->resp->info_3.fifo_max_event_count; + } + for (i = 0; i < ARRAY_SIZE(frequencies); i++) { + state->frequencies[2 * i] = frequencies[i] / 1000; + state->frequencies[2 * i + 1] = + (frequencies[i] % 1000) * 1000; + } + + if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { + /* + * Create a software buffer, feed by the EC FIFO. + * We can not use trigger here, as events are generated + * as soon as sample_frequency is set. + */ + struct iio_buffer *buffer; + + buffer = devm_iio_kfifo_allocate(dev); + if (!buffer) + return -ENOMEM; + + iio_device_attach_buffer(indio_dev, buffer); + indio_dev->modes = INDIO_BUFFER_SOFTWARE; + + ret = cros_ec_sensorhub_register_push_data( + sensor_hub, sensor_platform->sensor_num, + indio_dev, push_data); + if (ret) return ret; - } - state->type = state->resp->info.type; - state->loc = state->resp->info.location; - /* Set sign vector, only used for backward compatibility. */ - memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS); + ret = devm_add_action_or_reset( + dev, cros_ec_sensors_core_clean, pdev); + if (ret) + return ret; - for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++) - state->calib[i].scale = MOTION_SENSE_DEFAULT_SCALE; - - /* 0 is a correct value used to stop the device */ - if (state->msg->version < 3) { - get_default_min_max_freq(state->resp->info.type, - &frequencies[1], - &frequencies[2], - &state->fifo_max_event_count); - } else { - frequencies[1] = state->resp->info_3.min_frequency; - frequencies[2] = state->resp->info_3.max_frequency; - state->fifo_max_event_count = - state->resp->info_3.fifo_max_event_count; - } - for (i = 0; i < ARRAY_SIZE(frequencies); i++) { - state->frequencies[2 * i] = frequencies[i] / 1000; - state->frequencies[2 * i + 1] = - (frequencies[i] % 1000) * 1000; - } - - if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) { - /* - * Create a software buffer, feed by the EC FIFO. - * We can not use trigger here, as events are generated - * as soon as sample_frequency is set. - */ - struct iio_buffer *buffer; - - buffer = devm_iio_kfifo_allocate(dev); - if (!buffer) - return -ENOMEM; - - iio_device_attach_buffer(indio_dev, buffer); - indio_dev->modes = INDIO_BUFFER_SOFTWARE; - - ret = cros_ec_sensorhub_register_push_data( - sensor_hub, sensor_platform->sensor_num, - indio_dev, push_data); - if (ret) - return ret; - - ret = devm_add_action_or_reset( - dev, cros_ec_sensors_core_clean, pdev); - if (ret) - return ret; - - /* Timestamp coming from FIFO are in ns since boot. */ - ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); - if (ret) - return ret; - } else { - const struct attribute **fifo_attrs; - - if (has_hw_fifo) - fifo_attrs = cros_ec_sensor_fifo_attributes; - else - fifo_attrs = NULL; - - /* - * The only way to get samples in buffer is to set a - * software trigger (systrig, hrtimer). - */ - ret = devm_iio_triggered_buffer_setup_ext( - dev, indio_dev, NULL, trigger_capture, - NULL, fifo_attrs); - if (ret) - return ret; - } + /* Timestamp coming from FIFO are in ns since boot. */ + ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME); + if (ret) + return ret; + } else { + const struct attribute **fifo_attrs; + + if (has_hw_fifo) + fifo_attrs = cros_ec_sensor_fifo_attributes; + else + fifo_attrs = NULL; + + /* + * The only way to get samples in buffer is to set a + * software trigger (systrig, hrtimer). + */ + ret = devm_iio_triggered_buffer_setup_ext( + dev, indio_dev, NULL, trigger_capture, + NULL, fifo_attrs); + if (ret) + return ret; } return 0;
This whole code-block was put under one big if() condition/block. This change does an early return if the 'physical_device' boolean is false, thus unindenting the block by one level. No other functional change has been done. Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- .../cros_ec_sensors/cros_ec_sensors_core.c | 161 +++++++++--------- 1 file changed, 81 insertions(+), 80 deletions(-)