All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: alexandre.belloni@bootlin.com, robh+dt@kernel.org,
	 krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	chao.wei@sophgo.com,  unicorn_wang@outlook.com,
	paul.walmsley@sifive.com, palmer@dabbelt.com,
	 aou@eecs.berkeley.edu, linux-rtc@vger.kernel.org,
	devicetree@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org,  dlan@gentoo.org,
	inochiama@outlook.com
Subject: Re: [PATCH v6 3/3] riscv: dts: sophgo: add rtc dt node for CV1800
Date: Tue, 16 Jan 2024 22:41:44 +0800	[thread overview]
Message-ID: <CAJRtX8QFLoWnJBkepZrbneHX8qZdde=aw+zbdErVC91B=u==MA@mail.gmail.com> (raw)
In-Reply-To: <f2b3dff2-ce0d-4ddb-ad61-74abf2c3022d@linaro.org>

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

WARNING: multiple messages have this Message-ID (diff)
From: Jingbao Qiu <qiujingbao.dlmu@gmail.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: alexandre.belloni@bootlin.com, robh+dt@kernel.org,
	 krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	chao.wei@sophgo.com,  unicorn_wang@outlook.com,
	paul.walmsley@sifive.com, palmer@dabbelt.com,
	 aou@eecs.berkeley.edu, linux-rtc@vger.kernel.org,
	devicetree@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org,  dlan@gentoo.org,
	inochiama@outlook.com
Subject: Re: [PATCH v6 3/3] riscv: dts: sophgo: add rtc dt node for CV1800
Date: Tue, 16 Jan 2024 22:41:44 +0800	[thread overview]
Message-ID: <CAJRtX8QFLoWnJBkepZrbneHX8qZdde=aw+zbdErVC91B=u==MA@mail.gmail.com> (raw)
In-Reply-To: <f2b3dff2-ce0d-4ddb-ad61-74abf2c3022d@linaro.org>

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2024-01-16 14:41 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-15 16:05 [PATCH v6 0/3] riscv: rtc: sophgo: add mfd and rtc support for CV1800 Jingbao Qiu
2024-01-15 16:05 ` Jingbao Qiu
2024-01-15 16:05 ` [PATCH v6 1/3] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC Jingbao Qiu
2024-01-15 16:05   ` Jingbao Qiu
2024-01-15 16:05 ` [PATCH v6 2/3] rtc: sophgo: add rtc support for Sophgo CV1800 SoC Jingbao Qiu
2024-01-15 16:05   ` Jingbao Qiu
2024-01-16  7:46   ` Krzysztof Kozlowski
2024-01-16  7:46     ` Krzysztof Kozlowski
2024-01-15 16:06 ` [PATCH v6 3/3] riscv: dts: sophgo: add rtc dt node for CV1800 Jingbao Qiu
2024-01-15 16:06   ` Jingbao Qiu
2024-01-16  7:44   ` Krzysztof Kozlowski
2024-01-16  7:44     ` Krzysztof Kozlowski
2024-01-16 14:41     ` Jingbao Qiu [this message]
2024-01-16 14:41       ` Jingbao Qiu
2024-01-16 15:25       ` Krzysztof Kozlowski
2024-01-16 15:25         ` Krzysztof Kozlowski
2024-01-16 15:51         ` Jingbao Qiu
2024-01-16 15:51           ` Jingbao Qiu
2024-01-16 16:03           ` Krzysztof Kozlowski
2024-01-16 16:03             ` Krzysztof Kozlowski
2024-01-16 16:29             ` Jingbao Qiu
2024-01-16 16:29               ` Jingbao Qiu
2024-01-16 16:53               ` Alexandre Belloni
2024-01-16 16:53                 ` Alexandre Belloni
2024-01-17  2:54                 ` Jingbao Qiu
2024-01-17  2:54                   ` Jingbao Qiu
2024-01-17  9:01                   ` Alexandre Belloni
2024-01-17  9:01                     ` Alexandre Belloni
2024-01-17 10:12                     ` Jingbao Qiu
2024-01-17 10:12                       ` Jingbao Qiu
2024-01-16 16:58               ` Krzysztof Kozlowski
2024-01-16 16:58                 ` Krzysztof Kozlowski
2024-01-17  3:24                 ` Jingbao Qiu
2024-01-17  3:24                   ` Jingbao Qiu
2024-01-17  7:28                   ` Krzysztof Kozlowski
2024-01-17  7:28                     ` Krzysztof Kozlowski
2024-01-17  7:39                     ` Jingbao Qiu
2024-01-17  7:39                       ` Jingbao Qiu
2024-01-17 12:59     ` Conor Dooley
2024-01-17 12:59       ` Conor Dooley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJRtX8QFLoWnJBkepZrbneHX8qZdde=aw+zbdErVC91B=u==MA@mail.gmail.com' \
    --to=qiujingbao.dlmu@gmail.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=chao.wei@sophgo.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlan@gentoo.org \
    --cc=inochiama@outlook.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh+dt@kernel.org \
    --cc=unicorn_wang@outlook.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.