2023-12-17 11:10:06

by Jingbao Qiu

[permalink] [raw]
Subject: [PATCH v2 0/3] riscv: rtc: sophgo: add rtc support for CV1800

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

.../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



2023-12-17 11:10:19

by Jingbao Qiu

[permalink] [raw]
Subject: [PATCH v2 1/3] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

Add devicetree binding for Sophgo CV1800 SoC.

Signed-off-by: Jingbao Qiu <[email protected]>
---
.../bindings/rtc/sophgo,cv1800-rtc.yaml | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml

diff --git a/Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml b/Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml
new file mode 100644
index 000000000000..a9e1dcc2a5be
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/sophgo,cv1800-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Real Time Clock of the Sophgo CV1800 SoC
+
+maintainers:
+ - Jingbao Qiu <[email protected]>
+
+allOf:
+ - $ref: rtc.yaml#
+
+properties:
+ compatible:
+ const: sophgo,cv1800-rtc
+
+ reg:
+ items:
+ - description: data register
+ - description: control register
+
+ clocks:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - interrupts
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ rtc@5025000{
+ compatible = "sophgo,cv1800-rtc";
+ reg = <0x5025000 0x1000>, <0x5026000 0x1000>;
+ clocks = <&osc>;
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+ };
--
2.25.1


2023-12-17 11:10:33

by Jingbao Qiu

[permalink] [raw]
Subject: [PATCH v2 2/3] rtc: sophgo: add rtc support for Sophgo CV1800 SoC

Implement the RTC driver for CV1800, which able to provide time alarm
and calibrate functionality.

Signed-off-by: Jingbao Qiu <[email protected]>
---
drivers/rtc/Kconfig | 6 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-cv1800.c | 400 +++++++++++++++++++++++++++++++++++++++
3 files changed, 407 insertions(+)
create mode 100644 drivers/rtc/rtc-cv1800.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 3814e0845e77..bdcd9132ff1a 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1103,6 +1103,12 @@ config RTC_DRV_DS2404
This driver can also be built as a module. If so, the module
will be called rtc-ds2404.

+config RTC_DRV_CV1800
+ tristate "Sophgo CV1800 RTC"
+ depends on ARCH_SOPHGO || COMPILE_TEST
+ help
+ Say y here to support the RTC driver for Sophgo CV1800.
+
config RTC_DRV_DA9052
tristate "Dialog DA9052/DA9053 RTC"
depends on PMIC_DA9052
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 7b03c3abfd78..69005a2b7ac6 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_RTC_DRV_CADENCE) += rtc-cadence.o
obj-$(CONFIG_RTC_DRV_CMOS) += rtc-cmos.o
obj-$(CONFIG_RTC_DRV_CPCAP) += rtc-cpcap.o
obj-$(CONFIG_RTC_DRV_CROS_EC) += rtc-cros-ec.o
+obj-$(CONFIG_RTC_DRV_CV1800) += rtc-cv1800.o
obj-$(CONFIG_RTC_DRV_DA9052) += rtc-da9052.o
obj-$(CONFIG_RTC_DRV_DA9055) += rtc-da9055.o
obj-$(CONFIG_RTC_DRV_DA9063) += rtc-da9063.o
diff --git a/drivers/rtc/rtc-cv1800.c b/drivers/rtc/rtc-cv1800.c
new file mode 100644
index 000000000000..f4c76593ef80
--- /dev/null
+++ b/drivers/rtc/rtc-cv1800.c
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * rtc-cv1800.c: RTC driver for Sophgo cv1800 RTC
+ *
+ * Author: Jingbao Qiu <[email protected]>
+ */
+#include <linux/kernel.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/rtc.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+
+#define ANA_CALIB 0x0
+#define SEC_PULSE_GEN 0x4
+#define ALARM_TIME 0x8
+#define ALARM_ENABLE 0xC
+#define SET_SEC_CNTR_VAL 0x10
+#define SET_SEC_CNTR_TRIG 0x14
+#define SEC_CNTR_VAL 0x18
+#define APB_RDATA_SEL 0x3C
+#define POR_DB_MAGIC_KEY 0x68
+#define EN_PWR_WAKEUP 0xBC
+#define MACRO_DA_CLEAR_ALL 0x480
+#define MACRO_DA_SOC_READY 0x48C
+#define MACRO_RO_T 0x4A8
+#define MACRO_RG_SET_T 0x498
+
+#define CTRL 0x08
+#define FC_COARSE_EN 0x40
+#define FC_COARSE_CAL 0x44
+#define FC_FINE_EN 0x48
+#define FC_FINE_CAL 0x50
+#define CTRL_MODE_MASK BIT(10)
+#define CTRL_MODE_OSC32K 0x00UL
+#define CTRL_MODE_XTAL32K BIT(0)
+
+#define FC_COARSE_CAL_VAL_SHIFT 0
+#define FC_COARSE_CAL_VAL_MASK GENMASK(15, 0)
+#define FC_COARSE_CAL_TIME_SHIFT 16
+#define FC_COARSE_CAL_TIME_MASK GENMASK(31, 16)
+#define FC_FINE_CAL_VAL_SHIFT 0
+#define FC_FINE_CAL_VAL_MASK GENMASK(23, 0)
+#define FC_FINE_CAL_TIME_SHIFT 24
+#define FC_FINE_CAL_TIME_MASK GENMASK(31, 24)
+
+#define SEC_PULSE_GEN_INT_SHIFT 0
+#define SEC_PULSE_GEN_INT_MASK GENMASK(7, 0)
+#define SEC_PULSE_GEN_FRAC_SHIFT 8
+#define SEC_PULSE_GEN_FRAC_MASK GENMASK(24, 8)
+#define SEC_PULSE_GEN_SEL_SHIFT 31
+#define SEC_PULSE_GEN_SEL_MASK GENMASK(30, 0)
+
+#define CALIB_INIT_VAL (BIT(8) || BIT(16))
+#define CALIB_SEL_FTUNE_MASK GENMASK(30, 0)
+#define CALIB_OFFSET_INIT 128
+#define CALIB_OFFSET_SHIFT BIT(0)
+#define CALIB_FREQ 256000000000
+#define CALIB_FRAC_EXT 10000
+#define CALIB_FREQ_NS 40
+#define CALIB_FREQ_MULT 256
+#define CALIB_FC_COARSE_PLUS_OFFSET 770
+#define CALIB_FC_COARSE_SUB_OFFSET 755
+
+#define REG_ENABLE_FUN BIT(0)
+#define REG_DISABLE_FUN 0x00UL
+#define REG_INIT_TIMEOUT 100
+#define SEC_MAX_VAL 0xFFFFFFFF
+#define ALARM_ENABLE_MASK BIT(0)
+#define SET_SEC_CNTR_VAL_UPDATE (BIT(28) || BIT(29))
+#define DEALY_TIME_PREPARE 400
+#define DEALY_TIME_LOOP 100
+
+struct cv1800_priv {
+ struct rtc_device *dev;
+ void __iomem *base_data;
+ void __iomem *base_ctrl;
+ struct clk *clk;
+ spinlock_t rtc_lock;
+ int irq;
+};
+
+static int cv1800_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+ struct cv1800_priv *info = dev_get_drvdata(dev);
+
+ if (enabled)
+ writel(REG_ENABLE_FUN, info->base_data + ALARM_ENABLE);
+ else
+ writel(REG_DISABLE_FUN, info->base_data + ALARM_ENABLE);
+
+ return 0;
+}
+
+static int cv1800_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct cv1800_priv *info = dev_get_drvdata(dev);
+ unsigned long alarm_time;
+
+ alarm_time = rtc_tm_to_time64(&alrm->time);
+
+ if (alarm_time > SEC_MAX_VAL)
+ return -EINVAL;
+
+ writel(REG_DISABLE_FUN, info->base_data + ALARM_ENABLE);
+
+ udelay(DEALY_TIME_PREPARE);
+
+ writel(alarm_time, info->base_data + ALARM_TIME);
+ writel(REG_ENABLE_FUN, info->base_data + ALARM_ENABLE);
+
+ readl(info->base_data + SEC_CNTR_VAL);
+
+ return 0;
+}
+
+static int cv1800_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+ struct cv1800_priv *info = dev_get_drvdata(dev);
+
+ alarm->enabled = readl(info->base_data + ALARM_ENABLE) &
+ ALARM_ENABLE_MASK;
+
+ rtc_time64_to_tm(readl(info->base_data + ALARM_TIME), &alarm->time);
+
+ return 0;
+}
+
+static int cv1800_rtc_32k_coarse_val_calib(struct cv1800_priv *info)
+{
+ uint32_t calib_val = 0;
+ uint32_t coarse_val = 0;
+ uint32_t coarse_time_now = 0;
+ uint32_t coarse_time_next = 0;
+ uint32_t offset = CALIB_OFFSET_INIT;
+ uint32_t timeout = REG_INIT_TIMEOUT;
+ uint32_t get_val_timeout;
+ uint32_t sec_pulse_val;
+
+ writel(CALIB_INIT_VAL, info->base_data + ANA_CALIB);
+ udelay(DEALY_TIME_PREPARE);
+
+ /* Select 32K OSC tuning val source from sys */
+ sec_pulse_val = readl(info->base_data + SEC_PULSE_GEN) &
+ SEC_PULSE_GEN_SEL_MASK;
+ writel(sec_pulse_val, info->base_data + SEC_PULSE_GEN);
+
+ calib_val = readl(info->base_data + ANA_CALIB);
+
+ writel(REG_ENABLE_FUN, info->base_ctrl + FC_COARSE_EN);
+
+ while (--timeout) {
+ coarse_time_now = readl(info->base_ctrl + FC_COARSE_CAL) >>
+ FC_COARSE_CAL_TIME_SHIFT;
+
+ get_val_timeout = REG_INIT_TIMEOUT;
+
+ while (coarse_time_next <= coarse_time_now &&
+ --get_val_timeout) {
+ coarse_time_next =
+ readl(info->base_ctrl + FC_COARSE_CAL) >>
+ FC_COARSE_CAL_TIME_SHIFT;
+ udelay(DEALY_TIME_LOOP);
+ }
+
+ if (!get_val_timeout)
+ return -1;
+
+ udelay(DEALY_TIME_PREPARE);
+
+ coarse_val = readl(info->base_ctrl + FC_COARSE_CAL) &
+ FC_COARSE_CAL_VAL_MASK;
+
+ if (coarse_val > CALIB_FC_COARSE_PLUS_OFFSET) {
+ calib_val += offset;
+ offset >>= CALIB_OFFSET_SHIFT;
+ writel(calib_val, info->base_data + ANA_CALIB);
+ } else if (coarse_val < CALIB_FC_COARSE_SUB_OFFSET) {
+ calib_val -= offset;
+ offset >>= CALIB_OFFSET_SHIFT;
+ writel(calib_val, info->base_data + ANA_CALIB);
+ } else {
+ writel(REG_DISABLE_FUN, info->base_ctrl + FC_COARSE_EN);
+ break;
+ }
+
+ if (offset == 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+static int cv1800_rtc_32k_fine_val_calib(struct cv1800_priv *info)
+{
+ uint32_t fc_val;
+ uint64_t freq = CALIB_FREQ;
+ uint32_t sec_cnt;
+ uint32_t timeout = REG_INIT_TIMEOUT;
+ uint32_t fc_time_now = 0;
+ uint32_t fc_time_next = 0;
+
+ writel(REG_ENABLE_FUN, info->base_ctrl + FC_FINE_EN);
+
+ fc_time_now = readl(info->base_ctrl + FC_FINE_CAL) >>
+ FC_FINE_CAL_TIME_SHIFT;
+
+ while (fc_time_next <= fc_time_now && --timeout) {
+ fc_time_next = readl(info->base_ctrl + FC_FINE_CAL) >>
+ FC_FINE_CAL_TIME_SHIFT;
+ udelay(DEALY_TIME_LOOP);
+ }
+
+ if (!timeout)
+ return -1;
+
+ fc_val = readl(info->base_ctrl + FC_FINE_CAL) & FC_FINE_CAL_VAL_MASK;
+
+ do_div(freq, CALIB_FREQ_NS);
+ freq = freq * CALIB_FRAC_EXT;
+ do_div(freq, fc_val);
+
+ sec_cnt = ((do_div(freq, CALIB_FRAC_EXT) * CALIB_FREQ_MULT) /
+ CALIB_FRAC_EXT &
+ SEC_PULSE_GEN_INT_MASK) +
+ (freq << SEC_PULSE_GEN_FRAC_SHIFT);
+
+ writel(sec_cnt, info->base_data + SEC_PULSE_GEN);
+ writel(REG_DISABLE_FUN, info->base_ctrl + FC_FINE_EN);
+
+ return 0;
+}
+
+static void rtc_enable_sec_counter(struct cv1800_priv *info)
+{
+ uint32_t val;
+
+ /* select inner sec pulse and select reg set calibration val */
+ val = readl(info->base_data + SEC_PULSE_GEN) & SEC_PULSE_GEN_SEL_MASK;
+ writel(val, info->base_data + SEC_PULSE_GEN);
+
+ val = readl(info->base_data + ANA_CALIB) & CALIB_SEL_FTUNE_MASK;
+ writel(val, info->base_data + ANA_CALIB);
+
+ readl(info->base_data + SEC_CNTR_VAL);
+ writel(REG_DISABLE_FUN, info->base_data + ALARM_ENABLE);
+}
+
+static int cv1800_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct cv1800_priv *info = dev_get_drvdata(dev);
+ unsigned long sec;
+ unsigned long sec_ro_t;
+ unsigned long flag;
+
+ spin_lock_irqsave(&info->rtc_lock, flag);
+
+ sec = readl(info->base_data + SEC_CNTR_VAL);
+ sec_ro_t = readl(info->base_data + MACRO_RO_T);
+
+ if (sec_ro_t > SET_SEC_CNTR_VAL_UPDATE) {
+ sec = sec_ro_t;
+ writel(sec, info->base_data + SET_SEC_CNTR_VAL);
+ writel(REG_ENABLE_FUN, info->base_data + SET_SEC_CNTR_TRIG);
+ }
+
+ spin_unlock_irqrestore(&info->rtc_lock, flag);
+
+ rtc_time64_to_tm(sec, tm);
+
+ return 0;
+}
+
+static int cv1800_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct cv1800_priv *info = dev_get_drvdata(dev);
+ unsigned long sec;
+ int ret;
+ unsigned long flag;
+
+ ret = rtc_valid_tm(tm);
+ if (ret)
+ return ret;
+
+ sec = rtc_tm_to_time64(tm);
+
+ spin_lock_irqsave(&info->rtc_lock, flag);
+
+ writel(sec, info->base_data + SET_SEC_CNTR_VAL);
+ writel(REG_ENABLE_FUN, info->base_data + SET_SEC_CNTR_TRIG);
+
+ writel(sec, info->base_data + MACRO_RG_SET_T);
+
+ spin_unlock_irqrestore(&info->rtc_lock, flag);
+
+ return 0;
+}
+
+static irqreturn_t cv1800_irq_handler(int irq, void *dev_id)
+{
+ struct device *dev = dev_id;
+ struct cv1800_priv *info = dev_get_drvdata(dev);
+ struct rtc_wkalrm alrm;
+
+ writel(REG_DISABLE_FUN, info->base_data + ALARM_ENABLE);
+
+ rtc_read_alarm(info->dev, &alrm);
+ alrm.enabled = 0;
+ rtc_set_alarm(info->dev, &alrm);
+
+ return IRQ_HANDLED;
+}
+
+static const struct rtc_class_ops cv800b_ops = {
+ .read_time = cv1800_rtc_read_time,
+ .set_time = cv1800_rtc_set_time,
+ .read_alarm = cv1800_rtc_read_alarm,
+ .set_alarm = cv1800_rtc_set_alarm,
+ .alarm_irq_enable = cv1800_rtc_alarm_irq_enable,
+};
+
+static int cv1800_rtc_probe(struct platform_device *pdev)
+{
+ struct cv1800_priv *rtc;
+ int ret;
+
+ rtc = devm_kzalloc(&pdev->dev, sizeof(struct cv1800_priv), GFP_KERNEL);
+ if (!rtc)
+ return -ENOMEM;
+
+ rtc->base_ctrl = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(rtc->base_ctrl))
+ return PTR_ERR(rtc->base_ctrl);
+
+ rtc->base_data = devm_platform_ioremap_resource(pdev, 1);
+ if (IS_ERR(rtc->base_data))
+ return PTR_ERR(rtc->base_data);
+
+ rtc->irq = platform_get_irq(pdev, 0);
+ if (rtc->irq < 0)
+ return rtc->irq;
+
+ ret = devm_request_irq(&pdev->dev, rtc->irq, cv1800_irq_handler,
+ IRQF_TRIGGER_HIGH, "alarm", &pdev->dev);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "cannot register interrupt handler\n");
+
+ rtc->clk = devm_clk_get_enabled(&pdev->dev, NULL);
+ if (IS_ERR(rtc->clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(rtc->clk),
+ "clk not found\n");
+
+ platform_set_drvdata(pdev, rtc);
+
+ spin_lock_init(&rtc->rtc_lock);
+
+ rtc->dev = devm_rtc_device_register(&pdev->dev, dev_name(&pdev->dev),
+ &cv800b_ops, THIS_MODULE);
+ if (IS_ERR(rtc->dev))
+ return dev_err_probe(&pdev->dev, PTR_ERR(rtc->dev),
+ "can't register rtc device\n");
+
+ /* if use internal clk,so coarse calibrate rtc */
+ if ((readl(rtc->base_ctrl + CTRL) & CTRL_MODE_MASK) ==
+ CTRL_MODE_OSC32K) {
+ ret = cv1800_rtc_32k_coarse_val_calib(rtc);
+ if (ret)
+ dev_err(&pdev->dev, "failed to coarse RTC val !\n");
+
+ ret = cv1800_rtc_32k_fine_val_calib(rtc);
+ if (ret)
+ dev_err(&pdev->dev, "failed to fine RTC val !\n");
+ }
+
+ rtc_enable_sec_counter(rtc);
+
+ return 0;
+}
+
+static const struct of_device_id cv1800_dt_ids[] = {
+ { .compatible = "sophgo,cv1800-rtc" },
+ { /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, cv1800_dt_ids);
+
+static struct platform_driver cv1800_driver = {
+ .driver = {
+ .name = "cv1800-rtc",
+ .of_match_table = cv1800_dt_ids,
+ },
+ .probe = cv1800_rtc_probe,
+};
+
+module_platform_driver(cv1800_driver);
+MODULE_AUTHOR("Jingbao Qiu");
+MODULE_DESCRIPTION("Sophgo CV1800 RTC Driver");
+MODULE_LICENSE("GPL");
--
2.25.1


2023-12-17 11:10:47

by Jingbao Qiu

[permalink] [raw]
Subject: [PATCH v2 3/3] riscv: dts: sophgo: add rtc dt node for CV1800

Add the rtc device tree node to cv1800 SoC.

Signed-off-by: Jingbao Qiu <[email protected]>
---
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";
+ reg = <0x5025000 0x1000>, <0x5026000 0x1000>;
+ clocks = <&osc>;
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+ };
};
};
--
2.25.1


2023-12-17 12:26:42

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

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.

> + clocks = <&osc>;
> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> + };


Attachments:
(No filename) (563.00 B)
signature.asc (235.00 B)
Download all attachments

2023-12-17 13:17:03

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On Sun, Dec 17, 2023 at 8:26 PM Conor Dooley <[email protected]> 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>;
> > + };
>

2023-12-17 20:47:10

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On Sun, Dec 17, 2023 at 09:16:39PM +0800, jingbao qiu wrote:
> On Sun, Dec 17, 2023 at 8:26 PM Conor Dooley <[email protected]> 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.


Attachments:
(No filename) (1.30 kB)
signature.asc (235.00 B)
Download all attachments

2023-12-17 20:48:21

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] riscv: dts: sophgo: add rtc dt node for CV1800

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 <[email protected]>
> ---
> 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
>


Attachments:
(No filename) (938.00 B)
signature.asc (235.00 B)
Download all attachments

2023-12-17 22:48:10

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] rtc: sophgo: add rtc support for Sophgo CV1800 SoC

Hi Jingbao,

kernel test robot noticed the following build errors:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on robh/for-next linus/master v6.7-rc5 next-20231215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Jingbao-Qiu/dt-bindings-rtc-sophgo-add-RTC-support-for-Sophgo-CV1800-series-SoC/20231217-191123
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link: https://lore.kernel.org/r/20231217110952.78784-3-qiujingbao.dlmu%40gmail.com
patch subject: [PATCH v2 2/3] rtc: sophgo: add rtc support for Sophgo CV1800 SoC
config: sparc-allmodconfig (https://download.01.org/0day-ci/archive/20231218/[email protected]/config)
compiler: sparc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231218/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/rtc/rtc-cv1800.c: In function 'cv1800_rtc_alarm_irq_enable':
>> drivers/rtc/rtc-cv1800.c:89:17: error: implicit declaration of function 'writel' [-Werror=implicit-function-declaration]
89 | writel(REG_ENABLE_FUN, info->base_data + ALARM_ENABLE);
| ^~~~~~
drivers/rtc/rtc-cv1800.c: In function 'cv1800_rtc_set_alarm':
>> drivers/rtc/rtc-cv1800.c:113:9: error: implicit declaration of function 'readl' [-Werror=implicit-function-declaration]
113 | readl(info->base_data + SEC_CNTR_VAL);
| ^~~~~
cc1: some warnings being treated as errors


vim +/writel +89 drivers/rtc/rtc-cv1800.c

83
84 static int cv1800_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
85 {
86 struct cv1800_priv *info = dev_get_drvdata(dev);
87
88 if (enabled)
> 89 writel(REG_ENABLE_FUN, info->base_data + ALARM_ENABLE);
90 else
91 writel(REG_DISABLE_FUN, info->base_data + ALARM_ENABLE);
92
93 return 0;
94 }
95
96 static int cv1800_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
97 {
98 struct cv1800_priv *info = dev_get_drvdata(dev);
99 unsigned long alarm_time;
100
101 alarm_time = rtc_tm_to_time64(&alrm->time);
102
103 if (alarm_time > SEC_MAX_VAL)
104 return -EINVAL;
105
106 writel(REG_DISABLE_FUN, info->base_data + ALARM_ENABLE);
107
108 udelay(DEALY_TIME_PREPARE);
109
110 writel(alarm_time, info->base_data + ALARM_TIME);
111 writel(REG_ENABLE_FUN, info->base_data + ALARM_ENABLE);
112
> 113 readl(info->base_data + SEC_CNTR_VAL);
114
115 return 0;
116 }
117

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-12-18 02:06:10

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] riscv: dts: sophgo: add rtc dt node for CV1800

On Mon, Dec 18, 2023 at 4:48 AM Conor Dooley <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >

2023-12-18 02:07:11

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On Mon, Dec 18, 2023 at 4:47 AM Conor Dooley <[email protected]> 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 <[email protected]> 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

2023-12-18 03:42:27

by Inochi Amaoto

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

>On Sun, Dec 17, 2023 at 09:16:39PM +0800, jingbao qiu wrote:
>> On Sun, Dec 17, 2023 at 8:26=E2=80=AFPM Conor Dooley <[email protected]> w=
>rote:
>> >
>> > On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote:
>> >
>> > > + reg:
>> > > + items:
>> > > + - description: data register
>> > > + - description: control register
>> >
>> > > + rtc@5025000{
>> > > + compatible =3D "sophgo,cv1800-rtc";
>> > > + reg =3D <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.
>> >
>>=20
>> 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 divi=
>des
>> 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=EF=BC=9F
>
>If all of the cv1800 series devices have them sequentially, I would just
>make them one region.
>

I agree with using one region. The ctrl and core region are highly
releated.

Moreover, I suggest using syscon to describe this region, the reboot
device is also in this region.

2023-12-18 04:16:01

by Inochi Amaoto

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] riscv: dts: sophgo: add rtc dt node for CV1800

>Add the rtc device tree node to cv1800 SoC.
>
>Signed-off-by: Jingbao Qiu <[email protected]>
>---
> 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";
>+ reg = <0x5025000 0x1000>, <0x5026000 0x1000>;

>+ clocks = <&osc>;

IIRC, the clock is not osc, but the clock controller with id CLK_RTC_25M.
Please read the manual again and ensure this is the right clock.

>+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
>+ };
> };
> };
>--
>2.25.1
>

2023-12-18 05:53:17

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] riscv: rtc: sophgo: add rtc support for CV1800

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
>

2023-12-18 07:12:49

by Jingbao Qiu

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] riscv: rtc: sophgo: add rtc support for CV1800

On Mon, Dec 18, 2023 at 1:53 PM Jisheng Zhang <[email protected]> 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
> >

2023-12-20 21:36:41

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: rtc: sophgo: add RTC support for Sophgo CV1800 series SoC

On Mon, Dec 18, 2023 at 11:41:52AM +0800, Inochi Amaoto wrote:
> >On Sun, Dec 17, 2023 at 09:16:39PM +0800, jingbao qiu wrote:
> >> On Sun, Dec 17, 2023 at 8:26=E2=80=AFPM Conor Dooley <[email protected]> w=
> >rote:
> >> >
> >> > On Sun, Dec 17, 2023 at 07:09:50PM +0800, Jingbao Qiu wrote:
> >> >
> >> > > + reg:
> >> > > + items:
> >> > > + - description: data register
> >> > > + - description: control register
> >> >
> >> > > + rtc@5025000{
> >> > > + compatible =3D "sophgo,cv1800-rtc";
> >> > > + reg =3D <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.
> >> >
> >>=20
> >> 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 divi=
> >des
> >> 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=EF=BC=9F
> >
> >If all of the cv1800 series devices have them sequentially, I would just
> >make them one region.
> >
>
> I agree with using one region. The ctrl and core region are highly
> releated.
>
> Moreover, I suggest using syscon to describe this region, the reboot
> device is also in this region.

Then the description of the device is incomplete. Please describe the
whole block/device.

Rob