linux-kernel - Re: [PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <210871b8-4967-40c9-bbaf-338d2d6d9c63@baylibre.com>
Date: Fri, 5 Sep 2025 15:15:55 -0500
From: David Lechner <dlechner@...libre.com>
To: Petre Rodan <petre.rodan@...dimension.ro>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: Jonathan Cameron <jic23@...nel.org>, Nuno Sá
 <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
 Rob Herring <robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>
Subject: Re: [PATCH 01/10] dt-bindings: iio: accel: bosch,BMA220 improvements

On 9/1/25 2:47 PM, Petre Rodan wrote:
>  - fix title typo
>  - add optional watchdog setting that recovers the sensor from a stuck-low
>  SDA condition
>  - set correct SPI phase and polarity
>  - interrupt on rising edge. the level-based interrupt that is being
>  replaced was not actually implemented in the driver.
> 
> This set of changes should not negatively affect existing users.
> 
> Signed-off-by: Petre Rodan <petre.rodan@...dimension.ro>
> ---
>  .../bindings/iio/accel/bosch,bma220.yaml      | 20 +++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
> index ec643de031a3..f71b2320b010 100644
> --- a/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
> +++ b/Documentation/devicetree/bindings/iio/accel/bosch,bma220.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/iio/accel/bosch,bma220.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
> 
> -title: Bosch BMA220 Trixial Acceleration Sensor
> +title: Bosch BMA220 Triaxial Acceleration Sensor
> 
>  maintainers:
>    - Jonathan Cameron <Jonathan.Cameron@...wei.com>
> @@ -20,6 +20,20 @@ properties:
>    interrupts:
>      maxItems: 1
> 
> +  bosch,watchdog:
> +    description:
> +      In order to prevent the built-in I2C slave to lock-up the I2C bus, a
> +      watchdog timer is introduced. The WDT observes internal I2C signals and
> +      resets the I2C interface if the bus is locked-up by the BMA220.
> +      0 - off
> +      1 - 1ms
> +      2 - 10ms
> +    enum: [0, 1, 2]
> +    $ref: /schemas/types.yaml#/definitions/uint32

Why should this depend on how the chip is wired up? Normally, we don't have this
sort of control in devicetree. E.g. if it is useful, why shouldn't drivers just
always enable it?

If we can make the case that it belongs in the devicetree, it should use
standard units, e.g. property should be watchdog-timeout-ms with enum: [1, 10].
Maybe 0 for disabled is OK too - in that case should have default: 0.

> +
> +  spi-cpha: true
> +  spi-cpol: true
> +
>    vdda-supply: true
>    vddd-supply: true
>    vddio-supply: true
> @@ -44,8 +58,10 @@ examples:
>              compatible = "bosch,bma220";
>              reg = <0>;
>              spi-max-frequency = <2500000>;
> +            spi-cpol;
> +            spi-cpha;
>              interrupt-parent = <&gpio0>;
> -            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupts = <0 IRQ_TYPE_EDGE_RISING>;
>          };
>      };
>  ...
> --
> 2.49.1
>