This series adds rtc controller support for Sophgo CV1800B.
Jingbao Qiu (3):
dt-bindings: rtc: add binding for Sophgo CV1800B rtc controller
rtc: add rtc controller support for Sophgo CV1800B SoC
riscv: dts: sophgo: add rtc dt node for CV1800B
.../bindings/rtc/sophgo,cv1800b-rtc.yaml | 37 +++
arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 8 +
drivers/rtc/Kconfig | 10 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-cv1800b.c | 293 ++++++++++++++++++
5 files changed, 349 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
create mode 100644 drivers/rtc/rtc-cv1800b.c
--
2.25.1
Add the rtc device tree node to CV1800B SoC.
Signed-off-by: Jingbao Qiu <[email protected]>
---
arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
index df40e87ee063..89411c75b89a 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
@@ -119,5 +119,13 @@ clint: timer@74000000 {
reg = <0x74000000 0x10000>;
interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
};
+
+ rtc: rtc-controller@05026000 {
+ compatible = "sophgo,cv800b-rtc";
+ reg = <0x05026000 0x1000>;
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&plic>;
+ clocks = <&osc>;
+ };
};
};
--
2.25.1
Add devicetree binding for Sophgo CV1800B SoC rtc controller.
Signed-off-by: Jingbao Qiu <[email protected]>
---
.../bindings/rtc/sophgo,cv1800b-rtc.yaml | 37 +++++++++++++++++++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
diff --git a/Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml b/Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
new file mode 100644
index 000000000000..fefb1e70c45c
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/sophgo,cv1800b-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo CV1800B SoC RTC Controller
+
+maintainers:
+ - Jingbao Qiu <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - sophgo,cv1800b-rtc
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ rtc-controller@05026000{
+ compatible = "sophgo,cv800b-rtc";
+ reg = <0x05026000 0x1000>;
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&plic0>;
+ clocks = <&osc>;
+ };
--
2.25.1
Implement the RTC driver for CV1800B, which able to provide time and
alarm functionality.
Signed-off-by: Jingbao Qiu <[email protected]>
---
drivers/rtc/Kconfig | 10 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-cv1800b.c | 293 ++++++++++++++++++++++++++++++++++++++
3 files changed, 304 insertions(+)
create mode 100644 drivers/rtc/rtc-cv1800b.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 3814e0845e77..2089cceea38c 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1103,6 +1103,16 @@ 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_CV1800B
+ tristate "Sophgo CV1800B RTC"
+ depends on ARCH_SOPHGO || COMPILE_TEST
+ help
+ If you say yes here you will get support for the
+ RTC of the Sophgo CV1800B SOC.
+
+ This depend on ARCH_SOPHGO and COMPILE_TEST. Please
+ first config that.
+
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..3717d7ec8a4e 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_CV1800B) += rtc-cv1800b.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-cv1800b.c b/drivers/rtc/rtc-cv1800b.c
new file mode 100644
index 000000000000..d56132de30bb
--- /dev/null
+++ b/drivers/rtc/rtc-cv1800b.c
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * rtc-cv1800b.c: RTC driver for Sophgo CV1800B RTC
+ *
+ * Author: Jingbao Qiu <[email protected]>
+ */
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+
+#define RTC_ANA_CALIB 0x000
+#define RTC_SEC_PULSE_GEN 0x004
+#define RTC_ALARM_TIME 0x008
+#define RTC_ALARM_ENABLE 0x00c
+#define RTC_SET_SEC_CNTR_VALUE 0x010
+#define RTC_SET_SEC_CNTR_TRIG 0x014
+#define RTC_SEC_CNTR_VALUE 0x018
+#define RTC_INFO0 0x01c
+#define RTC_INFO1 0x020
+#define RTC_INFO2 0x024
+#define RTC_INFO3 0x028
+#define RTC_NOPOR_INFO0 0x02c
+#define RTC_NOPOR_INFO1 0x030
+#define RTC_NOPOR_INFO2 0x034
+#define RTC_NOPOR_INFO3 0x038
+#define RTC_DB_PWR_VBAT_DET 0x040
+#define RTC_DB_BUTTON1 0x048
+#define RTC_DB_PWR_ON 0x04c
+#define RTC_7SEC_RESET 0x050
+#define RTC_THM_SHDN_AUTO_REBOOT 0x064
+#define RTC_POR_DB_MAGIC_KEY 0x068
+#define RTC_DB_SEL_PWR 0x06c
+#define RTC_UP_SEQ0 0x070
+#define RTC_UP_SEQ1 0x074
+#define RTC_UP_SEQ2 0x078
+#define RTC_UP_SEQ3 0x07c
+#define RTC_UP_IF_EN 0x080
+#define RTC_UP_RSTN 0x084
+#define RTC_UP_MAX 0x088
+#define RTC_DN_SEQ0 0x090
+#define RTC_DN_SEQ1 0x094
+#define RTC_DN_SEQ2 0x098
+#define RTC_DN_SEQ3 0x09c
+#define RTC_DN_IF_EN 0x0a0
+#define RTC_DN_RSTN 0x0a4
+#define RTC_DN_MAX 0x0a8
+#define RTC_PWR_CYC_MAX 0x0b0
+#define RTC_WARM_RST_MAX 0x0b4
+#define RTC_EN_7SEC_RST 0x0b8
+#define RTC_EN_PWR_WAKEUP 0x0bc
+#define RTC_EN_SHDN_REQ 0x0c0
+#define RTC_EN_THM_SHDN 0x0c4
+#define RTC_EN_PWR_CYC_REQ 0x0c8
+#define RTC_EN_WARM_RST_REQ 0x0cc
+#define RTC_EN_PWR_VBAT_DET 0x0d0
+#define FSM_STATE 0x0d4
+#define RTC_EN_WDG_RST_REQ 0x0e0
+#define RTC_EN_SUSPEND_REQ 0x0e4
+#define RTC_DB_REQ_WDG_RST 0x0e8
+#define RTC_DB_REQ_SUSPEND 0x0ec
+#define RTC_PG_REG 0x0f0
+#define RTC_ST_ON_REASON 0x0f8
+#define RTC_ST_OFF_REASON 0x0fc
+#define RTC_EN_WAKEUP_REQ 0x120
+#define RTC_PWR_WAKEUP_POLARITY 0x128
+#define RTC_DB_SEL_REQ 0x130
+#define RTC_PWR_DET_SEL 0x140
+
+#define REG_DISABLE_FUN 0x00UL
+#define REG_ENABLE_FUN BIT(0)
+#define ACTIVATE_RTC_POR_DB_MAGIC_KEY 0x5af0
+#define INIT_LOAD_TIME 0xff
+
+struct cv1800b_rtc_priv {
+ struct rtc_device *rtc_dev;
+ void __iomem *core_map;
+ struct clk *clk;
+ int irq;
+};
+
+static int cv1800b_rtc_alarm_irq_enable(struct device *dev,
+ unsigned int enabled)
+{
+ struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
+
+ if (enabled)
+ writel_relaxed(REG_ENABLE_FUN, data->core_map + RTC_ALARM_ENABLE);
+ else
+ writel_relaxed(REG_DISABLE_FUN,
+ data->core_map + RTC_ALARM_ENABLE);
+
+ return 0;
+}
+
+static int cv1800b_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
+ unsigned long time = rtc_tm_to_time64(&alrm->time);
+
+ writel_relaxed(time, data->core_map + RTC_ALARM_TIME);
+
+ cv1800b_rtc_alarm_irq_enable(dev, 1);
+
+ return 0;
+}
+
+static int cv1800b_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ u32 alrm_time, now_time;
+ struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
+
+ alrm_time = readl(data->core_map + RTC_ALARM_TIME);
+ now_time = readl(data->core_map + RTC_SEC_CNTR_VALUE);
+ rtc_time64_to_tm(alrm_time, &alrm->time);
+ alrm->pending = now_time > alrm_time ? 1 : 0;
+ alrm->enabled = readl(data->core_map + RTC_ALARM_ENABLE);
+
+ return 0;
+}
+
+static int cv1800b_rtc_softinit(struct cv1800b_rtc_priv *dev)
+{
+ u32 timeout = 20;
+
+ writel(ACTIVATE_RTC_POR_DB_MAGIC_KEY,
+ dev->core_map + RTC_POR_DB_MAGIC_KEY);
+ writel(INIT_LOAD_TIME, dev->core_map + RTC_SET_SEC_CNTR_VALUE);
+ writel(REG_DISABLE_FUN, dev->core_map + RTC_SET_SEC_CNTR_TRIG);
+
+ while (readl(dev->core_map + RTC_SEC_CNTR_VALUE) == INIT_LOAD_TIME
+ && timeout--)
+ udelay(5);
+
+ if (!timeout)
+ return -1;
+ return 0;
+}
+
+static int cv1800b_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
+ u32 time = 0;
+
+ if (!data)
+ return -1;
+
+ time = readl_relaxed(data->core_map + RTC_SEC_CNTR_VALUE);
+ rtc_time64_to_tm(time, tm);
+
+ return 0;
+}
+
+static int cv1800b_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+ unsigned long time = rtc_tm_to_time64(tm);
+ struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
+
+ if (!data)
+ return -1;
+
+ writel(time, data->core_map + RTC_SET_SEC_CNTR_VALUE);
+ writel(REG_ENABLE_FUN, data->core_map + RTC_SET_SEC_CNTR_TRIG);
+
+ return 0;
+}
+
+static irqreturn_t cv1800b_rtc_irq_handler(int irq, void *dev_id)
+{
+ struct device *dev = dev_id;
+ struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
+
+ if (!data)
+ return -1;
+ writel(REG_DISABLE_FUN, data->core_map + RTC_ALARM_ENABLE);
+
+ return IRQ_HANDLED;
+}
+
+static const struct rtc_class_ops cv800b_rtc_ops = {
+ .read_time = cv1800b_rtc_read_time,
+ .set_time = cv1800b_rtc_set_time,
+ .read_alarm = cv1800b_rtc_read_alarm,
+ .set_alarm = cv1800b_rtc_set_alarm,
+ .alarm_irq_enable = cv1800b_rtc_alarm_irq_enable,
+};
+
+static int cv1800b_rtc_probe(struct platform_device *pdev)
+{
+ struct cv1800b_rtc_priv *rtc;
+ struct resource *res;
+ int ret;
+
+ rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+ if (!rtc)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ ret = -ENODEV;
+ goto err;
+ }
+
+ rtc->core_map = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(rtc->core_map)) {
+ ret = PTR_ERR(rtc->core_map);
+ goto err;
+ }
+
+ rtc->irq = platform_get_irq(pdev, 0);
+ platform_set_drvdata(pdev, rtc);
+ if (rtc->irq < 0) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ ret =
+ devm_request_irq(&pdev->dev, rtc->irq, cv1800b_rtc_irq_handler,
+ IRQF_SHARED, "rtc alarm", &pdev->dev);
+ if (ret)
+ goto err;
+
+ rtc->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(rtc->clk)) {
+ dev_err(&pdev->dev, "no clock");
+ ret = PTR_ERR(rtc->clk);
+ goto err;
+ }
+ ret = clk_prepare_enable(rtc->clk);
+ if (ret)
+ goto err;
+ ret = cv1800b_rtc_softinit(rtc);
+ if (ret)
+ goto err;
+ cv1800b_rtc_alarm_irq_enable(&pdev->dev, 1);
+ rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+ if (IS_ERR(rtc->rtc_dev)) {
+ ret = PTR_ERR(rtc->rtc_dev);
+ goto err;
+ }
+ rtc->rtc_dev->range_max = U32_MAX;
+ rtc->rtc_dev->ops = &cv800b_rtc_ops;
+
+ return rtc_register_device(rtc->rtc_dev);
+err:
+ return dev_err_probe(&pdev->dev, ret, "Failed to init cv1800b rtc\n");
+}
+
+static int __maybe_unused cv1800b_rtc_suspend_noirq(struct device *dev)
+{
+ struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(data->clk);
+
+ return 0;
+}
+
+static int __maybe_unused cv1800b_rtc_resume_noirq(struct device *dev)
+{
+ struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
+
+ clk_prepare_enable(data->clk);
+
+ return 0;
+}
+
+static const struct dev_pm_ops cv1800b_rtc_pm_ops = {
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(cv1800b_rtc_suspend_noirq,
+ cv1800b_rtc_resume_noirq)
+};
+
+static const struct of_device_id cv1800b_dt_ids[] = {
+ { .compatible = "sophgo,cv800b-rtc" },
+ { /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, cv1800b_dt_ids);
+
+static struct platform_driver cv1800b_rtc_driver = {
+ .driver = {
+ .name = "sophgo-cv800b-rtc",
+ .pm = &cv1800b_rtc_pm_ops,
+ .of_match_table = cv1800b_dt_ids,
+ },
+ .probe = cv1800b_rtc_probe,
+};
+
+module_platform_driver(cv1800b_rtc_driver);
+MODULE_AUTHOR("Jingbao Qiu");
+MODULE_DESCRIPTION("Sophgo CV1800B RTC Driver");
+MODULE_LICENSE("GPL");
--
2.25.1
On 21/11/2023 10:46, Jingbao Qiu wrote:
> Add devicetree binding for Sophgo CV1800B SoC rtc controller.
A nit, subject: drop second/last, redundant "binding for". The
"dt-bindings" prefix is already stating that these are bindings.
>
> Signed-off-by: Jingbao Qiu <[email protected]>
> ---
> .../bindings/rtc/sophgo,cv1800b-rtc.yaml | 37 +++++++++++++++++++
> 1 file changed, 37 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
>
> diff --git a/Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml b/Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
> new file mode 100644
> index 000000000000..fefb1e70c45c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
> @@ -0,0 +1,37 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/sophgo,cv1800b-rtc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sophgo CV1800B SoC RTC Controller
What is a RTC Controller? You have multiple RTCs there?
> +
> +maintainers:
> + - Jingbao Qiu <[email protected]>
> +
Missing ref to rtc.yaml. Unless it is not applicable but then why?
> +properties:
> + compatible:
> + enum:
> + - sophgo,cv1800b-rtc
Blank line
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +additionalProperties: false
unevaluatedProperties instead
> +
> +examples:
> + - |
> + rtc-controller@05026000{
The names is always "rtc", unless this is not RTC. If it isn't, please
add full description of the hardware.
> + compatible = "sophgo,cv800b-rtc";
> + reg = <0x05026000 0x1000>;
> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-parent = <&plic0>;
> + clocks = <&osc>;
Why do you send untested bindings? Review costs significant amount of
effort. Code was also not compiled? Warnings not fixed?
Best regards,
Krzysztof
On 21/11/2023 10:46, Jingbao Qiu wrote:
> Add the rtc device tree node to CV1800B SoC.
>
> Signed-off-by: Jingbao Qiu <[email protected]>
> ---
> arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> index df40e87ee063..89411c75b89a 100644
> --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> @@ -119,5 +119,13 @@ clint: timer@74000000 {
> reg = <0x74000000 0x10000>;
> interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> };
> +
> + rtc: rtc-controller@05026000 {
> + compatible = "sophgo,cv800b-rtc";
> + reg = <0x05026000 0x1000>;
> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-parent = <&plic>;
> + clocks = <&osc>;
> + };
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Best regards,
Krzysztof
On 21/11/2023 10:46, Jingbao Qiu wrote:
> Implement the RTC driver for CV1800B, which able to provide time and
> alarm functionality.
>
> Signed-off-by: Jingbao Qiu <[email protected]>
> ---
> drivers/rtc/Kconfig | 10 ++
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-cv1800b.c | 293 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
> create mode 100644 drivers/rtc/rtc-cv1800b.c
Bindings were not tested, so I assume you did not compile the code
either. Please confirm that you fixed all warnings pointed out by W=1
builds, smatch and sparse. All of them.
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 3814e0845e77..2089cceea38c 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1103,6 +1103,16 @@ 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_CV1800B
> + tristate "Sophgo CV1800B RTC"
> + depends on ARCH_SOPHGO || COMPILE_TEST
> + help
> + If you say yes here you will get support for the
> + RTC of the Sophgo CV1800B SOC.
> +
> + This depend on ARCH_SOPHGO and COMPILE_TEST. Please
> + first config that.
...
> +static int cv1800b_rtc_probe(struct platform_device *pdev)
> +{
> + struct cv1800b_rtc_priv *rtc;
> + struct resource *res;
> + int ret;
> +
> + rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> + if (!rtc)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + rtc->core_map = devm_ioremap_resource(&pdev->dev, res);
Use helper combining these two calls.
> + if (IS_ERR(rtc->core_map)) {
> + ret = PTR_ERR(rtc->core_map);
> + goto err;
> + }
> +
> + rtc->irq = platform_get_irq(pdev, 0);
> + platform_set_drvdata(pdev, rtc);
Your code has random order. First you get IRQ, then you check its value,
then you go further.
> + if (rtc->irq < 0) {
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + ret =
> + devm_request_irq(&pdev->dev, rtc->irq, cv1800b_rtc_irq_handler,
Wrong wrapping.
> + IRQF_SHARED, "rtc alarm", &pdev->dev);
Why shared?
> + if (ret)
> + goto err;
> +
> + rtc->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(rtc->clk)) {
> + dev_err(&pdev->dev, "no clock");
This code is not ready for upstream. There are multiple things wrong here.
First, syntax is return dev_err_probe.
Second, you do not have clocks and you do not allow them! Just open your
binding.
Third, use wrapper - devm_clk_get_enable or something like that.
> + ret = PTR_ERR(rtc->clk);
> + goto err;
> + }
Blank line.
> + ret = clk_prepare_enable(rtc->clk);
> + if (ret)
> + goto err;
Blank line.
> + ret = cv1800b_rtc_softinit(rtc);
> + if (ret)
> + goto err;
> + cv1800b_rtc_alarm_irq_enable(&pdev->dev, 1);
> + rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
> + if (IS_ERR(rtc->rtc_dev)) {
> + ret = PTR_ERR(rtc->rtc_dev);
> + goto err;
> + }
> + rtc->rtc_dev->range_max = U32_MAX;
> + rtc->rtc_dev->ops = &cv800b_rtc_ops;
> +
> + return rtc_register_device(rtc->rtc_dev);
> +err:
> + return dev_err_probe(&pdev->dev, ret, "Failed to init cv1800b rtc\n");
Drop, just return.
Best regards,
Krzysztof
On Tue, 21 Nov 2023 17:46:40 +0800, Jingbao Qiu wrote:
> Add devicetree binding for Sophgo CV1800B SoC rtc controller.
>
> Signed-off-by: Jingbao Qiu <[email protected]>
> ---
> .../bindings/rtc/sophgo,cv1800b-rtc.yaml | 37 +++++++++++++++++++
> 1 file changed, 37 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.example.dts:27.28-29 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Hi Jingbao,
kernel test robot noticed the following build errors:
[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on linus/master v6.7-rc2 next-20231121]
[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-add-binding-for-Sophgo-CV1800B-rtc-controller/20231121-174927
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link: https://lore.kernel.org/r/20231121094642.2973795-3-qiujingbao.dlmu%40gmail.com
patch subject: [PATCH 2/3] rtc: add rtc controller support for Sophgo CV1800B SoC
config: parisc-allmodconfig (https://download.01.org/0day-ci/archive/20231122/[email protected]/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231122/[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-cv1800b.c: In function 'cv1800b_rtc_probe':
>> drivers/rtc/rtc-cv1800b.c:246:16: error: implicit declaration of function 'rtc_register_device'; did you mean 'devm_rtc_register_device'? [-Werror=implicit-function-declaration]
246 | return rtc_register_device(rtc->rtc_dev);
| ^~~~~~~~~~~~~~~~~~~
| devm_rtc_register_device
cc1: some warnings being treated as errors
vim +246 drivers/rtc/rtc-cv1800b.c
189
190 static int cv1800b_rtc_probe(struct platform_device *pdev)
191 {
192 struct cv1800b_rtc_priv *rtc;
193 struct resource *res;
194 int ret;
195
196 rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
197 if (!rtc)
198 return -ENOMEM;
199
200 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
201 if (!res) {
202 ret = -ENODEV;
203 goto err;
204 }
205
206 rtc->core_map = devm_ioremap_resource(&pdev->dev, res);
207 if (IS_ERR(rtc->core_map)) {
208 ret = PTR_ERR(rtc->core_map);
209 goto err;
210 }
211
212 rtc->irq = platform_get_irq(pdev, 0);
213 platform_set_drvdata(pdev, rtc);
214 if (rtc->irq < 0) {
215 ret = -EINVAL;
216 goto err;
217 }
218
219 ret =
220 devm_request_irq(&pdev->dev, rtc->irq, cv1800b_rtc_irq_handler,
221 IRQF_SHARED, "rtc alarm", &pdev->dev);
222 if (ret)
223 goto err;
224
225 rtc->clk = devm_clk_get(&pdev->dev, NULL);
226 if (IS_ERR(rtc->clk)) {
227 dev_err(&pdev->dev, "no clock");
228 ret = PTR_ERR(rtc->clk);
229 goto err;
230 }
231 ret = clk_prepare_enable(rtc->clk);
232 if (ret)
233 goto err;
234 ret = cv1800b_rtc_softinit(rtc);
235 if (ret)
236 goto err;
237 cv1800b_rtc_alarm_irq_enable(&pdev->dev, 1);
238 rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
239 if (IS_ERR(rtc->rtc_dev)) {
240 ret = PTR_ERR(rtc->rtc_dev);
241 goto err;
242 }
243 rtc->rtc_dev->range_max = U32_MAX;
244 rtc->rtc_dev->ops = &cv800b_rtc_ops;
245
> 246 return rtc_register_device(rtc->rtc_dev);
247 err:
248 return dev_err_probe(&pdev->dev, ret, "Failed to init cv1800b rtc\n");
249 }
250
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Jingbao,
kernel test robot noticed the following build errors:
[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on linus/master v6.7-rc2 next-20231121]
[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-add-binding-for-Sophgo-CV1800B-rtc-controller/20231121-174927
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link: https://lore.kernel.org/r/20231121094642.2973795-3-qiujingbao.dlmu%40gmail.com
patch subject: [PATCH 2/3] rtc: add rtc controller support for Sophgo CV1800B SoC
config: sparc-allmodconfig (https://download.01.org/0day-ci/archive/20231122/[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/20231122/[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-cv1800b.c: In function 'cv1800b_rtc_alarm_irq_enable':
>> drivers/rtc/rtc-cv1800b.c:90:17: error: implicit declaration of function 'writel_relaxed' [-Werror=implicit-function-declaration]
90 | writel_relaxed(REG_ENABLE_FUN, data->core_map + RTC_ALARM_ENABLE);
| ^~~~~~~~~~~~~~
drivers/rtc/rtc-cv1800b.c: In function 'cv1800b_rtc_read_alarm':
>> drivers/rtc/rtc-cv1800b.c:115:21: error: implicit declaration of function 'readl' [-Werror=implicit-function-declaration]
115 | alrm_time = readl(data->core_map + RTC_ALARM_TIME);
| ^~~~~
drivers/rtc/rtc-cv1800b.c: In function 'cv1800b_rtc_softinit':
>> drivers/rtc/rtc-cv1800b.c:128:9: error: implicit declaration of function 'writel' [-Werror=implicit-function-declaration]
128 | writel(ACTIVATE_RTC_POR_DB_MAGIC_KEY,
| ^~~~~~
drivers/rtc/rtc-cv1800b.c: In function 'cv1800b_rtc_read_time':
>> drivers/rtc/rtc-cv1800b.c:150:16: error: implicit declaration of function 'readl_relaxed' [-Werror=implicit-function-declaration]
150 | time = readl_relaxed(data->core_map + RTC_SEC_CNTR_VALUE);
| ^~~~~~~~~~~~~
drivers/rtc/rtc-cv1800b.c: In function 'cv1800b_rtc_probe':
drivers/rtc/rtc-cv1800b.c:246:16: error: implicit declaration of function 'rtc_register_device'; did you mean 'devm_rtc_register_device'? [-Werror=implicit-function-declaration]
246 | return rtc_register_device(rtc->rtc_dev);
| ^~~~~~~~~~~~~~~~~~~
| devm_rtc_register_device
cc1: some warnings being treated as errors
vim +/writel_relaxed +90 drivers/rtc/rtc-cv1800b.c
83
84 static int cv1800b_rtc_alarm_irq_enable(struct device *dev,
85 unsigned int enabled)
86 {
87 struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
88
89 if (enabled)
> 90 writel_relaxed(REG_ENABLE_FUN, data->core_map + RTC_ALARM_ENABLE);
91 else
92 writel_relaxed(REG_DISABLE_FUN,
93 data->core_map + RTC_ALARM_ENABLE);
94
95 return 0;
96 }
97
98 static int cv1800b_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
99 {
100 struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
101 unsigned long time = rtc_tm_to_time64(&alrm->time);
102
103 writel_relaxed(time, data->core_map + RTC_ALARM_TIME);
104
105 cv1800b_rtc_alarm_irq_enable(dev, 1);
106
107 return 0;
108 }
109
110 static int cv1800b_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
111 {
112 u32 alrm_time, now_time;
113 struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
114
> 115 alrm_time = readl(data->core_map + RTC_ALARM_TIME);
116 now_time = readl(data->core_map + RTC_SEC_CNTR_VALUE);
117 rtc_time64_to_tm(alrm_time, &alrm->time);
118 alrm->pending = now_time > alrm_time ? 1 : 0;
119 alrm->enabled = readl(data->core_map + RTC_ALARM_ENABLE);
120
121 return 0;
122 }
123
124 static int cv1800b_rtc_softinit(struct cv1800b_rtc_priv *dev)
125 {
126 u32 timeout = 20;
127
> 128 writel(ACTIVATE_RTC_POR_DB_MAGIC_KEY,
129 dev->core_map + RTC_POR_DB_MAGIC_KEY);
130 writel(INIT_LOAD_TIME, dev->core_map + RTC_SET_SEC_CNTR_VALUE);
131 writel(REG_DISABLE_FUN, dev->core_map + RTC_SET_SEC_CNTR_TRIG);
132
133 while (readl(dev->core_map + RTC_SEC_CNTR_VALUE) == INIT_LOAD_TIME
134 && timeout--)
135 udelay(5);
136
137 if (!timeout)
138 return -1;
139 return 0;
140 }
141
142 static int cv1800b_rtc_read_time(struct device *dev, struct rtc_time *tm)
143 {
144 struct cv1800b_rtc_priv *data = dev_get_drvdata(dev);
145 u32 time = 0;
146
147 if (!data)
148 return -1;
149
> 150 time = readl_relaxed(data->core_map + RTC_SEC_CNTR_VALUE);
151 rtc_time64_to_tm(time, tm);
152
153 return 0;
154 }
155
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Tue, Nov 21, 2023 at 5:57 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 21/11/2023 10:46, Jingbao Qiu wrote:
> > Add devicetree binding for Sophgo CV1800B SoC rtc controller.
>
> A nit, subject: drop second/last, redundant "binding for". The
> "dt-bindings" prefix is already stating that these are bindings.
will fix.
>
> >
> > Signed-off-by: Jingbao Qiu <[email protected]>
> > ---
> > .../bindings/rtc/sophgo,cv1800b-rtc.yaml | 37 +++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml b/Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
> > new file mode 100644
> > index 000000000000..fefb1e70c45c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/sophgo,cv1800b-rtc.yaml
> > @@ -0,0 +1,37 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/rtc/sophgo,cv1800b-rtc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sophgo CV1800B SoC RTC Controller
>
> What is a RTC Controller? You have multiple RTCs there?
>
will drop "Controller", as I think RTC is not something like I2C, eMMC, USB,
which have the "controller <-> client/device" model.
> > +
> > +maintainers:
> > + - Jingbao Qiu <[email protected]>
> > +
>
> Missing ref to rtc.yaml. Unless it is not applicable but then why?
ok, I should ref this file.
>
> > +properties:
> > + compatible:
> > + enum:
> > + - sophgo,cv1800b-rtc
>
> Blank line
ok
>
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > +
> > +additionalProperties: false
>
> unevaluatedProperties instead
will fix .
>
> > +
> > +examples:
> > + - |
> > + rtc-controller@05026000{
>
> The names is always "rtc", unless this is not RTC. If it isn't, please
> add full description of the hardware.
I will use "rtc" replace "rtc-controller" .
>
> > + compatible = "sophgo,cv800b-rtc";
> > + reg = <0x05026000 0x1000>;
> > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-parent = <&plic0>;
> > + clocks = <&osc>;
>
> Why do you send untested bindings? Review costs significant amount of
> effort. Code was also not compiled? Warnings not fixed?
I will check it.
Leading 0 and referencing issues will be fixed.
>
> Best regards,
> Krzysztof
>
I'm sorry for taking so long to reply.
I took a few days off due to being infected with the flu.
Thank you again for your patient reply.
Best regards,
Jingbao Qiu
On Tue, Nov 21, 2023 at 6:01 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 21/11/2023 10:46, Jingbao Qiu wrote:
> > Implement the RTC driver for CV1800B, which able to provide time and
> > alarm functionality.
> >
> > Signed-off-by: Jingbao Qiu <[email protected]>
> > ---
> > drivers/rtc/Kconfig | 10 ++
> > drivers/rtc/Makefile | 1 +
> > drivers/rtc/rtc-cv1800b.c | 293 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 304 insertions(+)
> > create mode 100644 drivers/rtc/rtc-cv1800b.c
>
> Bindings were not tested, so I assume you did not compile the code
> either. Please confirm that you fixed all warnings pointed out by W=1
> builds, smatch and sparse. All of them.
will test & fix in next version
>
> >
> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> > index 3814e0845e77..2089cceea38c 100644
> > --- a/drivers/rtc/Kconfig
> > +++ b/drivers/rtc/Kconfig
> > @@ -1103,6 +1103,16 @@ 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_CV1800B
> > + tristate "Sophgo CV1800B RTC"
> > + depends on ARCH_SOPHGO || COMPILE_TEST
> > + help
> > + If you say yes here you will get support for the
> > + RTC of the Sophgo CV1800B SOC.
> > +
> > + This depend on ARCH_SOPHGO and COMPILE_TEST. Please
> > + first config that.
>
> ...
>
> > +static int cv1800b_rtc_probe(struct platform_device *pdev)
> > +{
> > + struct cv1800b_rtc_priv *rtc;
> > + struct resource *res;
> > + int ret;
> > +
> > + rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > + if (!rtc)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!res) {
> > + ret = -ENODEV;
> > + goto err;
> > + }
> > +
> > + rtc->core_map = devm_ioremap_resource(&pdev->dev, res);
>
> Use helper combining these two calls.
Ok, will use devm_platform_ioremap_resource() to replace it.
>
> > + if (IS_ERR(rtc->core_map)) {
> > + ret = PTR_ERR(rtc->core_map);
> > + goto err;
> > + }
> > +
> > + rtc->irq = platform_get_irq(pdev, 0);
> > + platform_set_drvdata(pdev, rtc);
>
> Your code has random order. First you get IRQ, then you check its value,
> then you go further.
ok
>
> > + if (rtc->irq < 0) {
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
> > + ret =
> > + devm_request_irq(&pdev->dev, rtc->irq, cv1800b_rtc_irq_handler,
>
> Wrong wrapping.
ok
>
> > + IRQF_SHARED, "rtc alarm", &pdev->dev);
>
> Why shared?
ok
>
> > + if (ret)
> > + goto err;
> > +
> > + rtc->clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(rtc->clk)) {
> > + dev_err(&pdev->dev, "no clock");
>
> This code is not ready for upstream. There are multiple things wrong here.
>
> First, syntax is return dev_err_probe.
>
> Second, you do not have clocks and you do not allow them! Just open your
> binding.
I'm not fully understanding here, can you elaborate more?
as there is clocks info like this in the dt-bindings:
clocks = <&osc>;
>
> Third, use wrapper - devm_clk_get_enable or something like that.
I will use devm_clk_get_enabled() to replace it.
>
>
> > + ret = PTR_ERR(rtc->clk);
> > + goto err;
> > + }
>
> Blank line.
ok
>
> > + ret = clk_prepare_enable(rtc->clk);
> > + if (ret)
> > + goto err;
>
> Blank line.
ok
>
> > + ret = cv1800b_rtc_softinit(rtc);
> > + if (ret)
> > + goto err;
> > + cv1800b_rtc_alarm_irq_enable(&pdev->dev, 1);
> > + rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
> > + if (IS_ERR(rtc->rtc_dev)) {
> > + ret = PTR_ERR(rtc->rtc_dev);
> > + goto err;
> > + }
> > + rtc->rtc_dev->range_max = U32_MAX;
> > + rtc->rtc_dev->ops = &cv800b_rtc_ops;
> > +
> > + return rtc_register_device(rtc->rtc_dev);
I find the commet of devm_rtc_device_register wirte
“This function is deprecated, use devm_rtc_allocate_device and
rtc_register_device instead”
but all of code about this, they all use devm_rtc_device_register
function. So which one do you suggest I use?
> > +err:
> > + return dev_err_probe(&pdev->dev, ret, "Failed to init cv1800b rtc\n");
>
> Drop, just return.
ok
>
> Best regards,
> Krzysztof
>
Best regards,
Jingbao Qiu
On Tue, Nov 21, 2023 at 6:00 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 21/11/2023 10:46, Jingbao Qiu wrote:
> > Add the rtc device tree node to CV1800B SoC.
> >
> > Signed-off-by: Jingbao Qiu <[email protected]>
> > ---
> > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > index df40e87ee063..89411c75b89a 100644
> > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > @@ -119,5 +119,13 @@ clint: timer@74000000 {
> > reg = <0x74000000 0x10000>;
> > interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> > };
> > +
> > + rtc: rtc-controller@05026000 {
> > + compatible = "sophgo,cv800b-rtc";
> > + reg = <0x05026000 0x1000>;
> > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-parent = <&plic>;
> > + clocks = <&osc>;
> > + };
>
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check W=1` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).
I will check it.
>
> Best regards,
> Krzysztof
>
Best regards,
Jingbao Qiu
On 28/11/2023 14:22, jingbao qiu wrote:
>>> + if (ret)
>>> + goto err;
>>> +
>>> + rtc->clk = devm_clk_get(&pdev->dev, NULL);
>>> + if (IS_ERR(rtc->clk)) {
>>> + dev_err(&pdev->dev, "no clock");
>>
>> This code is not ready for upstream. There are multiple things wrong here.
>>
>> First, syntax is return dev_err_probe.
>>
>> Second, you do not have clocks and you do not allow them! Just open your
>> binding.
>
> I'm not fully understanding here, can you elaborate more?
That the syntax is dev_err_probe() or that you do not have clocks?
> as there is clocks info like this in the dt-bindings:
> clocks = <&osc>;
Really?
Point me to the line in your patch:
+properties:
+ compatible:
+ enum:
+ - sophgo,cv1800b-rtc
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
Where are the clocks?
Best regards,
Krzysztof
On Tue, Nov 28, 2023 at 9:59 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 28/11/2023 14:22, jingbao qiu wrote:
> >>> + if (ret)
> >>> + goto err;
> >>> +
> >>> + rtc->clk = devm_clk_get(&pdev->dev, NULL);
> >>> + if (IS_ERR(rtc->clk)) {
> >>> + dev_err(&pdev->dev, "no clock");
> >>
> >> This code is not ready for upstream. There are multiple things wrong here.
> >>
> >> First, syntax is return dev_err_probe.
> >>
> >> Second, you do not have clocks and you do not allow them! Just open your
> >> binding.
> >
> > I'm not fully understanding here, can you elaborate more?
>
> That the syntax is dev_err_probe() or that you do not have clocks?
>
>
> > as there is clocks info like this in the dt-bindings:
> > clocks = <&osc>;
>
> Really?
>
> Point me to the line in your patch:
>
> +properties:
> + compatible:
> + enum:
> + - sophgo,cv1800b-rtc
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
>
> Where are the clocks?
I will fix properties.
>
>
> Best regards,
> Krzysztof
>
Best regards,
Jingbao Qiu
On 28/11/2023 21:22:52+0800, jingbao qiu wrote:
> >
> > > + ret = cv1800b_rtc_softinit(rtc);
> > > + if (ret)
> > > + goto err;
> > > + cv1800b_rtc_alarm_irq_enable(&pdev->dev, 1);
> > > + rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
> > > + if (IS_ERR(rtc->rtc_dev)) {
> > > + ret = PTR_ERR(rtc->rtc_dev);
> > > + goto err;
> > > + }
> > > + rtc->rtc_dev->range_max = U32_MAX;
> > > + rtc->rtc_dev->ops = &cv800b_rtc_ops;
> > > +
> > > + return rtc_register_device(rtc->rtc_dev);
>
> I find the commet of devm_rtc_device_register wirte
> “This function is deprecated, use devm_rtc_allocate_device and
> rtc_register_device instead”
> but all of code about this, they all use devm_rtc_device_register
They don't, they use devm_rtc_register_device
> function. So which one do you suggest I use?
>
> > > +err:
> > > + return dev_err_probe(&pdev->dev, ret, "Failed to init cv1800b rtc\n");
> >
> > Drop, just return.
>
> ok
>
> >
> > Best regards,
> > Krzysztof
> >
>
> Best regards,
> Jingbao Qiu
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com