linux-kernel - Re: [PATCH v5 3/4] iio: imu: bmi270: add support for motion events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75Ve93UPiE=STPLiGzfipWUe0WeQsER5X50sKbkdMWDR4bQ@mail.gmail.com>
Date: Sat, 30 Aug 2025 15:44:37 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Gustavo Silva <gustavograzs@...il.com>
Cc: lanzano.alex@...il.com, jic23@...nel.org, dlechner@...libre.com, 
	nuno.sa@...log.com, andy@...nel.org, linux-iio@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 3/4] iio: imu: bmi270: add support for motion events

On Sat, Aug 30, 2025 at 2:58 PM Gustavo Silva <gustavograzs@...il.com> wrote:
>
> Any-motion event can be enabled on a per-axis basis and triggers a
> combined event when motion is detected on any axis.
>
> No-motion event is triggered if the rate of change on all axes falls
> below a specified threshold for a configurable duration. A fake channel
> is used to report this event.
>
> Threshold and duration can be configured from userspace.

...

> +#define BMI270_INT_MICRO_TO_RAW(val, val2, scale) \
> +       ((val) * (scale) + ((val2) * (scale)) / MEGA)
> +#define BMI270_RAW_TO_MICRO(raw, scale) \
> +       ((((raw) % (scale)) * MEGA) / scale)

In the macro names "MICRO" in the implementation "MEGA", please make
this consistent.

...

> +static ssize_t bmi270_show_accel_value_avail(struct device *dev,
> +                                            struct device_attribute *attr,
> +                                            char *buf)
> +{
> +       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +       struct bmi270_data *data = iio_priv(indio_dev);
> +       int ret, scale, uscale;
> +       unsigned int step, max;
> +
> +       ret = bmi270_get_scale(data, IIO_ACCEL, &scale, &uscale);
> +       if (ret)
> +               return ret;
> +
> +       max = BMI270_G_MICRO_M_S_2 / uscale;
> +       step = max / BMI270_MOTION_THRES_FULL_SCALE;
> +
> +       return sysfs_emit(buf, "[0 %u %u]\n", step, max);

Do we need []? Is it common in IIO? The usual way in SW to have "$min
$max $step" (see `seq` in shell, range() in Python and so on).

> +}
> +
> +static IIO_DEVICE_ATTR(in_accel_value_available, 0444,
> +                      bmi270_show_accel_value_avail, NULL, 0);

IIO_DEVICE_ATTR_RO()

...

> +               return FIELD_GET(BMI270_INT_MAP_FEAT_STEP_CNT_WTRMRK_MSK, regval) ?
> +                                                                       1 : 0;

Make it one line by replacing ternary with !! (double exclamation mark).

...

> +                       return FIELD_GET(BMI270_INT_MAP_FEAT_NOMOTION_MSK,
> +                                        regval) ? 1 : 0;

Ditto.

-- 
With Best Regards,
Andy Shevchenko