linux-kernel - Re: [PATCH v4 3/3] 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: <j7vqnxx3d6cws2zsveqohr6iqtno5thlauypnowlijifiwx5ow@6arfipnjkfh7>
Date: Tue, 15 Jul 2025 20:52:41 -0300
From: Gustavo Silva <gustavograzs@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Alex Lanzano <lanzano.alex@...il.com>, 
	Jonathan Cameron <jic23@...nel.org>, David Lechner <dlechner@...libre.com>, 
	Nuno Sá <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>, linux-iio@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Lothar Rubusch <l.rubusch@...il.com>
Subject: Re: [PATCH v4 3/3] iio: imu: bmi270: add support for motion events

On Tue, Jul 15, 2025 at 10:37:21AM +0300, Andy Shevchenko wrote:
> On Fri, Jul 11, 2025 at 08:36:03PM -0300, Gustavo Silva 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.
> 
Hi Andy. Thanks for the review.

> ...
> 
> > +/* 9.81 * 1000000 m/s^2 */
> > +#define BMI270_G_MEGA_M_S_2				9810000
> 
> I thought this is MICRO...
> 
> ...
> 
> > +#define BMI270_INT_MICRO_TO_RAW(val, val2, scale) \
> > +	((val) * (scale) + ((val2) * (scale)) / MEGA)
> 
> Side note: Perhaps good candidate to have it in math.h
> (AFAIR there are more users of the same.)
> 
Sure. I can submit a follow-up patch for that.

> > +#define BMI270_RAW_TO_MICRO(raw, scale) \
> > +	((((raw) % (scale)) * MEGA) / scale)
> 
> > +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_MEGA_M_S_2 / uscale;
> 
> ...as here we divide 10^6/10^-6 semantically speaking.
> 
Agreed. It makes more sense to call it MICRO.

> > +	step = max / BMI270_MOTION_THRES_FULL_SCALE;
> > +
> > +	return sysfs_emit(buf, "[0 %u %u]\n", step, max);
> > +}
> 
> Is this already documented in ABI?
> 
Not yet. Should this have a device-specific ABI entry?

> -- 
> With Best Regards,
> Andy Shevchenko
> 
>