| Message ID | cover.1681370153.git.zhoubinbin@loongson.cn |
|---|---|
| Headers | show |
| Series | rtc: ls2x: Add support for the Loongson-2K/LS7A RTC | expand |
On 13/04/2023 09:57, Binbin Zhou wrote: > The LS2X RTC alarm depends on the associated registers in a separate > power management domain. > > In order to define the PM domain addresses of the different chips, a > more detailed description of compatible is required. This does not match your diff at all. > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > --- > Documentation/devicetree/bindings/rtc/trivial-rtc.yaml | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml > index a3603e638c37..2928811b83a0 100644 > --- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml > +++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml > @@ -47,8 +47,11 @@ properties: > - isil,isl1218 > # Intersil ISL12022 Real-time Clock > - isil,isl12022 > - # Loongson-2K Socs/LS7A bridge Real-time Clock > - - loongson,ls2x-rtc Why removing it? > + # Loongson LS7A bridge Real-time Clock > + - loongson,ls7a-rtc > + # Loongson-2K Socs Real-time Clock > + - loongson,ls2k0500-rtc > + - loongson,ls2k1000-rtc That's even more surprising... I don't understand what you are doing here at all. Best regards, Krzysztof
On 13/04/2023 15:57:32+0800, Binbin Zhou wrote: > Hi all: > > The initial DT-base ls2x rtc driver was written by Wang Xuerui, He has > released five versions of patchset before, and all related mail records > are shown below if you are interested: > > https://lore.kernel.org/all/?q=ls2x-rtc > > In this series of patches, based on the code above, I have added the > following support: > > 1. Add ACPI-related support, as Loongson-3A5000 + LS7A is now ACPI-base > by default under LoongArch architecture; > 2. Add rtc alarm/walarm related functions. > > I have tested on Loongson-3A5000LA+LS7A1000/LS7A2000, Loongson-2K1000LA > and Loongson-2K0500. > > BTW: > As LS1X does not support DT, we will merge LS2X and LS1X after keguang > has completed DT support for LS1X. Stop waiting and simply add it. > > ------- > Changes since v2: > - Rebase on the top of rtc-next. > patch(1/7): > - The new patch. > patch(2/7): > - Check patchset with "checkpatch.pl --strict"; > - Drop static inline functions which are used only once, such as > ls2x_rtc_regs_to_time; > - Remove the suruct ls2x_rtc_regs and regmap_bulk_xxx() is used to read > and write rtc registers; > - Clear the RTC wakeup interrupt manually; > - Enable the RTC in set_time() and check in read_time(); > - device_get_match_data() is used to get ls2x_pm_offset, for LS2k1000 > has the different value; > - Remove some inexact CONFIG_ACPI; > - remove()->remove_new(). > > Changes since v1: > 1. Rebased on top of latest loongarch-next; > 2. Add interrupt descriptions to the ls2k and ls7a DTS files to avoid > errors when the driver gets the IRQ number, Thanks to Qing Zhang for > testing; > 3. Remove some inexact CONFIG_ACPI. > > Thanks. > > Binbin Zhou (7): > dt-bindings: rtc: Subdivision of LS2X RTC compatible > rtc: Add support for the Loongson-2K/LS7A RTC > LoongArch: Enable LS2X RTC in loongson3_defconfig > MIPS: Loongson64: DTS: Add RTC support to LS7A > MIPS: Loongson: Enable LS2X RTC in loongson3_defconfig > MIPS: Loongson64: DTS: Add RTC support to Loongson-2K > MIPS: Loongson: Enable LS2X RTC in loongson2k_defconfig > > .../devicetree/bindings/rtc/trivial-rtc.yaml | 7 +- > arch/loongarch/configs/loongson3_defconfig | 1 + > .../boot/dts/loongson/loongson64-2k1000.dtsi | 7 + > arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 + > arch/mips/configs/loongson2k_defconfig | 1 + > arch/mips/configs/loongson3_defconfig | 1 + > drivers/rtc/Kconfig | 11 + > drivers/rtc/Makefile | 1 + > drivers/rtc/rtc-ls2x.c | 345 ++++++++++++++++++ > 9 files changed, 379 insertions(+), 2 deletions(-) > create mode 100644 drivers/rtc/rtc-ls2x.c > > -- > 2.39.1 >
On 19/04/2023 09:12, Binbin Zhou wrote: > On Mon, Apr 17, 2023 at 1:31 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 13/04/2023 09:57, Binbin Zhou wrote: >>> The LS2X RTC alarm depends on the associated registers in a separate >>> power management domain. >>> >>> In order to define the PM domain addresses of the different chips, a >>> more detailed description of compatible is required. >> >> This does not match your diff at all. >> >>> >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> >>> --- >>> Documentation/devicetree/bindings/rtc/trivial-rtc.yaml | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml >>> index a3603e638c37..2928811b83a0 100644 >>> --- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml >>> +++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml >>> @@ -47,8 +47,11 @@ properties: >>> - isil,isl1218 >>> # Intersil ISL12022 Real-time Clock >>> - isil,isl12022 >>> - # Loongson-2K Socs/LS7A bridge Real-time Clock >>> - - loongson,ls2x-rtc >> >> Why removing it? >> >>> + # Loongson LS7A bridge Real-time Clock >>> + - loongson,ls7a-rtc >>> + # Loongson-2K Socs Real-time Clock >>> + - loongson,ls2k0500-rtc >>> + - loongson,ls2k1000-rtc >> >> That's even more surprising... >> >> I don't understand what you are doing here at all. > Hi Krzysztof: > > Sorry, maybe my description was not accurate. > > Looking back at my V2 patchset, the first patch was to add ls2x-rtc compatible. > (https://lore.kernel.org/linux-rtc/0288efeb4209e4a49af07de6399045aaa00a970c.1673227292.git.zhoubinbin@loongson.cn/) > > In the process of modifying the V2 patchset, it was discovered that > the ACPI domain offset addresses on some of the socs (LS2K1000) were > different and I wanted to differentiate them by soc compatible. So I > was going to drop the ls2x-rtc compatible directly from the V3 patch > set. > However, when I rebased the V3 patchset, I found that the previous > ls2x-rtc compatible patch had been merged (commit 473a8ce756fd). So I > had to remove it and add soc compatible. Can all folks in Loongson stop adding wildcards as compatibles? Several compatibles were acked, because we do not know what 'x' stands for. Now, it turns out it's a wildcard which is not allowed. https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 > > How about the following description: > Since commit 473a8ce756fd (dt-bindings: rtc: Add Loongson LS2X RTC > support), the ls2x-rtc compatible has been added. But the specific soc > compatible is needed to be used to define different ACPI domain offset > addresses. > It's better. Anyway, SoC parts are rarely trivial devices, so this should be probably moved to its own schema. Best regards, Krzysztof