[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJRtX8ROH4R_s1=ML5ka340PAE0SWJKK24yVWHw5gCd+7d9pkA@mail.gmail.com>
Date: Tue, 16 Jan 2024 23:51:42 +0800
From: Jingbao Qiu <qiujingbao.dlmu@...il.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: alexandre.belloni@...tlin.com, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, chao.wei@...hgo.com,
unicorn_wang@...look.com, paul.walmsley@...ive.com, palmer@...belt.com,
aou@...s.berkeley.edu, linux-rtc@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
dlan@...too.org, inochiama@...look.com
Subject: Re: [PATCH v6 3/3] riscv: dts: sophgo: add rtc dt node for CV1800
On Tue, Jan 16, 2024 at 11:25 PM Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> wrote:
>
> On 16/01/2024 15:41, Jingbao Qiu wrote:
> > On Tue, Jan 16, 2024 at 3:44 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@...aro.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@...il.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@...00000 {
> >>> reg = <0x74000000 0x10000>;
> >>> interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> >>> };
> >>> +
> >>> + rtc: rtc@...5000 {
> >>> + 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
>
>
> I am not going to read pages of description. Please write concise replies.
Thanks, What I mean is that this hardware includes two functions, RTC
and POR. How should I describe their relationship?
>
> > 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
>
> This is DTS, not drivers. Please do not mix it.
Thank you, I will pay attention to it.
>
> > 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@...5000 {
> > 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@...5000 {
> > 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.
>
> So where did you delete it? I still see it in this patch.
Should I completely delete him? How can a por driver obtain device information?
>
> > 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@...5000 {
> > 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.
>
> "Will" is the future tense, so about which patch are we talking?
Sorry, this is my mistake.
>
> >
> > rtc: rtc@...5000 {
> > 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>;
> > };
>
> NAK, because:
>
> "so it should be deleted."
>
>
> >
> > My idea is that this register can be accessed through the syscon tag,
> > RTC driver, and reboot driver.
>
> Again, what drivers have anything to do here?
Of course, the corresponding driver for POR can provide power on/off and
restart functions.
>
> > 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@...5000 {
> > compatible = "sophgo,cv1800-rtc", "sophgo,cv1800-por";
>
> Device is only one thing, not two.
>
> > 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.
>
> I expect one device node. How many drivers you have does not matter: you
> can instantiate 100 Linux devices in 100 Linux device drivers.
I understand what you mean. A device node corresponds to multiple drivers.
Should I completely delete the POR device tree node and add it when
submitting the POR driver?
If that's the case, how can I explain that the rtc device tree node
uses the syscon tag?
How can I describe a POR device in DTS? POR is a submodule of RTC, and
it also has corresponding drivers.
It's just that his resources are only shared with RTC's Reg.
Looking forward to your reply.
Best regards,
Jingbao Qiu
Powered by blists - more mailing lists