1. Patchew
  2. linux
  3. [PATCH v2 0/3] BMI270: Add support for step counter and motion events
  4. View patch

[PATCH v2 1/3] iio: imu: bmi270: add channel for step counter

Gustavo Silva posted 3 patches 6 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 1/3] iio: imu: bmi270: add channel for step counter
Posted by Gustavo Silva 6 months, 2 weeks ago 
Add a channel for enabling/disabling the step counter, reading the
number of steps and resetting the counter.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>
---
 drivers/iio/imu/bmi270/bmi270_core.c | 138 +++++++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)

diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index a86be5af5ccb1f010f2282ee31c47f284c1bcc86..6056f7635c5a6e89b670322adfeae0cb7dc5cd9a 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -31,6 +31,8 @@
 #define BMI270_INT_STATUS_1_REG				0x1d
 #define BMI270_INT_STATUS_1_ACC_GYR_DRDY_MSK		GENMASK(7, 6)
 
+#define BMI270_SC_OUT_0_REG				0x1e
+
 #define BMI270_INTERNAL_STATUS_REG			0x21
 #define BMI270_INTERNAL_STATUS_MSG_MSK			GENMASK(3, 0)
 #define BMI270_INTERNAL_STATUS_MSG_INIT_OK		0x01
@@ -39,6 +41,8 @@
 
 #define BMI270_TEMPERATURE_0_REG			0x22
 
+#define BMI270_FEAT_PAGE_REG				0x2f
+
 #define BMI270_ACC_CONF_REG				0x40
 #define BMI270_ACC_CONF_ODR_MSK				GENMASK(3, 0)
 #define BMI270_ACC_CONF_ODR_100HZ			0x08
@@ -90,6 +94,9 @@
 #define BMI270_PWR_CTRL_ACCEL_EN_MSK			BIT(2)
 #define BMI270_PWR_CTRL_TEMP_EN_MSK			BIT(3)
 
+#define BMI270_STEP_SC26_RST_CNT_MSK			BIT(10)
+#define BMI270_STEP_SC26_EN_CNT_MSK			BIT(12)
+
 /* See datasheet section 4.6.14, Temperature Sensor */
 #define BMI270_TEMP_OFFSET				11776
 #define BMI270_TEMP_SCALE				1953125
@@ -111,6 +118,7 @@ struct bmi270_data {
 	struct iio_trigger *trig;
 	 /* Protect device's private data from concurrent access */
 	struct mutex mutex;
+	bool steps_enabled;
 
 	/*
 	 * Where IIO_DMA_MINALIGN may be larger than 8 bytes, align to
@@ -120,6 +128,11 @@ struct bmi270_data {
 		__le16 channels[6];
 		aligned_s64 timestamp;
 	} buffer __aligned(IIO_DMA_MINALIGN);
+	/*
+	 * Variable to access feature registers. It can be accessed concurrently
+	 * with the 'buffer' variable
+	 */
+	__le16 regval __aligned(IIO_DMA_MINALIGN);
 };
 
 enum bmi270_scan {
@@ -282,6 +295,107 @@ static const struct  bmi270_odr_item bmi270_odr_table[] = {
 	},
 };
 
+enum bmi270_feature_reg_id {
+	BMI270_SC_26_REG,
+};
+
+struct bmi270_feature_reg {
+	u8 page;
+	u8 addr;
+};
+
+static const struct bmi270_feature_reg bmi270_feature_regs[] = {
+	[BMI270_SC_26_REG] = {
+		.page = 6,
+		.addr = 0x32,
+	},
+};
+
+static int bmi270_write_feature_reg(struct bmi270_data *data,
+				    enum bmi270_feature_reg_id id,
+				    u16 val)
+{
+	const struct bmi270_feature_reg *reg = &bmi270_feature_regs[id];
+	int ret;
+
+	ret = regmap_write(data->regmap, BMI270_FEAT_PAGE_REG, reg->page);
+	if (ret)
+		return ret;
+
+	data->regval = cpu_to_le16(val);
+	return regmap_bulk_write(data->regmap, reg->addr, &data->regval,
+				 sizeof(data->regval));
+}
+
+static int bmi270_read_feature_reg(struct bmi270_data *data,
+				   enum bmi270_feature_reg_id id,
+				   u16 *val)
+{
+	const struct bmi270_feature_reg *reg = &bmi270_feature_regs[id];
+	int ret;
+
+	ret = regmap_write(data->regmap, BMI270_FEAT_PAGE_REG, reg->page);
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(data->regmap, reg->addr, &data->regval,
+			       sizeof(data->regval));
+	if (ret)
+		return ret;
+
+	*val = le16_to_cpu(data->regval);
+	return 0;
+}
+
+static int bmi270_update_feature_reg(struct bmi270_data *data,
+				     enum bmi270_feature_reg_id id,
+				     u16 mask, u16 val)
+{
+	u16 regval = 0;
+	int ret;
+
+	ret = bmi270_read_feature_reg(data, id, &regval);
+	if (ret)
+		return ret;
+
+	set_mask_bits(&regval, mask, val);
+
+	return bmi270_write_feature_reg(data, id, regval);
+}
+
+static int bmi270_enable_steps(struct bmi270_data *data, int val)
+{
+	int ret;
+
+	guard(mutex)(&data->mutex);
+	if (data->steps_enabled)
+		return 0;
+
+	ret = bmi270_update_feature_reg(data, BMI270_SC_26_REG,
+					BMI270_STEP_SC26_EN_CNT_MSK,
+					FIELD_PREP(BMI270_STEP_SC26_EN_CNT_MSK,
+						   val ? 1 : 0));
+	if (ret)
+		return ret;
+
+	data->steps_enabled = true;
+	return 0;
+}
+
+static int bmi270_read_steps(struct bmi270_data *data, int *val)
+{
+	__le16 steps_count;
+	int ret;
+
+	ret = regmap_bulk_read(data->regmap, BMI270_SC_OUT_0_REG, &steps_count,
+			       sizeof(steps_count));
+	if (ret)
+		return ret;
+
+	*val = sign_extend32(le16_to_cpu(steps_count), 15);
+	return IIO_VAL_INT;
+}
+
 static int bmi270_set_scale(struct bmi270_data *data, int chan_type, int uscale)
 {
 	int i;
@@ -551,6 +665,8 @@ static int bmi270_read_raw(struct iio_dev *indio_dev,
 	struct bmi270_data *data = iio_priv(indio_dev);
 
 	switch (mask) {
+	case IIO_CHAN_INFO_PROCESSED:
+		return bmi270_read_steps(data, val);
 	case IIO_CHAN_INFO_RAW:
 		if (!iio_device_claim_direct(indio_dev))
 			return -EBUSY;
@@ -571,6 +687,9 @@ static int bmi270_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		ret = bmi270_get_odr(data, chan->type, val, val2);
 		return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_ENABLE:
+		*val = data->steps_enabled ? 1 : 0;
+		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
 	}
@@ -596,6 +715,19 @@ static int bmi270_write_raw(struct iio_dev *indio_dev,
 		ret = bmi270_set_odr(data, chan->type, val, val2);
 		iio_device_release_direct(indio_dev);
 		return ret;
+	case IIO_CHAN_INFO_ENABLE:
+		return bmi270_enable_steps(data, val);
+	case IIO_CHAN_INFO_PROCESSED: {
+		if (val || !data->steps_enabled)
+			return -EINVAL;
+
+		guard(mutex)(&data->mutex);
+		/* Clear step counter value */
+		return bmi270_update_feature_reg(data, BMI270_SC_26_REG,
+						 BMI270_STEP_SC26_RST_CNT_MSK,
+						 FIELD_PREP(BMI270_STEP_SC26_RST_CNT_MSK,
+							    1));
+	}
 	default:
 		return -EINVAL;
 	}
@@ -698,6 +830,12 @@ static const struct iio_chan_spec bmi270_channels[] = {
 				      BIT(IIO_CHAN_INFO_OFFSET),
 		.scan_index = -1, /* No buffer support */
 	},
+	{
+		.type = IIO_STEPS,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE) |
+				      BIT(IIO_CHAN_INFO_PROCESSED),
+		.scan_index = -1, /* No buffer support */
+	},
 	IIO_CHAN_SOFT_TIMESTAMP(BMI270_SCAN_TIMESTAMP),
 };
 

-- 
2.49.0
Re: [PATCH v2 1/3] iio: imu: bmi270: add channel for step counter
Posted by Andy Shevchenko 6 months, 2 weeks ago 
On Thu, Jun 05, 2025 at 07:05:01PM -0300, Gustavo Silva wrote:
> Add a channel for enabling/disabling the step counter, reading the
> number of steps and resetting the counter.

...

> +static int bmi270_update_feature_reg(struct bmi270_data *data,
> +				     enum bmi270_feature_reg_id id,
> +				     u16 mask, u16 val)
> +{
> +	u16 regval = 0;

Redundant assignment.

> +	int ret;
> +
> +	ret = bmi270_read_feature_reg(data, id, &regval);
> +	if (ret)
> +		return ret;

> +	set_mask_bits(&regval, mask, val);

You can't do this on the 16-bit values on some architectures.
Maybe it's easy to implement cmpxchg() for 16-bit values there,
though.

> +	return bmi270_write_feature_reg(data, id, regval);
> +}

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 1/3] iio: imu: bmi270: add channel for step counter
Posted by Jonathan Cameron 6 months, 2 weeks ago 
On Fri, 6 Jun 2025 12:14:44 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, Jun 05, 2025 at 07:05:01PM -0300, Gustavo Silva wrote:
> > Add a channel for enabling/disabling the step counter, reading the
> > number of steps and resetting the counter.  
> 
> ...
> 
> > +static int bmi270_update_feature_reg(struct bmi270_data *data,
> > +				     enum bmi270_feature_reg_id id,
> > +				     u16 mask, u16 val)
> > +{
> > +	u16 regval = 0;  
> 
> Redundant assignment.
> 
> > +	int ret;
> > +
> > +	ret = bmi270_read_feature_reg(data, id, &regval);
> > +	if (ret)
> > +		return ret;  
> 
> > +	set_mask_bits(&regval, mask, val);  
> 
> You can't do this on the 16-bit values on some architectures.
> Maybe it's easy to implement cmpxchg() for 16-bit values there,
> though.

It doesn't need to be atomic, so stick to traditional

	regval &= ~mask;
	regval |= bits;

And avoid the fun of architectural corner cases entirely.
> 
> > +	return bmi270_write_feature_reg(data, id, regval);
> > +}  
>
Re: [PATCH v2 1/3] iio: imu: bmi270: add channel for step counter
Posted by Andy Shevchenko 6 months, 1 week ago 
On Sat, Jun 07, 2025 at 04:50:04PM +0100, Jonathan Cameron wrote:
> On Fri, 6 Jun 2025 12:14:44 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Jun 05, 2025 at 07:05:01PM -0300, Gustavo Silva wrote:

...

> > > +	set_mask_bits(&regval, mask, val);  
> > 
> > You can't do this on the 16-bit values on some architectures.
> > Maybe it's easy to implement cmpxchg() for 16-bit values there,
> > though.
> 
> It doesn't need to be atomic, so stick to traditional
> 
> 	regval &= ~mask;
> 	regval |= bits;
> 
> And avoid the fun of architectural corner cases entirely.

Standard pattern is

	regval = (regval & ~mask) | (val & mask);

this will be robust against any changes in the val.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 1/3] iio: imu: bmi270: add channel for step counter
Posted by kernel test robot 6 months, 2 weeks ago 
Hi Gustavo,

kernel test robot noticed the following build errors:

[auto build test ERROR on b475195fecc79a1a6e7fb0846aaaab0a1a4cb2e6]

url:    https://github.com/intel-lab-lkp/linux/commits/Gustavo-Silva/iio-imu-bmi270-add-channel-for-step-counter/20250606-061020
base:   b475195fecc79a1a6e7fb0846aaaab0a1a4cb2e6
patch link:    https://lore.kernel.org/r/20250605-bmi270-events-v2-1-8b2c07d0c213%40gmail.com
patch subject: [PATCH v2 1/3] iio: imu: bmi270: add channel for step counter
config: sh-randconfig-002-20250606 (https://download.01.org/0day-ci/archive/20250606/202506061501.4kluId3d-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250606/202506061501.4kluId3d-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506061501.4kluId3d-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in lib/tests/slub_kunit.o
>> ERROR: modpost: "__cmpxchg_called_with_bad_pointer" [drivers/iio/imu/bmi270/bmi270_core.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.