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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 16, 2024 at 11:25 PM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 16/01/2024 15:41, Jingbao Qiu wrote:
> > On Tue, Jan 16, 2024 at 3:44 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@xxxxxxxxxx> 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@xxxxxxxxx>
> >>> ---
> >>>  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@74000000 {
> >>>                       reg = <0x74000000 0x10000>;
> >>>                       interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> >>>               };
> >>> +
> >>> +             rtc: rtc@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
>
>
> 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@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@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.
>
> 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@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.
>
> "Will" is the future tense, so about which patch are we talking?

Sorry, this is my mistake.

>
> >
> > rtc: rtc@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>;
> > };
>
> 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@5025000 {
> >                     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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux