Add support for bosch bmi120.
BMI120 is an energy-efficient version of BMI160. Despite having a different
CHIPID value, this variant seems to be fully compatible with BMI160.
It could be find in many phones like xiaomi-vince or xiaomi-tissot.
Signed-off-by: Barnabás Czémán <[email protected]>
---
Danila Tikhonov (2):
iio: imu: bmi160: add support for bmi120
dt-bindings: iio: imu: bmi160: add bmi120
.../devicetree/bindings/iio/imu/bosch,bmi160.yaml | 4 +++-
drivers/iio/imu/bmi160/bmi160_core.c | 24 ++++++++++++++++++----
drivers/iio/imu/bmi160/bmi160_i2c.c | 3 +++
drivers/iio/imu/bmi160/bmi160_spi.c | 3 +++
4 files changed, 29 insertions(+), 5 deletions(-)
---
base-commit: 9221b2819b8a4196eecf5476d66201be60fbcf29
change-id: 20240504-bmi120-d3c88dcb3073
Best regards,
--
Barnabás Czémán <[email protected]>
From: Danila Tikhonov <[email protected]>
Add support for bmi120 low power variant of bmi160.
Signed-off-by: Danila Tikhonov <[email protected]>
Co-developed-by: Barnabás Czémán <[email protected]>
Signed-off-by: Barnabás Czémán <[email protected]>
---
drivers/iio/imu/bmi160/bmi160_core.c | 24 ++++++++++++++++++++----
drivers/iio/imu/bmi160/bmi160_i2c.c | 3 +++
drivers/iio/imu/bmi160/bmi160_spi.c | 3 +++
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index a77f1a8348ff..015801ad4d9a 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -26,6 +26,7 @@
#include "bmi160.h"
#define BMI160_REG_CHIP_ID 0x00
+#define BMI120_CHIP_ID_VAL 0xD3
#define BMI160_CHIP_ID_VAL 0xD1
#define BMI160_REG_PMU_STATUS 0x03
@@ -112,6 +113,11 @@
.ext_info = bmi160_ext_info, \
}
+const u8 bmi_chip_ids[] = {
+ BMI120_CHIP_ID_VAL,
+ BMI160_CHIP_ID_VAL,
+};
+
/* scan indexes follow DATA register order */
enum bmi160_scan_axis {
BMI160_SCAN_EXT_MAGN_X = 0,
@@ -704,6 +710,16 @@ static int bmi160_setup_irq(struct iio_dev *indio_dev, int irq,
return bmi160_probe_trigger(indio_dev, irq, irq_type);
}
+static int bmi160_check_chip_id(const u8 chip_id)
+{
+ for (int i = 0; i < ARRAY_SIZE(bmi_chip_ids); i++) {
+ if (chip_id == bmi_chip_ids[i])
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
{
int ret;
@@ -737,10 +753,10 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
dev_err(dev, "Error reading chip id\n");
goto disable_regulator;
}
- if (val != BMI160_CHIP_ID_VAL) {
- dev_err(dev, "Wrong chip id, got %x expected %x\n",
- val, BMI160_CHIP_ID_VAL);
- ret = -ENODEV;
+
+ ret = bmi160_check_chip_id(val);
+ if (ret) {
+ dev_err(dev, "Wrong chip id %x\n", val);
goto disable_regulator;
}
diff --git a/drivers/iio/imu/bmi160/bmi160_i2c.c b/drivers/iio/imu/bmi160/bmi160_i2c.c
index a081305254db..d0ec5301ad9a 100644
--- a/drivers/iio/imu/bmi160/bmi160_i2c.c
+++ b/drivers/iio/imu/bmi160/bmi160_i2c.c
@@ -37,6 +37,7 @@ static int bmi160_i2c_probe(struct i2c_client *client)
}
static const struct i2c_device_id bmi160_i2c_id[] = {
+ {"bmi120", 0},
{"bmi160", 0},
{}
};
@@ -52,12 +53,14 @@ static const struct acpi_device_id bmi160_acpi_match[] = {
* the affected devices are from 2021/2022.
*/
{"10EC5280", 0},
+ {"BMI0120", 0},
{"BMI0160", 0},
{ },
};
MODULE_DEVICE_TABLE(acpi, bmi160_acpi_match);
static const struct of_device_id bmi160_of_match[] = {
+ { .compatible = "bosch,bmi120" },
{ .compatible = "bosch,bmi160" },
{ },
};
diff --git a/drivers/iio/imu/bmi160/bmi160_spi.c b/drivers/iio/imu/bmi160/bmi160_spi.c
index 8b573ea99af2..9f40500132f7 100644
--- a/drivers/iio/imu/bmi160/bmi160_spi.c
+++ b/drivers/iio/imu/bmi160/bmi160_spi.c
@@ -34,18 +34,21 @@ static int bmi160_spi_probe(struct spi_device *spi)
}
static const struct spi_device_id bmi160_spi_id[] = {
+ {"bmi120", 0},
{"bmi160", 0},
{}
};
MODULE_DEVICE_TABLE(spi, bmi160_spi_id);
static const struct acpi_device_id bmi160_acpi_match[] = {
+ {"BMI0120", 0},
{"BMI0160", 0},
{ },
};
MODULE_DEVICE_TABLE(acpi, bmi160_acpi_match);
static const struct of_device_id bmi160_of_match[] = {
+ { .compatible = "bosch,bmi120" },
{ .compatible = "bosch,bmi160" },
{ },
};
--
2.45.0
From: Danila Tikhonov <[email protected]>
Document bosch,bmi120 compatible.
Signed-off-by: Danila Tikhonov <[email protected]>
Signed-off-by: Barnbás Czémán <[email protected]>
---
Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
index 47cfba939ca6..9ca874aea837 100644
--- a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
@@ -16,7 +16,9 @@ description: |
properties:
compatible:
- const: bosch,bmi160
+ enum:
+ - bosch,bmi120
+ - bosch,bmi160
reg:
maxItems: 1
--
2.45.0
On Sat, 04 May 2024 01:45:24 +0200
Barnabás Czémán <[email protected]> wrote:
> From: Danila Tikhonov <[email protected]>
>
> Add support for bmi120 low power variant of bmi160.
>
> Signed-off-by: Danila Tikhonov <[email protected]>
> Co-developed-by: Barnabás Czémán <[email protected]>
> Signed-off-by: Barnabás Czémán <[email protected]>
Hi. A comment inline that is more about us doing things wrong
in the past than the changes you are making. However, I would
like to fix that up whilst we are here.
Thanks,
Jonathan
> ---
> drivers/iio/imu/bmi160/bmi160_core.c | 24 ++++++++++++++++++++----
> drivers/iio/imu/bmi160/bmi160_i2c.c | 3 +++
> drivers/iio/imu/bmi160/bmi160_spi.c | 3 +++
> 3 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> index a77f1a8348ff..015801ad4d9a 100644
> --- a/drivers/iio/imu/bmi160/bmi160_core.c
> +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> @@ -26,6 +26,7 @@
> #include "bmi160.h"
>
> #define BMI160_REG_CHIP_ID 0x00
> +#define BMI120_CHIP_ID_VAL 0xD3
> #define BMI160_CHIP_ID_VAL 0xD1
>
> #define BMI160_REG_PMU_STATUS 0x03
> @@ -112,6 +113,11 @@
> .ext_info = bmi160_ext_info, \
> }
>
> +const u8 bmi_chip_ids[] = {
> + BMI120_CHIP_ID_VAL,
> + BMI160_CHIP_ID_VAL,
> +};
> +
> /* scan indexes follow DATA register order */
> enum bmi160_scan_axis {
> BMI160_SCAN_EXT_MAGN_X = 0,
> @@ -704,6 +710,16 @@ static int bmi160_setup_irq(struct iio_dev *indio_dev, int irq,
> return bmi160_probe_trigger(indio_dev, irq, irq_type);
> }
>
> +static int bmi160_check_chip_id(const u8 chip_id)
> +{
> + for (int i = 0; i < ARRAY_SIZE(bmi_chip_ids); i++) {
> + if (chip_id == bmi_chip_ids[i])
> + return 0;
This falls into a miss design issue on the original driver based on
my understanding of what we should do with chip IDs back then.
My current thinking after discussing with DT maintainers is we should
at most print a warning if we fail to match an ID.
The intent being to allow for cases exactly like this one to work with
an older kernel if a fallback compatible is in use.
So whilst you are here, please could you relax the below error path
to just do a dev_warn() or similar and return 0 anyway.
Or better yet, push that warning print to the caller...
> + }
> +
> + return -ENODEV;
> +}
> +
> static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
> {
> int ret;
> @@ -737,10 +753,10 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
> dev_err(dev, "Error reading chip id\n");
> goto disable_regulator;
> }
> - if (val != BMI160_CHIP_ID_VAL) {
> - dev_err(dev, "Wrong chip id, got %x expected %x\n",
> - val, BMI160_CHIP_ID_VAL);
> - ret = -ENODEV;
> +
> + ret = bmi160_check_chip_id(val);
> + if (ret) {
> + dev_err(dev, "Wrong chip id %x\n", val);
> goto disable_regulator;
For reasons given above, whilst we are hear I'd like this to change to dev_warn() and
carry on as if we did have a match.
> }
>
> diff --git a/drivers/iio/imu/bmi160/bmi160_i2c.c b/drivers/iio/imu/bmi160/bmi160_i2c.c
> index a081305254db..d0ec5301ad9a 100644
> --- a/drivers/iio/imu/bmi160/bmi160_i2c.c
> +++ b/drivers/iio/imu/bmi160/bmi160_i2c.c
> @@ -37,6 +37,7 @@ static int bmi160_i2c_probe(struct i2c_client *client)
> }
>
> static const struct i2c_device_id bmi160_i2c_id[] = {
> + {"bmi120", 0},
> {"bmi160", 0},
> {}
> };
> @@ -52,12 +53,14 @@ static const struct acpi_device_id bmi160_acpi_match[] = {
> * the affected devices are from 2021/2022.
> */
> {"10EC5280", 0},
> + {"BMI0120", 0},
> {"BMI0160", 0},
> { },
> };
> MODULE_DEVICE_TABLE(acpi, bmi160_acpi_match);
>
> static const struct of_device_id bmi160_of_match[] = {
> + { .compatible = "bosch,bmi120" },
> { .compatible = "bosch,bmi160" },
> { },
> };
> diff --git a/drivers/iio/imu/bmi160/bmi160_spi.c b/drivers/iio/imu/bmi160/bmi160_spi.c
> index 8b573ea99af2..9f40500132f7 100644
> --- a/drivers/iio/imu/bmi160/bmi160_spi.c
> +++ b/drivers/iio/imu/bmi160/bmi160_spi.c
> @@ -34,18 +34,21 @@ static int bmi160_spi_probe(struct spi_device *spi)
> }
>
> static const struct spi_device_id bmi160_spi_id[] = {
> + {"bmi120", 0},
> {"bmi160", 0},
> {}
> };
> MODULE_DEVICE_TABLE(spi, bmi160_spi_id);
>
> static const struct acpi_device_id bmi160_acpi_match[] = {
> + {"BMI0120", 0},
> {"BMI0160", 0},
> { },
> };
> MODULE_DEVICE_TABLE(acpi, bmi160_acpi_match);
>
> static const struct of_device_id bmi160_of_match[] = {
> + { .compatible = "bosch,bmi120" },
> { .compatible = "bosch,bmi160" },
> { },
> };
>
On Sat, 04 May 2024 01:45:25 +0200
Barnabás Czémán <[email protected]> wrote:
> From: Danila Tikhonov <[email protected]>
>
> Document bosch,bmi120 compatible.
>
> Signed-off-by: Danila Tikhonov <[email protected]>
> Signed-off-by: Barnbás Czémán <[email protected]>
> ---
> Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
> index 47cfba939ca6..9ca874aea837 100644
> --- a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
> @@ -16,7 +16,9 @@ description: |
>
> properties:
> compatible:
> - const: bosch,bmi160
> + enum:
> + - bosch,bmi120
> + - bosch,bmi160
Please use a fallback compatible for the bmi120 similar to handling
done for st,asm330lhhx in st,lsm6dsx.yaml
Whilst the driver would currently reject the chip ID, the binding should
assume the driver does fallback handling correctly.
Jonathan
>
> reg:
> maxItems: 1
>
Hi Barnab?s,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 9221b2819b8a4196eecf5476d66201be60fbcf29]
url: https://github.com/intel-lab-lkp/linux/commits/Barnab-s-Cz-m-n/iio-imu-bmi160-add-support-for-bmi120/20240504-074802
base: 9221b2819b8a4196eecf5476d66201be60fbcf29
patch link: https://lore.kernel.org/r/20240504-bmi120-v1-1-478470a85058%40gmail.com
patch subject: [PATCH 1/2] iio: imu: bmi160: add support for bmi120
config: x86_64-randconfig-121-20240505 (https://download.01.org/0day-ci/archive/20240505/[email protected]/config)
compiler: clang version 18.1.4 (https://github.com/llvm/llvm-project e6c3289804a67ea0bb6a86fadbe454dd93b8d855)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240505/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
sparse warnings: (new ones prefixed by >>)
>> drivers/iio/imu/bmi160/bmi160_core.c:116:10: sparse: sparse: symbol 'bmi_chip_ids' was not declared. Should it be static?
vim +/bmi_chip_ids +116 drivers/iio/imu/bmi160/bmi160_core.c
115
> 116 const u8 bmi_chip_ids[] = {
117 BMI120_CHIP_ID_VAL,
118 BMI160_CHIP_ID_VAL,
119 };
120
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki