| Message ID | 20231217110952.78784-1-qiujingbao.dlmu@gmail.com |
|---|---|
| Headers | show |
| Series | riscv: rtc: sophgo: add rtc support for CV1800 | expand |
On Sun, Dec 17, 2023 at 8:26 PM Conor Dooley <conor@kernel.org> wrote: > > On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote: > > > + reg: > > + items: > > + - description: data register > > + - description: control register > > > + rtc@5025000{ > > + compatible = "sophgo,cv1800-rtc"; > > + reg = <0x5025000 0x1000>, <0x5026000 0x1000>; > > Why are these two regions rather than just one, given they are located > next to one another? > Are they separate on one of the other devices in this family? > > Thanks, > Conor. > I think there are two reasons, the first one is to distinguish different logical , REG_ CTRL (base on 0x5025000) controls clock calibration, sleep,and other functions, RTC_ CORE (base on 0x5026000) has basic RTC functionality, The second is the maximum address used by RTC_CTRL (base on 0x5025000) is 0x0ac,which is much smaller than 0x1000. Therefore, the datasheet divides it into two parts for introduction, and I also divide it into two parts based on this introduction.So do you suggest that I merge them together？ Best regards, Jingbao Qiu > > + clocks = <&osc>; > > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; > > + }; >
On Sun, Dec 17, 2023 at 07:09:52PM +0800, 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 | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > index df40e87ee063..429bee76f677 100644 > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > @@ -119,5 +119,12 @@ clint: timer@74000000 { > reg = <0x74000000 0x10000>; > interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>; > }; > + > + rtc@5025000 { > + compatible = "sophgo,cv1800-rtc"; This is a cv1800b, not a cv1800. > + reg = <0x5025000 0x1000>, <0x5026000 0x1000>; > + clocks = <&osc>; > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; > + }; > }; > }; > -- > 2.25.1 >
On Mon, Dec 18, 2023 at 4:48 AM Conor Dooley <conor@kernel.org> wrote: > > On Sun, Dec 17, 2023 at 07:09:52PM +0800, 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 | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > index df40e87ee063..429bee76f677 100644 > > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > @@ -119,5 +119,12 @@ clint: timer@74000000 { > > reg = <0x74000000 0x10000>; > > interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>; > > }; > > + > > + rtc@5025000 { > > + compatible = "sophgo,cv1800-rtc"; > > This is a cv1800b, not a cv1800. > Thanks, I will fix it. Best regards, Jingbao Qiu > > + reg = <0x5025000 0x1000>, <0x5026000 0x1000>; > > + clocks = <&osc>; > > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > }; > > }; > > -- > > 2.25.1 > >
On Mon, Dec 18, 2023 at 4:47 AM Conor Dooley <conor@kernel.org> wrote: > > On Sun, Dec 17, 2023 at 09:16:39PM +0800, jingbao qiu wrote: > > On Sun, Dec 17, 2023 at 8:26 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote: > > > > > > > + reg: > > > > + items: > > > > + - description: data register > > > > + - description: control register > > > > > > > + rtc@5025000{ > > > > + compatible = "sophgo,cv1800-rtc"; > > > > + reg = <0x5025000 0x1000>, <0x5026000 0x1000>; > > > > > > Why are these two regions rather than just one, given they are located > > > next to one another? > > > Are they separate on one of the other devices in this family? > > > > > > Thanks, > > > Conor. > > > > > > > I think there are two reasons, the first one is to distinguish > > different logical , > > REG_ CTRL (base on 0x5025000) controls clock calibration, sleep,and other > > functions, RTC_ CORE (base on 0x5026000) has basic RTC functionality, > > The second is the maximum address used by RTC_CTRL (base on 0x5025000) > > is 0x0ac,which is much smaller than 0x1000. Therefore, the datasheet divides > > it into two parts for introduction, and I also divide it into two > > parts based on this > > introduction.So do you suggest that I merge them together？ > > If all of the cv1800 series devices have them sequentially, I would just > make them one region. Thanks, I will fix it. Best regards, Jingbao Qiu
On Sun, Dec 17, 2023 at 07:09:49PM +0800, Jingbao Qiu wrote: > This series adds rtc support for Sophgo CV1800. > > Changes since v1 > - fix duplicate names in subject > - using RTC replace RTC controller > - improve the properties of dt-bindings > - using `unevaluatedProperties` replace `additionalProperties` > - dt-bindings passed the test > - using `devm_platform_ioremap_resource()` replace > `platform_get_resource()` and `devm_ioremap_resource()` > - fix random order of the code > - fix wrong wrapping of the `devm_request_irq()` and map the flag with dts > - using devm_clk_get_enabled replace `devm_clk_get()` and > `clk_prepare_enable()` > - fix return style > - add rtc clock calibration function > - use spinlock when write register on read/set time > > Jingbao Qiu (3): > dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC > rtc: sophgo: add rtc support for Sophgo CV1800 SoC > riscv: dts: sophgo: add rtc dt node for CV1800 AFAICT, the rtc subsystem supports not only RTC function but also power/reboot controller, so modeling the rtc subsystem as RTC only doesn't match the HW. I expect a mfd here. > > .../bindings/rtc/sophgo,cv1800-rtc.yaml | 47 ++ > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 7 + > drivers/rtc/Kconfig | 6 + > drivers/rtc/Makefile | 1 + > drivers/rtc/rtc-cv1800.c | 400 ++++++++++++++++++ > 5 files changed, 461 insertions(+) > create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml > create mode 100644 drivers/rtc/rtc-cv1800.c > > -- > 2.25.1 >
On Mon, Dec 18, 2023 at 1:53 PM Jisheng Zhang <jszhang@kernel.org> wrote: > > On Sun, Dec 17, 2023 at 07:09:49PM +0800, Jingbao Qiu wrote: > > This series adds rtc support for Sophgo CV1800. > > > > Changes since v1 > > - fix duplicate names in subject > > - using RTC replace RTC controller > > - improve the properties of dt-bindings > > - using `unevaluatedProperties` replace `additionalProperties` > > - dt-bindings passed the test > > - using `devm_platform_ioremap_resource()` replace > > `platform_get_resource()` and `devm_ioremap_resource()` > > - fix random order of the code > > - fix wrong wrapping of the `devm_request_irq()` and map the flag with dts > > - using devm_clk_get_enabled replace `devm_clk_get()` and > > `clk_prepare_enable()` > > - fix return style > > - add rtc clock calibration function > > - use spinlock when write register on read/set time > > > > Jingbao Qiu (3): > > dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC > > rtc: sophgo: add rtc support for Sophgo CV1800 SoC > > riscv: dts: sophgo: add rtc dt node for CV1800 > > AFAICT, the rtc subsystem supports not only RTC function but also > power/reboot controller, so modeling the rtc subsystem as RTC only doesn't > match the HW. I expect a mfd here. > Thanks,I will improve it in the next version. Best regards, Jingbao Qiu > > > > .../bindings/rtc/sophgo,cv1800-rtc.yaml | 47 ++ > > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 7 + > > drivers/rtc/Kconfig | 6 + > > drivers/rtc/Makefile | 1 + > > drivers/rtc/rtc-cv1800.c | 400 ++++++++++++++++++ > > 5 files changed, 461 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml > > create mode 100644 drivers/rtc/rtc-cv1800.c > > > > -- > > 2.25.1 > >