On Tue, 29 Dec 2020 19:33:39 +0200
Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> 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?
It's not the same. As stated it's the inverse condition. Could add some brackets
and a ! or, flip the logic of the whole if condition and not return early.
Jonathan
>
