Hi Krzysztof, On Mon, Sep 18, 2023 at 5:55 PM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 18/09/2023 10:03, Jagath Jog J wrote: > > Add devicetree description document for Bosch BMI323, a 6-Axis IMU. > > I don't know why this is RFC and cover letter does not explain it. Shall > I just ignore it? Patch is no ready? Recently at least two times someone > was disappointed that his code marked as RFC received my review. Thank you for reviewing. This was the sensor's first patch series, so I initially submitted it as an RFC. I will mark it as "Patch" in the next series. > > A nit, subject: drop second/last, redundant "DT binding doc for". The > "dt-bindings" prefix is already stating that these are bindings. Four > words entirely redundant and duplicating what prefix is saying... Sure I will remove redundant words from the subject line. > > > > > > Signed-off-by: Jagath Jog J <jagathjog1996@xxxxxxxxx> > > --- > > .../bindings/iio/imu/bosch,bmi323.yaml | 81 +++++++++++++++++++ > > 1 file changed, 81 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bmi323.yaml > > > > diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi323.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi323.yaml > > new file mode 100644 > > index 000000000000..9c08988103c5 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi323.yaml > > @@ -0,0 +1,81 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/iio/imu/bosch,bmi323.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Bosch BMI323 6-Axis IMU > > + > > +maintainers: > > + - Jagath Jog J <jagathjog1996@xxxxxxxxx> > > + > > +description: > > + BMI323 is a 6-axis inertial measurement unit that supports acceleration and > > + gyroscopic measurements with hardware fifo buffering. Sensor also provides > > + events information such as motion, steps, orientation, single and double > > + tap detection. > > + > > +properties: > > + compatible: > > + const: bosch,bmi323 > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + interrupt-names: > > + enum: > > + - INT1 > > + - INT2 > > + description: | > > Do not need '|' unless you need to preserve formatting. > > > + set to "INT1" if INT1 pin should be used as interrupt input, set > > + to "INT2" if INT2 pin should be used instead > > And what happens with other INT pin? Remains floating?` Yes, the other pin is unconnected. The driver provides support for either INT1, INT2, or no interrupt configuration. I should have added minItems, maxItems, and const, I will add these in the next series. > > > + > > + drive-open-drain: > > + description: | > > Do not need '|' unless you need to preserve formatting. Sure I will remove this. > > > + set if the specified interrupt pin should be configured as > > + open drain. If not set, defaults to push-pull. > > Missing supplies. Are you sure device does not use any electric energy? Sorry, I missed adding supply, I will add it in the next series. > > > + > > +required: > > + - compatible > > + - reg > > + > > +allOf: > > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + // Example for I2C > > + #include <dt-bindings/interrupt-controller/irq.h> > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > +> + bmi323@68 { > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation I intend to utilize the generic term 'imu' (representing the accelerometer and gyrometer) even though it's not listed in the generic-names recommendation. Would this be acceptable? > > > + compatible = "bosch,bmi323"; > > + reg = <0x68>; > > + interrupt-parent = <&gpio1>; > > + interrupts = <29 IRQ_TYPE_EDGE_RISING>; > > + interrupt-names = "INT1"; > > + }; > > + }; > > + - | > > + // Example for SPI > > + #include <dt-bindings/interrupt-controller/irq.h> > > + spi { > > > It's the same as other example. No difference. Drop. Sure I will keep only one example. > > Best regards, > Krzysztof > Regards, Jagath
