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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJRtX8Qxvpf_CTJG41U6JC3_qLL9raFxX3LD0LoPNhve=MDyFA@mail.gmail.com>
Date: Wed, 17 Jan 2024 11:24:43 +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 Wed, Jan 17, 2024 at 12:58 AM Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> wrote:
>
> On 16/01/2024 17:29, Jingbao Qiu wrote:
> > On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@...aro.org> wrote:
> >>
> >> On 16/01/2024 16:51, Jingbao Qiu wrote:
> >>>>> 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?
> >>
> >> Your POR does not need to take any resources, so no need to describe any
> >> relationship.
> >>
> >> ...
> >>
> >>>>> 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?
> >>
> >> Delete completely.
> >>
> >> Device information? What is this? We already agreed you don't have any
> >> resources for 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?
> >>
> >> ? I wrote it in previous messages and twice in this thread. Completely
> >> delete. You do not add it back! Because if you ever intended to add it
> >> back, it should be added since beginning. I don't understand what
> >> submitting later would solve.
> >>
> >>> 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.
> >>
> >> I said, there is no need for POR in DTS, because you have nothing there.
> >> Why do you insist on putting it on DTS?
> >>
> >>> It's just that his resources are only shared with RTC's Reg.
> >>
> >> What resources? Reg? That's not a separate resource.
>
> I meant, separate from the RTC. I had impression that IO space is shared
> or mixed with RTC? If it is separate, why it wasn't listed?
>
> >
> > I'm very sorry about this.
> > But I found a binding file that only contains Reg and Compatible.
> >
> > rtc@...20000 {
> > compatible = "cirrus,ep9301-rtc";
> > reg = <0x80920000 0x100>;
> > };
> >
> > Link: Documentation/devicetree/bindings/rtc/cirrus,ep9301-rtc.yaml
>
> And?
>
> >
> >>
> >> To summarize: Drop POR from DTS and never bring it back, unless you come
> >> with some different arguments, which you did not say already.
> >>
> >
> > You are right, if there is no por device tree node, how can the por
> > driver obtain the Reg?
>
> The same as currently. Does your POR node has reg? No, so according to
> your logic it cannot obtain address space.
>
> Children Linux devices share regmap with parent device.
>

Thanks, Power-On-Reset/POR driver requires Reg to complete its functions.
The compatible of POR is required in DTS to load the corresponding driver.
The POR driver was not submitted in the patch. However, this patch requires
the addition of RTC in DTS. Considering the future addition of POR
driver, I added a POR node.
I'm not sure why the POR node needs to be deleted, just because it
only has the compatible attribute?
Or maybe it's because I didn't submit the POR driver, so I need to
delete the POR node.
I found an example.

st: timer@...ffd00 {
    compatible = "atmel,at91rm9200-st", "syscon", "simple-mfd";
    reg = <0xfffffd00 0x100>;
    interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
    clocks = <&slow_xtal>;
    watchdog {
      compatible = "atmel,at91rm9200-wdt";
    };
};

Link:arch/arm/boot/dts/microchip/at91rm9200.dtsi:114

Like this, when the por driver insmod is activated, the por driver can
obtain the regs of the parent device.
Thank you again.

Best regards,
Jingbao Qiu