Re: [PATCH V4 1/5] dt-bindings: rtc: Remove the LS2X from the trivial RTCs

[Conor Dooley <conor@kernel.org>]

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

Hey Binbin,

On Thu, May 25, 2023 at 08:55:23PM +0800, Binbin Zhou wrote:
> Move Loongson RTC bindings from trivial-rtc.yaml into loongson,rtc.yaml.
> 
> Also, we will discard the use of wildcards in compatible (ls2x-rtc),
> soc-based compatible is more accurate for hardware differences between
> chips.
> 
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  .../devicetree/bindings/rtc/loongson,rtc.yaml | 47 +++++++++++++++++++
>  .../devicetree/bindings/rtc/trivial-rtc.yaml  |  2 -
>  2 files changed, 47 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/loongson,rtc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/rtc/loongson,rtc.yaml b/Documentation/devicetree/bindings/rtc/loongson,rtc.yaml
> new file mode 100644
> index 000000000000..68e56829e390
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/loongson,rtc.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/loongson,rtc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson Real-Time Clock
> +
> +maintainers:
> +  - Binbin Zhou <zhoubinbin@loongson.cn>
> +
> +allOf:
> +  - $ref: rtc.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - loongson,ls1b-rtc
> +      - loongson,ls1c-rtc
> +      - loongson,ls7a-rtc
> +      - loongson,ls2k0500-rtc
> +      - loongson,ls2k1000-rtc
> +      - loongson,ls2k2000-rtc

|+static const struct of_device_id loongson_rtc_of_match[] = {
|+       { .compatible = "loongson,ls1b-rtc", .data = &ls1x_rtc_config },
|+       { .compatible = "loongson,ls1c-rtc", .data = &ls1x_rtc_config },
|+       { .compatible = "loongson,ls7a-rtc", .data = &generic_rtc_config },
|+       { .compatible = "loongson,ls2k0500-rtc", .data = &generic_rtc_config },
|+       { .compatible = "loongson,ls2k1000-rtc", .data = &ls2k1000_rtc_config },
|+       { .compatible = "loongson,ls2k2000-rtc", .data = &generic_rtc_config },
|+       { /* sentinel */ }
|+};

This is a sign to me that your compatibles here are could do with some
fallbacks. Both of the ls1 ones are compatible with each other & there
are three that are generic.

I would allow the following:
"loongson,ls1b-rtc"
"loongson,ls1c-rtc", "loongson,ls1b-rtc"
"loongson,ls7a-rtc"
"loongson,ls2k0500-rtc", "loongson,ls7a-rtc"
"loongson,ls2k2000-rtc", "loongson,ls7a-rtc"
"loongson,ls2k1000-rtc"

And then the driver only needs:
|+static const struct of_device_id loongson_rtc_of_match[] = {
|+       { .compatible = "loongson,ls1b-rtc", .data = &ls1x_rtc_config },
|+       { .compatible = "loongson,ls7a-rtc", .data = &generic_rtc_config },
|+       { .compatible = "loongson,ls2k1000-rtc", .data = &ls2k1000_rtc_config },
|+       { /* sentinel */ }
|+};

And ~if~when you add support for more devices in the future that are
compatible with the existing ones no code changes are required.

To maintain compatibility with the existing devicetrees, should the old
"loongson,ls2x-rtc" be kept in the driver?

Thanks,
Conor.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    rtc_dev: rtc@1fe27800 {
> +      compatible = "loongson,ls2k0500-rtc";
> +      reg = <0x1fe27800 0x100>;
> +
> +      interrupt-parent = <&liointc1>;
> +      interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
> index a3603e638c37..9af77f21bb7f 100644
> --- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
> +++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
> @@ -47,8 +47,6 @@ properties:
>        - isil,isl1218
>        # Intersil ISL12022 Real-time Clock
>        - isil,isl12022
> -      # Loongson-2K Socs/LS7A bridge Real-time Clock
> -      - loongson,ls2x-rtc
>        # Real Time Clock Module with I2C-Bus
>        - microcrystal,rv3029
>        # Real Time Clock
> -- 
> 2.39.1
> 

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