On Fri, May 26, 2023 at 1:05 AM Conor Dooley <conor@xxxxxxxxxx> wrote: > > 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@xxxxxxxxxxx> > > --- > > .../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@xxxxxxxxxxx> > > + > > +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. Hi Conor: Thanks for your reply. Yes, this is looking much cleaner. But it can't show every chip that supports that driver. As we know, Loongson is a family of chips: ls1b/ls1c represent the Loongson-1 family of CPU chips; ls7a represents the Loongson LS7A bridge chip; ls2k0500/ls2k1000/ls2k2000 represent the Loongson-2 family of CPU chips. Based on my previous conversations with Krzysztof, it seems that soc-based to order compatible is more popular, so I have listed all the chips that support that RTC driver. > > To maintain compatibility with the existing devicetrees, should the old > "loongson,ls2x-rtc" be kept in the driver? No, It seems that wildcards in compatible are not allowed." loongson,ls2x-rtc" itself was part of this patch series at one time, but apparently it is not the right way to describe these chips. Here is Krzysztof's previous reply: https://lore.kernel.org/linux-rtc/05ebf834-2220-d1e6-e07a-529b8f9cb100@xxxxxxxxxx/ Thanks. Binbin > > 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 > >
