[PATCH v2] media: dt-bindings: Convert Analog Devices ad5820 to DT schema

[PATCH v2] media: dt-bindings: Convert Analog Devices ad5820 to DT schema

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Convert the Analog Devices ad5820 to DT schema format.

Add the previously undocumented io-channel-cells property,
which can be omitted. If present, it must be set to 0,
as the device provides only one channel.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
Changes in v2:
- added MAINTAINERS entry for the binding
- documented why io-channel-cells got added into the binding.
- dropped io-channel-cells in required properties.
- adjusted example indentation to 4 spaces.
- Link to v1: https://lore.kernel.org/r/20250209203940.159088-1-david@ixit.cz
---
 .../devicetree/bindings/media/i2c/ad5820.txt       | 28 ----------
 .../devicetree/bindings/media/i2c/adi,ad5820.yaml  | 59 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 3 files changed, 60 insertions(+), 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
deleted file mode 100644
index 5764cbedf9b73387ad1bfa9acf99c643f959b84a..0000000000000000000000000000000000000000
--- a/Documentation/devicetree/bindings/media/i2c/ad5820.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-* Analog Devices AD5820 autofocus coil
-
-Required Properties:
-
-  - compatible: Must contain one of:
-		- "adi,ad5820"
-		- "adi,ad5821"
-		- "adi,ad5823"
-
-  - reg: I2C slave address
-
-  - VANA-supply: supply of voltage for VANA pin
-
-Optional properties:
-
-   - enable-gpios : GPIO spec for the XSHUTDOWN pin. The XSHUTDOWN signal is
-active low, a high level on the pin enables the device.
-
-Example:
-
-       ad5820: coil@c {
-               compatible = "adi,ad5820";
-               reg = <0x0c>;
-
-               VANA-supply = <&vaux4>;
-               enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>;
-       };
-
diff --git a/Documentation/devicetree/bindings/media/i2c/adi,ad5820.yaml b/Documentation/devicetree/bindings/media/i2c/adi,ad5820.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..7967b8dcd77a2171074829625d71dbb53a80fbc6
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/adi,ad5820.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/adi,ad5820.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5820 autofocus coil
+
+maintainers:
+  - Pavel Machek <pavel@ucw.cz>
+
+description:
+  The AD5820 is a current sink driver designed for precise control of
+  voice coil motors (VCMs) in camera autofocus systems.
+
+properties:
+  compatible:
+    enum:
+      - adi,ad5820
+      - adi,ad5821
+      - adi,ad5823
+
+  reg:
+    maxItems: 1
+
+  enable-gpios:
+    maxItems: 1
+    description:
+      GPIO spec for the XSHUTDOWN pin. The XSHUTDOWN signal is active low,
+      a high level on the pin enables the device.
+
+  VANA-supply:
+    description: supply of voltage for VANA pin
+
+  '#io-channel-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - VANA-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        coil@c {
+            compatible = "adi,ad5820";
+            reg = <0x0c>;
+
+            enable-gpios = <&msmgpio 26 GPIO_ACTIVE_HIGH>;
+            VANA-supply = <&vaux4>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index e4b6545aab3ec75f92c0daaabe2fc1bb1d483969..238c00d3abe2f53ea7b7e6ba265d560ad5c3fe86 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17073,6 +17073,7 @@ M:	Pavel Machek <pavel@kernel.org>
 M:	Sakari Ailus <sakari.ailus@iki.fi>
 L:	linux-media@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/media/i2c/adi,ad5820.yaml
 F:	drivers/media/i2c/ad5820.c
 F:	drivers/media/i2c/et8ek8
 

---
base-commit: da920b7df701770e006928053672147075587fb2
change-id: 20250314-b4-ad5820-dt-yaml-3220bf2f1e40

Best regards,
-- 
David Heidelberg <david@ixit.cz>

Re: [PATCH v2] media: dt-bindings: Convert Analog Devices ad5820 to DT schema

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

On Fri 2025-03-14 20:58:27, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
> 
> Convert the Analog Devices ad5820 to DT schema format.
> 
> Add the previously undocumented io-channel-cells property,
> which can be omitted. If present, it must be set to 0,
> as the device provides only one channel.
> 
Acked-by: Pavel Machek <pavel@ucw.cz>

Thanks,
							Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH v2] media: dt-bindings: Convert Analog Devices ad5820 to DT schema

[Sakari Ailus]

Hi David,

Thanks for converting this to YAML.

On Fri, Mar 14, 2025 at 08:58:27PM +0100, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
> 
> Convert the Analog Devices ad5820 to DT schema format.
> 
> Add the previously undocumented io-channel-cells property,
> which can be omitted. If present, it must be set to 0,
> as the device provides only one channel.

What's the purpose of this property? The driver doesn't use it nor I think
it provides any information on the hardware either. The above description
also appears to be saying it's redundant.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v2] media: dt-bindings: Convert Analog Devices ad5820 to DT schema

[David Heidelberg]

On 14/03/2025 21:57, Sakari Ailus wrote:
> Hi David,
> 
> Thanks for converting this to YAML.
> 
> On Fri, Mar 14, 2025 at 08:58:27PM +0100, David Heidelberg via B4 Relay wrote:
>> From: David Heidelberg <david@ixit.cz>
>>
>> Convert the Analog Devices ad5820 to DT schema format.
>>
>> Add the previously undocumented io-channel-cells property,
>> which can be omitted. If present, it must be set to 0,
>> as the device provides only one channel.
> 
> What's the purpose of this property? The driver doesn't use it nor I think
> it provides any information on the hardware either. The above description
> also appears to be saying it's redundant.

Hello Sakari,

from my understanding, you're right.

I would prefer to drop it, see [1].

Anyway from reading of documentation I understood it may be right to 
have the property empty, but also may be omitted. I saw both approaches 
in the code.

If you choose not liking this redudancy, I push [1] and drop the 
property here.

David



[1] 
https://patchwork.kernel.org/project/linux-omap/patch/20250213203208.93316-1-david@ixit.cz/

-- 
David Heidelberg

Re: [PATCH v2] media: dt-bindings: Convert Analog Devices ad5820 to DT schema

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2570 bytes --]

Hello,

On Sat, Mar 15, 2025 at 02:18:40PM +0100, David Heidelberg wrote:
> On 14/03/2025 21:57, Sakari Ailus wrote:
> > Thanks for converting this to YAML.
> > 
> > On Fri, Mar 14, 2025 at 08:58:27PM +0100, David Heidelberg via B4 Relay wrote:
> > > From: David Heidelberg <david@ixit.cz>
> > > 
> > > Convert the Analog Devices ad5820 to DT schema format.
> > > 
> > > Add the previously undocumented io-channel-cells property,
> > > which can be omitted. If present, it must be set to 0,
> > > as the device provides only one channel.
> > 
> > What's the purpose of this property? The driver doesn't use it nor I think
> > it provides any information on the hardware either. The above description
> > also appears to be saying it's redundant.
> 
> Hello Sakari,
> 
> from my understanding, you're right.
> 
> I would prefer to drop it, see [1].
> 
> Anyway from reading of documentation I understood it may be right to have
> the property empty, but also may be omitted. I saw both approaches in the
> code.
> 
> If you choose not liking this redudancy, I push [1] and drop the property
> here.
> 
> David
> 
> [1] https://patchwork.kernel.org/project/linux-omap/patch/20250213203208.93316-1-david@ixit.cz/

Like all DT properties starting with #, this is not about hardware,
but about parsing the device tree itself and used by core DT code:

$ git grep io-channels drivers/of/property.c
drivers/of/property.c:DEFINE_SIMPLE_PROP(io_channels, "io-channels", "#io-channel-cells")

Have a look at the following example DT:

iio_device1: iio-provider1 {
    #io-channel-cells = <2>;
};

iio_device2: iio-provider2 {
    #io-channel-cells = <1>;
};

iio_device3: iio-provider3 {
    #io-channel-cells = <0>;
};

example-iio-user {
    my-iio-reference = <&iio_device1 AREA0 CHANNEL2>, <&iio_device3>, <&iio_device2 CHAN0>;
};

While you can easily understand that without the io-channel-cells,
please have a look what it looks like after sending it through the
DT compiler. You will get something like this:

iio-provider1 {
    #io-channel-cells = <2>;
    phandle = <23>;
};

iio-provider2 {
    #io-channel-cells = <1>;
    phandle = <10>;
};

iio-provider3 {
    #io-channel-cells = <0>;
    phandle = <37>;
};

example-iio-user {
    my-iio-reference = <23 42 13 37 10 0>;
};

But you can safely drop it, when there are no references to the IIO
device. So it's obviously not required in all cases.

I hope I could explain the purpose well enough :)

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] media: dt-bindings: Convert Analog Devices ad5820 to DT schema

[David Heidelberg]

On 15/03/2025 16:49, Sebastian Reichel wrote:
> Hello,
> 
> On Sat, Mar 15, 2025 at 02:18:40PM +0100, David Heidelberg wrote:
>> On 14/03/2025 21:57, Sakari Ailus wrote:
>>> Thanks for converting this to YAML.
>>>
>>> On Fri, Mar 14, 2025 at 08:58:27PM +0100, David Heidelberg via B4 Relay wrote:
>>>> From: David Heidelberg <david@ixit.cz>
>>>>
>>>> Convert the Analog Devices ad5820 to DT schema format.
>>>>
>>>> Add the previously undocumented io-channel-cells property,
>>>> which can be omitted. If present, it must be set to 0,
>>>> as the device provides only one channel.
>>>
>>> What's the purpose of this property? The driver doesn't use it nor I think
>>> it provides any information on the hardware either. The above description
>>> also appears to be saying it's redundant.
>>
>> Hello Sakari,
>>
>> from my understanding, you're right.
>>
>> I would prefer to drop it, see [1].
>>
>> Anyway from reading of documentation I understood it may be right to have
>> the property empty, but also may be omitted. I saw both approaches in the
>> code.
>>
>> If you choose not liking this redudancy, I push [1] and drop the property
>> here.
>>
>> David
>>
>> [1] https://patchwork.kernel.org/project/linux-omap/patch/20250213203208.93316-1-david@ixit.cz/
> 
> Like all DT properties starting with #, this is not about hardware,
> but about parsing the device tree itself and used by core DT code:
> 
> $ git grep io-channels drivers/of/property.c
> drivers/of/property.c:DEFINE_SIMPLE_PROP(io_channels, "io-channels", "#io-channel-cells")
> 
> Have a look at the following example DT:
> 
> iio_device1: iio-provider1 {
>      #io-channel-cells = <2>;
> };
> 
> iio_device2: iio-provider2 {
>      #io-channel-cells = <1>;
> };
> 
> iio_device3: iio-provider3 {
>      #io-channel-cells = <0>;
> };
> 
> example-iio-user {
>      my-iio-reference = <&iio_device1 AREA0 CHANNEL2>, <&iio_device3>, <&iio_device2 CHAN0>;
> };
> 
> While you can easily understand that without the io-channel-cells,
> please have a look what it looks like after sending it through the
> DT compiler. You will get something like this:
> 
> iio-provider1 {
>      #io-channel-cells = <2>;
>      phandle = <23>;
> };
> 
> iio-provider2 {
>      #io-channel-cells = <1>;
>      phandle = <10>;
> };
> 
> iio-provider3 {
>      #io-channel-cells = <0>;
>      phandle = <37>;
> };
> 
> example-iio-user {
>      my-iio-reference = <23 42 13 37 10 0>;
> };
> 
> But you can safely drop it, when there are no references to the IIO
> device. So it's obviously not required in all cases.
> 
> I hope I could explain the purpose well enough :)

This looks like a very good explanation, it helped me a lot.

Any chance to sneak it into the docs somewhere?

David

> 
> Greetings,
> 
> -- Sebastian

-- 
David Heidelberg

Re: [PATCH v2] media: dt-bindings: Convert Analog Devices ad5820 to DT schema

[Krzysztof Kozlowski]

On Fri, Mar 14, 2025 at 08:58:27PM +0100, David Heidelberg wrote:
> Convert the Analog Devices ad5820 to DT schema format.
> 
> Add the previously undocumented io-channel-cells property,

You did not explain here *why* you added this property. Your patch, your
changes, anything should say why you are doing it.

> which can be omitted. If present, it must be set to 0,
> as the device provides only one channel.

Also, Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
> Changes in v2:
> - added MAINTAINERS entry for the binding
> - documented why io-channel-cells got added into the binding.
> - dropped io-channel-cells in required properties.
> - adjusted example indentation to 4 spaces.
> - Link to v1: https://lore.kernel.org/r/20250209203940.159088-1-david@ixit.cz

Best regards,
Krzysztof