Skip to contentSkip to conversation

iio: imu: bmi160: added regulator and mount-matrix support

PATCH Superseded Jonathan Albrieux 5 participants 25 messages May 19, 2020 – May 22, 2020

Subsystems (2)

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v2 cover iio: imu: bmi160: added regulator and mount-matrix support

v2: fixed missing description for iio: imu: bmi160: added regulator support

Convert txt format documentation to yaml.
Add documentation about vdd-supply, vddio-supply and mount-matrix.

Add vdd-supply and vddio-supply support. Without this support, vdd and vddio should be set to always-on in device tree.

Add mount-matrix binding support. As chip could have different orientations a mount matrix support is needed to correctly translate these differences

Jonathan Albrieux (4):
dt-bindings: iio: imu: bmi160: convert txt format to yaml
dt-bindings: iio: imu: bmi160: add regulators and mount-matrix
iio: imu: bmi160: added regulator support
iio: imu: bmi160: added mount-matrix support

 .../devicetree/bindings/iio/imu/bmi160.txt    |  37 ------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 105 ++++++++++++++++++
 drivers/iio/imu/bmi160/bmi160.h               |   3 +
 drivers/iio/imu/bmi160/bmi160_core.c          |  47 +++++++-
 4 files changed, 154 insertions(+), 38 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml
Signature · 1 line
2.17.1
#1 lore raw

PATCH v2 1/4 dt-bindings: iio: imu: bmi160: convert txt format to yaml

Converts documentation from txt format to yaml

---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml
Documentation/devicetree/bindings/iio/imu/bmi160.txt deleted +0 -37
deleted file mode 100644
index 900c169de00f..000000000000
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
-   input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
-   open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
-	compatible = "bosch,bmi160";
-	reg = <0x68>;
-
-	interrupt-parent = <&gpio4>;
-	interrupts = <12 IRQ_TYPE_EDGE_RISING>;
-	interrupt-names = "INT1";
-};
-
-bmi160@0 {
-	compatible = "bosch,bmi160";
-	reg = <0>;
-	spi-max-frequency = <10000000>;
-
-	interrupt-parent = <&gpio2>;
-	interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
-	interrupt-names = "INT2";
-};
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +84 -0

84 lines not on this page · raw message

Signature · 1 line
2.17.1
maintainer
#2 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 10 lines · Jonathan Albrieux

Converts documentation from txt format to yaml

---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml

My bot found errors running 'make dt_binding_check' on your patch:

Error: Documentation/devicetree/bindings/iio/imu/bmi160.example.dts:37.1-2 syntax error
FATAL ERROR: Unable to parse input tree
scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/iio/imu/bmi160.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/iio/imu/bmi160.example.dt.yaml] Error 1 Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1293085

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

#3 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 22 lines · Rob Herring
Quoted 10 lines · Jonathan Albrieux

Converts documentation from txt format to yaml

---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml

My bot found errors running 'make dt_binding_check' on your patch:

Error: Documentation/devicetree/bindings/iio/imu/bmi160.example.dts:37.1-2 syntax error
FATAL ERROR: Unable to parse input tree
scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/iio/imu/bmi160.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/iio/imu/bmi160.example.dt.yaml] Error 1 Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

Quoted 9 lines

See https://patchwork.ozlabs.org/patch/1293085

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

Uhm I don't get that error, will try to update to see if it gives me the same error, Thank you,

Best regards,
Jonathan Albrieux

#4 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

In reply to Jonathan Albrieux
Quoted 59 lines · On Tue, 19 May 2020 09:50:57 +0200 Jonathan Albrieux…

Converts documentation from txt format to yaml

---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml
Documentation/devicetree/bindings/iio/imu/bmi160.txt deleted +0 -37
deleted file mode 100644
index 900c169de00f..000000000000
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
-   input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
-   open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
-	compatible = "bosch,bmi160";
-	reg = <0x68>;
-
-	interrupt-parent = <&gpio4>;
-	interrupts = <12 IRQ_TYPE_EDGE_RISING>;
-	interrupt-names = "INT1";
-};
-
-bmi160@0 {
-	compatible = "bosch,bmi160";
-	reg = <0>;
-	spi-max-frequency = <10000000>;
-
-	interrupt-parent = <&gpio2>;
-	interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
-	interrupt-names = "INT2";
-};
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +0 -0
new file mode 100644
index 000000000000..6b464ce5ed0b

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 10

Quoted 10 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +10 -0
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bmi160.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI160
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>

Daniel is still active in the kernel, just not at Intel any more. +CC

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 23

Quoted 13 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +13 -0
+
+description: |
+  Inertial Measurement Unit with Accelerometer, Gyroscope and externally
+  connectable Magnetometer
+  https://www.bosch-sensortec.com/bst/products/all_products/bmi160
+
+properties:
+  compatible:
+    const: bosch,bmi160
+
+  reg:
+    maxItems: 1
+    description: the I2C address or SPI chip select number of the sensor

As standard for i2c and spi, usually no need to have a description line for this element.

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 27

Quoted 4 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +4 -0
+
+  spi-max-frequency:
+    maxItems: 1
+    description: set maximum clock frequency (required only for SPI)

Standard spi binding. Probably doesn't need to be included here.

Quoted 23 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +22 -0
+
+  interrupts:
+    maxItems: 1
+    description: interrupt mapping for IRQ
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 1
+    items:
+      enum:
+        - INT1
+        - INT2
+    description: |
+      set to "INT1" if INT1 pin should be used as interrupt input, set
+      to "INT2" if INT2 pin should be used instead
+
+  drive-open-drain:
+    description: |
+      set if the specified interrupt pin should be configured as
+      open drain. If not set, defaults to push-pull.
 
+
+required:

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 59

Quoted 10 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +10 -0
+  - compatible
+  - reg
+
+examples:
+  - |
+    // Example for I2C
+    i2c@78b7000 {
+        reg = <0x78b6000 0x600>;
+        #address-cells = <1>;
+        #size-cells = <0>;

Take a look at some of the other binding examples. We normally just focus on the driver so don't supply details for the bus.

e.g. https://elixir.bootlin.com/linux/v5.7-rc6/source/Documentation/devicetree/bindings/iio/adc/maxim,max1363.yaml#L39

Quoted 25 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +25 -0
+
+        bmi160@68 {
+                compatible = "bosch,bmi160";
+                reg = <0x68>;
+                interrupt-parent = <&gpio4>;
+                interrupts = <12 1>;
+                interrupt-names = "INT1";
+        };
+  - |
+    // Example for SPI
+    spi@78b7000 {
+        reg = <0x78b7000 0x600>,
+              <0x7884000 0x23000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bmi160@0 {
+                compatible = "bosch,bmi160";
+                reg = <0>;
+                spi-max-frequency = <10000000>;
+                interrupt-parent = <&gpio2>;
+                interrupts = <12 1>;
+                interrupt-names = "INT2";
+        };
+    };
#5 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 66 lines · Jonathan Cameron
Quoted 63 lines · On Tue, 19 May 2020 09:50:57 +0200 Jonathan Albrieux…

Converts documentation from txt format to yaml

---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml
Documentation/devicetree/bindings/iio/imu/bmi160.txt deleted +0 -37
deleted file mode 100644
index 900c169de00f..000000000000
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
-   input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
-   open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
-	compatible = "bosch,bmi160";
-	reg = <0x68>;
-
-	interrupt-parent = <&gpio4>;
-	interrupts = <12 IRQ_TYPE_EDGE_RISING>;
-	interrupt-names = "INT1";
-};
-
-bmi160@0 {
-	compatible = "bosch,bmi160";
-	reg = <0>;
-	spi-max-frequency = <10000000>;
-
-	interrupt-parent = <&gpio2>;
-	interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
-	interrupt-names = "INT2";
-};
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +4 -0
new file mode 100644
index 000000000000..6b464ce5ed0b
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bmi160.yaml#
Quoted 9 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 10

Quoted 6 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +6 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI160
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>

Daniel is still active in the kernel, just not at Intel any more. +CC

Oh ok thank you! Daniel are you still maintaining this driver?

Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +8 -0
+
+description: |
+  Inertial Measurement Unit with Accelerometer, Gyroscope and externally
+  connectable Magnetometer
+  https://www.bosch-sensortec.com/bst/products/all_products/bmi160
+
+properties:
+  compatible:
Quoted 9 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 23

Quoted 5 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +5 -0
+    const: bosch,bmi160
+
+  reg:
+    maxItems: 1
+    description: the I2C address or SPI chip select number of the sensor

As standard for i2c and spi, usually no need to have a description line for this element.

Thank you, will remove the description then.

Quoted 7 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 27

Quoted 4 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +4 -0
+
+  spi-max-frequency:
+    maxItems: 1
+    description: set maximum clock frequency (required only for SPI)

Standard spi binding. Probably doesn't need to be included here.

So should I completely remove it from properties?

Quoted 37 lines
Quoted 23 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +22 -0
+
+  interrupts:
+    maxItems: 1
+    description: interrupt mapping for IRQ
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 1
+    items:
+      enum:
+        - INT1
+        - INT2
+    description: |
+      set to "INT1" if INT1 pin should be used as interrupt input, set
+      to "INT2" if INT2 pin should be used instead
+
+  drive-open-drain:
+    description: |
+      set if the specified interrupt pin should be configured as
+      open drain. If not set, defaults to push-pull.
 
+
+required:

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 59

Quoted 10 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +10 -0
+  - compatible
+  - reg
+
+examples:
+  - |
+    // Example for I2C
+    i2c@78b7000 {
+        reg = <0x78b6000 0x600>;
+        #address-cells = <1>;
+        #size-cells = <0>;

Take a look at some of the other binding examples. We normally just focus on the driver so don't supply details for the bus.

Quoted 2 lines

e.g. https://elixir.bootlin.com/linux/v5.7-rc6/source/Documentation/devicetree/bindings/iio/adc/maxim,max1363.yaml#L39

Will check :-)

Quoted 17 lines
Quoted 17 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +17 -0
+
+        bmi160@68 {
+                compatible = "bosch,bmi160";
+                reg = <0x68>;
+                interrupt-parent = <&gpio4>;
+                interrupts = <12 1>;
+                interrupt-names = "INT1";
+        };
+  - |
+    // Example for SPI
+    spi@78b7000 {
+        reg = <0x78b7000 0x600>,
+              <0x7884000 0x23000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bmi160@0 {
Quoted 10 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 84

Quoted 8 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +8 -0
+                compatible = "bosch,bmi160";
+                reg = <0>;
+                spi-max-frequency = <10000000>;
+                interrupt-parent = <&gpio2>;
+                interrupts = <12 1>;
+                interrupt-names = "INT2";
+        };
+    };

Thank you,

Best regards,
Jonathan Albrieux

maintainer
#6 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 104 lines · On Wed, 20 May 2020 09:24:23 +0200 Jonathan Albrieux…
Quoted 66 lines · Jonathan Cameron
Quoted 63 lines · On Tue, 19 May 2020 09:50:57 +0200 Jonathan Albrieux…

Converts documentation from txt format to yaml

---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml
Documentation/devicetree/bindings/iio/imu/bmi160.txt deleted +0 -37
deleted file mode 100644
index 900c169de00f..000000000000
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
-   input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
-   open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
-	compatible = "bosch,bmi160";
-	reg = <0x68>;
-
-	interrupt-parent = <&gpio4>;
-	interrupts = <12 IRQ_TYPE_EDGE_RISING>;
-	interrupt-names = "INT1";
-};
-
-bmi160@0 {
-	compatible = "bosch,bmi160";
-	reg = <0>;
-	spi-max-frequency = <10000000>;
-
-	interrupt-parent = <&gpio2>;
-	interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
-	interrupt-names = "INT2";
-};
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +4 -0
new file mode 100644
index 000000000000..6b464ce5ed0b
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bmi160.yaml#
Quoted 9 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 10

Quoted 6 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +6 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI160
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>  

Daniel is still active in the kernel, just not at Intel any more. +CC

Oh ok thank you! Daniel are you still maintaining this driver?

Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +8 -0
+
+description: |
+  Inertial Measurement Unit with Accelerometer, Gyroscope and externally
+  connectable Magnetometer
+  https://www.bosch-sensortec.com/bst/products/all_products/bmi160
+
+properties:
+  compatible:
Quoted 9 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 23

Quoted 5 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +5 -0
+    const: bosch,bmi160
+
+  reg:
+    maxItems: 1
+    description: the I2C address or SPI chip select number of the sensor  

As standard for i2c and spi, usually no need to have a description line for this element.

Thank you, will remove the description then.

Quoted 5 lines
Quoted 4 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +4 -0
+
+  spi-max-frequency:
+    maxItems: 1
+    description: set maximum clock frequency (required only for SPI)  
Quoted 4 lines
Quoted 2 lines

Standard spi binding. Probably doesn't need to be included here.

So should I completely remove it from properties?

Yes

Thanks,

Jonathan

#7 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 110 lines · Jonathan Cameron
Quoted 107 lines · On Wed, 20 May 2020 09:24:23 +0200 Jonathan Albrieux…
Quoted 66 lines · Jonathan Cameron
Quoted 63 lines · On Tue, 19 May 2020 09:50:57 +0200 Jonathan Albrieux…

Converts documentation from txt format to yaml

---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml
Documentation/devicetree/bindings/iio/imu/bmi160.txt deleted +0 -37
deleted file mode 100644
index 900c169de00f..000000000000
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
-   input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
-   open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
-	compatible = "bosch,bmi160";
-	reg = <0x68>;
-
-	interrupt-parent = <&gpio4>;
-	interrupts = <12 IRQ_TYPE_EDGE_RISING>;
-	interrupt-names = "INT1";
-};
-
-bmi160@0 {
-	compatible = "bosch,bmi160";
-	reg = <0>;
-	spi-max-frequency = <10000000>;
-
-	interrupt-parent = <&gpio2>;
-	interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
-	interrupt-names = "INT2";
-};
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +4 -0
new file mode 100644
index 000000000000..6b464ce5ed0b
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bmi160.yaml#
Quoted 9 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 10

Quoted 6 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +6 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI160
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>  

Daniel is still active in the kernel, just not at Intel any more. +CC

Oh ok thank you! Daniel are you still maintaining this driver?

Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +8 -0
+
+description: |
+  Inertial Measurement Unit with Accelerometer, Gyroscope and externally
+  connectable Magnetometer
+  https://www.bosch-sensortec.com/bst/products/all_products/bmi160
+
+properties:
+  compatible:
Quoted 9 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 23

Quoted 5 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +5 -0
+    const: bosch,bmi160
+
+  reg:
+    maxItems: 1
+    description: the I2C address or SPI chip select number of the sensor  

As standard for i2c and spi, usually no need to have a description line for this element.

Thank you, will remove the description then.

Quoted 7 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 27

Quoted 4 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +4 -0
+
+  spi-max-frequency:
+    maxItems: 1
+    description: set maximum clock frequency (required only for SPI)  

Standard spi binding. Probably doesn't need to be included here.

Quoted 8 lines
Quoted 1 line

So should I completely remove it from properties?

Yes

Thanks,

Jonathan

Ok I will remove it completely in next patch,
Thank you,

Best regards,
Jonathan Albrieux

#8 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 10

Quoted 4 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +3 -0
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>

Daniel is still active in the kernel, just not at Intel any more. +CC

Oh ok thank you! Daniel are you still maintaining this driver?

I can do reviews if requested but I'm not actively maintaining this driver. If anyone wants

to take this over, will be more than happy.

Other than that we can add my gmail address: Daniel Baluta <[email protected]>

#9 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 7 lines · Daniel Baluta
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 10

Quoted 4 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +3 -0
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>

Daniel is still active in the kernel, just not at Intel any more. +CC

Oh ok thank you! Daniel are you still maintaining this driver?

Quoted 11 lines

I can do reviews if requested but I'm not actively maintaining this driver. If anyone wants

to take this over, will be more than happy.

Other than that we can add my gmail address: Daniel Baluta <[email protected]>

Well if you'd like to review this patch I'd really appreciate :-) Forgive me for not having understood your answer regarding the maintainer field, can I add you to this binding as maintainer or are you saying to not add you? Thank you and sorry for the repeated question,

Best regards,
Jonathan Albrieux

#10 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 17 lines · Jonathan Albrieux
Quoted 5 lines · Daniel Baluta
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>

Daniel is still active in the kernel, just not at Intel any more. +CC

Oh ok thank you! Daniel are you still maintaining this driver?

Quoted 11 lines

I can do reviews if requested but I'm not actively maintaining this driver. If anyone wants

to take this over, will be more than happy.

Other than that we can add my gmail address: Daniel Baluta <[email protected]>

Quoted 6 lines

Well if you'd like to review this patch I'd really appreciate :-) Forgive me for not having understood your answer regarding the maintainer field, can I add you to this binding as maintainer or are you saying to not add you? Thank you and sorry for the repeated question,

OK, so I think would be better not to add me as a maintainer because

this would set some expecation from people, and I most likely won't

have time to met them.

Can you instead add the linux-iio mailing list as a maintainer, not sure

if this is a common practice though.

#11 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 28 lines · Daniel Baluta
Quoted 17 lines · Jonathan Albrieux
Quoted 5 lines · Daniel Baluta
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>

Daniel is still active in the kernel, just not at Intel any more. +CC

Oh ok thank you! Daniel are you still maintaining this driver?

Quoted 11 lines

I can do reviews if requested but I'm not actively maintaining this driver. If anyone wants

to take this over, will be more than happy.

Other than that we can add my gmail address: Daniel Baluta <[email protected]>

Quoted 6 lines

Well if you'd like to review this patch I'd really appreciate :-) Forgive me for not having understood your answer regarding the maintainer field, can I add you to this binding as maintainer or are you saying to not add you? Thank you and sorry for the repeated question,

OK, so I think would be better not to add me as a maintainer because

Quoted 10 lines

this would set some expecation from people, and I most likely won't

have time to met them.

Can you instead add the linux-iio mailing list as a maintainer, not sure

if this is a common practice though.

Ok thank you! I don't know, maybe someone else could point out the right direction to take, it would be sad to drop yaml documentation format just for this.

Best regards,
Jonathan Albrieux

maintainer
#12 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

In reply to Jonathan Albrieux
Quoted 9 lines · Jonathan Albrieux

Converts documentation from txt format to yaml

---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml

Use compatible string for filename: bosch,bmi160.yaml

Quoted 50 lines
Documentation/devicetree/bindings/iio/imu/bmi160.txt deleted +0 -37
deleted file mode 100644
index 900c169de00f..000000000000
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
-   input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
-   open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
-	compatible = "bosch,bmi160";
-	reg = <0x68>;
-
-	interrupt-parent = <&gpio4>;
-	interrupts = <12 IRQ_TYPE_EDGE_RISING>;
-	interrupt-names = "INT1";
-};
-
-bmi160@0 {
-	compatible = "bosch,bmi160";
-	reg = <0>;
-	spi-max-frequency = <10000000>;
-
-	interrupt-parent = <&gpio2>;
-	interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
-	interrupt-names = "INT2";
-};
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +0 -0
new file mode 100644
index 000000000000..6b464ce5ed0b

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 10

Quoted 10 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +10 -0
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bmi160.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI160
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>

Would help to Cc him perhaps.

Quoted 11 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +11 -0
+
+description: |
+  Inertial Measurement Unit with Accelerometer, Gyroscope and externally
+  connectable Magnetometer
+  https://www.bosch-sensortec.com/bst/products/all_products/bmi160
+
+properties:
+  compatible:
+    const: bosch,bmi160
+
+  reg:

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 31

Quoted 10 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +10 -0
+    maxItems: 1
+    description: the I2C address or SPI chip select number of the sensor
+
+  spi-max-frequency:
+    maxItems: 1
+    description: set maximum clock frequency (required only for SPI)
+
+  interrupts:
+    maxItems: 1
+    description: interrupt mapping for IRQ

No need for description if not adding anything unique for this device.

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 39

Quoted 8 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +8 -0
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 1
+    items:
+      enum:
+        - INT1
+        - INT2

Just the enum is enough.

Quoted 48 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +45 -0
+    description: |
+      set to "INT1" if INT1 pin should be used as interrupt input, set
+      to "INT2" if INT2 pin should be used instead
+
+  drive-open-drain:
+    description: |
+      set if the specified interrupt pin should be configured as
+      open drain. If not set, defaults to push-pull.
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    // Example for I2C
+    i2c@78b7000 {
+        reg = <0x78b6000 0x600>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bmi160@68 {
+                compatible = "bosch,bmi160";
+                reg = <0x68>;
+                interrupt-parent = <&gpio4>;
+                interrupts = <12 1>;
+                interrupt-names = "INT1";
+        };
+  - |
+    // Example for SPI
+    spi@78b7000 {
+        reg = <0x78b7000 0x600>,
+              <0x7884000 0x23000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bmi160@0 {
+                compatible = "bosch,bmi160";
+                reg = <0>;
+                spi-max-frequency = <10000000>;
+                interrupt-parent = <&gpio2>;
+                interrupts = <12 1>;
+                interrupt-names = "INT2";
+        };
+    };
Signature · 1 line
2.17.1
#13 lore raw

Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml

Quoted 11 lines · Rob Herring
Quoted 9 lines · Jonathan Albrieux

Converts documentation from txt format to yaml

---
 .../devicetree/bindings/iio/imu/bmi160.txt    | 37 --------
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.yaml
Quoted 2 lines

Use compatible string for filename: bosch,bmi160.yaml

Ok thank you, will change filename.

Quoted 54 lines
Quoted 54 lines
Documentation/devicetree/bindings/iio/imu/bmi160.txt deleted +0 -37
deleted file mode 100644
index 900c169de00f..000000000000
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
-   input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
-   open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
-	compatible = "bosch,bmi160";
-	reg = <0x68>;
-
-	interrupt-parent = <&gpio4>;
-	interrupts = <12 IRQ_TYPE_EDGE_RISING>;
-	interrupt-names = "INT1";
-};
-
-bmi160@0 {
-	compatible = "bosch,bmi160";
-	reg = <0>;
-	spi-max-frequency = <10000000>;
-
-	interrupt-parent = <&gpio2>;
-	interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
-	interrupt-names = "INT2";
-};
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +4 -0
new file mode 100644
index 000000000000..6b464ce5ed0b
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bmi160.yaml#
Quoted 9 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 10

Quoted 6 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +6 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI160
+
+maintainers:
+  - can't find a mantainer, author is Daniel Baluta <[email protected]>

Would help to Cc him perhaps.

Thank you, I will add him too on the next version.

Quoted 15 lines
Quoted 15 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +15 -0
+
+description: |
+  Inertial Measurement Unit with Accelerometer, Gyroscope and externally
+  connectable Magnetometer
+  https://www.bosch-sensortec.com/bst/products/all_products/bmi160
+
+properties:
+  compatible:
+    const: bosch,bmi160
+
+  reg:
+    maxItems: 1
+    description: the I2C address or SPI chip select number of the sensor
+
+  spi-max-frequency:
Quoted 9 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 31

Quoted 6 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +6 -0
+    maxItems: 1
+    description: set maximum clock frequency (required only for SPI)
+
+  interrupts:
+    maxItems: 1
+    description: interrupt mapping for IRQ

No need for description if not adding anything unique for this device.

Will remove it then.

Quoted 11 lines

Documentation/devicetree/bindings/iio/imu/bmi160.yaml line 39

Quoted 8 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +8 -0
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 1
+    items:
+      enum:
+        - INT1
+        - INT2

Just the enum is enough.

Ok, will clean this.

Quoted 40 lines
Quoted 40 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +40 -0
+    description: |
+      set to "INT1" if INT1 pin should be used as interrupt input, set
+      to "INT2" if INT2 pin should be used instead
+
+  drive-open-drain:
+    description: |
+      set if the specified interrupt pin should be configured as
+      open drain. If not set, defaults to push-pull.
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    // Example for I2C
+    i2c@78b7000 {
+        reg = <0x78b6000 0x600>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bmi160@68 {
+                compatible = "bosch,bmi160";
+                reg = <0x68>;
+                interrupt-parent = <&gpio4>;
+                interrupts = <12 1>;
+                interrupt-names = "INT1";
+        };
+  - |
+    // Example for SPI
+    spi@78b7000 {
+        reg = <0x78b7000 0x600>,
+              <0x7884000 0x23000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bmi160@0 {
+                compatible = "bosch,bmi160";
+                reg = <0>;
+                spi-max-frequency = <10000000>;
Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml new file +5 -0
+                interrupt-parent = <&gpio2>;
+                interrupts = <12 1>;
+                interrupt-names = "INT2";
+        };
+    };
Signature · 1 line
2.17.1

Thank you for your help,

Best regards,
Jonathan Albrieux

#14 lore raw

PATCH v2 2/4 dt-bindings: iio: imu: bmi160: add regulators and mount-matrix

In reply to Jonathan Albrieux

Add vdd-supply and vddio-supply support.
Add mount-matrix support.

---
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
Documentation/devicetree/bindings/iio/imu/bmi160.yaml +21 -0
index 6b464ce5ed0b..5b13af7a209f 100644
@@ -46,6 +46,21 @@ properties:
       set if the specified interrupt pin should be configured as
       open drain. If not set, defaults to push-pull.
 
+  vdd-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide VDD power to
+      the sensor.
+
+  vddio-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide the VDD IO power to
+      the sensor.
+
+  mount-matrix:
+    description: an optional 3x3 mounting rotation matrix
+
 required:
   - compatible
   - reg
@@ -61,9 +76,15 @@ examples:
         bmi160@68 {
                 compatible = "bosch,bmi160";
                 reg = <0x68>;
+                vdd-supply = <&pm8916_l17>;
+                vddio-supply = <&pm8916_l6>;
                 interrupt-parent = <&gpio4>;
                 interrupts = <12 1>;
                 interrupt-names = "INT1";
+                mount-matrix = "0", "1", "0",
+                               "-1", "0", "0",
+                               "0", "0", "1";
+                };
         };
   - |
     // Example for SPI
Signature · 1 line
2.17.1
#15 lore raw

Re: [PATCH v2 2/4] dt-bindings: iio: imu: bmi160: add regulators and mount-matrix

Quoted 4 lines · On Tue, 19 May 2020 09:50:58 +0200 Jonathan Albrieux…

Add vdd-supply and vddio-supply support.
Add mount-matrix support.

A few minor comments inline.

Quoted 9 lines
---
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
Documentation/devicetree/bindings/iio/imu/bmi160.yaml +0 -0
index 6b464ce5ed0b..5b13af7a209f 100644

Documentation/devicetree/bindings/iio/imu/bmi160.yaml properties: · line 53

Quoted 8 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml +5 -0
@@ -46,6 +46,21 @@ properties:
       set if the specified interrupt pin should be configured as
       open drain. If not set, defaults to push-pull.
 
+  vdd-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide VDD power to
+      the sensor.

They aren't optional. Whether we specify them or rely on stub regulators being provided because they aren't controllable is the optional bit. That's clearly defined by them not being in the required list below. So say something li.e

description: |
provide VDD power to the sensor.

Quoted 29 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml +16 -0
+
+  vddio-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide the VDD IO power to
+      the sensor.
+
+  mount-matrix:
+    description: an optional 3x3 mounting rotation matrix
+
 required:
   - compatible
   - reg
@@ -61,9 +76,15 @@ examples:
         bmi160@68 {
                 compatible = "bosch,bmi160";
                 reg = <0x68>;
+                vdd-supply = <&pm8916_l17>;
+                vddio-supply = <&pm8916_l6>;
                 interrupt-parent = <&gpio4>;
                 interrupts = <12 1>;
                 interrupt-names = "INT1";
+                mount-matrix = "0", "1", "0",
+                               "-1", "0", "0",
+                               "0", "0", "1";
+                };
         };
   - |
     // Example for SPI
#16 lore raw

Re: [PATCH v2 2/4] dt-bindings: iio: imu: bmi160: add regulators and mount-matrix

Quoted 33 lines · Jonathan Cameron
Quoted 4 lines · On Tue, 19 May 2020 09:50:58 +0200 Jonathan Albrieux…

Add vdd-supply and vddio-supply support.
Add mount-matrix support.

A few minor comments inline.

Quoted 9 lines
---
 .../devicetree/bindings/iio/imu/bmi160.yaml   | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
Documentation/devicetree/bindings/iio/imu/bmi160.yaml +0 -0
index 6b464ce5ed0b..5b13af7a209f 100644

Documentation/devicetree/bindings/iio/imu/bmi160.yaml properties: · line 53

Quoted 8 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml +5 -0
@@ -46,6 +46,21 @@ properties:
       set if the specified interrupt pin should be configured as
       open drain. If not set, defaults to push-pull.
 
+  vdd-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide VDD power to
+      the sensor.

They aren't optional. Whether we specify them or rely on stub regulators being provided because they aren't controllable is the optional bit. That's clearly defined by them not being in the required list below. So say something li.e

Quoted 3 lines

description: |
provide VDD power to the sensor.

Oh ok thank you, will work on that

Quoted 31 lines
Quoted 29 lines
Documentation/devicetree/bindings/iio/imu/bmi160.yaml +16 -0
+
+  vddio-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide the VDD IO power to
+      the sensor.
+
+  mount-matrix:
+    description: an optional 3x3 mounting rotation matrix
+
 required:
   - compatible
   - reg
@@ -61,9 +76,15 @@ examples:
         bmi160@68 {
                 compatible = "bosch,bmi160";
                 reg = <0x68>;
+                vdd-supply = <&pm8916_l17>;
+                vddio-supply = <&pm8916_l6>;
                 interrupt-parent = <&gpio4>;
                 interrupts = <12 1>;
                 interrupt-names = "INT1";
+                mount-matrix = "0", "1", "0",
+                               "-1", "0", "0",
+                               "0", "0", "1";
+                };
         };
   - |
     // Example for SPI
#17 lore raw

PATCH v2 3/4 iio: imu: bmi160: added regulator support

In reply to Jonathan Albrieux

v2: fixed missing description

Add vdd-supply and vddio-supply support. Without this support vdd and vddio should be set to always-on in device tree

---
 drivers/iio/imu/bmi160/bmi160.h      |  2 ++
 drivers/iio/imu/bmi160/bmi160_core.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)
drivers/iio/imu/bmi160/bmi160.h +2 -0
index 621f5309d735..923c3b274fde 100644
@@ -3,10 +3,12 @@
 #define BMI160_H_
 
 #include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
 
 struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
+	struct regulator_bulk_data supplies[2];
 };
 
 extern const struct regmap_config bmi160_regmap_config;
drivers/iio/imu/bmi160/bmi160_core.c +26 -1
index 6af65d6f1d28..9bbe0d8e6720 100644
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/triggered_buffer.h>
@@ -709,6 +710,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	unsigned int val;
 	struct device *dev = regmap_get_device(data->regmap);
 
+	ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
 	if (ret)
 		return ret;
@@ -793,9 +800,17 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
 static void bmi160_chip_uninit(void *data)
 {
 	struct bmi160_data *bmi_data = data;
+	struct device *dev = regmap_get_device(bmi_data->regmap);
+	int ret;
 
 	bmi160_set_mode(bmi_data, BMI160_GYRO, false);
 	bmi160_set_mode(bmi_data, BMI160_ACCEL, false);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(bmi_data->supplies),
+				     bmi_data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to disable regulators: %d\n", ret);
+	}
 }
 
 int bmi160_core_probe(struct device *dev, struct regmap *regmap,
@@ -815,6 +830,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	dev_set_drvdata(dev, indio_dev);
 	data->regmap = regmap;
 
+	data->supplies[0].supply = "vdd";
+	data->supplies[1].supply = "vddio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(data->supplies),
+				      data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;
@@ -853,6 +878,6 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 }
 EXPORT_SYMBOL_GPL(bmi160_core_probe);
 
-MODULE_AUTHOR("Daniel Baluta <[email protected]");
+MODULE_AUTHOR("Daniel Baluta <[email protected]>");
 MODULE_DESCRIPTION("Bosch BMI160 driver");
 MODULE_LICENSE("GPL v2");
Signature · 1 line
2.17.1
#18 lore raw

Re: [PATCH v2 3/4] iio: imu: bmi160: added regulator support

Quoted 1 line · On Tue, 19 May 2020 09:50:59 +0200 Jonathan Albrieux…

v2: fixed missing description

Don't put change log here....

Quoted 3 lines

Add vdd-supply and vddio-supply support. Without this support vdd and vddio should be set to always-on in device tree

Kind of the opposite. If they are always on we don't have to provide them in the device tree.

A few trivial things inline.

Quoted 3 lines

---

Change log goes here so we don't end up keeping it in the git log.

Quoted 51 lines
 drivers/iio/imu/bmi160/bmi160.h      |  2 ++
 drivers/iio/imu/bmi160/bmi160_core.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)
drivers/iio/imu/bmi160/bmi160.h +2 -0
index 621f5309d735..923c3b274fde 100644
@@ -3,10 +3,12 @@
 #define BMI160_H_
 
 #include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
 
 struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
+	struct regulator_bulk_data supplies[2];
 };
 
 extern const struct regmap_config bmi160_regmap_config;
drivers/iio/imu/bmi160/bmi160_core.c +8 -0
index 6af65d6f1d28..9bbe0d8e6720 100644
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/triggered_buffer.h>
@@ -709,6 +710,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	unsigned int val;
 	struct device *dev = regmap_get_device(data->regmap);
 
+	ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
 	if (ret)
 		return ret;
@@ -793,9 +800,17 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
 static void bmi160_chip_uninit(void *data)
 {
 	struct bmi160_data *bmi_data = data;
+	struct device *dev = regmap_get_device(bmi_data->regmap);

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type) · line 813

Quoted 10 lines
drivers/iio/imu/bmi160/bmi160_core.c +7 -0
+	int ret;
 
 	bmi160_set_mode(bmi_data, BMI160_GYRO, false);
 	bmi160_set_mode(bmi_data, BMI160_ACCEL, false);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(bmi_data->supplies),
+				     bmi_data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to disable regulators: %d\n", ret);
+	}

No need for brackets around a 1 line if block

	if (ret)
		dev_err(dev, "failed to disable regulators: %d\n", ret);
Quoted 21 lines
drivers/iio/imu/bmi160/bmi160_core.c +10 -0
 }
 
 int bmi160_core_probe(struct device *dev, struct regmap *regmap,
@@ -815,6 +830,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	dev_set_drvdata(dev, indio_dev);
 	data->regmap = regmap;
 
+	data->supplies[0].supply = "vdd";
+	data->supplies[1].supply = "vddio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(data->supplies),
+				      data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_core_probe(struct device *dev, struct regmap *regmap, · line 881

Quoted 5 lines
drivers/iio/imu/bmi160/bmi160_core.c +1 -1
@@ -853,6 +878,6 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 }
 EXPORT_SYMBOL_GPL(bmi160_core_probe);
 
-MODULE_AUTHOR("Daniel Baluta <[email protected]");
+MODULE_AUTHOR("Daniel Baluta <[email protected]>");

Good fix but shouldn't be in this patch. Put it a separate patch on it's own.

Quoted 2 lines
drivers/iio/imu/bmi160/bmi160_core.c +0 -0
 MODULE_DESCRIPTION("Bosch BMI160 driver");
 MODULE_LICENSE("GPL v2");
#19 lore raw

Re: [PATCH v2 3/4] iio: imu: bmi160: added regulator support

Quoted 6 lines · Jonathan Cameron
Quoted 1 line · On Tue, 19 May 2020 09:50:59 +0200 Jonathan Albrieux…

v2: fixed missing description

Don't put change log here....

Yep I will put it in the cover letter

Quoted 7 lines
Quoted 3 lines

Add vdd-supply and vddio-supply support. Without this support vdd and vddio should be set to always-on in device tree

Kind of the opposite. If they are always on we don't have to provide them in the device tree.

I wrote that because, testing on msm8916, without setting the regulators to always on they were controlled by other components and it happened that the line wasn't ready during probe causing failure to load the module.

I will try to reword based on your comment, thank you.

Quoted 61 lines

A few trivial things inline.

Quoted 3 lines

---

Change log goes here so we don't end up keeping it in the git log.

Quoted 53 lines
 drivers/iio/imu/bmi160/bmi160.h      |  2 ++
 drivers/iio/imu/bmi160/bmi160_core.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)
drivers/iio/imu/bmi160/bmi160.h +2 -0
index 621f5309d735..923c3b274fde 100644
@@ -3,10 +3,12 @@
 #define BMI160_H_
 
 #include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
 
 struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
+	struct regulator_bulk_data supplies[2];
 };
 
 extern const struct regmap_config bmi160_regmap_config;
drivers/iio/imu/bmi160/bmi160_core.c +9 -0
index 6af65d6f1d28..9bbe0d8e6720 100644
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/triggered_buffer.h>
@@ -709,6 +710,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	unsigned int val;
 	struct device *dev = regmap_get_device(data->regmap);
 
+	ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
 	if (ret)
 		return ret;
@@ -793,9 +800,17 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
 static void bmi160_chip_uninit(void *data)
 {
 	struct bmi160_data *bmi_data = data;
+	struct device *dev = regmap_get_device(bmi_data->regmap);
+	int ret;
 
Quoted 10 lines

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type) · line 813

Quoted 8 lines
drivers/iio/imu/bmi160/bmi160_core.c +6 -0
 	bmi160_set_mode(bmi_data, BMI160_GYRO, false);
 	bmi160_set_mode(bmi_data, BMI160_ACCEL, false);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(bmi_data->supplies),
+				     bmi_data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to disable regulators: %d\n", ret);
+	}

No need for brackets around a 1 line if block

Thank you, I didn't noticed that :-)

Quoted 27 lines
	if (ret)
		dev_err(dev, "failed to disable regulators: %d\n", ret);
Quoted 24 lines
drivers/iio/imu/bmi160/bmi160_core.c +10 -0
 }
 
 int bmi160_core_probe(struct device *dev, struct regmap *regmap,
@@ -815,6 +830,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	dev_set_drvdata(dev, indio_dev);
 	data->regmap = regmap;
 
+	data->supplies[0].supply = "vdd";
+	data->supplies[1].supply = "vddio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(data->supplies),
+				      data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;
@@ -853,6 +878,6 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 }
 EXPORT_SYMBOL_GPL(bmi160_core_probe);
 
Quoted 5 lines

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_core_probe(struct device *dev, struct regmap *regmap, · line 881

Quoted 2 lines
drivers/iio/imu/bmi160/bmi160_core.c +1 -1
-MODULE_AUTHOR("Daniel Baluta <[email protected]");
+MODULE_AUTHOR("Daniel Baluta <[email protected]>");

Good fix but shouldn't be in this patch. Put it a separate patch on it's own.

Ok will separate this fix into another patch, thank you!

Quoted 4 lines

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_core_probe(struct device *dev, struct regmap *regmap, · line 883

Quoted 2 lines
drivers/iio/imu/bmi160/bmi160_core.c +0 -0
 MODULE_DESCRIPTION("Bosch BMI160 driver");
 MODULE_LICENSE("GPL v2");

Best regards,
Jonathan Albrieux

maintainer
#20 lore raw

Re: [PATCH v2 3/4] iio: imu: bmi160: added regulator support

Quoted 18 lines · On Wed, 20 May 2020 09:17:51 +0200 Jonathan Albrieux…
Quoted 6 lines · Jonathan Cameron
Quoted 1 line · On Tue, 19 May 2020 09:50:59 +0200 Jonathan Albrieux…

v2: fixed missing description

Don't put change log here....

Yep I will put it in the cover letter

Quoted 7 lines
Quoted 3 lines

Add vdd-supply and vddio-supply support. Without this support vdd and vddio should be set to always-on in device tree

Kind of the opposite. If they are always on we don't have to provide them in the device tree.

Quoted 5 lines

I wrote that because, testing on msm8916, without setting the regulators to always on they were controlled by other components and it happened that the line wasn't ready during probe causing failure to load the module.

I will try to reword based on your comment, thank you.

Ah. Understood. I'd give that explicit example in the patch description. I'd assumed this was the normal case of they weren't being described at all in DT, whereas you case is more complex.

Jonathan

Quoted 117 lines
Quoted 62 lines

A few trivial things inline.

Quoted 3 lines

---

Change log goes here so we don't end up keeping it in the git log.

Quoted 53 lines
 drivers/iio/imu/bmi160/bmi160.h      |  2 ++
 drivers/iio/imu/bmi160/bmi160_core.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)
drivers/iio/imu/bmi160/bmi160.h +2 -0
index 621f5309d735..923c3b274fde 100644
@@ -3,10 +3,12 @@
 #define BMI160_H_
 
 #include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
 
 struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
+	struct regulator_bulk_data supplies[2];
 };
 
 extern const struct regmap_config bmi160_regmap_config;
drivers/iio/imu/bmi160/bmi160_core.c +9 -0
index 6af65d6f1d28..9bbe0d8e6720 100644
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/triggered_buffer.h>
@@ -709,6 +710,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	unsigned int val;
 	struct device *dev = regmap_get_device(data->regmap);
 
+	ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
 	if (ret)
 		return ret;
@@ -793,9 +800,17 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
 static void bmi160_chip_uninit(void *data)
 {
 	struct bmi160_data *bmi_data = data;
+	struct device *dev = regmap_get_device(bmi_data->regmap);
+	int ret;
 
Quoted 10 lines

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type) · line 813

Quoted 8 lines
drivers/iio/imu/bmi160/bmi160_core.c +6 -0
 	bmi160_set_mode(bmi_data, BMI160_GYRO, false);
 	bmi160_set_mode(bmi_data, BMI160_ACCEL, false);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(bmi_data->supplies),
+				     bmi_data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to disable regulators: %d\n", ret);
+	}  

No need for brackets around a 1 line if block

Thank you, I didn't noticed that :-)

Quoted 27 lines
	if (ret)
		dev_err(dev, "failed to disable regulators: %d\n", ret);
Quoted 24 lines
drivers/iio/imu/bmi160/bmi160_core.c +10 -0
 }
 
 int bmi160_core_probe(struct device *dev, struct regmap *regmap,
@@ -815,6 +830,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	dev_set_drvdata(dev, indio_dev);
 	data->regmap = regmap;
 
+	data->supplies[0].supply = "vdd";
+	data->supplies[1].supply = "vddio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(data->supplies),
+				      data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;
@@ -853,6 +878,6 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 }
 EXPORT_SYMBOL_GPL(bmi160_core_probe);
 
Quoted 5 lines

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_core_probe(struct device *dev, struct regmap *regmap, · line 881

Quoted 2 lines
drivers/iio/imu/bmi160/bmi160_core.c +1 -1
-MODULE_AUTHOR("Daniel Baluta <[email protected]");
+MODULE_AUTHOR("Daniel Baluta <[email protected]>");  

Good fix but shouldn't be in this patch. Put it a separate patch on it's own.

Ok will separate this fix into another patch, thank you!

Quoted 4 lines

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_core_probe(struct device *dev, struct regmap *regmap, · line 883

Quoted 2 lines
drivers/iio/imu/bmi160/bmi160_core.c +0 -0
 MODULE_DESCRIPTION("Bosch BMI160 driver");
 MODULE_LICENSE("GPL v2");  

Best regards,
Jonathan Albrieux

#21 lore raw

Re: [PATCH v2 3/4] iio: imu: bmi160: added regulator support

Quoted 25 lines · Jonathan Cameron
Quoted 22 lines · On Wed, 20 May 2020 09:17:51 +0200 Jonathan Albrieux…
Quoted 6 lines · Jonathan Cameron
Quoted 1 line · On Tue, 19 May 2020 09:50:59 +0200 Jonathan Albrieux…

v2: fixed missing description

Don't put change log here....

Yep I will put it in the cover letter

Quoted 7 lines
Quoted 3 lines

Add vdd-supply and vddio-supply support. Without this support vdd and vddio should be set to always-on in device tree

Kind of the opposite. If they are always on we don't have to provide them in the device tree.

I wrote that because, testing on msm8916, without setting the regulators to always on they were controlled by other components and it happened that the line wasn't ready during probe causing failure to load the module.

Quoted 8 lines
Quoted 1 line

I will try to reword based on your comment, thank you.

Ah. Understood. I'd give that explicit example in the patch description. I'd assumed this was the normal case of they weren't being described at all in DT, whereas you case is more complex.

Jonathan

Yep, I omitted to describe the case I was in. I'll add it to next patch, thank you,

Quoted 110 lines
Quoted 110 lines
Quoted 62 lines

A few trivial things inline.

Quoted 3 lines

---

Change log goes here so we don't end up keeping it in the git log.

Quoted 53 lines
 drivers/iio/imu/bmi160/bmi160.h      |  2 ++
 drivers/iio/imu/bmi160/bmi160_core.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)
drivers/iio/imu/bmi160/bmi160.h +2 -0
index 621f5309d735..923c3b274fde 100644
@@ -3,10 +3,12 @@
 #define BMI160_H_
 
 #include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
 
 struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
+	struct regulator_bulk_data supplies[2];
 };
 
 extern const struct regmap_config bmi160_regmap_config;
drivers/iio/imu/bmi160/bmi160_core.c +9 -0
index 6af65d6f1d28..9bbe0d8e6720 100644
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/triggered_buffer.h>
@@ -709,6 +710,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	unsigned int val;
 	struct device *dev = regmap_get_device(data->regmap);
 
+	ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
 	if (ret)
 		return ret;
@@ -793,9 +800,17 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
 static void bmi160_chip_uninit(void *data)
 {
 	struct bmi160_data *bmi_data = data;
+	struct device *dev = regmap_get_device(bmi_data->regmap);
+	int ret;
 
Quoted 10 lines

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type) · line 813

Quoted 8 lines
drivers/iio/imu/bmi160/bmi160_core.c +6 -0
 	bmi160_set_mode(bmi_data, BMI160_GYRO, false);
 	bmi160_set_mode(bmi_data, BMI160_ACCEL, false);
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(bmi_data->supplies),
+				     bmi_data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to disable regulators: %d\n", ret);
+	}  

No need for brackets around a 1 line if block

Thank you, I didn't noticed that :-)

Quoted 27 lines
	if (ret)
		dev_err(dev, "failed to disable regulators: %d\n", ret);
Quoted 24 lines
drivers/iio/imu/bmi160/bmi160_core.c +10 -0
 }
 
 int bmi160_core_probe(struct device *dev, struct regmap *regmap,
@@ -815,6 +830,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	dev_set_drvdata(dev, indio_dev);
 	data->regmap = regmap;
 
+	data->supplies[0].supply = "vdd";
+	data->supplies[1].supply = "vddio";
+	ret = devm_regulator_bulk_get(dev,
+				      ARRAY_SIZE(data->supplies),
+				      data->supplies);
+	if (ret) {
+		dev_err(dev, "Failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;
@@ -853,6 +878,6 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 }
 EXPORT_SYMBOL_GPL(bmi160_core_probe);
 
Quoted 5 lines

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_core_probe(struct device *dev, struct regmap *regmap, · line 881

Quoted 2 lines
drivers/iio/imu/bmi160/bmi160_core.c +1 -1
-MODULE_AUTHOR("Daniel Baluta <[email protected]");
+MODULE_AUTHOR("Daniel Baluta <[email protected]>");  

Good fix but shouldn't be in this patch. Put it a separate patch on it's own.

Ok will separate this fix into another patch, thank you!

Quoted 8 lines
Quoted 7 lines
Quoted 4 lines

drivers/iio/imu/bmi160/bmi160_core.c int bmi160_core_probe(struct device *dev, struct regmap *regmap, · line 883

Quoted 2 lines
drivers/iio/imu/bmi160/bmi160_core.c +0 -0
 MODULE_DESCRIPTION("Bosch BMI160 driver");
 MODULE_LICENSE("GPL v2");  

Best regards,
Jonathan Albrieux

Best regards,
Jonathan Albrieux

#22 lore raw

PATCH v2 4/4 iio: imu: bmi160: added mount-matrix support

In reply to Jonathan Albrieux

Add mount-matrix binding support. As chip could have different orientations a mount matrix support is needed to correctly translate these differences

---
 drivers/iio/imu/bmi160/bmi160.h      |  1 +
 drivers/iio/imu/bmi160/bmi160_core.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)
drivers/iio/imu/bmi160/bmi160.h +1 -0
index 923c3b274fde..a82e040bd109 100644
@@ -9,6 +9,7 @@ struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
 	struct regulator_bulk_data supplies[2];
+	struct iio_mount_matrix orientation;
 };
 
 extern const struct regmap_config bmi160_regmap_config;
drivers/iio/imu/bmi160/bmi160_core.c +20 -0
index 9bbe0d8e6720..78c8ca962359 100644
@@ -110,6 +110,7 @@
 		.storagebits = 16,				\
 		.endianness = IIO_LE,				\
 	},							\
+	.ext_info = bmi160_ext_info,				\
 }
 
 /* scan indexes follow DATA register order */
@@ -265,6 +266,20 @@ static const struct  bmi160_odr_item bmi160_odr_table[] = {
 	},
 };
 
+static const struct iio_mount_matrix *
+bmi160_get_mount_matrix(const struct iio_dev *indio_dev,
+			const struct iio_chan_spec *chan)
+{
+	struct bmi160_data *data = iio_priv(indio_dev);
+
+	return &data->orientation;
+}
+
+static const struct iio_chan_spec_ext_info bmi160_ext_info[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bmi160_get_mount_matrix),
+	{ }
+};
+
 static const struct iio_chan_spec bmi160_channels[] = {
 	BMI160_CHANNEL(IIO_ACCEL, X, BMI160_SCAN_ACCEL_X),
 	BMI160_CHANNEL(IIO_ACCEL, Y, BMI160_SCAN_ACCEL_Y),
@@ -840,6 +855,11 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 		return ret;
 	}
 
+	ret = iio_read_mount_matrix(dev, "mount-matrix",
+				    &data->orientation);
+	if (ret)
+		return ret;
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;
Signature · 1 line
2.17.1
#23 lore raw

Re: [PATCH v2 4/4] iio: imu: bmi160: added mount-matrix support

Quoted 4 lines · On Tue, 19 May 2020 09:51:00 +0200 Jonathan Albrieux…

Add mount-matrix binding support. As chip could have different orientations a mount matrix support is needed to correctly translate these differences

Hi Jonathan,

Looks good to me. I'll pick this up once 1-3 are tidied up and we have a device tree review in for the binding doc. I'm rubbish at reviewing those as Rob will certify so may well have missed something!

Jonathan

Quoted 62 lines
---
 drivers/iio/imu/bmi160/bmi160.h      |  1 +
 drivers/iio/imu/bmi160/bmi160_core.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)
drivers/iio/imu/bmi160/bmi160.h +1 -0
index 923c3b274fde..a82e040bd109 100644
@@ -9,6 +9,7 @@ struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
 	struct regulator_bulk_data supplies[2];
+	struct iio_mount_matrix orientation;
 };
 
 extern const struct regmap_config bmi160_regmap_config;
drivers/iio/imu/bmi160/bmi160_core.c +20 -0
index 9bbe0d8e6720..78c8ca962359 100644
@@ -110,6 +110,7 @@
 		.storagebits = 16,				\
 		.endianness = IIO_LE,				\
 	},							\
+	.ext_info = bmi160_ext_info,				\
 }
 
 /* scan indexes follow DATA register order */
@@ -265,6 +266,20 @@ static const struct  bmi160_odr_item bmi160_odr_table[] = {
 	},
 };
 
+static const struct iio_mount_matrix *
+bmi160_get_mount_matrix(const struct iio_dev *indio_dev,
+			const struct iio_chan_spec *chan)
+{
+	struct bmi160_data *data = iio_priv(indio_dev);
+
+	return &data->orientation;
+}
+
+static const struct iio_chan_spec_ext_info bmi160_ext_info[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bmi160_get_mount_matrix),
+	{ }
+};
+
 static const struct iio_chan_spec bmi160_channels[] = {
 	BMI160_CHANNEL(IIO_ACCEL, X, BMI160_SCAN_ACCEL_X),
 	BMI160_CHANNEL(IIO_ACCEL, Y, BMI160_SCAN_ACCEL_Y),
@@ -840,6 +855,11 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 		return ret;
 	}
 
+	ret = iio_read_mount_matrix(dev, "mount-matrix",
+				    &data->orientation);
+	if (ret)
+		return ret;
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;
#24 lore raw

Re: [PATCH v2 4/4] iio: imu: bmi160: added mount-matrix support

Quoted 6 lines · Jonathan Cameron
Quoted 3 lines · On Tue, 19 May 2020 09:51:00 +0200 Jonathan Albrieux…

Add mount-matrix binding support. As chip could have different orientations a mount matrix support is needed to correctly translate these differences

Quoted 10 lines
Quoted 1 line

Hi Jonathan,

Looks good to me. I'll pick this up once 1-3 are tidied up and we have a device tree review in for the binding doc. I'm rubbish at reviewing those as Rob will certify so may well have missed something!

Jonathan

Thank you! I'm going to work on suggestions now,

Best regards,
Jonathan Albrieux

Quoted 65 lines
Quoted 63 lines
---
 drivers/iio/imu/bmi160/bmi160.h      |  1 +
 drivers/iio/imu/bmi160/bmi160_core.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)
drivers/iio/imu/bmi160/bmi160.h +1 -0
index 923c3b274fde..a82e040bd109 100644
@@ -9,6 +9,7 @@ struct bmi160_data {
 	struct regmap *regmap;
 	struct iio_trigger *trig;
 	struct regulator_bulk_data supplies[2];
+	struct iio_mount_matrix orientation;
 };
 
 extern const struct regmap_config bmi160_regmap_config;
drivers/iio/imu/bmi160/bmi160_core.c +20 -0
index 9bbe0d8e6720..78c8ca962359 100644
@@ -110,6 +110,7 @@
 		.storagebits = 16,				\
 		.endianness = IIO_LE,				\
 	},							\
+	.ext_info = bmi160_ext_info,				\
 }
 
 /* scan indexes follow DATA register order */
@@ -265,6 +266,20 @@ static const struct  bmi160_odr_item bmi160_odr_table[] = {
 	},
 };
 
+static const struct iio_mount_matrix *
+bmi160_get_mount_matrix(const struct iio_dev *indio_dev,
+			const struct iio_chan_spec *chan)
+{
+	struct bmi160_data *data = iio_priv(indio_dev);
+
+	return &data->orientation;
+}
+
+static const struct iio_chan_spec_ext_info bmi160_ext_info[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bmi160_get_mount_matrix),
+	{ }
+};
+
 static const struct iio_chan_spec bmi160_channels[] = {
 	BMI160_CHANNEL(IIO_ACCEL, X, BMI160_SCAN_ACCEL_X),
 	BMI160_CHANNEL(IIO_ACCEL, Y, BMI160_SCAN_ACCEL_Y),
@@ -840,6 +855,11 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 		return ret;
 	}
 
+	ret = iio_read_mount_matrix(dev, "mount-matrix",
+				    &data->orientation);
+	if (ret)
+		return ret;
+
 	ret = bmi160_chip_init(data, use_spi);
 	if (ret)
 		return ret;