On Wed, 18 Nov 2020 12:35:16 +0200 Alexandru Ardelean <ardeleana...@gmail.com> wrote:
> On Tue, Sep 29, 2020 at 6:40 PM Jonathan Cameron <ji...@kernel.org> wrote: > > > > On Tue, 29 Sep 2020 17:31:55 +0300 > > Alexandru Ardelean <ardeleana...@gmail.com> wrote: > > > > > On Tue, Sep 29, 2020 at 4:09 PM Andy Shevchenko > > > <andy.shevche...@gmail.com> wrote: > > > > > > > > On Tue, Sep 29, 2020 at 3:55 PM Alexandru Ardelean > > > > <alexandru.ardel...@analog.com> wrote: > > > > > > > > > This change switches to the new devm_iio_triggered_buffer_setup_ext() > > > > > function and removes the iio_buffer_set_attrs() call, for assigning > > > > > the > > > > > HW FIFO attributes to the buffer. > > > > > > > > Sorry, you were too fast with the version, below one nit. > > > > > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardel...@analog.com> > > > > > --- > > > > > .../common/cros_ec_sensors/cros_ec_sensors_core.c | 15 > > > > > +++++++++------ > > > > > 1 file changed, 9 insertions(+), 6 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 c62cacc04672..1eafcf04ad69 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 > > > > > @@ -353,19 +353,22 @@ int cros_ec_sensors_core_init(struct > > > > > platform_device *pdev, > > > > > 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( > > > > > > > > > + ret = devm_iio_triggered_buffer_setup_ext( > > > > > dev, indio_dev, NULL, > > > > > trigger_capture, > > > > > - NULL); > > > > > + NULL, fifo_attrs); > > > > > > > > Perhaps it's time to reformat a bit, i.e. move dev to the first line > > > > and do the rest accordingly? > > > > > > this feels like a mix of preferences here; > > > for once, the patch here [as-is], is the minimal form for this change > > > [in terms of patch-noise]; > > > so, some people would choose the least noisiest patch; > > > > > > also, this indentation was chosen [as-is here] from the start [for > > > this code block]; > > > not sure if it was preferred; i'd suspect it was due to the old 80-col > > > limit; > > > > > > i'd leave it as-is [for now], or defer the decision to a maintainer to > > > decide [either IIO or chromium]; > > > > The indenting of this whole code block is a bit too deep. > > > > Looks to me like we should flip the sense of the outer if statement > > > > if (!physical_device) > > return 0; > > > > That would lead to a whole bunch of reformatting around here including > > picking up this. > > > > For now I can just shuffle it a bit whilst applying. > > > > This set isn't likely to make the merge window anyway now as I'd like > > it to sit on list a little longer just because it touches several > > drivers with active maintainers and I'd like time for them to sanity > > check. > > > > ping on this; > should i do a V4 for this? Yes, probably worth sending out again. I'd like to see a few more acks on the individual drivers ideally and a v4 will get this to the top of peoples' inboxes. If we don't get them I won't let it block this series, but it's nice to try at least! Thanks, Jonathan > > this is related to the multiple IIO buffer support: > https://lore.kernel.org/linux-iio/20201117162340.43924-1-alexandru.ardel...@analog.com/T/#t > > it's one of the patchsets i could split away on it's own; > > > Jonathan > > > > > > > > > > > > > > > > if (ret) > > > > > return ret; > > > > > - > > > > > - if (has_hw_fifo) > > > > > - > > > > > iio_buffer_set_attrs(indio_dev->buffer, > > > > > - > > > > > cros_ec_sensor_fifo_attributes); > > > > > } > > > > > } > > > > > > > > > > -- > > > > > 2.17.1 > > > > > > > > > > > > > > > > > -- > > > > With Best Regards, > > > > Andy Shevchenko > >