Re: [PATCH 1/2] dt-bindings: iio: magnetometer: add ti tmag5273 documentation file

Gerald Loacker
2022-11-17T16:12:46Z

Am 15.11.2022 um 18:43 schrieb Jonathan Cameron:
> On Tue, 15 Nov 2022 08:37:17 +0100
> Gerald Loacker <gerald.loacker@wolfvision.net> wrote:
>
>> Add bindings documentation file for TI TMAG5273.
>>
>> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
>> ---
>> .../iio/magnetometer/ti,tmag5273.yaml | 72 +++++++++++++++++++
>> MAINTAINERS | 6 ++
>> 2 files changed, 78 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml b/Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml
>> new file mode 100644
>> index 000000000000..2f5b0a4d2f40
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml
>> @@ -0,0 +1,72 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: https://eur04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fiio%2Fmagnetometer%2Fti%2Ctmag5273.yaml%23&amp;data=05%7C01%7Cgerald.loacker%40wolfvision.net%7C9788e9788f344fcff9b808dac730f926%7Ce94ec9da9183471e83b351baa8eb804f%7C1%7C0%7C638041310400330990%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=nczO1QC74gD6eGXAkm%2B6LRrc7fyEsr62r%2B3aoW%2Bcfu4%3D&amp;reserved=0
>> +$schema: https://eur04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Cgerald.loacker%40wolfvision.net%7C9788e9788f344fcff9b808dac730f926%7Ce94ec9da9183471e83b351baa8eb804f%7C1%7C0%7C638041310400330990%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=raKUiSntfhvdSSnqiR1Wm%2Fqr9cI3XEu5HCprqvISlLE%3D&amp;reserved=0
>> +
>> +title: TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor
>> +
>> +maintainers:
>> + - Gerald Loacker <gerald.loacker@wolfvision.net>
>> +
>> +description:
>> + The TI TMAG5273 is a low-power linear 3D Hall-effect sensor. This device
>> + integrates three independent Hall-effect sensors in the X, Y, and Z axes.
>> + The device has an integrated temperature sensor available. The TMAG5273
>> + can be configured through the I2C interface to enable any combination of
>> + magnetic axes and temperature measurements. An integrated angle calculation
>> + engine (CORDIC) provides full 360° angular position information for both
>> + on-axis and off-axis angle measurement topologies. The angle calculation is
>> + performed using two user-selected magnetic axes.
>> +
>> +properties:
>> + $nodename:
>> + pattern: '^magnetometer@[0-9a-f]+$'
>
> What Krzysztof said on this ;)
>
>> +
>> + compatible:
>> + const: ti,tmag5273
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + "#io-channel-cells":
>> + const: 1
>> +
>> + ti,angle-enable:
>> + description:
>> + Enables angle measurement in the selected plane.
>> + 0 = OFF
>> + 1 = X-Y (default)
>> + 2 = Y-Z
>> + 3 = X-Z
>
> This feels like something we should be configuring at runtime rather that
> DT, or is it driven by board design or similar?
>

We use this sensor for a zoom wheel application, there is an EVM from TI
for this as well. So this is for setting the mounting position of the wheel.

>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 0
>> + maximum: 3
>> +
>> + vcc-supply:
>> + description:
>> + A regulator providing 1.7 V to 3.6 V supply voltage on the VCC pin,
>> + typically 3.3 V.
>> +
>
> The dt binding should attempt to describe the hardware, not what we happen
> to support in the driver so far. So I'd expect to also see an interrupt.
> That way if someone ships a dts file today, and we enable it sometime in the
> future they will be ready for it.
>

Is it fine to add just the description without example then? The
interrupt has many options such as low active or low pulse.

>> +required:
>> + - compatible
>> + - reg
>> + - vcc-supply
>
> Ah. This is presumably as side effect of having the driver set the voltage.
> Normally we only need to 'require' a supply if we need to read it's voltage
> (for scaling on ADCs and similar). That's not the case here so I wouldn't
> expect to see it.
>
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + i2c-0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + magnetometer@35 {
>> + compatible = "ti,tmag5273";
>> + reg = <0x35>;
>> + #io-channel-cells = <1>;
>> + ti,angle-enable = <3>;
>> + vcc-supply = <&vcc3v3>;
>> + };
>> + };
>> +...
>
>

Thanks for the review,
Gerald