Real Time Clock (RTC) is an independently powered module
within the chip, which includes a 32KHz oscillator and
a Power On Reset/POR submodule. It can be used for time
display and timed alarm generation.
Power On Reset/POR submodule only using register resources
so it should be empty. The 32KHz oscillator only provides
pulses for RTC in hardware.
Changes since v5:
- remove unnecessary lock
- fix cv1800_rtc_alarm_irq_enable()
- remove duplicate checks
- using alrm->enabled instead of unconditionally
enabling
- remove disable alarms on probe
- using rtc_update_irq() replace mess of alarm
- remove leak clk
- useing devm_rtc_allocate_device() and
devm_rtc_register_device() instead old way
- add judgment for rtc_enable_sec_counter()
- add POR nodes in DTS. This POR device shares
the register region with the RTC device
v5: https://lore.kernel.org/all/[email protected]/
Changes since v4:
- remove POR dt-bindings because it empty
- remove MFD dt-bindings because SoC does
not have MFDs
- add syscon attribute to share registers
with POR
v4: https://lore.kernel.org/all/[email protected]/
Changes since v3:
- temporarily not submitting RTC driver code
waiting for communication with IC designer
- add MFD dt-bindings
- add POR dt-bindings
v3: https://lore.kernel.org/all/[email protected]/
Changes since v2:
- add mfd support for CV1800
- add rtc to mfd
- using regmap replace iomap
- merge register address in dts
v2: https://lore.kernel.org/lkml/[email protected]/
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
v1: https://lore.kernel.org/lkml/[email protected]/
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 | 55 +++
arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 12 +
drivers/rtc/Kconfig | 7 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-cv1800.c | 397 ++++++++++++++++++
5 files changed, 472 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml
create mode 100644 drivers/rtc/rtc-cv1800.c
base-commit: e3d3fe7e7bf08820a83c9d9a4c38c7b29a2927f1
--
2.43.0
Add RTC devicetree binding for Sophgo CV1800 SoC.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Jingbao Qiu <[email protected]>
---
.../bindings/rtc/sophgo,cv1800-rtc.yaml | 55 +++++++++++++++++++
1 file changed, 55 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..e54abd59c193
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/sophgo,cv1800-rtc.yaml
@@ -0,0 +1,55 @@
+# 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
+
+description:
+ Real Time Clock (RTC) is an independently powered module
+ within the chip, which includes a 32KHz oscillator and a
+ Power On Reset/POR submodule. It can be used for time display
+ and timed alarm generation. In addition, the hardware state
+ machine provides triggering and timing control for chip
+ power on, off, and reset.
+
+maintainers:
+ - Jingbao Qiu <[email protected]>
+
+allOf:
+ - $ref: rtc.yaml#
+
+properties:
+ compatible:
+ items:
+ - const: sophgo,cv1800-rtc
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ rtc@5025000 {
+ compatible = "sophgo,cv1800-rtc", "syscon";
+ reg = <0x5025000 0x2000>;
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&osc>;
+ };
--
2.43.0
Implement the RTC driver for CV1800, which able to provide time alarm
and calibrate functionality.
Signed-off-by: Jingbao Qiu <[email protected]>
---
Depends on https://lore.kernel.org/all/IA1PR20MB4953C774D41EDF1EADB6EC18BB6D2@IA1PR20MB4953.namprd20.prod.outlook.com/
---
drivers/rtc/Kconfig | 7 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-cv1800.c | 397 +++++++++++++++++++++++++++++++++++++++
3 files changed, 405 insertions(+)
create mode 100644 drivers/rtc/rtc-cv1800.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 1ea9c7600150..3633f45c7fa0 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1115,6 +1115,13 @@ 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
+ If you say Y here you will get support for the RTC found on
+ 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 31c4606fd9bf..a5fc78ac0094 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..836f15a881e0
--- /dev/null
+++ b/drivers/rtc/rtc-cv1800.c
@@ -0,0 +1,397 @@
+// 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/irq.h>
+#include <linux/delay.h>
+#include <linux/rtc.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/of.h>
+
+#define ANA_CALIB 0x1000
+#define SEC_PULSE_GEN 0x1004
+#define ALARM_TIME 0x1008
+#define ALARM_ENABLE 0x100C
+#define SET_SEC_CNTR_VAL 0x1010
+#define SET_SEC_CNTR_TRIG 0x1014
+#define SEC_CNTR_VAL 0x1018
+#define APB_RDATA_SEL 0x103C
+#define POR_DB_MAGIC_KEY 0x1068
+#define EN_PWR_WAKEUP 0x10BC
+
+/*
+ * When in VDDBKUP domain, this MACRO register
+ * does not power down
+ */
+#define MACRO_DA_CLEAR_ALL 0x1480
+#define MACRO_DA_SOC_READY 0x148C
+#define MACRO_RO_T 0x14A8
+#define MACRO_RG_SET_T 0x1498
+#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 SEC_PULSE_SEL_INNER BIT(31)
+#define CALIB_INIT_VAL (BIT(8) || BIT(16))
+#define CALIB_SEL_FTUNE_MASK GENMASK(30, 0)
+#define CALIB_SEL_FTUNE_INNER 0x00UL
+#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_INIT (BIT(28) || BIT(29))
+#define DEALY_TIME_PREPARE 400
+#define DEALY_TIME_LOOP 100
+
+struct cv1800_rtc_priv {
+ struct rtc_device *rtc_dev;
+ struct device *dev;
+ struct regmap *rtc_map;
+ struct clk *clk;
+ int irq;
+};
+
+static int cv1800_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+ struct cv1800_rtc_priv *info = dev_get_drvdata(dev);
+
+ regmap_write(info->rtc_map, ALARM_ENABLE, enabled);
+
+ return 0;
+}
+
+static int cv1800_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct cv1800_rtc_priv *info = dev_get_drvdata(dev);
+ unsigned long alarm_time;
+
+ alarm_time = rtc_tm_to_time64(&alrm->time);
+
+ cv1800_rtc_alarm_irq_enable(dev, REG_DISABLE_FUN);
+
+ regmap_write(info->rtc_map, ALARM_TIME, alarm_time);
+
+ cv1800_rtc_alarm_irq_enable(dev, alrm->enabled);
+
+ return 0;
+}
+
+static int cv1800_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+ struct cv1800_rtc_priv *info = dev_get_drvdata(dev);
+ uint32_t enabled;
+ uint32_t time;
+
+ regmap_read(info->rtc_map, ALARM_ENABLE, &enabled);
+
+ alarm->enabled = enabled & ALARM_ENABLE_MASK;
+
+ regmap_read(info->rtc_map, ALARM_TIME, &time);
+
+ rtc_time64_to_tm(time, &alarm->time);
+
+ return 0;
+}
+
+/**
+ * cv1800_rtc_32k_coarse_val_calib() - Using an external
+ * clock to coarse calibrate the crystal oscillator
+ * @info: the device of calibrated
+ *
+ * @return 0 on success, or -1 on fail
+ *
+ * This RTC has an independent 32KHz oscillator. However,
+ * the accuracy of this oscillator is easily affected by
+ * external environmental interference,resulting in lower
+ * accuracy than the internal oscillator.Therefore, a 25M
+ * crystal oscillator is used as a reference source to
+ * calibrate the RTC and improve its accuracy.Calibration
+ * is completed through two steps, namely rough calibration
+ * and fine calibration.
+ */
+static int cv1800_rtc_32k_coarse_val_calib(struct cv1800_rtc_priv *info)
+{
+ uint32_t calib_val = 0;
+ uint32_t coarse_val = 0;
+ uint32_t time_now = 0;
+ uint32_t time_next = 0;
+ uint32_t offset = CALIB_OFFSET_INIT;
+ uint32_t coarse_timeout = REG_INIT_TIMEOUT;
+ uint32_t get_val_timeout = 0;
+
+ regmap_write(info->rtc_map, ANA_CALIB, CALIB_INIT_VAL);
+
+ udelay(DEALY_TIME_PREPARE);
+
+ /* Select 32K OSC tuning val source from rtc_sys */
+ regmap_update_bits(info->rtc_map, SEC_PULSE_GEN,
+ (unsigned int)(~SEC_PULSE_GEN_SEL_MASK),
+ (unsigned int)(~SEC_PULSE_SEL_INNER));
+
+ regmap_read(info->rtc_map, ANA_CALIB, &calib_val);
+
+ regmap_write(info->rtc_map, FC_COARSE_EN, REG_ENABLE_FUN);
+
+ while (--coarse_timeout) {
+ regmap_read(info->rtc_map, FC_COARSE_CAL, &time_now);
+ time_now >>= FC_COARSE_CAL_TIME_SHIFT;
+
+ get_val_timeout = REG_INIT_TIMEOUT;
+
+ while (time_next <= time_now && --get_val_timeout) {
+ regmap_read(info->rtc_map, FC_COARSE_CAL, &time_next);
+ time_next >>= FC_COARSE_CAL_TIME_SHIFT;
+ udelay(DEALY_TIME_LOOP);
+ }
+
+ if (!get_val_timeout)
+ return -1;
+
+ udelay(DEALY_TIME_PREPARE);
+
+ regmap_read(info->rtc_map, FC_COARSE_CAL, &coarse_val);
+ coarse_val &= FC_COARSE_CAL_VAL_MASK;
+
+ if (coarse_val > CALIB_FC_COARSE_PLUS_OFFSET) {
+ calib_val += offset;
+ offset >>= CALIB_OFFSET_SHIFT;
+ regmap_write(info->rtc_map, ANA_CALIB, calib_val);
+ } else if (coarse_val < CALIB_FC_COARSE_SUB_OFFSET) {
+ calib_val -= offset;
+ offset >>= CALIB_OFFSET_SHIFT;
+ regmap_write(info->rtc_map, ANA_CALIB, calib_val);
+ } else {
+ regmap_write(info->rtc_map, FC_COARSE_EN,
+ REG_DISABLE_FUN);
+ break;
+ }
+
+ if (offset == 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+static int cv1800_rtc_32k_fine_val_calib(struct cv1800_rtc_priv *info)
+{
+ uint32_t val;
+ uint64_t freq = CALIB_FREQ;
+ uint32_t sec_cnt;
+ uint32_t timeout = REG_INIT_TIMEOUT;
+ uint32_t time_now = 0;
+ uint32_t time_next = 0;
+
+ regmap_write(info->rtc_map, FC_FINE_EN, REG_ENABLE_FUN);
+
+ regmap_read(info->rtc_map, FC_FINE_CAL, &time_now);
+ time_now >>= FC_FINE_CAL_TIME_SHIFT;
+
+ while (time_next <= time_now && --timeout) {
+ regmap_read(info->rtc_map, FC_FINE_CAL, &time_next);
+ time_next >>= FC_FINE_CAL_TIME_SHIFT;
+ udelay(DEALY_TIME_LOOP);
+ }
+
+ if (!timeout)
+ return -1;
+
+ regmap_read(info->rtc_map, FC_FINE_CAL, &val);
+ val &= FC_FINE_CAL_VAL_MASK;
+
+ do_div(freq, CALIB_FREQ_NS);
+ freq = freq * CALIB_FRAC_EXT;
+ do_div(freq, 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);
+
+ regmap_write(info->rtc_map, SEC_PULSE_GEN, sec_cnt);
+ regmap_write(info->rtc_map, FC_FINE_EN, REG_DISABLE_FUN);
+
+ return 0;
+}
+
+static void rtc_enable_sec_counter(struct cv1800_rtc_priv *info)
+{
+ unsigned int sec_ro_t;
+ unsigned int sec;
+
+ /* select inner sec pulse and select reg set calibration val */
+ regmap_update_bits(info->rtc_map, SEC_PULSE_GEN,
+ (unsigned int)(~SEC_PULSE_GEN_SEL_MASK),
+ (unsigned int)(~SEC_PULSE_SEL_INNER));
+
+ regmap_update_bits(info->rtc_map, ANA_CALIB,
+ (unsigned int)(~CALIB_SEL_FTUNE_MASK),
+ CALIB_SEL_FTUNE_INNER);
+
+ sec = SET_SEC_CNTR_VAL_INIT;
+
+ /* load from MACRO register */
+ regmap_read(info->rtc_map, MACRO_RO_T, &sec_ro_t);
+ if (sec_ro_t > (SET_SEC_CNTR_VAL_INIT))
+ sec = sec_ro_t;
+
+ regmap_write(info->rtc_map, SET_SEC_CNTR_VAL, sec);
+ regmap_write(info->rtc_map, SET_SEC_CNTR_TRIG, REG_ENABLE_FUN);
+}
+
+static int cv1800_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct cv1800_rtc_priv *info = dev_get_drvdata(dev);
+ unsigned int sec;
+
+ regmap_read(info->rtc_map, SEC_CNTR_VAL, &sec);
+
+ rtc_time64_to_tm(sec, tm);
+
+ return 0;
+}
+
+static int cv1800_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct cv1800_rtc_priv *info = dev_get_drvdata(dev);
+ unsigned long sec;
+
+ sec = rtc_tm_to_time64(tm);
+
+ regmap_write(info->rtc_map, SET_SEC_CNTR_VAL, sec);
+ regmap_write(info->rtc_map, SET_SEC_CNTR_TRIG, REG_ENABLE_FUN);
+
+ regmap_write(info->rtc_map, MACRO_RG_SET_T, sec);
+
+ return 0;
+}
+
+static irqreturn_t cv1800_rtc_irq_handler(int irq, void *dev_id)
+{
+ struct rtc_device *rtc = dev_id;
+
+ rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+
+ return IRQ_HANDLED;
+}
+
+static const struct rtc_class_ops cv800b_rtc_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_rtc_priv *rtc;
+ uint32_t ctrl_val;
+ int ret;
+
+ rtc = devm_kzalloc(&pdev->dev, sizeof(struct cv1800_rtc_priv),
+ GFP_KERNEL);
+ if (!rtc)
+ return -ENOMEM;
+
+ rtc->dev = &pdev->dev;
+
+ rtc->rtc_map = syscon_node_to_regmap(rtc->dev->of_node->parent);
+ if (IS_ERR(rtc->rtc_map))
+ return PTR_ERR(rtc->rtc_map);
+
+ rtc->irq = platform_get_irq(pdev, 0);
+ if (rtc->irq < 0)
+ return rtc->irq;
+
+ ret = devm_request_irq(&pdev->dev, rtc->irq, cv1800_rtc_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);
+
+ rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+ if (IS_ERR(rtc->rtc_dev))
+ return PTR_ERR(rtc->rtc_dev);
+
+ rtc->rtc_dev->ops = &cv800b_rtc_ops;
+ rtc->rtc_dev->range_max = U32_MAX;
+
+ /* if use internal clk,so coarse calibrate rtc */
+ regmap_read(rtc->rtc_map, CTRL, &ctrl_val);
+ ctrl_val &= CTRL_MODE_MASK;
+
+ if (ctrl_val == 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 devm_rtc_register_device(rtc->rtc_dev);
+}
+
+static const struct of_device_id cv1800_dt_ids[] = {
+ { .compatible = "sophgo,cv1800b-rtc" },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, cv1800_dt_ids);
+
+static struct platform_driver cv1800_rtc_driver = {
+ .driver = {
+ .name = "sophgo-cv800b-rtc",
+ .of_match_table = cv1800_dt_ids,
+ },
+ .probe = cv1800_rtc_probe,
+};
+
+module_platform_driver(cv1800_rtc_driver);
+MODULE_AUTHOR("Jingbao Qiu");
+MODULE_DESCRIPTION("Sophgo cv1800 RTC Driver");
+MODULE_LICENSE("GPL");
--
2.43.0
Add the rtc device tree node to cv1800 SoC.
Signed-off-by: Jingbao Qiu <[email protected]>
---
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";
+ sophgo,rtc-sysreg = <&rtc>;
+ };
};
};
--
2.43.0
On 15/01/2024 17:06, 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 | 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
Best regards,
Krzysztof
On 15/01/2024 17:05, Jingbao Qiu wrote:
> Implement the RTC driver for CV1800, which able to provide time alarm
> and calibrate functionality.
>
> Signed-off-by: Jingbao Qiu <[email protected]>
> ---
> +
> +static int cv1800_rtc_probe(struct platform_device *pdev)
> +{
> + struct cv1800_rtc_priv *rtc;
> + uint32_t ctrl_val;
> + int ret;
> +
> + rtc = devm_kzalloc(&pdev->dev, sizeof(struct cv1800_rtc_priv),
> + GFP_KERNEL);
> + if (!rtc)
> + return -ENOMEM;
> +
> + rtc->dev = &pdev->dev;
> +
> + rtc->rtc_map = syscon_node_to_regmap(rtc->dev->of_node->parent);
> + if (IS_ERR(rtc->rtc_map))
> + return PTR_ERR(rtc->rtc_map);
Why do you take the parent? Your bindings say this device is the syscon,
not parent. Your DTS shows there is no parent syscon! This wasn't tested.
Best regards,
Krzysztof
On Tue, Jan 16, 2024 at 3:44 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 15/01/2024 17:06, 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 | 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
> <[email protected]> wrote:
>>
>> On 15/01/2024 17:06, 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 | 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 Tue, Jan 16, 2024 at 11:25 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 16/01/2024 15:41, Jingbao Qiu wrote:
> > On Tue, Jan 16, 2024 at 3:44 PM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 15/01/2024 17:06, 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 | 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.
Thanks, What I mean is that this hardware includes two functions, RTC
and POR. How should I describe their relationship?
>
> > 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.
Thank you, I will pay attention to 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.
Should I completely delete him? How can a por driver obtain device information?
>
> > 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?
Sorry, this is my mistake.
>
> >
> > 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?
Of course, the corresponding driver for POR can provide power on/off and
restart functions.
>
> > 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.
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?
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.
It's just that his resources are only shared with RTC's Reg.
Looking forward to your reply.
Best regards,
Jingbao Qiu
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
<[email protected]> 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 17/01/2024 00:29:28+0800, Jingbao Qiu wrote:
> On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski
> <[email protected]> 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?
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 16/01/2024 17:29, Jingbao Qiu wrote:
> On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski
> <[email protected]> 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.
Best regards,
Krzysztof
On Wed, Jan 17, 2024 at 12:53 AM Alexandre Belloni
<[email protected]> wrote:
>
> On 17/01/2024 00:29:28+0800, Jingbao Qiu wrote:
> > On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski
> > <[email protected]> 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
<[email protected]> wrote:
>
> On 16/01/2024 17:29, Jingbao Qiu wrote:
> > On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski
> > <[email protected]> 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
> <[email protected]> wrote:
>>
>> On 16/01/2024 17:29, Jingbao Qiu wrote:
>>> On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski
>>> <[email protected]> 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
<[email protected]> wrote:
>
> On 17/01/2024 04:24, Jingbao Qiu wrote:
> > On Wed, Jan 17, 2024 at 12:58 AM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 16/01/2024 17:29, Jingbao Qiu wrote:
> >>> On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski
> >>> <[email protected]> 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
> <[email protected]> wrote:
> >
> > On 17/01/2024 00:29:28+0800, Jingbao Qiu wrote:
> > > On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski
> > > <[email protected]> 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
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Wed, Jan 17, 2024 at 5:01 PM Alexandre Belloni
<[email protected]> wrote:
>
> On 17/01/2024 10:54:08+0800, Jingbao Qiu wrote:
> > On Wed, Jan 17, 2024 at 12:53 AM Alexandre Belloni
> > <[email protected]> wrote:
> > >
> > > On 17/01/2024 00:29:28+0800, Jingbao Qiu wrote:
> > > > On Wed, Jan 17, 2024 at 12:03 AM Krzysztof Kozlowski
> > > > <[email protected]> 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 <[email protected]>
> > ---
> > 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.