On Tue, Dec 29, 2020 at 7:24 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Thu, 17 Dec 2020 00:38:06 +0200
> Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
>
> > On Wed, Dec 16, 2020 at 11:41 PM Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote:
> > >
> > > As some firmware does not notify on illuminance changes, add a
> > > trigger to be able to query light via software (sysfs-trigger or
> > > hrtrigger).
> > > Add a hardware trigger set as the default trigger to maintain backward
> > > compatibility.
> > >
> > > Check iio_info reports the sensor as buffer capable:
> > > iio:device0: acpi-als (buffer capable)
> > >
> > > To test, check we can get data on demand on an Intel based chromebook:
> > >
> > > IIO_DEV="iio:device0"
> > > echo 1 > iio_sysfs_trigger/add_trigger
> > > cat trigger2/name > ${IIO_DEV}/trigger/current_trigger
> > > for i in ${IIO_DEV}/scan_elements/*_en ${IIO_DEV}/buffer/enable ; do
> > > echo 1 > $i
> > > done
> > > od -x /dev/${IIO_DEV} &
> > > echo 1 > trigger2/trigger_now
> >
> > Few nitpicks below. After addressing, take my
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> > assuming that backward compatibility has been tested as well.
...
> > > + if (!iio_buffer_enabled(indio_dev) ||
> > > + !iio_trigger_using_own(indio_dev))
> >
> > I guess it can be located on one line.
> >
> > I hope those functions have no side effects. In that case you may
> > invert logic (save 2 characters)
> >
> > if (iio_buffer_enabled(indio_dev) && iio_trigger_using_own(indio_dev))
>
> You can but at cost of indenting the whole following block on level further.
> I'm not that fussed, but in general that doesn't seem like a good idea to
> save two characters here.
I didn't get it. The proposed change, in case of no side effect, is an
equivalent to the existing one, just 2 characters less.
How does it affect code block indentation?
--
With Best Regards,
Andy Shevchenko
