2022-11-15 08:05:19

by Gerald Loacker

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

Add bindings documentation file for TI TMAG5273.

Signed-off-by: Gerald Loacker <[email protected]>
---
.../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: http://devicetree.org/schemas/iio/magnetometer/ti,tmag5273.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor
+
+maintainers:
+ - Gerald Loacker <[email protected]>
+
+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]+$'
+
+ 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
+ $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.
+
+required:
+ - compatible
+ - reg
+ - vcc-supply
+
+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>;
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index cf0f18502372..ea7acec52f8b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20613,6 +20613,12 @@ L: [email protected] (moderated for non-subscribers)
S: Odd Fixes
F: sound/soc/codecs/tas571x*

+TI TMAG5273 MAGNETOMETER DRIVER
+M: Gerald Loacker <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml
+
TI TRF7970A NFC DRIVER
M: Mark Greer <[email protected]>
L: [email protected]
--
2.37.2



2022-11-15 08:36:22

by Krzysztof Kozlowski

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

On 15/11/2022 08:37, Gerald Loacker wrote:
> Add bindings documentation file for TI TMAG5273.

Subject - drop "documentation file".

>
> Signed-off-by: Gerald Loacker <[email protected]>
> ---
> .../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: http://devicetree.org/schemas/iio/magnetometer/ti,tmag5273.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor
> +
> +maintainers:
> + - Gerald Loacker <[email protected]>
> +
> +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]+$'

Device schemas do not need to enforce the names.

> +
> + compatible:
> + const: ti,tmag5273
> +
> + reg:
> + maxItems: 1
> +
> + "#io-channel-cells":
> + const: 1
> +
> + ti,angle-enable:

ti,angle-measurement

> + description:
> + Enables angle measurement in the selected plane.
> + 0 = OFF
> + 1 = X-Y (default)
> + 2 = Y-Z
> + 3 = X-Z

Why not strings which are easier for humans? off/x-y/y-z/x-z? How anyone
reading DTS can remember what is "3" in this and in thousands of other
devices?

> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 3

default: 1

> +
> + vcc-supply:
> + description:
> + A regulator providing 1.7 V to 3.6 V supply voltage on the VCC pin,
> + typically 3.3 V.
> +
> +required:
> + - compatible
> + - reg
> + - vcc-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c-0 {

Just: i2c


Best regards,
Krzysztof


2022-11-15 18:31:32

by Jonathan Cameron

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

On Tue, 15 Nov 2022 08:37:17 +0100
Gerald Loacker <[email protected]> wrote:

> Add bindings documentation file for TI TMAG5273.
>
> Signed-off-by: Gerald Loacker <[email protected]>
> ---
> .../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: http://devicetree.org/schemas/iio/magnetometer/ti,tmag5273.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor
> +
> +maintainers:
> + - Gerald Loacker <[email protected]>
> +
> +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?

> + $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.

> +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>;
> + };
> + };
> +...



2022-11-17 16:29:03

by Gerald Loacker

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



Am 15.11.2022 um 18:43 schrieb Jonathan Cameron:
> On Tue, 15 Nov 2022 08:37:17 +0100
> Gerald Loacker <[email protected]> wrote:
>
>> Add bindings documentation file for TI TMAG5273.
>>
>> Signed-off-by: Gerald Loacker <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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

2022-11-17 16:30:38

by Krzysztof Kozlowski

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

On 17/11/2022 17:12, Gerald Loacker wrote:
>>
>>> +
>>> + 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.

That's ok, but does not explain why choice of angle measurement should
be a property of the hardware. I could imagine configuring device to
measure sometimes X-Y and sometimes X-Z, depending on the use case. Use
case can change runtime.


Best regards,
Krzysztof


2022-11-17 17:03:59

by Jonathan Cameron

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

On Thu, 17 Nov 2022 17:12:33 +0100
Gerald Loacker <[email protected]> wrote:

> Am 15.11.2022 um 18:43 schrieb Jonathan Cameron:
> > On Tue, 15 Nov 2022 08:37:17 +0100
> > Gerald Loacker <[email protected]> wrote:
> >
> >> Add bindings documentation file for TI TMAG5273.
> >>
> >> Signed-off-by: Gerald Loacker <[email protected]>
> >> ---
> >> .../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 <[email protected]>
> >> +
> >> +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.

Ok. Thanks for explanation. Makes sense.

>
> >> + $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.

Yes. Just add description. The details of type etc depend on wiring
to some degree (inverters in the path etc). Nice to add an example later
but for now we can assume anyone who is providing the interrupt set it
up correctly.

Jonathan

2022-11-17 17:17:19

by Jonathan Cameron

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

On Thu, 17 Nov 2022 17:17:24 +0100
Krzysztof Kozlowski <[email protected]> wrote:

> On 17/11/2022 17:12, Gerald Loacker wrote:
> >>
> >>> +
> >>> + 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.
>
> That's ok, but does not explain why choice of angle measurement should
> be a property of the hardware. I could imagine configuring device to
> measure sometimes X-Y and sometimes X-Z, depending on the use case. Use
> case can change runtime.

If it's part of a physical device mounting, may well not be changeable at runtime
(at least not with out a screw driver / hacksaw etc. Not really different form
arguing someone might rewire a sensor at runtime. In theory possible but if they
do they are on their own - mostly we don't bother supporting them doing that.

So I think this is probably valid as a DT property.


>
>
> Best regards,
> Krzysztof
>


2022-11-17 17:19:08

by Michael Riesch

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

Hi Krzysztof,

On 11/17/22 17:17, Krzysztof Kozlowski wrote:
> On 17/11/2022 17:12, Gerald Loacker wrote:
>>>
>>>> +
>>>> + 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.
>
> That's ok, but does not explain why choice of angle measurement should
> be a property of the hardware. I could imagine configuring device to
> measure sometimes X-Y and sometimes X-Z, depending on the use case. Use
> case can change runtime.

If I may chime in here: in our use case the angle channel is used
directly as an input to adc-joystick, so that the combination of the two
is an input device. We feel that in this scenario this angle measurement
setting *has* to be a hardware property because the correct function of
the input device depends on the correct choice of the angle property
(which does not change during runtime, of course).

If we were to create a different input device in which the magnetometer
was tilted by 90° (for example), then the angle property could be easily
changed in the device tree. The user space, on the other hand, couldn't
possibly know the correct angle property.

That said, I agree that there may be use cases in which the angle
property should be changed during runtime. Would it be acceptable to
create an IIO property that is initialized by the device tree property?
(Please note that the implementation of the IIO property may not be in
our scope, though)

Best regards,
Michael

2022-11-17 17:38:41

by Krzysztof Kozlowski

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

On 17/11/2022 18:01, Michael Riesch wrote:
> Hi Krzysztof,
>
> On 11/17/22 17:17, Krzysztof Kozlowski wrote:
>> On 17/11/2022 17:12, Gerald Loacker wrote:
>>>>
>>>>> +
>>>>> + 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.
>>
>> That's ok, but does not explain why choice of angle measurement should
>> be a property of the hardware. I could imagine configuring device to
>> measure sometimes X-Y and sometimes X-Z, depending on the use case. Use
>> case can change runtime.
>
> If I may chime in here: in our use case the angle channel is used
> directly as an input to adc-joystick, so that the combination of the two
> is an input device. We feel that in this scenario this angle measurement
> setting *has* to be a hardware property because the correct function of
> the input device depends on the correct choice of the angle property
> (which does not change during runtime, of course).
>
> If we were to create a different input device in which the magnetometer
> was tilted by 90° (for example), then the angle property could be easily
> changed in the device tree. The user space, on the other hand, couldn't
> possibly know the correct angle property.
>
> That said, I agree that there may be use cases in which the angle
> property should be changed during runtime. Would it be acceptable to
> create an IIO property that is initialized by the device tree property?
> (Please note that the implementation of the IIO property may not be in
> our scope, though)

It's fine. Thanks for the explanations. For runtime-configurable setups
this still can be changed via some other interface.

Best regards,
Krzysztof