diff mbox series

[v4,2/5] dt-bindings: rtc: abx80x: convert to yaml

Message ID 20240202-add-am64-som-v4-2-5f8b12af5e71@solid-run.com
State Superseded
Headers show
Series arm64: dts: add description for solidrun am642 som and hummingboard evb | expand

Commit Message

Josua Mayer Feb. 2, 2024, 4:10 p.m. UTC
Convert the abracon abx80x rtc text bindings to dt-schema format.

In addition to the text description reference generic interrupts
properties and add an example.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 .../devicetree/bindings/rtc/abracon,abx80x.txt     | 31 ---------
 .../devicetree/bindings/rtc/abracon,abx80x.yaml    | 74 ++++++++++++++++++++++
 2 files changed, 74 insertions(+), 31 deletions(-)

Comments

Rob Herring (Arm) Feb. 2, 2024, 5:35 p.m. UTC | #1
On Fri, 02 Feb 2024 17:10:49 +0100, Josua Mayer wrote:
> Convert the abracon abx80x rtc text bindings to dt-schema format.
> 
> In addition to the text description reference generic interrupts
> properties and add an example.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  .../devicetree/bindings/rtc/abracon,abx80x.txt     | 31 ---------
>  .../devicetree/bindings/rtc/abracon,abx80x.yaml    | 74 ++++++++++++++++++++++
>  2 files changed, 74 insertions(+), 31 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/rtc/abracon,abx80x.example.dts:28.13-26: Warning (reg_format): /example-0/rtc@69:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/rtc/abracon,abx80x.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/rtc/abracon,abx80x.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/rtc/abracon,abx80x.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/rtc/abracon,abx80x.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/rtc/abracon,abx80x.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240202-add-am64-som-v4-2-5f8b12af5e71@solid-run.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Conor Dooley Feb. 3, 2024, 3:08 p.m. UTC | #2
Hey,

On Fri, Feb 02, 2024 at 05:10:49PM +0100, Josua Mayer wrote:
> Convert the abracon abx80x rtc text bindings to dt-schema format.
> 
> In addition to the text description reference generic interrupts
> properties and add an example.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  .../devicetree/bindings/rtc/abracon,abx80x.txt     | 31 ---------
>  .../devicetree/bindings/rtc/abracon,abx80x.yaml    | 74 ++++++++++++++++++++++
>  2 files changed, 74 insertions(+), 31 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> deleted file mode 100644
> index 2405e35a1bc0..000000000000
> --- a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -Abracon ABX80X I2C ultra low power RTC/Alarm chip
> -
> -The Abracon ABX80X family consist of the ab0801, ab0803, ab0804, ab0805, ab1801,
> -ab1803, ab1804 and ab1805. The ab0805 is the superset of ab080x and the ab1805
> -is the superset of ab180x.
> -
> -Required properties:
> -
> - - "compatible": should one of:
> -        "abracon,abx80x"
> -        "abracon,ab0801"
> -        "abracon,ab0803"
> -        "abracon,ab0804"
> -        "abracon,ab0805"
> -        "abracon,ab1801"
> -        "abracon,ab1803"
> -        "abracon,ab1804"
> -        "abracon,ab1805"
> -        "microcrystal,rv1805"
> -	Using "abracon,abx80x" will enable chip autodetection.
> - - "reg": I2C bus address of the device
> -
> -Optional properties:
> -
> -The abx804 and abx805 have a trickle charger that is able to charge the
> -connected battery or supercap. Both the following properties have to be defined
> -and valid to enable charging:
> -
> - - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V)
> - - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the output
> -                          resistor, the other values are in kOhm.
> diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
> new file mode 100644
> index 000000000000..405b386a54b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/abracon,abx80x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Abracon ABX80X I2C ultra low power RTC/Alarm chip
> +
> +maintainers:
> +  - devicetree@vger.kernel.org

Ideally you put someone here, not the DT list. Usually the original
author is a good choice, which I think happens to be the subsystem
maintainer... Failing that, the rtc subsystem list is likely a better
choice than the DT one.

> +
> +allOf:
> +  - $ref: rtc.yaml#

> +  - $ref: /schemas/interrupts.yaml#

This should not be need.

> +
> +properties:
> +  compatible:
> +    description:
> +      Select a specific compatible chip.

I'd drop this line.

> +      'abracon,abx80x' has special meaning,
> +      it provides auto-dection based on ID register.

And reword this. The compatible itself does not provide auto-detection,
it's the opposite - the driver must perform auto detection if this
compatible is used.

> +    enum:
> +      - abracon,abx80x
> +      - abracon,ab0801
> +      - abracon,ab0803
> +      - abracon,ab0804
> +      - abracon,ab0805
> +      - abracon,ab1801
> +      - abracon,ab1803
> +      - abracon,ab1804
> +      - abracon,ab1805
> +      - microcrystal,rv1805
> +
> +  reg:
> +    maxItems: 1
> +
> +  abracon,tc-diode:
> +    description:
> +      Trickle-charge diode type.
> +      Required to enable charging backup battery.
> +
> +      Supported are 'standard' diodes with a 0.6V drop
> +      and 'schottky' diodes with a 0.3V drop.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - standard
> +      - schottky
> +
> +  abracon,tc-resistor:
> +    description:
> +      Trickle-charge resistor value in kOhm.
> +      Required to enable charging backup battery.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 3, 6, 11]
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    rtc@69 {
> +        compatible = "abracon,abx80x";
> +        reg = <0x69>;

You'll have to make a "fake" i2c bus here to satisfy the tooling. There
should be lots of examples for how to do this in other rtc bindings.

Thanks,
Conor.

> +        abracon,tc-diode = "schottky";
> +        abracon,tc-resistor = <3>;
> +        interrupt-parent = <&fake_intc0>;
> +        interrupts = <44 IRQ_TYPE_EDGE_FALLING>;
> +    };
> 
> -- 
> 2.35.3
>
Conor Dooley Feb. 3, 2024, 3:10 p.m. UTC | #3
On Sat, Feb 03, 2024 at 03:08:59PM +0000, Conor Dooley wrote:
> Hey,
> 
> On Fri, Feb 02, 2024 at 05:10:49PM +0100, Josua Mayer wrote:
> > Convert the abracon abx80x rtc text bindings to dt-schema format.
> > 
> > In addition to the text description reference generic interrupts
> > properties and add an example.
> > 
> > Signed-off-by: Josua Mayer <josua@solid-run.com>
> > ---
> >  .../devicetree/bindings/rtc/abracon,abx80x.txt     | 31 ---------
> >  .../devicetree/bindings/rtc/abracon,abx80x.yaml    | 74 ++++++++++++++++++++++
> >  2 files changed, 74 insertions(+), 31 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> > deleted file mode 100644
> > index 2405e35a1bc0..000000000000
> > --- a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> > +++ /dev/null
> > @@ -1,31 +0,0 @@
> > -Abracon ABX80X I2C ultra low power RTC/Alarm chip
> > -
> > -The Abracon ABX80X family consist of the ab0801, ab0803, ab0804, ab0805, ab1801,
> > -ab1803, ab1804 and ab1805. The ab0805 is the superset of ab080x and the ab1805
> > -is the superset of ab180x.
> > -
> > -Required properties:
> > -
> > - - "compatible": should one of:
> > -        "abracon,abx80x"
> > -        "abracon,ab0801"
> > -        "abracon,ab0803"
> > -        "abracon,ab0804"
> > -        "abracon,ab0805"
> > -        "abracon,ab1801"
> > -        "abracon,ab1803"
> > -        "abracon,ab1804"
> > -        "abracon,ab1805"
> > -        "microcrystal,rv1805"
> > -	Using "abracon,abx80x" will enable chip autodetection.
> > - - "reg": I2C bus address of the device
> > -
> > -Optional properties:
> > -
> > -The abx804 and abx805 have a trickle charger that is able to charge the
> > -connected battery or supercap. Both the following properties have to be defined
> > -and valid to enable charging:
> > -
> > - - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V)
> > - - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the output
> > -                          resistor, the other values are in kOhm.
> > diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
> > new file mode 100644
> > index 000000000000..405b386a54b0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
> > @@ -0,0 +1,74 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/rtc/abracon,abx80x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Abracon ABX80X I2C ultra low power RTC/Alarm chip
> > +
> > +maintainers:
> > +  - devicetree@vger.kernel.org
> 
> Ideally you put someone here, not the DT list. Usually the original
> author is a good choice, which I think happens to be the subsystem
> maintainer... Failing that, the rtc subsystem list is likely a better
> choice than the DT one.
> 
> > +
> > +allOf:
> > +  - $ref: rtc.yaml#
> 
> > +  - $ref: /schemas/interrupts.yaml#
> 
> This should not be need.

Ahh I now realise what your intent was here. All you need to do is add
| interrupts:
|   maxItems: 1
to your binding and it should do what you're looking for.
Josua Mayer Feb. 6, 2024, 2:44 p.m. UTC | #4
Am 03.02.24 um 16:10 schrieb Conor Dooley:
> On Sat, Feb 03, 2024 at 03:08:59PM +0000, Conor Dooley wrote:
>> Hey,
>>
>> On Fri, Feb 02, 2024 at 05:10:49PM +0100, Josua Mayer wrote:
>>> Convert the abracon abx80x rtc text bindings to dt-schema format.
>>>
>>> In addition to the text description reference generic interrupts
>>> properties and add an example.
>>>
>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>> ---
>>>  .../devicetree/bindings/rtc/abracon,abx80x.txt     | 31 ---------
>>>  .../devicetree/bindings/rtc/abracon,abx80x.yaml    | 74 ++++++++++++++++++++++
>>>  2 files changed, 74 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
>>> deleted file mode 100644
>>> index 2405e35a1bc0..000000000000
>>> --- a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
>>> +++ /dev/null
>>> @@ -1,31 +0,0 @@
>>> -Abracon ABX80X I2C ultra low power RTC/Alarm chip
>>> -
>>> -The Abracon ABX80X family consist of the ab0801, ab0803, ab0804, ab0805, ab1801,
>>> -ab1803, ab1804 and ab1805. The ab0805 is the superset of ab080x and the ab1805
>>> -is the superset of ab180x.
>>> -
>>> -Required properties:
>>> -
>>> - - "compatible": should one of:
>>> -        "abracon,abx80x"
>>> -        "abracon,ab0801"
>>> -        "abracon,ab0803"
>>> -        "abracon,ab0804"
>>> -        "abracon,ab0805"
>>> -        "abracon,ab1801"
>>> -        "abracon,ab1803"
>>> -        "abracon,ab1804"
>>> -        "abracon,ab1805"
>>> -        "microcrystal,rv1805"
>>> -	Using "abracon,abx80x" will enable chip autodetection.
>>> - - "reg": I2C bus address of the device
>>> -
>>> -Optional properties:
>>> -
>>> -The abx804 and abx805 have a trickle charger that is able to charge the
>>> -connected battery or supercap. Both the following properties have to be defined
>>> -and valid to enable charging:
>>> -
>>> - - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V)
>>> - - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the output
>>> -                          resistor, the other values are in kOhm.
>>> diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
>>> new file mode 100644
>>> index 000000000000..405b386a54b0
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
>>> @@ -0,0 +1,74 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/rtc/abracon,abx80x.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Abracon ABX80X I2C ultra low power RTC/Alarm chip
>>> +
>>> +maintainers:
>>> +  - devicetree@vger.kernel.org
>> Ideally you put someone here, not the DT list. Usually the original
>> author is a good choice, which I think happens to be the subsystem
>> maintainer... Failing that, the rtc subsystem list is likely a better
>> choice than the DT one.
>>
>>> +
>>> +allOf:
>>> +  - $ref: rtc.yaml#
>>> +  - $ref: /schemas/interrupts.yaml#
I was intentionally looking for some feedback on this idea,
for solving dtbs_check complaints for both interrupts and
interrupt-parent properties:

arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dtb: rtc@69:
Unevaluated properties are not allowed ('interrupt-parent', 'interrupts' were unexpected)
       from schema $id: http://devicetree.org/schemas/rtc/abracon,abx80x.yaml#

> Ahh I now realise what your intent was here. All you need to do is add
> | interrupts:
> |   maxItems: 1
> to your binding and it should do what you're looking for.

Yes, that is in line with everything else.

What bugs me is what to do about interrupt-parent,
and whether to include it in example.
Conor Dooley Feb. 7, 2024, 4:22 p.m. UTC | #5
On Tue, Feb 06, 2024 at 02:44:24PM +0000, Josua Mayer wrote:

> > Ahh I now realise what your intent was here. All you need to do is add
> > | interrupts:
> > |   maxItems: 1
> > to your binding and it should do what you're looking for.
> 
> Yes, that is in line with everything else.
> 
> What bugs me is what to do about interrupt-parent,
> and whether to include it in example.

I am pretty sure you don't need to add it.
Josua Mayer Feb. 11, 2024, 2:57 p.m. UTC | #6
Am 03.02.24 um 16:08 schrieb Conor Dooley:
> Hey,
>
> On Fri, Feb 02, 2024 at 05:10:49PM +0100, Josua Mayer wrote:
>> Convert the abracon abx80x rtc text bindings to dt-schema format.
>>
>> In addition to the text description reference generic interrupts
>> properties and add an example.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
...
>> diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
>> new file mode 100644
>> index 000000000000..405b386a54b0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
>> @@ -0,0 +1,74 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/rtc/abracon,abx80x.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Abracon ABX80X I2C ultra low power RTC/Alarm chip
>> +
>> +maintainers:
>> +  - devicetree@vger.kernel.org
> Ideally you put someone here, not the DT list. Usually the original
> author is a good choice, which I think happens to be the subsystem
> maintainer... Failing that, the rtc subsystem list is likely a better
> choice than the DT one.
rtc-abx80x.c mentions:
MODULE_AUTHOR("Philippe De Muyter <phdm@macqel.be>");
MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@bootlin.com>");

I personally prefer to put a list, since I don't know status of first author,
and second author / rtc subsystem maintainer is automatic.

So v5 will feature linux-rtc@vger.kernel.org.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
deleted file mode 100644
index 2405e35a1bc0..000000000000
--- a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
+++ /dev/null
@@ -1,31 +0,0 @@ 
-Abracon ABX80X I2C ultra low power RTC/Alarm chip
-
-The Abracon ABX80X family consist of the ab0801, ab0803, ab0804, ab0805, ab1801,
-ab1803, ab1804 and ab1805. The ab0805 is the superset of ab080x and the ab1805
-is the superset of ab180x.
-
-Required properties:
-
- - "compatible": should one of:
-        "abracon,abx80x"
-        "abracon,ab0801"
-        "abracon,ab0803"
-        "abracon,ab0804"
-        "abracon,ab0805"
-        "abracon,ab1801"
-        "abracon,ab1803"
-        "abracon,ab1804"
-        "abracon,ab1805"
-        "microcrystal,rv1805"
-	Using "abracon,abx80x" will enable chip autodetection.
- - "reg": I2C bus address of the device
-
-Optional properties:
-
-The abx804 and abx805 have a trickle charger that is able to charge the
-connected battery or supercap. Both the following properties have to be defined
-and valid to enable charging:
-
- - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V)
- - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the output
-                          resistor, the other values are in kOhm.
diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
new file mode 100644
index 000000000000..405b386a54b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
@@ -0,0 +1,74 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/abracon,abx80x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Abracon ABX80X I2C ultra low power RTC/Alarm chip
+
+maintainers:
+  - devicetree@vger.kernel.org
+
+allOf:
+  - $ref: rtc.yaml#
+  - $ref: /schemas/interrupts.yaml#
+
+properties:
+  compatible:
+    description:
+      Select a specific compatible chip.
+
+      'abracon,abx80x' has special meaning,
+      it provides auto-dection based on ID register.
+    enum:
+      - abracon,abx80x
+      - abracon,ab0801
+      - abracon,ab0803
+      - abracon,ab0804
+      - abracon,ab0805
+      - abracon,ab1801
+      - abracon,ab1803
+      - abracon,ab1804
+      - abracon,ab1805
+      - microcrystal,rv1805
+
+  reg:
+    maxItems: 1
+
+  abracon,tc-diode:
+    description:
+      Trickle-charge diode type.
+      Required to enable charging backup battery.
+
+      Supported are 'standard' diodes with a 0.6V drop
+      and 'schottky' diodes with a 0.3V drop.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - standard
+      - schottky
+
+  abracon,tc-resistor:
+    description:
+      Trickle-charge resistor value in kOhm.
+      Required to enable charging backup battery.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 3, 6, 11]
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    rtc@69 {
+        compatible = "abracon,abx80x";
+        reg = <0x69>;
+        abracon,tc-diode = "schottky";
+        abracon,tc-resistor = <3>;
+        interrupt-parent = <&fake_intc0>;
+        interrupts = <44 IRQ_TYPE_EDGE_FALLING>;
+    };