Re: [PATCH v8 2/2] iio: accel: Add support for the Bosch-Sensortec BMI088

2021-02-06 Thread Jonathan Cameron
On Sun, 31 Jan 2021 12:16:45 +
Jonathan Cameron  wrote:

> On Mon, 25 Jan 2021 16:07:32 +0100
> Mike Looijmans  wrote:
> 
> > The BMI088 is a combined module with both accelerometer and gyroscope.
> > This adds the accelerometer driver support for the SPI interface.
> > The gyroscope part is already supported by the BMG160 driver.
> > 
> > Signed-off-by: Mike Looijmans   
> Hi Mike,
> 
> I'll drop the include  as no acpi stuff in
> here that I can see.
> 
> Otherwise, looks good to me.  Will give a bit of time for Rob or
> anyone else to take another look at this and the binding patch.
> 
I couple of warnings popped up during my local build tests with W=1 C=1

drivers/iio/accel/bmi088-accel-core.c: In function ‘bmi088_accel_power_up’:
drivers/iio/accel/bmi088-accel-core.c:153:17: warning: unused variable ‘dev’ 
[-Wunused-variable]
  153 |  struct device *dev = regmap_get_device(data->regmap);
  | ^~~
drivers/iio/accel/bmi088-accel-core.c: In function ‘bmi088_accel_power_down’:
drivers/iio/accel/bmi088-accel-core.c:177:17: warning: unused variable ‘dev’ 
[-Wunused-variable]
  177 |  struct device *dev = regmap_get_device(data->regmap);

As they are both obvious, I fixed them by dropping both those lines.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to poke at it.

Thanks for persisting with this Mike!

Jonathan

> Thanks,
> 
> Jonathan
> 
> > 
> > ---
> > 
> > Changes in v8:
> > include order asm/ after linux/
> > Suspend/resume redesigned, use runtime PM for both cases. Removed the
> > pm wrappers and let runtime PM handle power up/down. This also removed
> > the need for an internal mutex, thus reducing code further.
> > 
> > Changes in v7:
> > Change bmi088_accel to bmi088-accel
> > Order includes alphabetically
> > Suspend and disable on remove
> > Make bmi088_regmap_spi_{read|write} static
> > 
> > Changes in v6:
> > Hope you have good memory - v5 was almost a year ago now
> > Remove superfluous *val=0
> > Make sample_frequency selection into read_avail list
> > 
> > Changes in v5:
> > Add includes and forward defines in header
> > BIT(7) instead of 0x80
> > Reset already sets defaults, do not set them again
> > Remove now unused bmi088_accel_set_bw
> > Remove unused AXIS_MAX
> > Use MASK define for ODR setting
> > Explain buffer use and alignment
> > Split bmi088_accel_set_power_state into "on" and "off" parts
> > Cosmetic changes to improve readability
> > 
> > Changes in v4:
> > Remove unused #include directives
> > Remove unused #defines for event and irq
> > Replace (ret < 0) with (ret) for all regmap calls
> > Consistent checking of IO errors in probe and init
> > Removed #ifdef CONFIG_PM guard
> > Use bitops for set_frequency instead of loop with shift
> > s/__s16/s16/g
> > Remove excess blank lines
> > Don't return -EAGAIN in pm_runtime
> > 
> > Changes in v3:
> > Processed comments from Jonathan Cameron and Lars-Peter Clausen
> > implement runtime PM (tested by code tracing) and sleep
> > fix scale and offset factors for accel and temperature and
> > return raw values instead of pre-scaled ones
> > Use iio_device_{claim,release}_direct_mode
> > Remove unused code and structs
> > Use a cache-aligned buffer for bulk read
> > Configure and enable caching register values
> > 
> > Changes in v2:
> > Remove unused typedefs and variables
> > Fix error return when iio_device_register fails
> > 
> >  drivers/iio/accel/Kconfig |  18 +
> >  drivers/iio/accel/Makefile|   2 +
> >  drivers/iio/accel/bmi088-accel-core.c | 570 ++
> >  drivers/iio/accel/bmi088-accel-spi.c  |  83 
> >  drivers/iio/accel/bmi088-accel.h  |  18 +
> >  5 files changed, 691 insertions(+)
> >  create mode 100644 drivers/iio/accel/bmi088-accel-core.c
> >  create mode 100644 drivers/iio/accel/bmi088-accel-spi.c
> >  create mode 100644 drivers/iio/accel/bmi088-accel.h
> > 
> > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> > index 2e0c62c39155..cceda3cecbcf 100644
> > --- a/drivers/iio/accel/Kconfig
> > +++ b/drivers/iio/accel/Kconfig
> > @@ -157,6 +157,24 @@ config BMC150_ACCEL_SPI
> > tristate
> > select REGMAP_SPI
> >  
> > +config BMI088_ACCEL
> > +   tristate "Bosch BMI088 Accelerometer Driver"
> > +   depends on SPI
> > +   select IIO_BUFFER
> > +   select IIO_TRIGGERED_BUFFER
> > +   select REGMAP
> > +   select BMI088_ACCEL_SPI
> > +   help
> > + Say yes here to build support for the Bosch BMI088 accelerometer.
> > +
> > + This is a combo module with both accelerometer and gyroscope. This
> > + driver only implements the accelerometer part, which has its own
> > + address and register map. BMG160 provides the gyroscope driver.
> > +
> > +config BMI088_ACCEL_SPI
> > +   tristate
> > +   select REGMAP_SPI
> > +
> >  config DA280
> > tristate "MiraMEMS DA280 3-axis 14-bit digital accelerometer driver"
> > depends on I2C
> > diff --git 

Re: [PATCH v8 2/2] iio: accel: Add support for the Bosch-Sensortec BMI088

2021-01-31 Thread Jonathan Cameron
On Mon, 25 Jan 2021 16:07:32 +0100
Mike Looijmans  wrote:

> The BMI088 is a combined module with both accelerometer and gyroscope.
> This adds the accelerometer driver support for the SPI interface.
> The gyroscope part is already supported by the BMG160 driver.
> 
> Signed-off-by: Mike Looijmans 
Hi Mike,

I'll drop the include  as no acpi stuff in
here that I can see.

Otherwise, looks good to me.  Will give a bit of time for Rob or
anyone else to take another look at this and the binding patch.

Thanks,

Jonathan

> 
> ---
> 
> Changes in v8:
> include order asm/ after linux/
> Suspend/resume redesigned, use runtime PM for both cases. Removed the
> pm wrappers and let runtime PM handle power up/down. This also removed
> the need for an internal mutex, thus reducing code further.
> 
> Changes in v7:
> Change bmi088_accel to bmi088-accel
> Order includes alphabetically
> Suspend and disable on remove
> Make bmi088_regmap_spi_{read|write} static
> 
> Changes in v6:
> Hope you have good memory - v5 was almost a year ago now
> Remove superfluous *val=0
> Make sample_frequency selection into read_avail list
> 
> Changes in v5:
> Add includes and forward defines in header
> BIT(7) instead of 0x80
> Reset already sets defaults, do not set them again
> Remove now unused bmi088_accel_set_bw
> Remove unused AXIS_MAX
> Use MASK define for ODR setting
> Explain buffer use and alignment
> Split bmi088_accel_set_power_state into "on" and "off" parts
> Cosmetic changes to improve readability
> 
> Changes in v4:
> Remove unused #include directives
> Remove unused #defines for event and irq
> Replace (ret < 0) with (ret) for all regmap calls
> Consistent checking of IO errors in probe and init
> Removed #ifdef CONFIG_PM guard
> Use bitops for set_frequency instead of loop with shift
> s/__s16/s16/g
> Remove excess blank lines
> Don't return -EAGAIN in pm_runtime
> 
> Changes in v3:
> Processed comments from Jonathan Cameron and Lars-Peter Clausen
> implement runtime PM (tested by code tracing) and sleep
> fix scale and offset factors for accel and temperature and
> return raw values instead of pre-scaled ones
> Use iio_device_{claim,release}_direct_mode
> Remove unused code and structs
> Use a cache-aligned buffer for bulk read
> Configure and enable caching register values
> 
> Changes in v2:
> Remove unused typedefs and variables
> Fix error return when iio_device_register fails
> 
>  drivers/iio/accel/Kconfig |  18 +
>  drivers/iio/accel/Makefile|   2 +
>  drivers/iio/accel/bmi088-accel-core.c | 570 ++
>  drivers/iio/accel/bmi088-accel-spi.c  |  83 
>  drivers/iio/accel/bmi088-accel.h  |  18 +
>  5 files changed, 691 insertions(+)
>  create mode 100644 drivers/iio/accel/bmi088-accel-core.c
>  create mode 100644 drivers/iio/accel/bmi088-accel-spi.c
>  create mode 100644 drivers/iio/accel/bmi088-accel.h
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 2e0c62c39155..cceda3cecbcf 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -157,6 +157,24 @@ config BMC150_ACCEL_SPI
>   tristate
>   select REGMAP_SPI
>  
> +config BMI088_ACCEL
> + tristate "Bosch BMI088 Accelerometer Driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + select REGMAP
> + select BMI088_ACCEL_SPI
> + help
> +   Say yes here to build support for the Bosch BMI088 accelerometer.
> +
> +   This is a combo module with both accelerometer and gyroscope. This
> +   driver only implements the accelerometer part, which has its own
> +   address and register map. BMG160 provides the gyroscope driver.
> +
> +config BMI088_ACCEL_SPI
> + tristate
> + select REGMAP_SPI
> +
>  config DA280
>   tristate "MiraMEMS DA280 3-axis 14-bit digital accelerometer driver"
>   depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 4f6c1ebe13b0..32cd1342a31a 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -20,6 +20,8 @@ obj-$(CONFIG_BMA400_SPI) += bma400_spi.o
>  obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
>  obj-$(CONFIG_BMC150_ACCEL_I2C) += bmc150-accel-i2c.o
>  obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
> +obj-$(CONFIG_BMI088_ACCEL) += bmi088-accel-core.o
> +obj-$(CONFIG_BMI088_ACCEL_SPI) += bmi088-accel-spi.o
>  obj-$(CONFIG_DA280)  += da280.o
>  obj-$(CONFIG_DA311)  += da311.o
>  obj-$(CONFIG_DMARD06)+= dmard06.o
> diff --git a/drivers/iio/accel/bmi088-accel-core.c 
> b/drivers/iio/accel/bmi088-accel-core.c
> new file mode 100644
> index ..f86010a3cda3
> --- /dev/null
> +++ b/drivers/iio/accel/bmi088-accel-core.c
> @@ -0,0 +1,570 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * 3-axis accelerometer driver supporting following Bosch-Sensortec chips:
> + *  - BMI088
> + *
> + * Copyright (c) 2018-2021, Topic Embedded Products
> + */
> +
> +#include 


Re: [PATCH v8 2/2] iio: accel: Add support for the Bosch-Sensortec BMI088

2021-01-26 Thread Linus Walleij
On Mon, Jan 25, 2021 at 4:07 PM Mike Looijmans  wrote:

> The BMI088 is a combined module with both accelerometer and gyroscope.
> This adds the accelerometer driver support for the SPI interface.
> The gyroscope part is already supported by the BMG160 driver.
>
> Signed-off-by: Mike Looijmans 

Excellent!
Reviewed-by: Linus Walleij 

Yours,
Linus Walleij


[PATCH v8 2/2] iio: accel: Add support for the Bosch-Sensortec BMI088

2021-01-25 Thread Mike Looijmans
The BMI088 is a combined module with both accelerometer and gyroscope.
This adds the accelerometer driver support for the SPI interface.
The gyroscope part is already supported by the BMG160 driver.

Signed-off-by: Mike Looijmans 

---

Changes in v8:
include order asm/ after linux/
Suspend/resume redesigned, use runtime PM for both cases. Removed the
pm wrappers and let runtime PM handle power up/down. This also removed
the need for an internal mutex, thus reducing code further.

Changes in v7:
Change bmi088_accel to bmi088-accel
Order includes alphabetically
Suspend and disable on remove
Make bmi088_regmap_spi_{read|write} static

Changes in v6:
Hope you have good memory - v5 was almost a year ago now
Remove superfluous *val=0
Make sample_frequency selection into read_avail list

Changes in v5:
Add includes and forward defines in header
BIT(7) instead of 0x80
Reset already sets defaults, do not set them again
Remove now unused bmi088_accel_set_bw
Remove unused AXIS_MAX
Use MASK define for ODR setting
Explain buffer use and alignment
Split bmi088_accel_set_power_state into "on" and "off" parts
Cosmetic changes to improve readability

Changes in v4:
Remove unused #include directives
Remove unused #defines for event and irq
Replace (ret < 0) with (ret) for all regmap calls
Consistent checking of IO errors in probe and init
Removed #ifdef CONFIG_PM guard
Use bitops for set_frequency instead of loop with shift
s/__s16/s16/g
Remove excess blank lines
Don't return -EAGAIN in pm_runtime

Changes in v3:
Processed comments from Jonathan Cameron and Lars-Peter Clausen
implement runtime PM (tested by code tracing) and sleep
fix scale and offset factors for accel and temperature and
return raw values instead of pre-scaled ones
Use iio_device_{claim,release}_direct_mode
Remove unused code and structs
Use a cache-aligned buffer for bulk read
Configure and enable caching register values

Changes in v2:
Remove unused typedefs and variables
Fix error return when iio_device_register fails

 drivers/iio/accel/Kconfig |  18 +
 drivers/iio/accel/Makefile|   2 +
 drivers/iio/accel/bmi088-accel-core.c | 570 ++
 drivers/iio/accel/bmi088-accel-spi.c  |  83 
 drivers/iio/accel/bmi088-accel.h  |  18 +
 5 files changed, 691 insertions(+)
 create mode 100644 drivers/iio/accel/bmi088-accel-core.c
 create mode 100644 drivers/iio/accel/bmi088-accel-spi.c
 create mode 100644 drivers/iio/accel/bmi088-accel.h

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 2e0c62c39155..cceda3cecbcf 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -157,6 +157,24 @@ config BMC150_ACCEL_SPI
tristate
select REGMAP_SPI
 
+config BMI088_ACCEL
+   tristate "Bosch BMI088 Accelerometer Driver"
+   depends on SPI
+   select IIO_BUFFER
+   select IIO_TRIGGERED_BUFFER
+   select REGMAP
+   select BMI088_ACCEL_SPI
+   help
+ Say yes here to build support for the Bosch BMI088 accelerometer.
+
+ This is a combo module with both accelerometer and gyroscope. This
+ driver only implements the accelerometer part, which has its own
+ address and register map. BMG160 provides the gyroscope driver.
+
+config BMI088_ACCEL_SPI
+   tristate
+   select REGMAP_SPI
+
 config DA280
tristate "MiraMEMS DA280 3-axis 14-bit digital accelerometer driver"
depends on I2C
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 4f6c1ebe13b0..32cd1342a31a 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -20,6 +20,8 @@ obj-$(CONFIG_BMA400_SPI) += bma400_spi.o
 obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
 obj-$(CONFIG_BMC150_ACCEL_I2C) += bmc150-accel-i2c.o
 obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
+obj-$(CONFIG_BMI088_ACCEL) += bmi088-accel-core.o
+obj-$(CONFIG_BMI088_ACCEL_SPI) += bmi088-accel-spi.o
 obj-$(CONFIG_DA280)+= da280.o
 obj-$(CONFIG_DA311)+= da311.o
 obj-$(CONFIG_DMARD06)  += dmard06.o
diff --git a/drivers/iio/accel/bmi088-accel-core.c 
b/drivers/iio/accel/bmi088-accel-core.c
new file mode 100644
index ..f86010a3cda3
--- /dev/null
+++ b/drivers/iio/accel/bmi088-accel-core.c
@@ -0,0 +1,570 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 3-axis accelerometer driver supporting following Bosch-Sensortec chips:
+ *  - BMI088
+ *
+ * Copyright (c) 2018-2021, Topic Embedded Products
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "bmi088-accel.h"
+
+#define BMI088_ACCEL_REG_CHIP_ID   0x00
+#define BMI088_ACCEL_REG_ERROR 0x02
+
+#define BMI088_ACCEL_REG_INT_STATUS0x1D
+#define BMI088_ACCEL_INT_STATUS_BIT_DRDY   BIT(7)
+
+#define BMI088_ACCEL_REG_RESET 0x7E
+#define BMI088_ACCEL_RESET_VAL 0xB6