linux-kernel - Re: [PATCH 2/3] dt-bindings: iio: magnetometer: document the Allegro MicroSystems ALS31300 3-D Linear Hall Effect Sensor

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: <20fc022c-874f-489f-8ac8-843db2726393@linaro.org>
Date: Fri, 18 Oct 2024 12:34:09 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-iio@...r.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: iio: magnetometer: document the Allegro
 MicroSystems ALS31300 3-D Linear Hall Effect Sensor

Hi Jonathan,

On 12/10/2024 16:20, Jonathan Cameron wrote:
> On Mon, 07 Oct 2024 15:14:39 +0200
> Neil Armstrong <neil.armstrong@...aro.org> wrote:
> 
>> Document the bindings for the Allegro MicroSystems ALS31300 3-D Linear Hall
>> Effect Sensor controller by an I2C interface, mainly used in 3D head-on
>> motion sensing applications.
>>
>> The device can be configured with different sensitivities in factory,
>> but the sensitivity value used to calculate value into the Gauss
>> unit is not available from registers, thus the sensitivity is
>> provided by the compatible/device-id string which is based
>> on the part number as described in the datasheet page 2.
>>
>> The datasheet is available on the product website at [1].
>>
>> [1] https://www.allegromicro.com/en/products/sense/linear-and-angular-position/linear-position-sensor-ics/als31300
> Use Datasheet tag. It's not that common but it makes it clear what this is and some scripting
> can pick it up.

Ack sure I'll use that instead

> 
>>
> Datasheet: https://www.allegromicro.com/en/products/sense/linear-and-angular-position/linear-position-sensor-ics/als31300
>> Signed-off-by: Neil Armstrong <neil.armstrong@...aro.org>
> Hi Neil,
> 
>> ---
>>   .../iio/magnetometer/allegro,als31300.yaml         | 43 ++++++++++++++++++++++
>>   1 file changed, 43 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/allegro,als31300.yaml b/Documentation/devicetree/bindings/iio/magnetometer/allegro,als31300.yaml
>> new file mode 100644
>> index 000000000000..0a08e769f3aa
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/magnetometer/allegro,als31300.yaml
> 
> Rob's bot has better eyes than me.  Filename needs to be allegromicro,als31300.yaml

Yeah I did modifications without testing it, bad habit....

> 
>> @@ -0,0 +1,43 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/magnetometer/allegromicro,als31300.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Allegro MicroSystems ALS31300 3-D Linear Hall Effect sensor
>> +
>> +maintainers:
>> +  - Neil Armstrong <neil.armstrong@...aro.org>
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: '^magnetometer@[0-9a-f]+$'
>> +
>> +  compatible:
>> +    enum:
>> +      - allegromicro,als31300-500 # Factory configured at 500 Gauss input range
>> +      - allegromicro,als31300-1000 # Factory configured at 1000 Gauss input range
>> +      - allegromicro,als31300-2000 # Factory configured at 2000 Gauss input range
> 
> I was wondering if the range should be a separate property, but given these
> are the part numbers the parts are sold under, I think compatibles are fine.

Yeah, I had the same conclusion

>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  vcc-supply:
>> +    description: 5.5V supply
> A quick glance at the pinout google fed me suggests an interrupt pin.
> Even though the driver doesn't yet support it (I assume as I've not looked at that yet)
> the binding should include it.

Sure I'll add it even if how it's configured in the epprom, it's barely usable by default.

> 
>> +
>> +required:
>> +  - compatible
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    i2c {
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +      sensor@61 {
> 
> magnetometer@61 {

Ack

> 
>> +        compatible = "allegromicro,als31300";
>> +        reg = <0x61>;
>> +        vcc-supply = <&hall_vcc>;
>> +      };
>> +    };
>>
> 

Thanks for the review,
Neil