2022-06-16 10:52:42

by Dmitry Rokosov

[permalink] [raw]
Subject: [PATCH v3 0/3] iio: accel: add MSA311 accelerometer driver

MSA311 is a tri-axial, low-g accelerometer with I2C digital output for
sensitivity consumer applications. It has dynamical user selectable full
scales range of +-2g/+-4g/+-8g/+-16g and allows acceleration measurements
with output data rates from 1Hz to 1000Hz.

Datasheet can be found at following URL:
https://cdn-shop.adafruit.com/product-files/5309/MSA311-V1.1-ENG.pdf

This driver supports following MSA311 features:
- IIO interface
- Different power modes: NORMAL and SUSPEND (using pm_runtime)
- ODR (Output Data Rate) selection
- Scale and samp_freq selection
- IIO triggered buffer, IIO reg access
- NEW_DATA interrupt + trigger

Below features to be done:
- Motion Events: ACTIVE, TAP, ORIENT, FREEFALL
- Low Power mode

Also this patchset has new vendor prefix for MEMSensing Microsystems and
MSA311 dt-binding schema.

You can test msa311 driver using libiio and gnuplot following below
instructions:
$ # Create hrtimer trigger object
$ mkdir /sys/kernel/config/iio/triggers/hrtimer/iio_hrtimer_trigger
$ # Read 4K samples using msa311-new-data trigger (irq) and
$ # buffer with depth equals to 64 samples and rotate device a little bit
$ iio_readdev -u "local:" -b 64 -s 4096 -t msa311-new-data -T 0 \
$ msa311 > /tmp/msa311.dat
$ # Or using hrtimer trigger instead of msa311-new-data trigger
$ iio_readdev -u "local:" -b 64 -s 4096 -t iio_hrtimer_trigger -T 0 \
$ msa311 > /data/local/tmp/msa311.dat
$ cat <<EOF >> msa311_data.gnu
set title "MSA311 Accel Data"

set key below

set xdata time
set format x "%H:%M\n%.4S"
set xlabel "timestamp"

set autoscale y

plot 'msa311.dat' binary endian=little \
format='%int16%int16%int16%uint16%uint64' using \
(\$5/1000000000):(int(\$1)/16) title "acc_x" \
with lines,\\
'msa311.dat' binary endian=little \
format='%int16%int16%int16%uint16%uint64' using \
(\$5/1000000000):(int(\$2)/16) title "acc_y" \
with lines,\\
'msa311.dat' binary endian=little \
format='%int16%int16%int16%uint16%uint64' using \
(\$5/1000000000):(int(\$3)/16) title "acc_z" with lines
EOF
$ gnuplot --persist msa311_data.gnu

Changes:
* v2->v3:
- removed MSA311_TIMESTAMP_CHANNEL() macro, used IIO_CHAN_SOFT_TIMESTAMP
directly
- do not call dev_err_probe() inside functions, which is used not only
from probe() path
- simplified error handling a little bit
- used iio_device_claim_direct_mode() and
iio_device_release_direct_mode() to lock attributes when buffer mode
is enabled
- prohibited sampling frequency changing during buffer usage because
otherwise sometimes MSA311 returns outliers when frequency values
grow up in the read operation moment
- allowed scale value changing when buffer mode is enabled
- removed IRQF_TRIGGER_RISING irq flag from irg registration because
it's provided from device tree directly
- do not switch off autosuspend from powerdown() devm callback,
because it's already done from pm_runtime_disable() during
devm pm_runtime actions
- provided more information why we need force suspend state for MSA311
in the powerdown flow
- reworked comments stuff: removed obvious extra comments, provided
more details in the complex driver code places

* v1->v2:
- memsensing vendor prefix was moved to right place by
alphabetical order
- LOW mode mention was deleted, because LOW mode isn't supported
in the current driver version
- reworked some enums with gaps to defines
- reworked register names as Jonathan mentioned in the v1
- do not use regmap_field API for entire registers
- deleted all extra comments
- supported info_mask_*_avail bitmaps instead of explicit IIO attrs
definitions, implemented read_avail() callback for samp_freq and
scale values
- msa311 mutex is still used to protect msa311 power transitions,
samp_freq/scale tune and axes data handling; described this lock
more informative
- ask new_data interruption status from appropriate register,
do not hold atomic variable for that
- optimized reads of axes data by I2C using regmap_bulk API
- use dev_err_probe() instead of dev_err() for all probe() code paths
- from now all I2C bus communication failures are interpreted as errors
- described wait_from_next() semantic better
- deleted all unneeded pm wrappers
- interpreter all axes data as __le16 type and adjust them to right
format (endianness + sign) for raw() flow only
- redesigned msa311_fs_table[] to 2D matrix (it's more comfortable
format for read_avail() callback)
- align and initialize msa311 buffer before pushing properly
- use pm_runtime resume and suspend from buffer preenable/postdisable,
deleted them from trigger set_state
- supported multiple trigger usage (tested with external hrtimer
trigger and internal new_data trigger)
- moved all irq related stuff to msa311_setup_interrupts() routine
- implemented msa311_powerdown() devm release action
- reworked initialization of pm_runtime msa311 flow, use
autosuspend logic
- purged driver remove() callback, because of devm release logic runs
all deinit stuff fully
- fixed dts bindings problems
- changed irq type in the dt-binding description, because interrupt
type for msa311 should have the same type as i2c irq, for example
using the gpio_intc it's IRQ_TYPE_EDGE_RISING usually. Otherwise
we may lose irq map on the second and further insmod attempts

Dmitry Rokosov (3):
dt-bindings: vendor-prefixes: add MEMSensing Microsystems Co., Ltd.
iio: add MEMSensing MSA311 3-axis accelerometer driver
dt-bindings: iio: accel: add dt-binding schema for msa311 accel driver

.../bindings/iio/accel/memsensing,msa311.yaml | 52 +
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 7 +
drivers/iio/accel/Kconfig | 13 +
drivers/iio/accel/Makefile | 2 +
drivers/iio/accel/msa311.c | 1312 +++++++++++++++++
6 files changed, 1388 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
create mode 100644 drivers/iio/accel/msa311.c

--
2.36.0


2022-06-16 11:07:46

by Dmitry Rokosov

[permalink] [raw]
Subject: [PATCH v3 3/3] dt-bindings: iio: accel: add dt-binding schema for msa311 accel driver

Introduce devicetree binding json-schema for MSA311 tri-axial,
low-g accelerometer driver.

Signed-off-by: Dmitry Rokosov <[email protected]>
---
.../bindings/iio/accel/memsensing,msa311.yaml | 52 +++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 53 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml

diff --git a/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml b/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
new file mode 100644
index 000000000000..072632708d42
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/iio/accel/memsensing,msa311.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: MEMSensing digital 3-Axis accelerometer
+
+maintainers:
+ - Dmitry Rokosov <[email protected]>
+
+description: |
+ MSA311 is a tri-axial, low-g accelerometer with I2C digital output for
+ sensitivity consumer applications. It has dynamical user selectable full
+ scales range of +-2g/+-4g/+-8g/+-16g and allows acceleration measurements
+ with output data rates from 1Hz to 1000Hz.
+ Datasheet can be found at following URL
+ https://cdn-shop.adafruit.com/product-files/5309/MSA311-V1.1-ENG.pdf
+
+properties:
+ compatible:
+ const: memsensing,msa311
+
+ reg:
+ maxItems: 1
+ description: I2C registers address
+
+ interrupts:
+ maxItems: 1
+ description: optional I2C int pin can be freely mapped to specific func
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ accelerometer@62 {
+ compatible = "memsensing,msa311";
+ reg = <0x62>;
+ interrupt-parent = <&gpio_intc>;
+ interrupts = <29 IRQ_TYPE_EDGE_RISING>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 55aeb25c004c..be39e5c214fe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12482,6 +12482,7 @@ MEMSENSING MICROSYSTEMS MSA311 ACCELEROMETER DRIVER
M: Dmitry Rokosov <[email protected]>
L: [email protected]
S: Maintained
+F: Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
F: drivers/iio/accel/msa311.c

MEN A21 WATCHDOG DRIVER
--
2.36.0

2022-06-19 12:06:14

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] dt-bindings: iio: accel: add dt-binding schema for msa311 accel driver

On Thu, 16 Jun 2022 10:42:17 +0000
Dmitry Rokosov <[email protected]> wrote:

> Introduce devicetree binding json-schema for MSA311 tri-axial,
> low-g accelerometer driver.
>
> Signed-off-by: Dmitry Rokosov <[email protected]>
Hi Dmitry,

A few trivial suggestions to drop description entries that don't
useful information.

One thing we often end up adding very soon after new bindings are
introduced is power supplies. If sensible to do so, it's better
to introduce them at the start and save on the noise.
Looks like this one just needs a property entry of

vdd-supply: true

Obviously you then need to get it in the driver and turn it on
(+ off via a devm_add_action_or_reset() call) as minimal support.

Thanks,

Jonathan

> ---
> .../bindings/iio/accel/memsensing,msa311.yaml | 52 +++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml b/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
> new file mode 100644
> index 000000000000..072632708d42
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/iio/accel/memsensing,msa311.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: MEMSensing digital 3-Axis accelerometer
> +
> +maintainers:
> + - Dmitry Rokosov <[email protected]>
> +
> +description: |
> + MSA311 is a tri-axial, low-g accelerometer with I2C digital output for
> + sensitivity consumer applications. It has dynamical user selectable full
> + scales range of +-2g/+-4g/+-8g/+-16g and allows acceleration measurements
> + with output data rates from 1Hz to 1000Hz.
> + Datasheet can be found at following URL
> + https://cdn-shop.adafruit.com/product-files/5309/MSA311-V1.1-ENG.pdf
> +
> +properties:
> + compatible:
> + const: memsensing,msa311
> +
> + reg:
> + maxItems: 1
> + description: I2C registers address

No need for description. It always means that for i2c devices.

> +
> + interrupts:
> + maxItems: 1
> + description: optional I2C int pin can be freely mapped to specific func

Why I2C int? Is there anything associating it with i2c specifically?

I'm not sure the description adds anything useful for this so I'd
drop it.

> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + accelerometer@62 {
> + compatible = "memsensing,msa311";
> + reg = <0x62>;
> + interrupt-parent = <&gpio_intc>;
> + interrupts = <29 IRQ_TYPE_EDGE_RISING>;
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 55aeb25c004c..be39e5c214fe 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12482,6 +12482,7 @@ MEMSENSING MICROSYSTEMS MSA311 ACCELEROMETER DRIVER
> M: Dmitry Rokosov <[email protected]>
> L: [email protected]
> S: Maintained
> +F: Documentation/devicetree/bindings/iio/accel/memsensing,msa311.yaml
> F: drivers/iio/accel/msa311.c
>
> MEN A21 WATCHDOG DRIVER