| Message ID | 20240115160600.5444-4-qiujingbao.dlmu@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: rtc: sophgo: add mfd and rtc support for CV1800 | expand |
On Tue, Jan 16, 2024 at 3:44 PM Krzysztof Kozlowski <krzysztof.kozlowski@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@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@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 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@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. 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. 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>; }; 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@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
On 16/01/2024 15:41, Jingbao Qiu wrote: > On Tue, Jan 16, 2024 at 3:44 PM Krzysztof Kozlowski > <krzysztof.kozlowski@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@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@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. > 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. > 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. > 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? > > 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? > 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. Best regards, Krzysztof
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. To summarize: Drop POR from DTS and never bring it back, unless you come with some different arguments, which you did not say already. Best regards, Krzysztof
On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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'm very sorry about this. But I found a binding file that only contains Reg and Compatible. rtc@80920000 { compatible = "cirrus,ep9301-rtc"; reg = <0x80920000 0x100>; }; Link: Documentation/devicetree/bindings/rtc/cirrus,ep9301-rtc.yaml > > 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? Thank you again. Best regards, Jingbao Qiu
On Wed, Jan 17, 2024 at 12:53 AM Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > On 17/01/2024 00:29:28+0800, Jingbao Qiu wrote: > > On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.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'm very sorry about this. > > But I found a binding file that only contains Reg and Compatible. > > > > rtc@80920000 { > > compatible = "cirrus,ep9301-rtc"; > > reg = <0x80920000 0x100>; > > }; > > > > Link: Documentation/devicetree/bindings/rtc/cirrus,ep9301-rtc.yaml > > > > > > > > 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? > > I guess the question is why don't you register everything from the RTC > driver? Thanks, POR provides power off and restart functions as a child node of RTC. So, I think it should be placed in the power/reset directory. Best regards, Jingbao Qiu
On Wed, Jan 17, 2024 at 12:58 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 16/01/2024 17:29, Jingbao Qiu wrote: > > On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.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@80920000 { > > 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@fffffd00 { 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
On 17/01/2024 04:24, Jingbao Qiu wrote: > On Wed, Jan 17, 2024 at 12:58 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 16/01/2024 17:29, Jingbao Qiu wrote: >>> On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski >>> <krzysztof.kozlowski@linaro.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@80920000 { >>> 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. No, it is not needed. I also wrote to in previous messages to keep drivers out of this. They are not related to this topic and don't use them as arguments. > 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 No. Bindings *MUST BE COMPLETE*, not depend on some other drivers. Submit *COMPLETE* bindings for entire hardware. Then submit complete DTS. I don't care about the drivers and they do not have to be complete. > 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? This is like a tenth message and I was explaining it multiple times. Go back to previous emails. > Or maybe it's because I didn't submit the POR driver, so I need to > delete the POR node. Please don't mention drivers anymore in this discussions. It only confuses you. > I found an example. > > st: timer@fffffd00 { > 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. Example of what? What is the question? I found a bug in Linux, so can I create such bug again? This discussion is fruitless and tiresome. You are repeating the same issues and asking the same questions. Best regards, Krzysztof
On Wed, Jan 17, 2024 at 3:28 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 17/01/2024 04:24, Jingbao Qiu wrote: > > On Wed, Jan 17, 2024 at 12:58 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 16/01/2024 17:29, Jingbao Qiu wrote: > >>> On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski > >>> <krzysztof.kozlowski@linaro.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@80920000 { > >>> 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. > > No, it is not needed. I also wrote to in previous messages to keep > drivers out of this. They are not related to this topic and don't use > them as arguments. > > > > 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 > > No. Bindings *MUST BE COMPLETE*, not depend on some other drivers. > Submit *COMPLETE* bindings for entire hardware. Then submit complete > DTS. I don't care about the drivers and they do not have to be complete. > > > > 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? > > This is like a tenth message and I was explaining it multiple times. Go > back to previous emails. > > > Or maybe it's because I didn't submit the POR driver, so I need to > > delete the POR node. > > Please don't mention drivers anymore in this discussions. It only > confuses you. > > > I found an example. > > > > st: timer@fffffd00 { > > 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. > > Example of what? What is the question? I found a bug in Linux, so can I > create such bug again? > > This discussion is fruitless and tiresome. You are repeating the same > issues and asking the same questions. > Thank you, I have been misled by historical legacy code. I will completely delete POR. Sorry again for wasting your time on repetitive discussions. Best regards, Jingbao Qiu
On 17/01/2024 10:54:08+0800, Jingbao Qiu wrote: > On Wed, Jan 17, 2024 at 12:53 AM Alexandre Belloni > <alexandre.belloni@bootlin.com> wrote: > > > > On 17/01/2024 00:29:28+0800, Jingbao Qiu wrote: > > > On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski > > > <krzysztof.kozlowski@linaro.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'm very sorry about this. > > > But I found a binding file that only contains Reg and Compatible. > > > > > > rtc@80920000 { > > > compatible = "cirrus,ep9301-rtc"; > > > reg = <0x80920000 0x100>; > > > }; > > > > > > Link: Documentation/devicetree/bindings/rtc/cirrus,ep9301-rtc.yaml > > > > > > > > > > > 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? > > > > I guess the question is why don't you register everything from the RTC > > driver? > > Thanks, POR provides power off and restart functions as a child node of RTC. > So, I think it should be placed in the power/reset directory. No it doesn't, have a look at the jz4740 rtc driver > > Best regards, > Jingbao Qiu
On Wed, Jan 17, 2024 at 5:01 PM Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > On 17/01/2024 10:54:08+0800, Jingbao Qiu wrote: > > On Wed, Jan 17, 2024 at 12:53 AM Alexandre Belloni > > <alexandre.belloni@bootlin.com> wrote: > > > > > > On 17/01/2024 00:29:28+0800, Jingbao Qiu wrote: > > > > On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski > > > > <krzysztof.kozlowski@linaro.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'm very sorry about this. > > > > But I found a binding file that only contains Reg and Compatible. > > > > > > > > rtc@80920000 { > > > > compatible = "cirrus,ep9301-rtc"; > > > > reg = <0x80920000 0x100>; > > > > }; > > > > > > > > Link: Documentation/devicetree/bindings/rtc/cirrus,ep9301-rtc.yaml > > > > > > > > > > > > > > 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? > > > > > > I guess the question is why don't you register everything from the RTC > > > driver? > > > > Thanks, POR provides power off and restart functions as a child node of RTC. > > So, I think it should be placed in the power/reset directory. > > No it doesn't, have a look at the jz4740 rtc driver Thank you for your suggestion. Best regards, Jingbao Qiu
On Tue, Jan 16, 2024 at 08:44:12AM +0100, Krzysztof Kozlowski 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@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@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 It doesn't pass dtbs_check, so it's automatically not a runner. Please make sure that your series adds no additional warnings at dtbs_check with W=1. Thanks, Conor.
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"; + sophgo,rtc-sysreg = <&rtc>; + }; }; };
Add the rtc device tree node to cv1800 SoC. Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> --- arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 12 ++++++++++++ 1 file changed, 12 insertions(+)