| 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
| ||
|
Message-ID: <20250112162715.16249380@jic23-huawei>
Date: Sun, 12 Jan 2025 16:27:15 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Gustavo Silva <gustavograzs@...il.com>
Cc: Alex Lanzano <lanzano.alex@...il.com>, Lars-Peter Clausen
<lars@...afoo.de>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: imu: bmi270: add temperature channel
On Sat, 11 Jan 2025 14:26:18 -0300
Gustavo Silva <gustavograzs@...il.com> wrote:
> The BMI270 IMU includes a temperature sensor. Add a channel for reading
> the temperature.
>
> Signed-off-by: Gustavo Silva <gustavograzs@...il.com>
Hi Gustavo,
A few minor comments inline,
Thanks,
Jonathan
> ---
> drivers/iio/imu/bmi270/bmi270_core.c | 49 ++++++++++++++++++++++++++++++++----
> 1 file changed, 44 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
> index 7fec52e0b48624f07031b63a9caf6c318f33f5dc..c7853923aa69f83a829a71979135f1f7a7ef29ec 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -5,6 +5,7 @@
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/regmap.h>
> +#include <linux/units.h>
>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> @@ -28,10 +29,11 @@
> #define BMI270_INTERNAL_STATUS_REG 0x21
> #define BMI270_INTERNAL_STATUS_MSG_MSK GENMASK(3, 0)
> #define BMI270_INTERNAL_STATUS_MSG_INIT_OK 0x01
> -
> #define BMI270_INTERNAL_STATUS_AXES_REMAP_ERR_MSK BIT(5)
> #define BMI270_INTERNAL_STATUS_ODR_50HZ_ERR_MSK BIT(6)
>
> +#define BMI270_TEMPERATURE_0_REG 0x22
> +
> #define BMI270_ACC_CONF_REG 0x40
> #define BMI270_ACC_CONF_ODR_MSK GENMASK(3, 0)
> #define BMI270_ACC_CONF_ODR_100HZ 0x08
> @@ -69,6 +71,10 @@
> #define BMI270_PWR_CTRL_ACCEL_EN_MSK BIT(2)
> #define BMI270_PWR_CTRL_TEMP_EN_MSK BIT(3)
>
> +/* See datasheet section 4.6.14, Temperature Sensor */
> +#define BMI270_TEMP_OFFSET 11776
> +#define BMI270_TEMP_SCALE 1953125
> +
> #define BMI260_INIT_DATA_FILE "bmi260-init-data.fw"
> #define BMI270_INIT_DATA_FILE "bmi270-init-data.fw"
>
> @@ -109,6 +115,7 @@ EXPORT_SYMBOL_NS_GPL(bmi270_chip_info, "IIO_BMI270");
> enum bmi270_sensor_type {
> BMI270_ACCEL = 0,
> BMI270_GYRO,
> + BMI270_TEMP,
> };
>
> struct bmi270_scale {
> @@ -136,6 +143,10 @@ static const struct bmi270_scale bmi270_gyro_scale[] = {
> { 0, 66 },
> };
>
> +static const struct bmi270_scale bmi270_temp_scale[] = {
> + {BMI270_TEMP_SCALE / MEGA, BMI270_TEMP_SCALE % MEGA},
{ BMI270_TEMP_SCALE / MEGA, BMI270_TEMP_SCALE % MEGA },
Maybe MICRO is the better choice (and same value).
> +};
> +
> struct bmi270_scale_item {
> const struct bmi270_scale *tbl;
> int num;
> @@ -150,6 +161,10 @@ static const struct bmi270_scale_item bmi270_scale_table[] = {
> .tbl = bmi270_gyro_scale,
> .num = ARRAY_SIZE(bmi270_gyro_scale),
> },
> + [BMI270_TEMP] = {
> + .tbl = bmi270_temp_scale,
> + .num = ARRAY_SIZE(bmi270_temp_scale),
> + },
> };
>
> static const struct bmi270_odr bmi270_accel_odr[] = {
> @@ -255,7 +270,7 @@ static int bmi270_set_scale(struct bmi270_data *data, int chan_type, int uscale)
> }
>
> static int bmi270_get_scale(struct bmi270_data *bmi270_device, int chan_type,
> - int *uscale)
> + int *scale, int *uscale)
> {
> int ret;
> unsigned int val;
> @@ -280,6 +295,10 @@ static int bmi270_get_scale(struct bmi270_data *bmi270_device, int chan_type,
> val = FIELD_GET(BMI270_GYR_CONF_RANGE_MSK, val);
> bmi270_scale_item = bmi270_scale_table[BMI270_GYRO];
> break;
> + case IIO_TEMP:
> + val = 0;
> + bmi270_scale_item = bmi270_scale_table[BMI270_TEMP];
> + break;
> default:
> return -EINVAL;
> }
> @@ -287,6 +306,7 @@ static int bmi270_get_scale(struct bmi270_data *bmi270_device, int chan_type,
> if (val >= bmi270_scale_item.num)
> return -EINVAL;
>
> + *scale = bmi270_scale_item.tbl[val].scale;
> *uscale = bmi270_scale_item.tbl[val].uscale;
> return 0;
> }
> @@ -399,6 +419,9 @@ static int bmi270_get_data(struct bmi270_data *bmi270_device,
> case IIO_ANGL_VEL:
> reg = BMI270_ANG_VEL_X_REG + (axis - IIO_MOD_X) * 2;
> break;
> + case IIO_TEMP:
> + reg = BMI270_TEMPERATURE_0_REG;
> + break;
> default:
> return -EINVAL;
> }
> @@ -427,12 +450,20 @@ static int bmi270_read_raw(struct iio_dev *indio_dev,
>
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> - *val = 0;
> - ret = bmi270_get_scale(bmi270_device, chan->type, val2);
> + ret = bmi270_get_scale(bmi270_device, chan->type, val, val2);
> return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
> + case IIO_CHAN_INFO_OFFSET:
> + switch (chan->type) {
> + case IIO_TEMP:
> + *val = BMI270_TEMP_OFFSET;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> case IIO_CHAN_INFO_SAMP_FREQ:
> ret = bmi270_get_odr(bmi270_device, chan->type, val, val2);
> return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
> +
Unrelated change. Good to remove these before posting a patch as it
just adds noise. If you want to tidy up white space that is fine but
not in the same patch as anything making functional changes.
> default:
> return -EINVAL;
> }
> @@ -544,6 +575,13 @@ static const struct iio_chan_spec bmi270_channels[] = {
> BMI270_ANG_VEL_CHANNEL(X),
> BMI270_ANG_VEL_CHANNEL(Y),
> BMI270_ANG_VEL_CHANNEL(Z),
> + {
> + .type = IIO_TEMP,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET),
> + .scan_index = -1, /* No buffer support */
> + },
> IIO_CHAN_SOFT_TIMESTAMP(BMI270_SCAN_TIMESTAMP),
> };
>
> @@ -646,7 +684,8 @@ static int bmi270_configure_imu(struct bmi270_data *bmi270_device)
> ret = regmap_set_bits(regmap, BMI270_PWR_CTRL_REG,
> BMI270_PWR_CTRL_AUX_EN_MSK |
> BMI270_PWR_CTRL_GYR_EN_MSK |
> - BMI270_PWR_CTRL_ACCEL_EN_MSK);
> + BMI270_PWR_CTRL_ACCEL_EN_MSK |
> + BMI270_PWR_CTRL_TEMP_EN_MSK);
> if (ret)
> return dev_err_probe(dev, ret, "Failed to enable accelerometer and gyroscope");
>
>
> ---
> base-commit: 577a66e2e634f712384c57a98f504c44ea4b47da
> change-id: 20250111-bmi270-temp-e9d253619180
>
> Best regards,
Powered by blists - more mailing lists