[PATCH v6 3/3] riscv: dts: sophgo: add rtc dt node for CV1800

[Jingbao Qiu]

On Tue, Jan 16, 2024 at 3:44 PM Krzysztof Kozlowski
<krzysztof.kozlowski at linaro.org> wrote:
>
> On 15/01/2024 17:06, Jingbao Qiu wrote:
> > Add the rtc device tree node to cv1800 SoC.
> >
> > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu at gmail.com>
> > ---
> >  arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > index df40e87ee063..66bb4a752b91 100644
> > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > @@ -119,5 +119,17 @@ clint: timer at 74000000 {
> >                       reg = <0x74000000 0x10000>;
> >                       interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> >               };
> > +
> > +             rtc: rtc at 5025000 {
> > +                     compatible = "sophgo,cv1800-rtc", "syscon";
> > +                     reg = <0x5025000 0x2000>;
> > +                     interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> > +                     clocks = <&osc>;
> > +             };
> > +
> > +             por {
> > +                     compatible = "sophgo,cv1800-por";
>
> What is this? Why is it here, how did it even appear? It misses unit
> address and reg or is clearly placed in wrong place. It seems you
> entirely ignored out previous discussion.
>
> NAK
>

I'm very sorry for wasting your time. Furthermore, we would like to
thank you for your patient response.
Let me realize the rigor of Linux kernel code. I greatly admire
this.Please allow me to briefly review
our previous discussions.

CV1800 is a RISCV based SOC that includes an RTC module. The RTC
module has an OSC oscillator
and POR submodule inside.This OSC oscillator is only for RTC use, so
it does not need to be described
as a dt node. The POR submodule provides power off/on and restart
functions for CV1800. So I need
two drivers corresponding to RTC and POR respectively. RTC requires
the use of irq and clk resources
in addition to registers, while POR only requires Reg resources. The
current problem is how to describe
the relationship between RTC and POR, and how to make registers shared
by these two drivers.

In v3, I thought RTC was an MFD device because it not only had RTC
functionality but also restart and
power on/off capabilities.So my example is shown below.

syscon at 5025000 {
  compatible = "sophgo,cv1800b-subsys", "syscon", "simple-mfd";
  reg = <0x05025000 0x2000>;
  rtc {
    compatible = "sophgo,cv1800b-rtc";
    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&clk CLK_RTC_25M>;
  };
}

There were two suggestions you made at the time. Firstly, I only
described RTC and did not describe
the POR submodule. Secondly, regarding the name issue, system
controllers should not be placed
in the mfd file.Afterwards, I released the 4th version, in which I
described the POR submodule, which
is included side by side with RTC in the misc device. Then, by sharing
the register, both RTC and
POR drivers can access the registers.

misc at 5025000 {
  compatible = "sophgo,cv1800-misc", "syscon", "simple-mfd";
  reg = <0x05025000 0x2000>;
  rtc  {
    compatible = "sophgo,cv1800-rtc";
    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&clk 15>;
  };
  por  {
    compatible = "sophgo,cv1800-por";
  };
};

Your suggestion is, firstly, the por submodule does not have any
resources, so it should be deleted.
The second issue is still the name, misc is any device.
Afterwards, I released the 5th edition. In this version, I removed the
POR submodule in RTC.

rtc at 5025000 {
    compatible = "sophgo,cv1800-rtc", "syscon";
    reg = <0x5025000 0x2000>;
    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&clk 15>;
};

The question you raised is why syscon child nodes are used.
In this version, I will try the following methods.

rtc: rtc at 5025000 {
                    compatible = "sophgo,cv1800-rtc", "syscon";
                    reg = <0x5025000 0x2000>;
                    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
                    clocks = <&osc>;
};
por {
                    compatible = "sophgo,cv1800-por";
                    sophgo,rtc-sysreg = <&rtc>;
};

My idea is that this register can be accessed through the syscon tag,
RTC driver, and reboot driver.
Then complete their functions.
I'm sorry if it was due to language differences that caused my misunderstanding.
Perhaps I can accomplish it through the following methods.
rtc: rtc at 5025000 {
                    compatible = "sophgo,cv1800-rtc", "sophgo,cv1800-por";
                    reg = <0x5025000 0x2000>;
                    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
                    clocks = <&osc>;
};
However, in reality, the POR submodule does not use IRQ and CLK.
Please do not hesitate to teach. Thanks.


Best regards,
Jingbao Qiu