[PATCH V2 0/7] rtc: ls2x: Add support for the Loongson-2K/LS7A RTC

[PATCH V2 0/7] rtc: ls2x: Add support for the Loongson-2K/LS7A RTC

[Binbin Zhou]

Hi all:

The initial DT-base ls2x rtc driver was written by Wang Xuerui, He has
released five versions of patchset before, and all related mail records
are shown below if you are interested:

https://lore.kernel.org/all/?q=ls2x-rtc

In this series of patches, based on the code above, I have added the
following support:

1. Add ACPI-related support, as Loongson-3A5000 + LS7A is now ACPI-base
   by default under LoongArch architecture;
2. Add rtc alarm/walarm related functions.

I have tested on Loongson-3A5000LA+LS7A1000/LS7A2000, Loongson-2K1000LA
and Loongson-2K0500.

BTW:
There have been discussions about merging the rtc drivers of ls1x and
ls2x, but the following reasons made the merger difficult to achieve:

1. ls1x does not support ACPI, for it is only on MIPS-based system;
2. ls1x does not support alarm function.

Thanks.

-------
Changes since v1:
1. Rebased on top of latest loongarch-next;
2. Add interrupt descriptions to the ls2k and ls7a DTS files to avoid
errors when the driver gets the IRQ number, Thanks to Qing Zhang for
testing;
3. Remove some inexact CONFIG_ACPI.

Binbin Zhou (4):
  rtc: Add support for the Loongson-2K/LS7A RTC
  LoongArch: Enable LS2X RTC in loongson3_defconfig
  MIPS: Loongson64: DTS: Add RTC support to LS7A
  MIPS: Loongson64: DTS: Add RTC support to Loongson-2K

WANG Xuerui (3):
  dt-bindings: rtc: Add Loongson LS2X RTC support
  MIPS: Loongson: Enable LS2X RTC in loongson3_defconfig
  MIPS: Loongson: Enable LS2X RTC in loongson2k_defconfig

 .../devicetree/bindings/rtc/trivial-rtc.yaml  |   2 +
 arch/loongarch/configs/loongson3_defconfig    |   1 +
 .../boot/dts/loongson/loongson64-2k1000.dtsi  |   7 +
 arch/mips/boot/dts/loongson/ls7a-pch.dtsi     |   7 +
 arch/mips/configs/loongson2k_defconfig        |   1 +
 arch/mips/configs/loongson3_defconfig         |   1 +
 drivers/rtc/Kconfig                           |  11 +
 drivers/rtc/Makefile                          |   1 +
 drivers/rtc/rtc-ls2x.c                        | 379 ++++++++++++++++++
 9 files changed, 410 insertions(+)
 create mode 100644 drivers/rtc/rtc-ls2x.c

-- 
2.31.1

[PATCH V2 1/7] dt-bindings: rtc: Add Loongson LS2X RTC support

[Binbin Zhou]

From: WANG Xuerui <git@xen0n.name>

Document the binding for the LS2X RTC block found on the Loongson-2K SoC
and the LS7A bridge, originally appearing on the Loongson-2H.

Signed-off-by: WANG Xuerui <git@xen0n.name>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/rtc/trivial-rtc.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
index d9fc120c61cc..0ef1ffd54655 100644
--- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
@@ -47,6 +47,8 @@ properties:
       - isil,isl1218
       # Intersil ISL12022 Real-time Clock
       - isil,isl12022
+      # Loongson-2K Socs/LS7A bridge Real-time Clock
+      - loongson,ls2x-rtc
       # Real Time Clock Module with I2C-Bus
       - microcrystal,rv3028
       # Real Time Clock Module with I2C-Bus
-- 
2.31.1

[PATCH V2 2/7] rtc: Add support for the Loongson-2K/LS7A RTC

[Binbin Zhou]

This RTC module is integrated into the Loongson-2K SoC and the LS7A
bridge chip. This version is almost entirely rewritten to make use of
current kernel API, and it supports both ACPI and DT.

This driver is shared by MIPS-based Loongson-3A4000 system (use FDT) and
LoongArch-based Loongson-3A5000 system (use ACPI).

Signed-off-by: Huacai Chen <chenhuacai@kernel.org>
Signed-off-by: WANG Xuerui <git@xen0n.name>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/rtc/Kconfig    |  11 ++
 drivers/rtc/Makefile   |   1 +
 drivers/rtc/rtc-ls2x.c | 379 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 391 insertions(+)
 create mode 100644 drivers/rtc/rtc-ls2x.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index bb63edb507da..f8586aa00fce 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1735,6 +1735,17 @@ config RTC_DRV_LPC32XX
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-lpc32xx.
 
+config RTC_DRV_LS2X
+	tristate "Loongson LS2X RTC"
+	depends on MACH_LOONGSON64 || COMPILE_TEST
+	select REGMAP_MMIO
+	help
+	  If you say yes here you get support for the RTC on the Loongson-2K
+	  SoC and LS7A bridge, which first appeared on the Loongson-2H.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-ls2x.
+
 config RTC_DRV_PM8XXX
 	tristate "Qualcomm PMIC8XXX RTC"
 	depends on MFD_PM8XXX || MFD_SPMI_PMIC || COMPILE_TEST
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index aab22bc63432..d5a467e9eec8 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_RTC_DRV_LOONGSON1)	+= rtc-ls1x.o
 obj-$(CONFIG_RTC_DRV_LP8788)	+= rtc-lp8788.o
 obj-$(CONFIG_RTC_DRV_LPC24XX)	+= rtc-lpc24xx.o
 obj-$(CONFIG_RTC_DRV_LPC32XX)	+= rtc-lpc32xx.o
+obj-$(CONFIG_RTC_DRV_LS2X)	+= rtc-ls2x.o
 obj-$(CONFIG_RTC_DRV_M41T80)	+= rtc-m41t80.o
 obj-$(CONFIG_RTC_DRV_M41T93)	+= rtc-m41t93.o
 obj-$(CONFIG_RTC_DRV_M41T94)	+= rtc-m41t94.o
diff --git a/drivers/rtc/rtc-ls2x.c b/drivers/rtc/rtc-ls2x.c
new file mode 100644
index 000000000000..06ef249a9485
--- /dev/null
+++ b/drivers/rtc/rtc-ls2x.c
@@ -0,0 +1,379 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Loongson-2K/LS7A RTC driver
+ *
+ * Based on the original out-of-tree Loongson-2H RTC driver for Linux 2.6.32,
+ * by Shaozong Liu <liushaozong@loongson.cn>.
+ *
+ * Maintained out-of-tree by Huacai Chen <chenhuacai@kernel.org>.
+ * Rewritten for mainline by WANG Xuerui <git@xen0n.name>.
+ *                           Binbin Zhou <zhoubinbin@loongson.cn>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/rtc.h>
+#include <linux/acpi.h>
+
+/* Time Of Year(TOY) counters registers */
+#define TOY_TRIM_REG		0x20 /* Must be initialized to 0 */
+#define TOY_WRITE0_REG		0x24 /* TOY low 32-bit value (write-only) */
+#define TOY_WRITE1_REG		0x28 /* TOY high 32-bit value (write-only) */
+#define TOY_READ0_REG		0x2c /* TOY low 32-bit value (read-only) */
+#define TOY_READ1_REG		0x30 /* TOY high 32-bit value (read-only) */
+#define TOY_MATCH0_REG		0x34 /* TOY timing interrupt 0 */
+#define TOY_MATCH1_REG		0x38 /* TOY timing interrupt 1 */
+#define TOY_MATCH2_REG		0x3c /* TOY timing interrupt 2 */
+
+/* RTC counters registers */
+#define RTC_CTRL_REG		0x40 /* TOY and RTC control register */
+#define RTC_TRIM_REG		0x60 /* Must be initialized to 0 */
+#define RTC_WRITE0_REG		0x64 /* RTC counters value (write-only) */
+#define RTC_READ0_REG		0x68 /* RTC counters value (read-only) */
+#define RTC_MATCH0_REG		0x6c /* RTC timing interrupt 0 */
+#define RTC_MATCH1_REG		0x70 /* RTC timing interrupt 1 */
+#define RTC_MATCH2_REG		0x74 /* RTC timing interrupt 2 */
+
+/* TOY_WRITE0_REG bitmask */
+#define TOY_MON			GENMASK(31, 26)
+#define TOY_DAY			GENMASK(25, 21)
+#define TOY_HOUR		GENMASK(20, 16)
+#define TOY_MIN			GENMASK(15, 10)
+#define TOY_SEC			GENMASK(9, 4)
+#define TOY_MSEC		GENMASK(3, 0)
+
+/* TOY_MATCH0/1/2_REG bitmask */
+#define TOY_MATCH_YEAR		GENMASK(31, 26)
+#define TOY_MATCH_MON		GENMASK(25, 22)
+#define TOY_MATCH_DAY		GENMASK(21, 17)
+#define TOY_MATCH_HOUR		GENMASK(16, 12)
+#define TOY_MATCH_MIN		GENMASK(11, 6)
+#define TOY_MATCH_SEC		GENMASK(5, 0)
+
+/* RTC_CTRL_REG bitmask */
+#define RTC_ENABLE		BIT(13) /* 1: RTC counters enable */
+#define TOY_ENABLE		BIT(11) /* 1: TOY counters enable */
+#define OSC_ENABLE		BIT(8) /* 1: 32.768k crystal enable */
+
+/* Offset of PM domain from RTC domain */
+#define PM_RTC_OFFSET		0x100
+
+/* PM domain registers */
+#define PM1_STS_REG		0x0c /* Power management 1 status register */
+#define RTC_STS			BIT(10) /* RTC status */
+#define PM1_EN_REG		0x10 /* Power management 1 enable register */
+#define RTC_EN			BIT(10) /* RTC event enable */
+
+struct ls2x_rtc_priv {
+	spinlock_t		rtc_reglock;
+	int			irq;
+	struct rtc_device	*rtcdev;
+	struct regmap		*regmap;
+	void __iomem		*acpi_base;
+};
+
+static const struct regmap_config ls2x_rtc_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+struct ls2x_rtc_regs {
+	u32 reg0;
+	u32 reg1;
+};
+
+/* IRQ Handlers */
+static irqreturn_t ls2x_rtc_isr(int irq, void *id)
+{
+	struct ls2x_rtc_priv *priv = (struct ls2x_rtc_priv *)id;
+
+	rtc_update_irq(priv->rtcdev, 1, RTC_AF | RTC_IRQF);
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_ACPI
+static u32 ls2x_acpi_fix_handler(void *id)
+{
+	int ret;
+	struct ls2x_rtc_priv *priv = (struct ls2x_rtc_priv *)id;
+
+	spin_lock(&priv->rtc_reglock);
+
+	/* Disable acpi rtc enabled */
+	ret = readl(priv->acpi_base + PM1_EN_REG) & ~RTC_EN;
+	writel(ret, priv->acpi_base + PM1_EN_REG);
+
+	/* Clear acpi rtc interrupt Status */
+	writel(RTC_STS, priv->acpi_base + PM1_STS_REG);
+
+	spin_unlock(&priv->rtc_reglock);
+
+	/*
+	 * The TOY_MATCH0_REG should be cleared 0 here,
+	 * otherwise the interrupt cannot be cleared.
+	 * Because the match condition is still satisfied
+	 */
+	ret = regmap_write(priv->regmap, TOY_MATCH0_REG, 0);
+	if (ret < 0)
+		return ret;
+
+	rtc_update_irq(priv->rtcdev, 1, RTC_AF | RTC_IRQF);
+	return 0;
+}
+#endif
+
+static inline void ls2x_rtc_regs_to_time(struct ls2x_rtc_regs *regs,
+					 struct rtc_time *tm)
+{
+	tm->tm_year = regs->reg1;
+	tm->tm_sec = FIELD_GET(TOY_SEC, regs->reg0);
+	tm->tm_min = FIELD_GET(TOY_MIN, regs->reg0);
+	tm->tm_hour = FIELD_GET(TOY_HOUR, regs->reg0);
+	tm->tm_mday = FIELD_GET(TOY_DAY, regs->reg0);
+	tm->tm_mon = FIELD_GET(TOY_MON, regs->reg0) - 1;
+}
+
+static inline void ls2x_rtc_time_to_regs(struct rtc_time *tm,
+					 struct ls2x_rtc_regs *regs)
+{
+	regs->reg0 = FIELD_PREP(TOY_SEC, tm->tm_sec);
+	regs->reg0 |= FIELD_PREP(TOY_MIN, tm->tm_min);
+	regs->reg0 |= FIELD_PREP(TOY_HOUR, tm->tm_hour);
+	regs->reg0 |= FIELD_PREP(TOY_DAY, tm->tm_mday);
+	regs->reg0 |= FIELD_PREP(TOY_MON, tm->tm_mon + 1);
+	regs->reg1 = tm->tm_year;
+}
+
+static inline void ls2x_rtc_alarm_regs_to_time(struct ls2x_rtc_regs *regs,
+					 struct rtc_time *tm)
+{
+	tm->tm_sec = FIELD_GET(TOY_MATCH_SEC, regs->reg0);
+	tm->tm_min = FIELD_GET(TOY_MATCH_MIN, regs->reg0);
+	tm->tm_hour = FIELD_GET(TOY_MATCH_HOUR, regs->reg0);
+	tm->tm_mday = FIELD_GET(TOY_MATCH_DAY, regs->reg0);
+	tm->tm_mon = FIELD_GET(TOY_MATCH_MON, regs->reg0) - 1;
+	/*
+	 * The rtc SYS_TOYMATCH0/YEAR bit field is only 6 bits, so it means 63
+	 * years at most. Therefore, The RTC alarm years can be set from 1900
+	 * to 1963. This causes the initialization of alarm fail during call
+	 * __rtc_read_alarm.
+	 * We add 64 years offset to ls2x_rtc_read_alarm. After adding the
+	 * offset, the RTC alarm clock can be set from 1964 to 2027.
+	 */
+	tm->tm_year = FIELD_GET(TOY_MATCH_YEAR, regs->reg0) + 64;
+}
+
+static inline void ls2x_rtc_time_to_alarm_regs(struct rtc_time *tm,
+					 struct ls2x_rtc_regs *regs)
+{
+	regs->reg0 = FIELD_PREP(TOY_MATCH_SEC, tm->tm_sec);
+	regs->reg0 |= FIELD_PREP(TOY_MATCH_MIN, tm->tm_min);
+	regs->reg0 |= FIELD_PREP(TOY_MATCH_HOUR, tm->tm_hour);
+	regs->reg0 |= FIELD_PREP(TOY_MATCH_DAY, tm->tm_mday);
+	regs->reg0 |= FIELD_PREP(TOY_MATCH_MON, tm->tm_mon + 1);
+	regs->reg0 |= FIELD_PREP(TOY_MATCH_YEAR, tm->tm_year);
+}
+
+static int ls2x_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	int ret;
+	struct ls2x_rtc_regs regs;
+	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
+
+	ret = regmap_read(priv->regmap, TOY_READ1_REG, &regs.reg1);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(priv->regmap, TOY_READ0_REG, &regs.reg0);
+	if (ret < 0)
+		return ret;
+
+	ls2x_rtc_regs_to_time(&regs, tm);
+	return 0;
+}
+
+static int ls2x_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	int ret;
+	struct ls2x_rtc_regs regs;
+	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
+
+	ls2x_rtc_time_to_regs(tm, &regs);
+
+	ret = regmap_write(priv->regmap, TOY_WRITE0_REG, regs.reg0);
+	if (ret < 0)
+		return ret;
+
+	return regmap_write(priv->regmap, TOY_WRITE1_REG, regs.reg1);
+}
+
+static int ls2x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	int ret;
+	struct ls2x_rtc_regs regs;
+	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
+
+	ret = regmap_read(priv->regmap, TOY_MATCH0_REG, &regs.reg0);
+	if (ret < 0)
+		return ret;
+
+	ls2x_rtc_alarm_regs_to_time(&regs, &alrm->time);
+	alrm->enabled = !!(readl(priv->acpi_base + PM1_EN_REG) & RTC_EN);
+
+	return 0;
+}
+
+static int ls2x_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	u32 val;
+	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
+
+	spin_lock(&priv->rtc_reglock);
+	val = readl(priv->acpi_base + PM1_EN_REG);
+
+	/* Enalbe RTC alarm */
+	writel((enabled ? val | RTC_EN : val & ~RTC_EN),
+	       priv->acpi_base + PM1_EN_REG);
+	spin_unlock(&priv->rtc_reglock);
+
+	return 0;
+}
+
+static int ls2x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	int ret;
+	struct ls2x_rtc_regs regs;
+	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
+
+	ls2x_rtc_time_to_alarm_regs(&alrm->time, &regs);
+
+	ret = regmap_write(priv->regmap, TOY_MATCH0_REG, regs.reg0);
+	if (ret < 0)
+		return ret;
+
+	return ls2x_rtc_alarm_irq_enable(dev, alrm->enabled);
+}
+
+static const struct rtc_class_ops ls2x_rtc_ops = {
+	.read_time = ls2x_rtc_read_time,
+	.set_time = ls2x_rtc_set_time,
+	.read_alarm = ls2x_rtc_read_alarm,
+	.set_alarm = ls2x_rtc_set_alarm,
+	.alarm_irq_enable = ls2x_rtc_alarm_irq_enable,
+};
+
+static int ls2x_enable_rtc(struct ls2x_rtc_priv *priv)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read(priv->regmap, RTC_CTRL_REG, &val);
+	if (ret < 0)
+		return ret;
+
+	return regmap_write(priv->regmap, RTC_CTRL_REG,
+			    val | TOY_ENABLE | OSC_ENABLE);
+}
+
+static int ls2x_rtc_probe(struct platform_device *pdev)
+{
+	int ret;
+	void __iomem *regs;
+	struct ls2x_rtc_priv *priv;
+	struct device *dev = &pdev->dev;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->irq = platform_get_irq(pdev, 0);
+	if (priv->irq < 0)
+		return dev_err_probe(dev, priv->irq, "platform_get_irq failed\n");
+
+	platform_set_drvdata(pdev, priv);
+
+	regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(regs))
+		return dev_err_probe(dev, PTR_ERR(regs),
+				     "devm_platform_ioremap_resource failed\n");
+
+	priv->regmap = devm_regmap_init_mmio(dev, regs,
+					     &ls2x_rtc_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return dev_err_probe(dev, PTR_ERR(priv->regmap),
+				     "devm_regmap_init_mmio failed\n");
+
+	priv->rtcdev = devm_rtc_allocate_device(dev);
+	if (IS_ERR(priv->rtcdev))
+		return dev_err_probe(dev, PTR_ERR(priv->rtcdev),
+				     "devm_rtc_allocate_device failed\n");
+
+	/* Due to hardware erratum, all years multiple of 4 are considered
+	 * leap year, so only years 2000 through 2099 are usable.
+	 *
+	 * Previous out-of-tree versions of this driver wrote tm_year directly
+	 * into the year register, so epoch 2000 must be used to preserve
+	 * semantics on shipped systems.
+	 */
+	priv->rtcdev->range_min = RTC_TIMESTAMP_BEGIN_2000;
+	priv->rtcdev->range_max = RTC_TIMESTAMP_END_2099;
+	priv->rtcdev->ops = &ls2x_rtc_ops;
+	priv->acpi_base = regs - PM_RTC_OFFSET;
+	spin_lock_init(&priv->rtc_reglock);
+	clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, priv->rtcdev->features);
+
+#ifdef CONFIG_ACPI
+	if (!acpi_disabled)
+		acpi_install_fixed_event_handler(ACPI_EVENT_RTC,
+						 ls2x_acpi_fix_handler, priv);
+#endif
+
+	ret = ls2x_enable_rtc(priv);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "ls2x_enable_rtc failed\n");
+
+	ret = devm_request_threaded_irq(dev, priv->irq, NULL, ls2x_rtc_isr,
+					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+					"ls2x-alarm", priv);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Unable to request irq %d\n",
+				     priv->irq);
+
+	if (!device_can_wakeup(&pdev->dev))
+		device_init_wakeup(dev, 1);
+
+	return devm_rtc_register_device(priv->rtcdev);
+}
+
+static const struct of_device_id ls2x_rtc_of_match[] = {
+	{ .compatible = "loongson,ls2x-rtc" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ls2x_rtc_of_match);
+
+static const struct acpi_device_id ls2x_rtc_acpi_match[] = {
+	{ "LOON0001" }, /* Loongson LS7A */
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, ls2x_rtc_acpi_match);
+
+static struct platform_driver ls2x_rtc_driver = {
+	.probe		= ls2x_rtc_probe,
+	.driver		= {
+		.name	= "ls2x-rtc",
+		.of_match_table = ls2x_rtc_of_match,
+		.acpi_match_table = ls2x_rtc_acpi_match,
+	},
+};
+
+module_platform_driver(ls2x_rtc_driver);
+
+MODULE_DESCRIPTION("Loongson LS2X RTC driver");
+MODULE_AUTHOR("WANG Xuerui <git@xen0n.name>");
+MODULE_AUTHOR("Huacai Chen <chenhuacai@kernel.org>");
+MODULE_AUTHOR("Binbin Zhou <zhoubinbin@loongson.cn>");
+MODULE_LICENSE("GPL");
-- 
2.31.1

Re: [PATCH V2 2/7] rtc: Add support for the Loongson-2K/LS7A RTC

[Jiaxun Yang]

在2023年1月9日一月 上午1:35，Binbin Zhou写道：
> This RTC module is integrated into the Loongson-2K SoC and the LS7A
> bridge chip. This version is almost entirely rewritten to make use of
> current kernel API, and it supports both ACPI and DT.
>
> This driver is shared by MIPS-based Loongson-3A4000 system (use FDT) and
> LoongArch-based Loongson-3A5000 system (use ACPI).
>
> Signed-off-by: Huacai Chen <chenhuacai@kernel.org>
> Signed-off-by: WANG Xuerui <git@xen0n.name>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>

Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com> # MIPS Loongson64 + DT

> ---
>  drivers/rtc/Kconfig    |  11 ++
>  drivers/rtc/Makefile   |   1 +
>  drivers/rtc/rtc-ls2x.c | 379 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 391 insertions(+)
>  create mode 100644 drivers/rtc/rtc-ls2x.c
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index bb63edb507da..f8586aa00fce 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1735,6 +1735,17 @@ config RTC_DRV_LPC32XX
>  	  This driver can also be built as a module. If so, the module
>  	  will be called rtc-lpc32xx.
> 
> +config RTC_DRV_LS2X
> +	tristate "Loongson LS2X RTC"
> +	depends on MACH_LOONGSON64 || COMPILE_TEST
> +	select REGMAP_MMIO
> +	help
> +	  If you say yes here you get support for the RTC on the Loongson-2K
> +	  SoC and LS7A bridge, which first appeared on the Loongson-2H.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called rtc-ls2x.
> +
>  config RTC_DRV_PM8XXX
>  	tristate "Qualcomm PMIC8XXX RTC"
>  	depends on MFD_PM8XXX || MFD_SPMI_PMIC || COMPILE_TEST
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index aab22bc63432..d5a467e9eec8 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_RTC_DRV_LOONGSON1)	+= rtc-ls1x.o
>  obj-$(CONFIG_RTC_DRV_LP8788)	+= rtc-lp8788.o
>  obj-$(CONFIG_RTC_DRV_LPC24XX)	+= rtc-lpc24xx.o
>  obj-$(CONFIG_RTC_DRV_LPC32XX)	+= rtc-lpc32xx.o
> +obj-$(CONFIG_RTC_DRV_LS2X)	+= rtc-ls2x.o
>  obj-$(CONFIG_RTC_DRV_M41T80)	+= rtc-m41t80.o
>  obj-$(CONFIG_RTC_DRV_M41T93)	+= rtc-m41t93.o
>  obj-$(CONFIG_RTC_DRV_M41T94)	+= rtc-m41t94.o
> diff --git a/drivers/rtc/rtc-ls2x.c b/drivers/rtc/rtc-ls2x.c
> new file mode 100644
> index 000000000000..06ef249a9485
> --- /dev/null
> +++ b/drivers/rtc/rtc-ls2x.c
> @@ -0,0 +1,379 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Loongson-2K/LS7A RTC driver
> + *
> + * Based on the original out-of-tree Loongson-2H RTC driver for Linux 2.6.32,
> + * by Shaozong Liu <liushaozong@loongson.cn>.
> + *
> + * Maintained out-of-tree by Huacai Chen <chenhuacai@kernel.org>.
> + * Rewritten for mainline by WANG Xuerui <git@xen0n.name>.
> + *                           Binbin Zhou <zhoubinbin@loongson.cn>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/rtc.h>
> +#include <linux/acpi.h>
> +
> +/* Time Of Year(TOY) counters registers */
> +#define TOY_TRIM_REG		0x20 /* Must be initialized to 0 */
> +#define TOY_WRITE0_REG		0x24 /* TOY low 32-bit value (write-only) */
> +#define TOY_WRITE1_REG		0x28 /* TOY high 32-bit value (write-only) */
> +#define TOY_READ0_REG		0x2c /* TOY low 32-bit value (read-only) */
> +#define TOY_READ1_REG		0x30 /* TOY high 32-bit value (read-only) */
> +#define TOY_MATCH0_REG		0x34 /* TOY timing interrupt 0 */
> +#define TOY_MATCH1_REG		0x38 /* TOY timing interrupt 1 */
> +#define TOY_MATCH2_REG		0x3c /* TOY timing interrupt 2 */
> +
> +/* RTC counters registers */
> +#define RTC_CTRL_REG		0x40 /* TOY and RTC control register */
> +#define RTC_TRIM_REG		0x60 /* Must be initialized to 0 */
> +#define RTC_WRITE0_REG		0x64 /* RTC counters value (write-only) */
> +#define RTC_READ0_REG		0x68 /* RTC counters value (read-only) */
> +#define RTC_MATCH0_REG		0x6c /* RTC timing interrupt 0 */
> +#define RTC_MATCH1_REG		0x70 /* RTC timing interrupt 1 */
> +#define RTC_MATCH2_REG		0x74 /* RTC timing interrupt 2 */
> +
> +/* TOY_WRITE0_REG bitmask */
> +#define TOY_MON			GENMASK(31, 26)
> +#define TOY_DAY			GENMASK(25, 21)
> +#define TOY_HOUR		GENMASK(20, 16)
> +#define TOY_MIN			GENMASK(15, 10)
> +#define TOY_SEC			GENMASK(9, 4)
> +#define TOY_MSEC		GENMASK(3, 0)
> +
> +/* TOY_MATCH0/1/2_REG bitmask */
> +#define TOY_MATCH_YEAR		GENMASK(31, 26)
> +#define TOY_MATCH_MON		GENMASK(25, 22)
> +#define TOY_MATCH_DAY		GENMASK(21, 17)
> +#define TOY_MATCH_HOUR		GENMASK(16, 12)
> +#define TOY_MATCH_MIN		GENMASK(11, 6)
> +#define TOY_MATCH_SEC		GENMASK(5, 0)
> +
> +/* RTC_CTRL_REG bitmask */
> +#define RTC_ENABLE		BIT(13) /* 1: RTC counters enable */
> +#define TOY_ENABLE		BIT(11) /* 1: TOY counters enable */
> +#define OSC_ENABLE		BIT(8) /* 1: 32.768k crystal enable */
> +
> +/* Offset of PM domain from RTC domain */
> +#define PM_RTC_OFFSET		0x100
> +
> +/* PM domain registers */
> +#define PM1_STS_REG		0x0c /* Power management 1 status register */
> +#define RTC_STS			BIT(10) /* RTC status */
> +#define PM1_EN_REG		0x10 /* Power management 1 enable register */
> +#define RTC_EN			BIT(10) /* RTC event enable */
> +
> +struct ls2x_rtc_priv {
> +	spinlock_t		rtc_reglock;
> +	int			irq;
> +	struct rtc_device	*rtcdev;
> +	struct regmap		*regmap;
> +	void __iomem		*acpi_base;
> +};
> +
> +static const struct regmap_config ls2x_rtc_regmap_config = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +};
> +
> +struct ls2x_rtc_regs {
> +	u32 reg0;
> +	u32 reg1;
> +};
> +
> +/* IRQ Handlers */
> +static irqreturn_t ls2x_rtc_isr(int irq, void *id)
> +{
> +	struct ls2x_rtc_priv *priv = (struct ls2x_rtc_priv *)id;
> +
> +	rtc_update_irq(priv->rtcdev, 1, RTC_AF | RTC_IRQF);
> +	return IRQ_HANDLED;
> +}
> +
> +#ifdef CONFIG_ACPI
> +static u32 ls2x_acpi_fix_handler(void *id)
> +{
> +	int ret;
> +	struct ls2x_rtc_priv *priv = (struct ls2x_rtc_priv *)id;
> +
> +	spin_lock(&priv->rtc_reglock);
> +
> +	/* Disable acpi rtc enabled */
> +	ret = readl(priv->acpi_base + PM1_EN_REG) & ~RTC_EN;
> +	writel(ret, priv->acpi_base + PM1_EN_REG);
> +
> +	/* Clear acpi rtc interrupt Status */
> +	writel(RTC_STS, priv->acpi_base + PM1_STS_REG);
> +
> +	spin_unlock(&priv->rtc_reglock);
> +
> +	/*
> +	 * The TOY_MATCH0_REG should be cleared 0 here,
> +	 * otherwise the interrupt cannot be cleared.
> +	 * Because the match condition is still satisfied
> +	 */
> +	ret = regmap_write(priv->regmap, TOY_MATCH0_REG, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	rtc_update_irq(priv->rtcdev, 1, RTC_AF | RTC_IRQF);
> +	return 0;
> +}
> +#endif
> +
> +static inline void ls2x_rtc_regs_to_time(struct ls2x_rtc_regs *regs,
> +					 struct rtc_time *tm)
> +{
> +	tm->tm_year = regs->reg1;
> +	tm->tm_sec = FIELD_GET(TOY_SEC, regs->reg0);
> +	tm->tm_min = FIELD_GET(TOY_MIN, regs->reg0);
> +	tm->tm_hour = FIELD_GET(TOY_HOUR, regs->reg0);
> +	tm->tm_mday = FIELD_GET(TOY_DAY, regs->reg0);
> +	tm->tm_mon = FIELD_GET(TOY_MON, regs->reg0) - 1;
> +}
> +
> +static inline void ls2x_rtc_time_to_regs(struct rtc_time *tm,
> +					 struct ls2x_rtc_regs *regs)
> +{
> +	regs->reg0 = FIELD_PREP(TOY_SEC, tm->tm_sec);
> +	regs->reg0 |= FIELD_PREP(TOY_MIN, tm->tm_min);
> +	regs->reg0 |= FIELD_PREP(TOY_HOUR, tm->tm_hour);
> +	regs->reg0 |= FIELD_PREP(TOY_DAY, tm->tm_mday);
> +	regs->reg0 |= FIELD_PREP(TOY_MON, tm->tm_mon + 1);
> +	regs->reg1 = tm->tm_year;
> +}
> +
> +static inline void ls2x_rtc_alarm_regs_to_time(struct ls2x_rtc_regs *regs,
> +					 struct rtc_time *tm)
> +{
> +	tm->tm_sec = FIELD_GET(TOY_MATCH_SEC, regs->reg0);
> +	tm->tm_min = FIELD_GET(TOY_MATCH_MIN, regs->reg0);
> +	tm->tm_hour = FIELD_GET(TOY_MATCH_HOUR, regs->reg0);
> +	tm->tm_mday = FIELD_GET(TOY_MATCH_DAY, regs->reg0);
> +	tm->tm_mon = FIELD_GET(TOY_MATCH_MON, regs->reg0) - 1;
> +	/*
> +	 * The rtc SYS_TOYMATCH0/YEAR bit field is only 6 bits, so it means 63
> +	 * years at most. Therefore, The RTC alarm years can be set from 1900
> +	 * to 1963. This causes the initialization of alarm fail during call
> +	 * __rtc_read_alarm.
> +	 * We add 64 years offset to ls2x_rtc_read_alarm. After adding the
> +	 * offset, the RTC alarm clock can be set from 1964 to 2027.
> +	 */
> +	tm->tm_year = FIELD_GET(TOY_MATCH_YEAR, regs->reg0) + 64;
> +}
> +
> +static inline void ls2x_rtc_time_to_alarm_regs(struct rtc_time *tm,
> +					 struct ls2x_rtc_regs *regs)
> +{
> +	regs->reg0 = FIELD_PREP(TOY_MATCH_SEC, tm->tm_sec);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_MIN, tm->tm_min);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_HOUR, tm->tm_hour);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_DAY, tm->tm_mday);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_MON, tm->tm_mon + 1);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_YEAR, tm->tm_year);
> +}
> +
> +static int ls2x_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	int ret;
> +	struct ls2x_rtc_regs regs;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	ret = regmap_read(priv->regmap, TOY_READ1_REG, &regs.reg1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_read(priv->regmap, TOY_READ0_REG, &regs.reg0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ls2x_rtc_regs_to_time(&regs, tm);
> +	return 0;
> +}
> +
> +static int ls2x_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	int ret;
> +	struct ls2x_rtc_regs regs;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	ls2x_rtc_time_to_regs(tm, &regs);
> +
> +	ret = regmap_write(priv->regmap, TOY_WRITE0_REG, regs.reg0);
> +	if (ret < 0)
> +		return ret;
> +
> +	return regmap_write(priv->regmap, TOY_WRITE1_REG, regs.reg1);
> +}
> +
> +static int ls2x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	int ret;
> +	struct ls2x_rtc_regs regs;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	ret = regmap_read(priv->regmap, TOY_MATCH0_REG, &regs.reg0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ls2x_rtc_alarm_regs_to_time(&regs, &alrm->time);
> +	alrm->enabled = !!(readl(priv->acpi_base + PM1_EN_REG) & RTC_EN);
> +
> +	return 0;
> +}
> +
> +static int ls2x_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> +{
> +	u32 val;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	spin_lock(&priv->rtc_reglock);
> +	val = readl(priv->acpi_base + PM1_EN_REG);
> +
> +	/* Enalbe RTC alarm */
> +	writel((enabled ? val | RTC_EN : val & ~RTC_EN),
> +	       priv->acpi_base + PM1_EN_REG);
> +	spin_unlock(&priv->rtc_reglock);
> +
> +	return 0;
> +}
> +
> +static int ls2x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	int ret;
> +	struct ls2x_rtc_regs regs;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	ls2x_rtc_time_to_alarm_regs(&alrm->time, &regs);
> +
> +	ret = regmap_write(priv->regmap, TOY_MATCH0_REG, regs.reg0);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ls2x_rtc_alarm_irq_enable(dev, alrm->enabled);
> +}
> +
> +static const struct rtc_class_ops ls2x_rtc_ops = {
> +	.read_time = ls2x_rtc_read_time,
> +	.set_time = ls2x_rtc_set_time,
> +	.read_alarm = ls2x_rtc_read_alarm,
> +	.set_alarm = ls2x_rtc_set_alarm,
> +	.alarm_irq_enable = ls2x_rtc_alarm_irq_enable,
> +};
> +
> +static int ls2x_enable_rtc(struct ls2x_rtc_priv *priv)
> +{
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(priv->regmap, RTC_CTRL_REG, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return regmap_write(priv->regmap, RTC_CTRL_REG,
> +			    val | TOY_ENABLE | OSC_ENABLE);
> +}
> +
> +static int ls2x_rtc_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	void __iomem *regs;
> +	struct ls2x_rtc_priv *priv;
> +	struct device *dev = &pdev->dev;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->irq = platform_get_irq(pdev, 0);
> +	if (priv->irq < 0)
> +		return dev_err_probe(dev, priv->irq, "platform_get_irq failed\n");
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(regs))
> +		return dev_err_probe(dev, PTR_ERR(regs),
> +				     "devm_platform_ioremap_resource failed\n");
> +
> +	priv->regmap = devm_regmap_init_mmio(dev, regs,
> +					     &ls2x_rtc_regmap_config);
> +	if (IS_ERR(priv->regmap))
> +		return dev_err_probe(dev, PTR_ERR(priv->regmap),
> +				     "devm_regmap_init_mmio failed\n");
> +
> +	priv->rtcdev = devm_rtc_allocate_device(dev);
> +	if (IS_ERR(priv->rtcdev))
> +		return dev_err_probe(dev, PTR_ERR(priv->rtcdev),
> +				     "devm_rtc_allocate_device failed\n");
> +
> +	/* Due to hardware erratum, all years multiple of 4 are considered
> +	 * leap year, so only years 2000 through 2099 are usable.
> +	 *
> +	 * Previous out-of-tree versions of this driver wrote tm_year directly
> +	 * into the year register, so epoch 2000 must be used to preserve
> +	 * semantics on shipped systems.
> +	 */
> +	priv->rtcdev->range_min = RTC_TIMESTAMP_BEGIN_2000;
> +	priv->rtcdev->range_max = RTC_TIMESTAMP_END_2099;
> +	priv->rtcdev->ops = &ls2x_rtc_ops;
> +	priv->acpi_base = regs - PM_RTC_OFFSET;
> +	spin_lock_init(&priv->rtc_reglock);
> +	clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, priv->rtcdev->features);
> +
> +#ifdef CONFIG_ACPI
> +	if (!acpi_disabled)
> +		acpi_install_fixed_event_handler(ACPI_EVENT_RTC,
> +						 ls2x_acpi_fix_handler, priv);
> +#endif
> +
> +	ret = ls2x_enable_rtc(priv);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "ls2x_enable_rtc failed\n");
> +
> +	ret = devm_request_threaded_irq(dev, priv->irq, NULL, ls2x_rtc_isr,
> +					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +					"ls2x-alarm", priv);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Unable to request irq %d\n",
> +				     priv->irq);
> +
> +	if (!device_can_wakeup(&pdev->dev))
> +		device_init_wakeup(dev, 1);
> +
> +	return devm_rtc_register_device(priv->rtcdev);
> +}
> +
> +static const struct of_device_id ls2x_rtc_of_match[] = {
> +	{ .compatible = "loongson,ls2x-rtc" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ls2x_rtc_of_match);
> +
> +static const struct acpi_device_id ls2x_rtc_acpi_match[] = {
> +	{ "LOON0001" }, /* Loongson LS7A */
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, ls2x_rtc_acpi_match);
> +
> +static struct platform_driver ls2x_rtc_driver = {
> +	.probe		= ls2x_rtc_probe,
> +	.driver		= {
> +		.name	= "ls2x-rtc",
> +		.of_match_table = ls2x_rtc_of_match,
> +		.acpi_match_table = ls2x_rtc_acpi_match,
> +	},
> +};
> +
> +module_platform_driver(ls2x_rtc_driver);
> +
> +MODULE_DESCRIPTION("Loongson LS2X RTC driver");
> +MODULE_AUTHOR("WANG Xuerui <git@xen0n.name>");
> +MODULE_AUTHOR("Huacai Chen <chenhuacai@kernel.org>");
> +MODULE_AUTHOR("Binbin Zhou <zhoubinbin@loongson.cn>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.31.1

-- 
- Jiaxun

Re: [PATCH V2 2/7] rtc: Add support for the Loongson-2K/LS7A RTC

[Alexandre Belloni]

Hello,


On 09/01/2023 09:35:12+0800, Binbin Zhou wrote:
> This RTC module is integrated into the Loongson-2K SoC and the LS7A
> bridge chip. This version is almost entirely rewritten to make use of
> current kernel API, and it supports both ACPI and DT.
> 
> This driver is shared by MIPS-based Loongson-3A4000 system (use FDT) and
> LoongArch-based Loongson-3A5000 system (use ACPI).
> 

checkpatch.pl --strict complains, please fix the warnings and checks


> +#ifdef CONFIG_ACPI
> +static u32 ls2x_acpi_fix_handler(void *id)
> +{
> +	int ret;
> +	struct ls2x_rtc_priv *priv = (struct ls2x_rtc_priv *)id;
> +
> +	spin_lock(&priv->rtc_reglock);
> +
> +	/* Disable acpi rtc enabled */
> +	ret = readl(priv->acpi_base + PM1_EN_REG) & ~RTC_EN;
> +	writel(ret, priv->acpi_base + PM1_EN_REG);
> +
> +	/* Clear acpi rtc interrupt Status */
> +	writel(RTC_STS, priv->acpi_base + PM1_STS_REG);
> +
> +	spin_unlock(&priv->rtc_reglock);
> +
> +	/*
> +	 * The TOY_MATCH0_REG should be cleared 0 here,
> +	 * otherwise the interrupt cannot be cleared.
> +	 * Because the match condition is still satisfied
> +	 */
> +	ret = regmap_write(priv->regmap, TOY_MATCH0_REG, 0);
> +	if (ret < 0)
> +		return ret;

How is this an ACPI related issue? I guess the same would happen on
!ACPI.

> +
> +	rtc_update_irq(priv->rtcdev, 1, RTC_AF | RTC_IRQF);

This is not useful, at that time, userspace has had no chance to open
the RTC device file as it is not created yet.

> +	return 0;
> +}
> +#endif
> +
> +static inline void ls2x_rtc_regs_to_time(struct ls2x_rtc_regs *regs,

Those static inline functions seem to be used only once, you should just
put the code in the proper location.

> +					 struct rtc_time *tm)
> +{
> +	tm->tm_year = regs->reg1;
> +	tm->tm_sec = FIELD_GET(TOY_SEC, regs->reg0);
> +	tm->tm_min = FIELD_GET(TOY_MIN, regs->reg0);
> +	tm->tm_hour = FIELD_GET(TOY_HOUR, regs->reg0);
> +	tm->tm_mday = FIELD_GET(TOY_DAY, regs->reg0);
> +	tm->tm_mon = FIELD_GET(TOY_MON, regs->reg0) - 1;
> +}
> +
> +static inline void ls2x_rtc_time_to_regs(struct rtc_time *tm,
> +					 struct ls2x_rtc_regs *regs)
> +{
> +	regs->reg0 = FIELD_PREP(TOY_SEC, tm->tm_sec);
> +	regs->reg0 |= FIELD_PREP(TOY_MIN, tm->tm_min);
> +	regs->reg0 |= FIELD_PREP(TOY_HOUR, tm->tm_hour);
> +	regs->reg0 |= FIELD_PREP(TOY_DAY, tm->tm_mday);
> +	regs->reg0 |= FIELD_PREP(TOY_MON, tm->tm_mon + 1);
> +	regs->reg1 = tm->tm_year;
> +}
> +
> +static inline void ls2x_rtc_alarm_regs_to_time(struct ls2x_rtc_regs *regs,
> +					 struct rtc_time *tm)
> +{
> +	tm->tm_sec = FIELD_GET(TOY_MATCH_SEC, regs->reg0);
> +	tm->tm_min = FIELD_GET(TOY_MATCH_MIN, regs->reg0);
> +	tm->tm_hour = FIELD_GET(TOY_MATCH_HOUR, regs->reg0);
> +	tm->tm_mday = FIELD_GET(TOY_MATCH_DAY, regs->reg0);
> +	tm->tm_mon = FIELD_GET(TOY_MATCH_MON, regs->reg0) - 1;
> +	/*
> +	 * The rtc SYS_TOYMATCH0/YEAR bit field is only 6 bits, so it means 63
> +	 * years at most. Therefore, The RTC alarm years can be set from 1900
> +	 * to 1963. This causes the initialization of alarm fail during call
> +	 * __rtc_read_alarm.
> +	 * We add 64 years offset to ls2x_rtc_read_alarm. After adding the
> +	 * offset, the RTC alarm clock can be set from 1964 to 2027.
> +	 */
> +	tm->tm_year = FIELD_GET(TOY_MATCH_YEAR, regs->reg0) + 64;

This is not symmetric with ls2x_rtc_time_to_alarm_regs, how can it work?

> +}
> +
> +static inline void ls2x_rtc_time_to_alarm_regs(struct rtc_time *tm,
> +					 struct ls2x_rtc_regs *regs)
> +{
> +	regs->reg0 = FIELD_PREP(TOY_MATCH_SEC, tm->tm_sec);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_MIN, tm->tm_min);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_HOUR, tm->tm_hour);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_DAY, tm->tm_mday);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_MON, tm->tm_mon + 1);
> +	regs->reg0 |= FIELD_PREP(TOY_MATCH_YEAR, tm->tm_year);
> +}
> +
> +static int ls2x_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	int ret;
> +	struct ls2x_rtc_regs regs;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	ret = regmap_read(priv->regmap, TOY_READ1_REG, &regs.reg1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_read(priv->regmap, TOY_READ0_REG, &regs.reg0);
> +	if (ret < 0)
> +		return ret;
> +

I never got a reply to my question:

"
I'm actually wondering why you read first TOY_READ1_REG then
TOY_READ0_REG. ls1x does the opposite and the ls1c datasheet I found
doesn't mention any latching happening. So unless latching is done on
TOY_READ1_REG, you could use regmap_bulk_read and simply avoid struct
ls2x_rtc_regs.
If there is no latching, you may need to read TOY_READ0_REG at least
twice. Because TOY_READ1_REG only contains the year, it is an issue only
on 31 of December and it will not be easy to reproduce.
"


> +	ls2x_rtc_regs_to_time(&regs, tm);
> +	return 0;
> +}
> +
> +static int ls2x_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	int ret;
> +	struct ls2x_rtc_regs regs;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	ls2x_rtc_time_to_regs(tm, &regs);
> +
> +	ret = regmap_write(priv->regmap, TOY_WRITE0_REG, regs.reg0);
> +	if (ret < 0)
> +		return ret;
> +
> +	return regmap_write(priv->regmap, TOY_WRITE1_REG, regs.reg1);
> +}
> +
> +static int ls2x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	int ret;
> +	struct ls2x_rtc_regs regs;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	ret = regmap_read(priv->regmap, TOY_MATCH0_REG, &regs.reg0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ls2x_rtc_alarm_regs_to_time(&regs, &alrm->time);
> +	alrm->enabled = !!(readl(priv->acpi_base + PM1_EN_REG) & RTC_EN);
> +
> +	return 0;
> +}
> +
> +static int ls2x_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> +{
> +	u32 val;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	spin_lock(&priv->rtc_reglock);
> +	val = readl(priv->acpi_base + PM1_EN_REG);
> +
> +	/* Enalbe RTC alarm */
Typo

> +	writel((enabled ? val | RTC_EN : val & ~RTC_EN),
> +	       priv->acpi_base + PM1_EN_REG);
> +	spin_unlock(&priv->rtc_reglock);
> +
> +	return 0;
> +}
> +
> +static int ls2x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	int ret;
> +	struct ls2x_rtc_regs regs;
> +	struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	ls2x_rtc_time_to_alarm_regs(&alrm->time, &regs);
> +
> +	ret = regmap_write(priv->regmap, TOY_MATCH0_REG, regs.reg0);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ls2x_rtc_alarm_irq_enable(dev, alrm->enabled);
> +}
> +
> +static const struct rtc_class_ops ls2x_rtc_ops = {
> +	.read_time = ls2x_rtc_read_time,
> +	.set_time = ls2x_rtc_set_time,
> +	.read_alarm = ls2x_rtc_read_alarm,
> +	.set_alarm = ls2x_rtc_set_alarm,
> +	.alarm_irq_enable = ls2x_rtc_alarm_irq_enable,
> +};
> +
> +static int ls2x_enable_rtc(struct ls2x_rtc_priv *priv)
> +{
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read(priv->regmap, RTC_CTRL_REG, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return regmap_write(priv->regmap, RTC_CTRL_REG,
> +			    val | TOY_ENABLE | OSC_ENABLE);
> +}
> +
> +static int ls2x_rtc_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	void __iomem *regs;
> +	struct ls2x_rtc_priv *priv;
> +	struct device *dev = &pdev->dev;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->irq = platform_get_irq(pdev, 0);
> +	if (priv->irq < 0)
> +		return dev_err_probe(dev, priv->irq, "platform_get_irq failed\n");
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(regs))
> +		return dev_err_probe(dev, PTR_ERR(regs),
> +				     "devm_platform_ioremap_resource failed\n");
> +
> +	priv->regmap = devm_regmap_init_mmio(dev, regs,
> +					     &ls2x_rtc_regmap_config);
> +	if (IS_ERR(priv->regmap))
> +		return dev_err_probe(dev, PTR_ERR(priv->regmap),
> +				     "devm_regmap_init_mmio failed\n");
> +
> +	priv->rtcdev = devm_rtc_allocate_device(dev);
> +	if (IS_ERR(priv->rtcdev))
> +		return dev_err_probe(dev, PTR_ERR(priv->rtcdev),
> +				     "devm_rtc_allocate_device failed\n");
> +
> +	/* Due to hardware erratum, all years multiple of 4 are considered
> +	 * leap year, so only years 2000 through 2099 are usable.
> +	 *
> +	 * Previous out-of-tree versions of this driver wrote tm_year directly
> +	 * into the year register, so epoch 2000 must be used to preserve
> +	 * semantics on shipped systems.
> +	 */
> +	priv->rtcdev->range_min = RTC_TIMESTAMP_BEGIN_2000;
> +	priv->rtcdev->range_max = RTC_TIMESTAMP_END_2099;
> +	priv->rtcdev->ops = &ls2x_rtc_ops;
> +	priv->acpi_base = regs - PM_RTC_OFFSET;
> +	spin_lock_init(&priv->rtc_reglock);
> +	clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, priv->rtcdev->features);

Why?

> +
> +#ifdef CONFIG_ACPI
> +	if (!acpi_disabled)
> +		acpi_install_fixed_event_handler(ACPI_EVENT_RTC,
> +						 ls2x_acpi_fix_handler, priv);
> +#endif
> +
> +	ret = ls2x_enable_rtc(priv);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "ls2x_enable_rtc failed\n");

This should not be done in probe but on the first set_time. This then
allows you to know whether the time has been set and is valid in
read_time. Please add the check.

> +
> +	ret = devm_request_threaded_irq(dev, priv->irq, NULL, ls2x_rtc_isr,
> +					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +					"ls2x-alarm", priv);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Unable to request irq %d\n",
> +				     priv->irq);
> +
> +	if (!device_can_wakeup(&pdev->dev))
> +		device_init_wakeup(dev, 1);
> +
> +	return devm_rtc_register_device(priv->rtcdev);
> +}
> +
> +static const struct of_device_id ls2x_rtc_of_match[] = {
> +	{ .compatible = "loongson,ls2x-rtc" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ls2x_rtc_of_match);
> +
> +static const struct acpi_device_id ls2x_rtc_acpi_match[] = {
> +	{ "LOON0001" }, /* Loongson LS7A */
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, ls2x_rtc_acpi_match);
> +
> +static struct platform_driver ls2x_rtc_driver = {
> +	.probe		= ls2x_rtc_probe,
> +	.driver		= {
> +		.name	= "ls2x-rtc",
> +		.of_match_table = ls2x_rtc_of_match,
> +		.acpi_match_table = ls2x_rtc_acpi_match,
> +	},
> +};
> +
> +module_platform_driver(ls2x_rtc_driver);
> +
> +MODULE_DESCRIPTION("Loongson LS2X RTC driver");
> +MODULE_AUTHOR("WANG Xuerui <git@xen0n.name>");
> +MODULE_AUTHOR("Huacai Chen <chenhuacai@kernel.org>");
> +MODULE_AUTHOR("Binbin Zhou <zhoubinbin@loongson.cn>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.31.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH V2 2/7] rtc: Add support for the Loongson-2K/LS7A RTC

[Binbin Zhou]

Hi Alexandre:

Sorry for the late reply.

On Tue, Jan 24, 2023 at 7:37 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hello,
>
>
> On 09/01/2023 09:35:12+0800, Binbin Zhou wrote:
> > This RTC module is integrated into the Loongson-2K SoC and the LS7A
> > bridge chip. This version is almost entirely rewritten to make use of
> > current kernel API, and it supports both ACPI and DT.
> >
> > This driver is shared by MIPS-based Loongson-3A4000 system (use FDT) and
> > LoongArch-based Loongson-3A5000 system (use ACPI).
> >
>
> checkpatch.pl --strict complains, please fix the warnings and checks

Got.
>
>
> > +#ifdef CONFIG_ACPI
> > +static u32 ls2x_acpi_fix_handler(void *id)
> > +{
> > +     int ret;
> > +     struct ls2x_rtc_priv *priv = (struct ls2x_rtc_priv *)id;
> > +
> > +     spin_lock(&priv->rtc_reglock);
> > +
> > +     /* Disable acpi rtc enabled */
> > +     ret = readl(priv->acpi_base + PM1_EN_REG) & ~RTC_EN;
> > +     writel(ret, priv->acpi_base + PM1_EN_REG);
> > +
> > +     /* Clear acpi rtc interrupt Status */
> > +     writel(RTC_STS, priv->acpi_base + PM1_STS_REG);
> > +
> > +     spin_unlock(&priv->rtc_reglock);
> > +
> > +     /*
> > +      * The TOY_MATCH0_REG should be cleared 0 here,
> > +      * otherwise the interrupt cannot be cleared.
> > +      * Because the match condition is still satisfied
> > +      */
> > +     ret = regmap_write(priv->regmap, TOY_MATCH0_REG, 0);
> > +     if (ret < 0)
> > +             return ret;
>
> How is this an ACPI related issue? I guess the same would happen on
> !ACPI.

I just assumed that the function would only be called under CONFIG_ACPI.
>
> > +
> > +     rtc_update_irq(priv->rtcdev, 1, RTC_AF | RTC_IRQF);
>
> This is not useful, at that time, userspace has had no chance to open
> the RTC device file as it is not created yet.
>
> > +     return 0;
> > +}
> > +#endif
> > +
> > +static inline void ls2x_rtc_regs_to_time(struct ls2x_rtc_regs *regs,
>
> Those static inline functions seem to be used only once, you should just
> put the code in the proper location.

I will do it in the next version.
>
> > +                                      struct rtc_time *tm)
> > +{
> > +     tm->tm_year = regs->reg1;
> > +     tm->tm_sec = FIELD_GET(TOY_SEC, regs->reg0);
> > +     tm->tm_min = FIELD_GET(TOY_MIN, regs->reg0);
> > +     tm->tm_hour = FIELD_GET(TOY_HOUR, regs->reg0);
> > +     tm->tm_mday = FIELD_GET(TOY_DAY, regs->reg0);
> > +     tm->tm_mon = FIELD_GET(TOY_MON, regs->reg0) - 1;
> > +}
> > +
> > +static inline void ls2x_rtc_time_to_regs(struct rtc_time *tm,
> > +                                      struct ls2x_rtc_regs *regs)
> > +{
> > +     regs->reg0 = FIELD_PREP(TOY_SEC, tm->tm_sec);
> > +     regs->reg0 |= FIELD_PREP(TOY_MIN, tm->tm_min);
> > +     regs->reg0 |= FIELD_PREP(TOY_HOUR, tm->tm_hour);
> > +     regs->reg0 |= FIELD_PREP(TOY_DAY, tm->tm_mday);
> > +     regs->reg0 |= FIELD_PREP(TOY_MON, tm->tm_mon + 1);
> > +     regs->reg1 = tm->tm_year;
> > +}
> > +
> > +static inline void ls2x_rtc_alarm_regs_to_time(struct ls2x_rtc_regs *regs,
> > +                                      struct rtc_time *tm)
> > +{
> > +     tm->tm_sec = FIELD_GET(TOY_MATCH_SEC, regs->reg0);
> > +     tm->tm_min = FIELD_GET(TOY_MATCH_MIN, regs->reg0);
> > +     tm->tm_hour = FIELD_GET(TOY_MATCH_HOUR, regs->reg0);
> > +     tm->tm_mday = FIELD_GET(TOY_MATCH_DAY, regs->reg0);
> > +     tm->tm_mon = FIELD_GET(TOY_MATCH_MON, regs->reg0) - 1;
> > +     /*
> > +      * The rtc SYS_TOYMATCH0/YEAR bit field is only 6 bits, so it means 63
> > +      * years at most. Therefore, The RTC alarm years can be set from 1900
> > +      * to 1963. This causes the initialization of alarm fail during call
> > +      * __rtc_read_alarm.
> > +      * We add 64 years offset to ls2x_rtc_read_alarm. After adding the
> > +      * offset, the RTC alarm clock can be set from 1964 to 2027.
> > +      */
> > +     tm->tm_year = FIELD_GET(TOY_MATCH_YEAR, regs->reg0) + 64;
>
> This is not symmetric with ls2x_rtc_time_to_alarm_regs, how can it work?

This is to avoid an "invalid alarm value" at boot time, which of
course should not be a good solution.
When the alarm value is read at boot time, "year" is not yet set to
the proper value so the year is always set to 1900.

How about just  "tm->tm_year = -1", as the alarm is now only read when booting?

>
> > +}
> > +
> > +static inline void ls2x_rtc_time_to_alarm_regs(struct rtc_time *tm,
> > +                                      struct ls2x_rtc_regs *regs)
> > +{
> > +     regs->reg0 = FIELD_PREP(TOY_MATCH_SEC, tm->tm_sec);
> > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_MIN, tm->tm_min);
> > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_HOUR, tm->tm_hour);
> > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_DAY, tm->tm_mday);
> > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_MON, tm->tm_mon + 1);
> > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_YEAR, tm->tm_year);
> > +}
> > +
> > +static int ls2x_rtc_read_time(struct device *dev, struct rtc_time *tm)
> > +{
> > +     int ret;
> > +     struct ls2x_rtc_regs regs;
> > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > +
> > +     ret = regmap_read(priv->regmap, TOY_READ1_REG, &regs.reg1);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = regmap_read(priv->regmap, TOY_READ0_REG, &regs.reg0);
> > +     if (ret < 0)
> > +             return ret;
> > +
>
> I never got a reply to my question:
>
> "
> I'm actually wondering why you read first TOY_READ1_REG then
> TOY_READ0_REG. ls1x does the opposite and the ls1c datasheet I found
> doesn't mention any latching happening. So unless latching is done on
> TOY_READ1_REG, you could use regmap_bulk_read and simply avoid struct
> ls2x_rtc_regs.
> If there is no latching, you may need to read TOY_READ0_REG at least
> twice. Because TOY_READ1_REG only contains the year, it is an issue only
> on 31 of December and it will not be easy to reproduce.
> "
>

The LS7A and Loongson-2K datasheets also do not mention any latching
happening. Reading TOY_READ1_REG first is probably just a matter of
habit.
I tried using regmap_bulk_xxx() and it also reads and writes time
properly. In the next version I will rewrite this part of the code.

Example:

#define LS2X_NUM_TIME_REGS      2

u32 rtc_data[LS2X_NUM_TIME_REGS];
struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);

ret = regmap_bulk_read(priv->regmap, TOY_READ0_REG, rtc_data,
LS2X_NUM_TIME_REGS);


>
> > +     ls2x_rtc_regs_to_time(&regs, tm);
> > +     return 0;
> > +}
> > +
> > +static int ls2x_rtc_set_time(struct device *dev, struct rtc_time *tm)
> > +{
> > +     int ret;
> > +     struct ls2x_rtc_regs regs;
> > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > +
> > +     ls2x_rtc_time_to_regs(tm, &regs);
> > +
> > +     ret = regmap_write(priv->regmap, TOY_WRITE0_REG, regs.reg0);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     return regmap_write(priv->regmap, TOY_WRITE1_REG, regs.reg1);
> > +}
> > +
> > +static int ls2x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> > +{
> > +     int ret;
> > +     struct ls2x_rtc_regs regs;
> > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > +
> > +     ret = regmap_read(priv->regmap, TOY_MATCH0_REG, &regs.reg0);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ls2x_rtc_alarm_regs_to_time(&regs, &alrm->time);
> > +     alrm->enabled = !!(readl(priv->acpi_base + PM1_EN_REG) & RTC_EN);
> > +
> > +     return 0;
> > +}
> > +
> > +static int ls2x_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> > +{
> > +     u32 val;
> > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > +
> > +     spin_lock(&priv->rtc_reglock);
> > +     val = readl(priv->acpi_base + PM1_EN_REG);
> > +
> > +     /* Enalbe RTC alarm */
> Typo
>
> > +     writel((enabled ? val | RTC_EN : val & ~RTC_EN),
> > +            priv->acpi_base + PM1_EN_REG);
> > +     spin_unlock(&priv->rtc_reglock);
> > +
> > +     return 0;
> > +}
> > +
> > +static int ls2x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> > +{
> > +     int ret;
> > +     struct ls2x_rtc_regs regs;
> > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > +
> > +     ls2x_rtc_time_to_alarm_regs(&alrm->time, &regs);
> > +
> > +     ret = regmap_write(priv->regmap, TOY_MATCH0_REG, regs.reg0);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     return ls2x_rtc_alarm_irq_enable(dev, alrm->enabled);
> > +}
> > +
> > +static const struct rtc_class_ops ls2x_rtc_ops = {
> > +     .read_time = ls2x_rtc_read_time,
> > +     .set_time = ls2x_rtc_set_time,
> > +     .read_alarm = ls2x_rtc_read_alarm,
> > +     .set_alarm = ls2x_rtc_set_alarm,
> > +     .alarm_irq_enable = ls2x_rtc_alarm_irq_enable,
> > +};
> > +
> > +static int ls2x_enable_rtc(struct ls2x_rtc_priv *priv)
> > +{
> > +     u32 val;
> > +     int ret;
> > +
> > +     ret = regmap_read(priv->regmap, RTC_CTRL_REG, &val);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     return regmap_write(priv->regmap, RTC_CTRL_REG,
> > +                         val | TOY_ENABLE | OSC_ENABLE);
> > +}
> > +
> > +static int ls2x_rtc_probe(struct platform_device *pdev)
> > +{
> > +     int ret;
> > +     void __iomem *regs;
> > +     struct ls2x_rtc_priv *priv;
> > +     struct device *dev = &pdev->dev;
> > +
> > +     priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +     if (!priv)
> > +             return -ENOMEM;
> > +
> > +     priv->irq = platform_get_irq(pdev, 0);
> > +     if (priv->irq < 0)
> > +             return dev_err_probe(dev, priv->irq, "platform_get_irq failed\n");
> > +
> > +     platform_set_drvdata(pdev, priv);
> > +
> > +     regs = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(regs))
> > +             return dev_err_probe(dev, PTR_ERR(regs),
> > +                                  "devm_platform_ioremap_resource failed\n");
> > +
> > +     priv->regmap = devm_regmap_init_mmio(dev, regs,
> > +                                          &ls2x_rtc_regmap_config);
> > +     if (IS_ERR(priv->regmap))
> > +             return dev_err_probe(dev, PTR_ERR(priv->regmap),
> > +                                  "devm_regmap_init_mmio failed\n");
> > +
> > +     priv->rtcdev = devm_rtc_allocate_device(dev);
> > +     if (IS_ERR(priv->rtcdev))
> > +             return dev_err_probe(dev, PTR_ERR(priv->rtcdev),
> > +                                  "devm_rtc_allocate_device failed\n");
> > +
> > +     /* Due to hardware erratum, all years multiple of 4 are considered
> > +      * leap year, so only years 2000 through 2099 are usable.
> > +      *
> > +      * Previous out-of-tree versions of this driver wrote tm_year directly
> > +      * into the year register, so epoch 2000 must be used to preserve
> > +      * semantics on shipped systems.
> > +      */
> > +     priv->rtcdev->range_min = RTC_TIMESTAMP_BEGIN_2000;
> > +     priv->rtcdev->range_max = RTC_TIMESTAMP_END_2099;
> > +     priv->rtcdev->ops = &ls2x_rtc_ops;
> > +     priv->acpi_base = regs - PM_RTC_OFFSET;
> > +     spin_lock_init(&priv->rtc_reglock);
> > +     clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, priv->rtcdev->features);
>
> Why?

If you don't clear UIE, on the Loongson-2k, the shutdown will turn
into a reboot after setting up the wakealarm.

>
> > +
> > +#ifdef CONFIG_ACPI
> > +     if (!acpi_disabled)
> > +             acpi_install_fixed_event_handler(ACPI_EVENT_RTC,
> > +                                              ls2x_acpi_fix_handler, priv);
> > +#endif
> > +
> > +     ret = ls2x_enable_rtc(priv);
> > +     if (ret < 0)
> > +             return dev_err_probe(dev, ret, "ls2x_enable_rtc failed\n");
>
> This should not be done in probe but on the first set_time. This then
> allows you to know whether the time has been set and is valid in
> read_time. Please add the check.

Actually I don't quite understand the point you are making, the
function just enables the TOY counter, would there be any particular
problem with calling it in the probe function?

Thanks.

Binbin
>
> > +
> > +     ret = devm_request_threaded_irq(dev, priv->irq, NULL, ls2x_rtc_isr,
> > +                                     IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > +                                     "ls2x-alarm", priv);
> > +     if (ret < 0)
> > +             return dev_err_probe(dev, ret, "Unable to request irq %d\n",
> > +                                  priv->irq);
> > +
> > +     if (!device_can_wakeup(&pdev->dev))
> > +             device_init_wakeup(dev, 1);
> > +
> > +     return devm_rtc_register_device(priv->rtcdev);
> > +}
> > +
> > +static const struct of_device_id ls2x_rtc_of_match[] = {
> > +     { .compatible = "loongson,ls2x-rtc" },
> > +     { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, ls2x_rtc_of_match);
> > +
> > +static const struct acpi_device_id ls2x_rtc_acpi_match[] = {
> > +     { "LOON0001" }, /* Loongson LS7A */
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(acpi, ls2x_rtc_acpi_match);
> > +
> > +static struct platform_driver ls2x_rtc_driver = {
> > +     .probe          = ls2x_rtc_probe,
> > +     .driver         = {
> > +             .name   = "ls2x-rtc",
> > +             .of_match_table = ls2x_rtc_of_match,
> > +             .acpi_match_table = ls2x_rtc_acpi_match,
> > +     },
> > +};
> > +
> > +module_platform_driver(ls2x_rtc_driver);
> > +
> > +MODULE_DESCRIPTION("Loongson LS2X RTC driver");
> > +MODULE_AUTHOR("WANG Xuerui <git@xen0n.name>");
> > +MODULE_AUTHOR("Huacai Chen <chenhuacai@kernel.org>");
> > +MODULE_AUTHOR("Binbin Zhou <zhoubinbin@loongson.cn>");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.31.1
> >
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>

Re: [PATCH V2 2/7] rtc: Add support for the Loongson-2K/LS7A RTC

[Alexandre Belloni]

On 01/02/2023 17:16:12+0800, Binbin Zhou wrote:
> Hi Alexandre:
> 
> Sorry for the late reply.
> 
> On Tue, Jan 24, 2023 at 7:37 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> >
> > Hello,
> >
> >
> > On 09/01/2023 09:35:12+0800, Binbin Zhou wrote:
> > > This RTC module is integrated into the Loongson-2K SoC and the LS7A
> > > bridge chip. This version is almost entirely rewritten to make use of
> > > current kernel API, and it supports both ACPI and DT.
> > >
> > > This driver is shared by MIPS-based Loongson-3A4000 system (use FDT) and
> > > LoongArch-based Loongson-3A5000 system (use ACPI).
> > >
> >
> > checkpatch.pl --strict complains, please fix the warnings and checks
> 
> Got.
> >
> >
> > > +#ifdef CONFIG_ACPI
> > > +static u32 ls2x_acpi_fix_handler(void *id)
> > > +{
> > > +     int ret;
> > > +     struct ls2x_rtc_priv *priv = (struct ls2x_rtc_priv *)id;
> > > +
> > > +     spin_lock(&priv->rtc_reglock);
> > > +
> > > +     /* Disable acpi rtc enabled */
> > > +     ret = readl(priv->acpi_base + PM1_EN_REG) & ~RTC_EN;
> > > +     writel(ret, priv->acpi_base + PM1_EN_REG);
> > > +
> > > +     /* Clear acpi rtc interrupt Status */
> > > +     writel(RTC_STS, priv->acpi_base + PM1_STS_REG);
> > > +
> > > +     spin_unlock(&priv->rtc_reglock);
> > > +
> > > +     /*
> > > +      * The TOY_MATCH0_REG should be cleared 0 here,
> > > +      * otherwise the interrupt cannot be cleared.
> > > +      * Because the match condition is still satisfied
> > > +      */
> > > +     ret = regmap_write(priv->regmap, TOY_MATCH0_REG, 0);
> > > +     if (ret < 0)
> > > +             return ret;
> >
> > How is this an ACPI related issue? I guess the same would happen on
> > !ACPI.
> 
> I just assumed that the function would only be called under CONFIG_ACPI.
> >
> > > +
> > > +     rtc_update_irq(priv->rtcdev, 1, RTC_AF | RTC_IRQF);
> >
> > This is not useful, at that time, userspace has had no chance to open
> > the RTC device file as it is not created yet.
> >
> > > +     return 0;
> > > +}
> > > +#endif
> > > +
> > > +static inline void ls2x_rtc_regs_to_time(struct ls2x_rtc_regs *regs,
> >
> > Those static inline functions seem to be used only once, you should just
> > put the code in the proper location.
> 
> I will do it in the next version.
> >
> > > +                                      struct rtc_time *tm)
> > > +{
> > > +     tm->tm_year = regs->reg1;
> > > +     tm->tm_sec = FIELD_GET(TOY_SEC, regs->reg0);
> > > +     tm->tm_min = FIELD_GET(TOY_MIN, regs->reg0);
> > > +     tm->tm_hour = FIELD_GET(TOY_HOUR, regs->reg0);
> > > +     tm->tm_mday = FIELD_GET(TOY_DAY, regs->reg0);
> > > +     tm->tm_mon = FIELD_GET(TOY_MON, regs->reg0) - 1;
> > > +}
> > > +
> > > +static inline void ls2x_rtc_time_to_regs(struct rtc_time *tm,
> > > +                                      struct ls2x_rtc_regs *regs)
> > > +{
> > > +     regs->reg0 = FIELD_PREP(TOY_SEC, tm->tm_sec);
> > > +     regs->reg0 |= FIELD_PREP(TOY_MIN, tm->tm_min);
> > > +     regs->reg0 |= FIELD_PREP(TOY_HOUR, tm->tm_hour);
> > > +     regs->reg0 |= FIELD_PREP(TOY_DAY, tm->tm_mday);
> > > +     regs->reg0 |= FIELD_PREP(TOY_MON, tm->tm_mon + 1);
> > > +     regs->reg1 = tm->tm_year;
> > > +}
> > > +
> > > +static inline void ls2x_rtc_alarm_regs_to_time(struct ls2x_rtc_regs *regs,
> > > +                                      struct rtc_time *tm)
> > > +{
> > > +     tm->tm_sec = FIELD_GET(TOY_MATCH_SEC, regs->reg0);
> > > +     tm->tm_min = FIELD_GET(TOY_MATCH_MIN, regs->reg0);
> > > +     tm->tm_hour = FIELD_GET(TOY_MATCH_HOUR, regs->reg0);
> > > +     tm->tm_mday = FIELD_GET(TOY_MATCH_DAY, regs->reg0);
> > > +     tm->tm_mon = FIELD_GET(TOY_MATCH_MON, regs->reg0) - 1;
> > > +     /*
> > > +      * The rtc SYS_TOYMATCH0/YEAR bit field is only 6 bits, so it means 63
> > > +      * years at most. Therefore, The RTC alarm years can be set from 1900
> > > +      * to 1963. This causes the initialization of alarm fail during call
> > > +      * __rtc_read_alarm.
> > > +      * We add 64 years offset to ls2x_rtc_read_alarm. After adding the
> > > +      * offset, the RTC alarm clock can be set from 1964 to 2027.
> > > +      */
> > > +     tm->tm_year = FIELD_GET(TOY_MATCH_YEAR, regs->reg0) + 64;
> >
> > This is not symmetric with ls2x_rtc_time_to_alarm_regs, how can it work?
> 
> This is to avoid an "invalid alarm value" at boot time, which of
> course should not be a good solution.
> When the alarm value is read at boot time, "year" is not yet set to
> the proper value so the year is always set to 1900.

Why isn't it set at boot time? Isn't the register persisting after a
reboot?
Setting a bogus alarm is not a solution.

> 
> How about just  "tm->tm_year = -1", as the alarm is now only read when booting?
> 
> >
> > > +}
> > > +
> > > +static inline void ls2x_rtc_time_to_alarm_regs(struct rtc_time *tm,
> > > +                                      struct ls2x_rtc_regs *regs)
> > > +{
> > > +     regs->reg0 = FIELD_PREP(TOY_MATCH_SEC, tm->tm_sec);
> > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_MIN, tm->tm_min);
> > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_HOUR, tm->tm_hour);
> > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_DAY, tm->tm_mday);
> > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_MON, tm->tm_mon + 1);
> > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_YEAR, tm->tm_year);
> > > +}
> > > +
> > > +static int ls2x_rtc_read_time(struct device *dev, struct rtc_time *tm)
> > > +{
> > > +     int ret;
> > > +     struct ls2x_rtc_regs regs;
> > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > +
> > > +     ret = regmap_read(priv->regmap, TOY_READ1_REG, &regs.reg1);
> > > +     if (ret < 0)
> > > +             return ret;
> > > +
> > > +     ret = regmap_read(priv->regmap, TOY_READ0_REG, &regs.reg0);
> > > +     if (ret < 0)
> > > +             return ret;
> > > +
> >
> > I never got a reply to my question:
> >
> > "
> > I'm actually wondering why you read first TOY_READ1_REG then
> > TOY_READ0_REG. ls1x does the opposite and the ls1c datasheet I found
> > doesn't mention any latching happening. So unless latching is done on
> > TOY_READ1_REG, you could use regmap_bulk_read and simply avoid struct
> > ls2x_rtc_regs.
> > If there is no latching, you may need to read TOY_READ0_REG at least
> > twice. Because TOY_READ1_REG only contains the year, it is an issue only
> > on 31 of December and it will not be easy to reproduce.
> > "
> >
> 
> The LS7A and Loongson-2K datasheets also do not mention any latching
> happening. Reading TOY_READ1_REG first is probably just a matter of
> habit.
> I tried using regmap_bulk_xxx() and it also reads and writes time
> properly. In the next version I will rewrite this part of the code.
> 
> Example:
> 
> #define LS2X_NUM_TIME_REGS      2
> 
> u32 rtc_data[LS2X_NUM_TIME_REGS];
> struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> 
> ret = regmap_bulk_read(priv->regmap, TOY_READ0_REG, rtc_data,
> LS2X_NUM_TIME_REGS);
> 

Doing a bulk read doesn't guarantee the atomicity of the operation. You
really must check whether a register changed while reading the other
one.

> 
> >
> > > +     ls2x_rtc_regs_to_time(&regs, tm);
> > > +     return 0;
> > > +}
> > > +
> > > +static int ls2x_rtc_set_time(struct device *dev, struct rtc_time *tm)
> > > +{
> > > +     int ret;
> > > +     struct ls2x_rtc_regs regs;
> > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > +
> > > +     ls2x_rtc_time_to_regs(tm, &regs);
> > > +
> > > +     ret = regmap_write(priv->regmap, TOY_WRITE0_REG, regs.reg0);
> > > +     if (ret < 0)
> > > +             return ret;
> > > +
> > > +     return regmap_write(priv->regmap, TOY_WRITE1_REG, regs.reg1);
> > > +}
> > > +
> > > +static int ls2x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> > > +{
> > > +     int ret;
> > > +     struct ls2x_rtc_regs regs;
> > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > +
> > > +     ret = regmap_read(priv->regmap, TOY_MATCH0_REG, &regs.reg0);
> > > +     if (ret < 0)
> > > +             return ret;
> > > +
> > > +     ls2x_rtc_alarm_regs_to_time(&regs, &alrm->time);
> > > +     alrm->enabled = !!(readl(priv->acpi_base + PM1_EN_REG) & RTC_EN);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int ls2x_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> > > +{
> > > +     u32 val;
> > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > +
> > > +     spin_lock(&priv->rtc_reglock);
> > > +     val = readl(priv->acpi_base + PM1_EN_REG);
> > > +
> > > +     /* Enalbe RTC alarm */
> > Typo
> >
> > > +     writel((enabled ? val | RTC_EN : val & ~RTC_EN),
> > > +            priv->acpi_base + PM1_EN_REG);
> > > +     spin_unlock(&priv->rtc_reglock);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int ls2x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> > > +{
> > > +     int ret;
> > > +     struct ls2x_rtc_regs regs;
> > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > +
> > > +     ls2x_rtc_time_to_alarm_regs(&alrm->time, &regs);
> > > +
> > > +     ret = regmap_write(priv->regmap, TOY_MATCH0_REG, regs.reg0);
> > > +     if (ret < 0)
> > > +             return ret;
> > > +
> > > +     return ls2x_rtc_alarm_irq_enable(dev, alrm->enabled);
> > > +}
> > > +
> > > +static const struct rtc_class_ops ls2x_rtc_ops = {
> > > +     .read_time = ls2x_rtc_read_time,
> > > +     .set_time = ls2x_rtc_set_time,
> > > +     .read_alarm = ls2x_rtc_read_alarm,
> > > +     .set_alarm = ls2x_rtc_set_alarm,
> > > +     .alarm_irq_enable = ls2x_rtc_alarm_irq_enable,
> > > +};
> > > +
> > > +static int ls2x_enable_rtc(struct ls2x_rtc_priv *priv)
> > > +{
> > > +     u32 val;
> > > +     int ret;
> > > +
> > > +     ret = regmap_read(priv->regmap, RTC_CTRL_REG, &val);
> > > +     if (ret < 0)
> > > +             return ret;
> > > +
> > > +     return regmap_write(priv->regmap, RTC_CTRL_REG,
> > > +                         val | TOY_ENABLE | OSC_ENABLE);
> > > +}
> > > +
> > > +static int ls2x_rtc_probe(struct platform_device *pdev)
> > > +{
> > > +     int ret;
> > > +     void __iomem *regs;
> > > +     struct ls2x_rtc_priv *priv;
> > > +     struct device *dev = &pdev->dev;
> > > +
> > > +     priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > +     if (!priv)
> > > +             return -ENOMEM;
> > > +
> > > +     priv->irq = platform_get_irq(pdev, 0);
> > > +     if (priv->irq < 0)
> > > +             return dev_err_probe(dev, priv->irq, "platform_get_irq failed\n");
> > > +
> > > +     platform_set_drvdata(pdev, priv);
> > > +
> > > +     regs = devm_platform_ioremap_resource(pdev, 0);
> > > +     if (IS_ERR(regs))
> > > +             return dev_err_probe(dev, PTR_ERR(regs),
> > > +                                  "devm_platform_ioremap_resource failed\n");
> > > +
> > > +     priv->regmap = devm_regmap_init_mmio(dev, regs,
> > > +                                          &ls2x_rtc_regmap_config);
> > > +     if (IS_ERR(priv->regmap))
> > > +             return dev_err_probe(dev, PTR_ERR(priv->regmap),
> > > +                                  "devm_regmap_init_mmio failed\n");
> > > +
> > > +     priv->rtcdev = devm_rtc_allocate_device(dev);
> > > +     if (IS_ERR(priv->rtcdev))
> > > +             return dev_err_probe(dev, PTR_ERR(priv->rtcdev),
> > > +                                  "devm_rtc_allocate_device failed\n");
> > > +
> > > +     /* Due to hardware erratum, all years multiple of 4 are considered
> > > +      * leap year, so only years 2000 through 2099 are usable.
> > > +      *
> > > +      * Previous out-of-tree versions of this driver wrote tm_year directly
> > > +      * into the year register, so epoch 2000 must be used to preserve
> > > +      * semantics on shipped systems.
> > > +      */
> > > +     priv->rtcdev->range_min = RTC_TIMESTAMP_BEGIN_2000;
> > > +     priv->rtcdev->range_max = RTC_TIMESTAMP_END_2099;
> > > +     priv->rtcdev->ops = &ls2x_rtc_ops;
> > > +     priv->acpi_base = regs - PM_RTC_OFFSET;
> > > +     spin_lock_init(&priv->rtc_reglock);
> > > +     clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, priv->rtcdev->features);
> >
> > Why?
> 
> If you don't clear UIE, on the Loongson-2k, the shutdown will turn
> into a reboot after setting up the wakealarm.
> 

Why? I don't get how UIE can have this effect.

> >
> > > +
> > > +#ifdef CONFIG_ACPI
> > > +     if (!acpi_disabled)
> > > +             acpi_install_fixed_event_handler(ACPI_EVENT_RTC,
> > > +                                              ls2x_acpi_fix_handler, priv);
> > > +#endif
> > > +
> > > +     ret = ls2x_enable_rtc(priv);
> > > +     if (ret < 0)
> > > +             return dev_err_probe(dev, ret, "ls2x_enable_rtc failed\n");
> >
> > This should not be done in probe but on the first set_time. This then
> > allows you to know whether the time has been set and is valid in
> > read_time. Please add the check.
> 
> Actually I don't quite understand the point you are making, the
> function just enables the TOY counter, would there be any particular
> problem with calling it in the probe function?
> 

Yes, you are losing an important bit of information which is that if
TOY_ENABLE is not set, then you know the time has not been set. Else,
you allow reading an known invalid time.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH V2 2/7] rtc: Add support for the Loongson-2K/LS7A RTC

[Binbin Zhou]

On Wed, Feb 15, 2023 at 7:13 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 01/02/2023 17:16:12+0800, Binbin Zhou wrote:
> > Hi Alexandre:
> >
> > Sorry for the late reply.
> >
> > On Tue, Jan 24, 2023 at 7:37 AM Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> > >
> > > Hello,
> > >
> > >
> > > On 09/01/2023 09:35:12+0800, Binbin Zhou wrote:
> > > > This RTC module is integrated into the Loongson-2K SoC and the LS7A
> > > > bridge chip. This version is almost entirely rewritten to make use of
> > > > current kernel API, and it supports both ACPI and DT.
> > > >
> > > > This driver is shared by MIPS-based Loongson-3A4000 system (use FDT) and
> > > > LoongArch-based Loongson-3A5000 system (use ACPI).
> > > >
> > >
> > > checkpatch.pl --strict complains, please fix the warnings and checks
> >
> > Got.
> > >
> > >
> > > > +#ifdef CONFIG_ACPI
> > > > +static u32 ls2x_acpi_fix_handler(void *id)
> > > > +{
> > > > +     int ret;
> > > > +     struct ls2x_rtc_priv *priv = (struct ls2x_rtc_priv *)id;
> > > > +
> > > > +     spin_lock(&priv->rtc_reglock);
> > > > +
> > > > +     /* Disable acpi rtc enabled */
> > > > +     ret = readl(priv->acpi_base + PM1_EN_REG) & ~RTC_EN;
> > > > +     writel(ret, priv->acpi_base + PM1_EN_REG);
> > > > +
> > > > +     /* Clear acpi rtc interrupt Status */
> > > > +     writel(RTC_STS, priv->acpi_base + PM1_STS_REG);
> > > > +
> > > > +     spin_unlock(&priv->rtc_reglock);
> > > > +
> > > > +     /*
> > > > +      * The TOY_MATCH0_REG should be cleared 0 here,
> > > > +      * otherwise the interrupt cannot be cleared.
> > > > +      * Because the match condition is still satisfied
> > > > +      */
> > > > +     ret = regmap_write(priv->regmap, TOY_MATCH0_REG, 0);
> > > > +     if (ret < 0)
> > > > +             return ret;
> > >
> > > How is this an ACPI related issue? I guess the same would happen on
> > > !ACPI.
> >
> > I just assumed that the function would only be called under CONFIG_ACPI.
> > >
> > > > +
> > > > +     rtc_update_irq(priv->rtcdev, 1, RTC_AF | RTC_IRQF);
> > >
> > > This is not useful, at that time, userspace has had no chance to open
> > > the RTC device file as it is not created yet.
> > >
> > > > +     return 0;
> > > > +}
> > > > +#endif
> > > > +
> > > > +static inline void ls2x_rtc_regs_to_time(struct ls2x_rtc_regs *regs,
> > >
> > > Those static inline functions seem to be used only once, you should just
> > > put the code in the proper location.
> >
> > I will do it in the next version.
> > >
> > > > +                                      struct rtc_time *tm)
> > > > +{
> > > > +     tm->tm_year = regs->reg1;
> > > > +     tm->tm_sec = FIELD_GET(TOY_SEC, regs->reg0);
> > > > +     tm->tm_min = FIELD_GET(TOY_MIN, regs->reg0);
> > > > +     tm->tm_hour = FIELD_GET(TOY_HOUR, regs->reg0);
> > > > +     tm->tm_mday = FIELD_GET(TOY_DAY, regs->reg0);
> > > > +     tm->tm_mon = FIELD_GET(TOY_MON, regs->reg0) - 1;
> > > > +}
> > > > +
> > > > +static inline void ls2x_rtc_time_to_regs(struct rtc_time *tm,
> > > > +                                      struct ls2x_rtc_regs *regs)
> > > > +{
> > > > +     regs->reg0 = FIELD_PREP(TOY_SEC, tm->tm_sec);
> > > > +     regs->reg0 |= FIELD_PREP(TOY_MIN, tm->tm_min);
> > > > +     regs->reg0 |= FIELD_PREP(TOY_HOUR, tm->tm_hour);
> > > > +     regs->reg0 |= FIELD_PREP(TOY_DAY, tm->tm_mday);
> > > > +     regs->reg0 |= FIELD_PREP(TOY_MON, tm->tm_mon + 1);
> > > > +     regs->reg1 = tm->tm_year;
> > > > +}
> > > > +
> > > > +static inline void ls2x_rtc_alarm_regs_to_time(struct ls2x_rtc_regs *regs,
> > > > +                                      struct rtc_time *tm)
> > > > +{
> > > > +     tm->tm_sec = FIELD_GET(TOY_MATCH_SEC, regs->reg0);
> > > > +     tm->tm_min = FIELD_GET(TOY_MATCH_MIN, regs->reg0);
> > > > +     tm->tm_hour = FIELD_GET(TOY_MATCH_HOUR, regs->reg0);
> > > > +     tm->tm_mday = FIELD_GET(TOY_MATCH_DAY, regs->reg0);
> > > > +     tm->tm_mon = FIELD_GET(TOY_MATCH_MON, regs->reg0) - 1;
> > > > +     /*
> > > > +      * The rtc SYS_TOYMATCH0/YEAR bit field is only 6 bits, so it means 63
> > > > +      * years at most. Therefore, The RTC alarm years can be set from 1900
> > > > +      * to 1963. This causes the initialization of alarm fail during call
> > > > +      * __rtc_read_alarm.
> > > > +      * We add 64 years offset to ls2x_rtc_read_alarm. After adding the
> > > > +      * offset, the RTC alarm clock can be set from 1964 to 2027.
> > > > +      */
> > > > +     tm->tm_year = FIELD_GET(TOY_MATCH_YEAR, regs->reg0) + 64;
> > >
> > > This is not symmetric with ls2x_rtc_time_to_alarm_regs, how can it work?
> >
> > This is to avoid an "invalid alarm value" at boot time, which of
> > course should not be a good solution.
> > When the alarm value is read at boot time, "year" is not yet set to
> > the proper value so the year is always set to 1900.
>
> Why isn't it set at boot time? Isn't the register persisting after a
> reboot?
> Setting a bogus alarm is not a solution.
>

Hi, Alexandre:

Sorry, I seem to have misled the issue.
This is a hardware bug, as we know from the datasheet, the year field
in the TOY_MATCH register has only 6 bits (bit[31:26]), so the maximum
value is 63. For example, 2023 can only be read here as 1959,
resulting in an invalid alarm.
The current workaround: after reading the year field in
ls2x_rtc_read_alarm(), manually add 64 or a multiple of 64 (equivalent
to completing the high bits), which also ensures that the reading and
writing is consistent.

> >
> > How about just  "tm->tm_year = -1", as the alarm is now only read when booting?
> >
> > >
> > > > +}
> > > > +
> > > > +static inline void ls2x_rtc_time_to_alarm_regs(struct rtc_time *tm,
> > > > +                                      struct ls2x_rtc_regs *regs)
> > > > +{
> > > > +     regs->reg0 = FIELD_PREP(TOY_MATCH_SEC, tm->tm_sec);
> > > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_MIN, tm->tm_min);
> > > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_HOUR, tm->tm_hour);
> > > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_DAY, tm->tm_mday);
> > > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_MON, tm->tm_mon + 1);
> > > > +     regs->reg0 |= FIELD_PREP(TOY_MATCH_YEAR, tm->tm_year);
> > > > +}
> > > > +
> > > > +static int ls2x_rtc_read_time(struct device *dev, struct rtc_time *tm)
> > > > +{
> > > > +     int ret;
> > > > +     struct ls2x_rtc_regs regs;
> > > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > > +
> > > > +     ret = regmap_read(priv->regmap, TOY_READ1_REG, &regs.reg1);
> > > > +     if (ret < 0)
> > > > +             return ret;
> > > > +
> > > > +     ret = regmap_read(priv->regmap, TOY_READ0_REG, &regs.reg0);
> > > > +     if (ret < 0)
> > > > +             return ret;
> > > > +
> > >
> > > I never got a reply to my question:
> > >
> > > "
> > > I'm actually wondering why you read first TOY_READ1_REG then
> > > TOY_READ0_REG. ls1x does the opposite and the ls1c datasheet I found
> > > doesn't mention any latching happening. So unless latching is done on
> > > TOY_READ1_REG, you could use regmap_bulk_read and simply avoid struct
> > > ls2x_rtc_regs.
> > > If there is no latching, you may need to read TOY_READ0_REG at least
> > > twice. Because TOY_READ1_REG only contains the year, it is an issue only
> > > on 31 of December and it will not be easy to reproduce.
> > > "
> > >
> >
> > The LS7A and Loongson-2K datasheets also do not mention any latching
> > happening. Reading TOY_READ1_REG first is probably just a matter of
> > habit.
> > I tried using regmap_bulk_xxx() and it also reads and writes time
> > properly. In the next version I will rewrite this part of the code.
> >
> > Example:
> >
> > #define LS2X_NUM_TIME_REGS      2
> >
> > u32 rtc_data[LS2X_NUM_TIME_REGS];
> > struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> >
> > ret = regmap_bulk_read(priv->regmap, TOY_READ0_REG, rtc_data,
> > LS2X_NUM_TIME_REGS);
> >
>
> Doing a bulk read doesn't guarantee the atomicity of the operation. You
> really must check whether a register changed while reading the other
> one.
>

How about protecting with mutex?

> >
> > >
> > > > +     ls2x_rtc_regs_to_time(&regs, tm);
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static int ls2x_rtc_set_time(struct device *dev, struct rtc_time *tm)
> > > > +{
> > > > +     int ret;
> > > > +     struct ls2x_rtc_regs regs;
> > > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > > +
> > > > +     ls2x_rtc_time_to_regs(tm, &regs);
> > > > +
> > > > +     ret = regmap_write(priv->regmap, TOY_WRITE0_REG, regs.reg0);
> > > > +     if (ret < 0)
> > > > +             return ret;
> > > > +
> > > > +     return regmap_write(priv->regmap, TOY_WRITE1_REG, regs.reg1);
> > > > +}
> > > > +
> > > > +static int ls2x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> > > > +{
> > > > +     int ret;
> > > > +     struct ls2x_rtc_regs regs;
> > > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > > +
> > > > +     ret = regmap_read(priv->regmap, TOY_MATCH0_REG, &regs.reg0);
> > > > +     if (ret < 0)
> > > > +             return ret;
> > > > +
> > > > +     ls2x_rtc_alarm_regs_to_time(&regs, &alrm->time);
> > > > +     alrm->enabled = !!(readl(priv->acpi_base + PM1_EN_REG) & RTC_EN);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static int ls2x_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> > > > +{
> > > > +     u32 val;
> > > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > > +
> > > > +     spin_lock(&priv->rtc_reglock);
> > > > +     val = readl(priv->acpi_base + PM1_EN_REG);
> > > > +
> > > > +     /* Enalbe RTC alarm */
> > > Typo
> > >
> > > > +     writel((enabled ? val | RTC_EN : val & ~RTC_EN),
> > > > +            priv->acpi_base + PM1_EN_REG);
> > > > +     spin_unlock(&priv->rtc_reglock);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static int ls2x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> > > > +{
> > > > +     int ret;
> > > > +     struct ls2x_rtc_regs regs;
> > > > +     struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > > > +
> > > > +     ls2x_rtc_time_to_alarm_regs(&alrm->time, &regs);
> > > > +
> > > > +     ret = regmap_write(priv->regmap, TOY_MATCH0_REG, regs.reg0);
> > > > +     if (ret < 0)
> > > > +             return ret;
> > > > +
> > > > +     return ls2x_rtc_alarm_irq_enable(dev, alrm->enabled);
> > > > +}
> > > > +
> > > > +static const struct rtc_class_ops ls2x_rtc_ops = {
> > > > +     .read_time = ls2x_rtc_read_time,
> > > > +     .set_time = ls2x_rtc_set_time,
> > > > +     .read_alarm = ls2x_rtc_read_alarm,
> > > > +     .set_alarm = ls2x_rtc_set_alarm,
> > > > +     .alarm_irq_enable = ls2x_rtc_alarm_irq_enable,
> > > > +};
> > > > +
> > > > +static int ls2x_enable_rtc(struct ls2x_rtc_priv *priv)
> > > > +{
> > > > +     u32 val;
> > > > +     int ret;
> > > > +
> > > > +     ret = regmap_read(priv->regmap, RTC_CTRL_REG, &val);
> > > > +     if (ret < 0)
> > > > +             return ret;
> > > > +
> > > > +     return regmap_write(priv->regmap, RTC_CTRL_REG,
> > > > +                         val | TOY_ENABLE | OSC_ENABLE);
> > > > +}
> > > > +
> > > > +static int ls2x_rtc_probe(struct platform_device *pdev)
> > > > +{
> > > > +     int ret;
> > > > +     void __iomem *regs;
> > > > +     struct ls2x_rtc_priv *priv;
> > > > +     struct device *dev = &pdev->dev;
> > > > +
> > > > +     priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > > +     if (!priv)
> > > > +             return -ENOMEM;
> > > > +
> > > > +     priv->irq = platform_get_irq(pdev, 0);
> > > > +     if (priv->irq < 0)
> > > > +             return dev_err_probe(dev, priv->irq, "platform_get_irq failed\n");
> > > > +
> > > > +     platform_set_drvdata(pdev, priv);
> > > > +
> > > > +     regs = devm_platform_ioremap_resource(pdev, 0);
> > > > +     if (IS_ERR(regs))
> > > > +             return dev_err_probe(dev, PTR_ERR(regs),
> > > > +                                  "devm_platform_ioremap_resource failed\n");
> > > > +
> > > > +     priv->regmap = devm_regmap_init_mmio(dev, regs,
> > > > +                                          &ls2x_rtc_regmap_config);
> > > > +     if (IS_ERR(priv->regmap))
> > > > +             return dev_err_probe(dev, PTR_ERR(priv->regmap),
> > > > +                                  "devm_regmap_init_mmio failed\n");
> > > > +
> > > > +     priv->rtcdev = devm_rtc_allocate_device(dev);
> > > > +     if (IS_ERR(priv->rtcdev))
> > > > +             return dev_err_probe(dev, PTR_ERR(priv->rtcdev),
> > > > +                                  "devm_rtc_allocate_device failed\n");
> > > > +
> > > > +     /* Due to hardware erratum, all years multiple of 4 are considered
> > > > +      * leap year, so only years 2000 through 2099 are usable.
> > > > +      *
> > > > +      * Previous out-of-tree versions of this driver wrote tm_year directly
> > > > +      * into the year register, so epoch 2000 must be used to preserve
> > > > +      * semantics on shipped systems.
> > > > +      */
> > > > +     priv->rtcdev->range_min = RTC_TIMESTAMP_BEGIN_2000;
> > > > +     priv->rtcdev->range_max = RTC_TIMESTAMP_END_2099;
> > > > +     priv->rtcdev->ops = &ls2x_rtc_ops;
> > > > +     priv->acpi_base = regs - PM_RTC_OFFSET;
> > > > +     spin_lock_init(&priv->rtc_reglock);
> > > > +     clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, priv->rtcdev->features);
> > >
> > > Why?
> >
> > If you don't clear UIE, on the Loongson-2k, the shutdown will turn
> > into a reboot after setting up the wakealarm.
> >
>
> Why? I don't get how UIE can have this effect.
>

In fact, we need to clear the RTC wakeup interrupt manually.
I will fix this in the next release and remove clear_bit().

> > >
> > > > +
> > > > +#ifdef CONFIG_ACPI
> > > > +     if (!acpi_disabled)
> > > > +             acpi_install_fixed_event_handler(ACPI_EVENT_RTC,
> > > > +                                              ls2x_acpi_fix_handler, priv);
> > > > +#endif
> > > > +
> > > > +     ret = ls2x_enable_rtc(priv);
> > > > +     if (ret < 0)
> > > > +             return dev_err_probe(dev, ret, "ls2x_enable_rtc failed\n");
> > >
> > > This should not be done in probe but on the first set_time. This then
> > > allows you to know whether the time has been set and is valid in
> > > read_time. Please add the check.
> >
> > Actually I don't quite understand the point you are making, the
> > function just enables the TOY counter, would there be any particular
> > problem with calling it in the probe function?
> >
>
> Yes, you are losing an important bit of information which is that if
> TOY_ENABLE is not set, then you know the time has not been set. Else,
> you allow reading an known invalid time.
>

I will enable the RTC in set_time() and check in read_time() that it
has been enabled

Thanks.


Binbin

>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH V2 2/7] rtc: Add support for the Loongson-2K/LS7A RTC

[Alexandre Belloni]

On 27/02/2023 10:26:09+0800, Binbin Zhou wrote:
> > > > > +static inline void ls2x_rtc_alarm_regs_to_time(struct ls2x_rtc_regs *regs,
> > > > > +                                      struct rtc_time *tm)
> > > > > +{
> > > > > +     tm->tm_sec = FIELD_GET(TOY_MATCH_SEC, regs->reg0);
> > > > > +     tm->tm_min = FIELD_GET(TOY_MATCH_MIN, regs->reg0);
> > > > > +     tm->tm_hour = FIELD_GET(TOY_MATCH_HOUR, regs->reg0);
> > > > > +     tm->tm_mday = FIELD_GET(TOY_MATCH_DAY, regs->reg0);
> > > > > +     tm->tm_mon = FIELD_GET(TOY_MATCH_MON, regs->reg0) - 1;
> > > > > +     /*
> > > > > +      * The rtc SYS_TOYMATCH0/YEAR bit field is only 6 bits, so it means 63
> > > > > +      * years at most. Therefore, The RTC alarm years can be set from 1900
> > > > > +      * to 1963. This causes the initialization of alarm fail during call
> > > > > +      * __rtc_read_alarm.
> > > > > +      * We add 64 years offset to ls2x_rtc_read_alarm. After adding the
> > > > > +      * offset, the RTC alarm clock can be set from 1964 to 2027.
> > > > > +      */
> > > > > +     tm->tm_year = FIELD_GET(TOY_MATCH_YEAR, regs->reg0) + 64;
> > > >
> > > > This is not symmetric with ls2x_rtc_time_to_alarm_regs, how can it work?
> > >
> > > This is to avoid an "invalid alarm value" at boot time, which of
> > > course should not be a good solution.
> > > When the alarm value is read at boot time, "year" is not yet set to
> > > the proper value so the year is always set to 1900.
> >
> > Why isn't it set at boot time? Isn't the register persisting after a
> > reboot?
> > Setting a bogus alarm is not a solution.
> >
> 
> Hi, Alexandre:
> 
> Sorry, I seem to have misled the issue.
> This is a hardware bug, as we know from the datasheet, the year field
> in the TOY_MATCH register has only 6 bits (bit[31:26]), so the maximum
> value is 63. For example, 2023 can only be read here as 1959,
> resulting in an invalid alarm.
> The current workaround: after reading the year field in
> ls2x_rtc_read_alarm(), manually add 64 or a multiple of 64 (equivalent
> to completing the high bits), which also ensures that the reading and
> writing is consistent.

My first complain was that this is not symmetric with
ls2x_rtc_time_to_alarm_regs. If you are adding 64 when reading the
alarm, you need to remove 64 when setting the alarm.

Now I get that FIELD_PREP will drop the overflowing bits.
Instead of having support for the 1964 to 2027 range, you should
probably aim for 2000 to 2064.

Also, this makes me realize that you are not setting the year properly,
the datasheet I have says that the supported year goes from 00 to 99.
This is also what you set in .probe.
Removing 100 from tm_year when setting and adding it back when reading
would fix all of that.

> > > The LS7A and Loongson-2K datasheets also do not mention any latching
> > > happening. Reading TOY_READ1_REG first is probably just a matter of
> > > habit.
> > > I tried using regmap_bulk_xxx() and it also reads and writes time
> > > properly. In the next version I will rewrite this part of the code.
> > >
> > > Example:
> > >
> > > #define LS2X_NUM_TIME_REGS      2
> > >
> > > u32 rtc_data[LS2X_NUM_TIME_REGS];
> > > struct ls2x_rtc_priv *priv = dev_get_drvdata(dev);
> > >
> > > ret = regmap_bulk_read(priv->regmap, TOY_READ0_REG, rtc_data,
> > > LS2X_NUM_TIME_REGS);
> > >
> >
> > Doing a bulk read doesn't guarantee the atomicity of the operation. You
> > really must check whether a register changed while reading the other
> > one.
> >
> 
> How about protecting with mutex?
> 

No, this would fix multiple processes accessing a variable, here what
you have are two unsynchronized hardware registers.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH V2 3/7] LoongArch: Enable LS2X RTC in loongson3_defconfig

[Binbin Zhou]

This is now supported, enable for Loongson-3 systems.
Other systems are unaffected.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 arch/loongarch/configs/loongson3_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/loongarch/configs/loongson3_defconfig b/arch/loongarch/configs/loongson3_defconfig
index 5677c4f8576e..43f46cfc73af 100644
--- a/arch/loongarch/configs/loongson3_defconfig
+++ b/arch/loongarch/configs/loongson3_defconfig
@@ -714,6 +714,7 @@ CONFIG_UCSI_ACPI=m
 CONFIG_INFINIBAND=m
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_EFI=y
+CONFIG_RTC_DRV_LS2X=y
 CONFIG_DMADEVICES=y
 CONFIG_UIO=m
 CONFIG_UIO_PDRV_GENIRQ=m
-- 
2.31.1

[PATCH V2 4/7] MIPS: Loongson64: DTS: Add RTC support to LS7A

[Binbin Zhou]

The LS7A RTC module is now supported, enable it.

Signed-off-by: WANG Xuerui <git@xen0n.name>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
index 2f45fce2cdc4..ec9a4bb95342 100644
--- a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
+++ b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
@@ -19,6 +19,13 @@ pic: interrupt-controller@10000000 {
 			#interrupt-cells = <2>;
 		};
 
+		rtc0: rtc@100d0100 {
+			compatible = "loongson,ls2x-rtc";
+			reg = <0 0x100d0100 0 0x78>;
+			interrupt-parent = <&pic>;
+			interrupts = <52 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
 		ls7a_uart0: serial@10080000 {
 			compatible = "ns16550a";
 			reg = <0 0x10080000 0 0x100>;
-- 
2.31.1

[PATCH V2 5/7] MIPS: Loongson: Enable LS2X RTC in loongson3_defconfig

[Binbin Zhou]

From: WANG Xuerui <git@xen0n.name>

This is now supported, enable for Loongson-3 systems.
Other systems are unaffected.

Signed-off-by: WANG Xuerui <git@xen0n.name>
---
 arch/mips/configs/loongson3_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/configs/loongson3_defconfig b/arch/mips/configs/loongson3_defconfig
index aca66a5f330d..4f55a02de9d8 100644
--- a/arch/mips/configs/loongson3_defconfig
+++ b/arch/mips/configs/loongson3_defconfig
@@ -319,6 +319,7 @@ CONFIG_USB_SERIAL_OPTION=m
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_CMOS=y
 CONFIG_RTC_DRV_GOLDFISH=y
+CONFIG_RTC_DRV_LS2X=y
 CONFIG_DMADEVICES=y
 CONFIG_VIRTIO_PCI=y
 CONFIG_VIRTIO_BALLOON=m
-- 
2.31.1

[PATCH V2 6/7] MIPS: Loongson64: DTS: Add RTC support to Loongson-2K

[Binbin Zhou]

The Loongson-2K RTC module is now supported, enable it.

The MMIO address is unclear from the Loongson 2K1000 user manual, I took
it from Loongson's out-of-tree fork of Linux 4.19.

Signed-off-by: WANG Xuerui <git@xen0n.name>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
index 8143a61111e3..c22414595140 100644
--- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
@@ -57,6 +57,13 @@ pm: reset-controller@1fe07000 {
 			reg = <0 0x1fe07000 0 0x422>;
 		};
 
+		rtc0: rtc@1fe07800 {
+			compatible = "loongson,ls2x-rtc";
+			reg = <0 0x1fe07800 0 0x78>;
+			interrupt-parent = <&liointc0>;
+			interrupts = <60 IRQ_TYPE_LEVEL_LOW>;
+		};
+
 		liointc0: interrupt-controller@1fe11400 {
 			compatible = "loongson,liointc-2.0";
 			reg = <0 0x1fe11400 0 0x40>,
-- 
2.31.1

Re: [PATCH V2 6/7] MIPS: Loongson64: DTS: Add RTC support to Loongson-2K

[Jiaxun Yang]

在2023年1月9日一月 上午1:36，Binbin Zhou写道：
> The Loongson-2K RTC module is now supported, enable it.
>
> The MMIO address is unclear from the Loongson 2K1000 user manual, I took
> it from Loongson's out-of-tree fork of Linux 4.19.

Can confirm this MMIO address is correct. It matches current confbus BAR
setting by PMON.

>
> Signed-off-by: WANG Xuerui <git@xen0n.name>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>

Acked-by: Jiaxun Yang <jiaxun.yang@flygoat.com>


> ---
>  arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi 
> b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
> index 8143a61111e3..c22414595140 100644
> --- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
> +++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi
> @@ -57,6 +57,13 @@ pm: reset-controller@1fe07000 {
>  			reg = <0 0x1fe07000 0 0x422>;
>  		};
> 
> +		rtc0: rtc@1fe07800 {
> +			compatible = "loongson,ls2x-rtc";
> +			reg = <0 0x1fe07800 0 0x78>;
> +			interrupt-parent = <&liointc0>;
> +			interrupts = <60 IRQ_TYPE_LEVEL_LOW>;
> +		};
> +
>  		liointc0: interrupt-controller@1fe11400 {
>  			compatible = "loongson,liointc-2.0";
>  			reg = <0 0x1fe11400 0 0x40>,
> -- 
> 2.31.1

-- 
- Jiaxun

[PATCH V2 7/7] MIPS: Loongson: Enable LS2X RTC in loongson2k_defconfig

[Binbin Zhou]

From: WANG Xuerui <git@xen0n.name>

This is now supported, enable for Loongson-2K systems.
Other systems are unaffected.

Signed-off-by: WANG Xuerui <git@xen0n.name>
---
 arch/mips/configs/loongson2k_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/configs/loongson2k_defconfig b/arch/mips/configs/loongson2k_defconfig
index 728bef666f7a..8000cf838c17 100644
--- a/arch/mips/configs/loongson2k_defconfig
+++ b/arch/mips/configs/loongson2k_defconfig
@@ -278,6 +278,7 @@ CONFIG_USB_SERIAL=m
 CONFIG_USB_SERIAL_OPTION=m
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_CMOS=y
+CONFIG_RTC_DRV_LS2X=y
 CONFIG_DMADEVICES=y
 # CONFIG_CPU_HWMON is not set
 CONFIG_PM_DEVFREQ=y
-- 
2.31.1

Re: [PATCH V2 0/7] rtc: ls2x: Add support for the Loongson-2K/LS7A RTC

[Alexandre Belloni]

On 09/01/2023 09:35:10+0800, Binbin Zhou wrote:
> Hi all:
> 
> The initial DT-base ls2x rtc driver was written by Wang Xuerui, He has
> released five versions of patchset before, and all related mail records
> are shown below if you are interested:
> 
> https://lore.kernel.org/all/?q=ls2x-rtc
> 
> In this series of patches, based on the code above, I have added the
> following support:
> 
> 1. Add ACPI-related support, as Loongson-3A5000 + LS7A is now ACPI-base
>    by default under LoongArch architecture;
> 2. Add rtc alarm/walarm related functions.
> 
> I have tested on Loongson-3A5000LA+LS7A1000/LS7A2000, Loongson-2K1000LA
> and Loongson-2K0500.
> 
> BTW:
> There have been discussions about merging the rtc drivers of ls1x and
> ls2x, but the following reasons made the merger difficult to achieve:
> 
> 1. ls1x does not support ACPI, for it is only on MIPS-based system;

This is not a good justification, you have to support both in your
driver anyway, as shown by your CONFIG_ACPI ifdefery.

> 2. ls1x does not support alarm function.

It is just a matter of clearing a single bit, this is not difficult at
all.

> 
> Thanks.
> 
> -------
> Changes since v1:
> 1. Rebased on top of latest loongarch-next;
> 2. Add interrupt descriptions to the ls2k and ls7a DTS files to avoid
> errors when the driver gets the IRQ number, Thanks to Qing Zhang for
> testing;
> 3. Remove some inexact CONFIG_ACPI.
> 
> Binbin Zhou (4):
>   rtc: Add support for the Loongson-2K/LS7A RTC
>   LoongArch: Enable LS2X RTC in loongson3_defconfig
>   MIPS: Loongson64: DTS: Add RTC support to LS7A
>   MIPS: Loongson64: DTS: Add RTC support to Loongson-2K
> 
> WANG Xuerui (3):
>   dt-bindings: rtc: Add Loongson LS2X RTC support
>   MIPS: Loongson: Enable LS2X RTC in loongson3_defconfig
>   MIPS: Loongson: Enable LS2X RTC in loongson2k_defconfig
> 
>  .../devicetree/bindings/rtc/trivial-rtc.yaml  |   2 +
>  arch/loongarch/configs/loongson3_defconfig    |   1 +
>  .../boot/dts/loongson/loongson64-2k1000.dtsi  |   7 +
>  arch/mips/boot/dts/loongson/ls7a-pch.dtsi     |   7 +
>  arch/mips/configs/loongson2k_defconfig        |   1 +
>  arch/mips/configs/loongson3_defconfig         |   1 +
>  drivers/rtc/Kconfig                           |  11 +
>  drivers/rtc/Makefile                          |   1 +
>  drivers/rtc/rtc-ls2x.c                        | 379 ++++++++++++++++++
>  9 files changed, 410 insertions(+)
>  create mode 100644 drivers/rtc/rtc-ls2x.c
> 
> -- 
> 2.31.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH V2 0/7] rtc: ls2x: Add support for the Loongson-2K/LS7A RTC

[Binbin Zhou]

Hi Kelvin:

Excuse me.
I am submitting the Loongson-2K/LS7A RTC driver and Alexandre would
like me to merge the ls1x rtc driver in parallel.
Unfortunately I found out that the loongson-1 does not yet support DT
and would like to ask if you have any plans to support DT?

I think this is the prerequisite for the merge.

Regards.


Binbin




On Tue, Jan 24, 2023 at 7:24 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 09/01/2023 09:35:10+0800, Binbin Zhou wrote:
> > Hi all:
> >
> > The initial DT-base ls2x rtc driver was written by Wang Xuerui, He has
> > released five versions of patchset before, and all related mail records
> > are shown below if you are interested:
> >
> > https://lore.kernel.org/all/?q=ls2x-rtc
> >
> > In this series of patches, based on the code above, I have added the
> > following support:
> >
> > 1. Add ACPI-related support, as Loongson-3A5000 + LS7A is now ACPI-base
> >    by default under LoongArch architecture;
> > 2. Add rtc alarm/walarm related functions.
> >
> > I have tested on Loongson-3A5000LA+LS7A1000/LS7A2000, Loongson-2K1000LA
> > and Loongson-2K0500.
> >
> > BTW:
> > There have been discussions about merging the rtc drivers of ls1x and
> > ls2x, but the following reasons made the merger difficult to achieve:
> >
> > 1. ls1x does not support ACPI, for it is only on MIPS-based system;
>
> This is not a good justification, you have to support both in your
> driver anyway, as shown by your CONFIG_ACPI ifdefery.
>
> > 2. ls1x does not support alarm function.
>
> It is just a matter of clearing a single bit, this is not difficult at
> all.
>
> >
> > Thanks.
> >
> > -------
> > Changes since v1:
> > 1. Rebased on top of latest loongarch-next;
> > 2. Add interrupt descriptions to the ls2k and ls7a DTS files to avoid
> > errors when the driver gets the IRQ number, Thanks to Qing Zhang for
> > testing;
> > 3. Remove some inexact CONFIG_ACPI.
> >
> > Binbin Zhou (4):
> >   rtc: Add support for the Loongson-2K/LS7A RTC
> >   LoongArch: Enable LS2X RTC in loongson3_defconfig
> >   MIPS: Loongson64: DTS: Add RTC support to LS7A
> >   MIPS: Loongson64: DTS: Add RTC support to Loongson-2K
> >
> > WANG Xuerui (3):
> >   dt-bindings: rtc: Add Loongson LS2X RTC support
> >   MIPS: Loongson: Enable LS2X RTC in loongson3_defconfig
> >   MIPS: Loongson: Enable LS2X RTC in loongson2k_defconfig
> >
> >  .../devicetree/bindings/rtc/trivial-rtc.yaml  |   2 +
> >  arch/loongarch/configs/loongson3_defconfig    |   1 +
> >  .../boot/dts/loongson/loongson64-2k1000.dtsi  |   7 +
> >  arch/mips/boot/dts/loongson/ls7a-pch.dtsi     |   7 +
> >  arch/mips/configs/loongson2k_defconfig        |   1 +
> >  arch/mips/configs/loongson3_defconfig         |   1 +
> >  drivers/rtc/Kconfig                           |  11 +
> >  drivers/rtc/Makefile                          |   1 +
> >  drivers/rtc/rtc-ls2x.c                        | 379 ++++++++++++++++++
> >  9 files changed, 410 insertions(+)
> >  create mode 100644 drivers/rtc/rtc-ls2x.c
> >
> > --
> > 2.31.1
> >
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>

Re: [PATCH V2 0/7] rtc: ls2x: Add support for the Loongson-2K/LS7A RTC

[Binbin Zhou]

On Tue, Jan 31, 2023 at 8:59 PM Binbin Zhou <zhoubb.aaron@gmail.com> wrote:
>
> Hi Kelvin:
>
> Excuse me.
> I am submitting the Loongson-2K/LS7A RTC driver and Alexandre would
> like me to merge the ls1x rtc driver in parallel.
> Unfortunately I found out that the loongson-1 does not yet support DT
> and would like to ask if you have any plans to support DT?
>
> I think this is the prerequisite for the merge.
>
> Regards.
> Binbin
>
>

Hi Alexandre:

Unfortunately there has been no reply from Keguang for the past week
or so. Can we try rtc-ls2x and rtc-ls1x to coexist until Loongson-1
supports DT?
Later on, if Keguang or someone else familiar with Loongson-1 adds DT
support, I would be happy to continue trying to merge the two drivers.

Regards.
Binbin

>
>
> On Tue, Jan 24, 2023 at 7:24 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> >
> > On 09/01/2023 09:35:10+0800, Binbin Zhou wrote:
> > > Hi all:
> > >
> > > The initial DT-base ls2x rtc driver was written by Wang Xuerui, He has
> > > released five versions of patchset before, and all related mail records
> > > are shown below if you are interested:
> > >
> > > https://lore.kernel.org/all/?q=ls2x-rtc
> > >
> > > In this series of patches, based on the code above, I have added the
> > > following support:
> > >
> > > 1. Add ACPI-related support, as Loongson-3A5000 + LS7A is now ACPI-base
> > >    by default under LoongArch architecture;
> > > 2. Add rtc alarm/walarm related functions.
> > >
> > > I have tested on Loongson-3A5000LA+LS7A1000/LS7A2000, Loongson-2K1000LA
> > > and Loongson-2K0500.
> > >
> > > BTW:
> > > There have been discussions about merging the rtc drivers of ls1x and
> > > ls2x, but the following reasons made the merger difficult to achieve:
> > >
> > > 1. ls1x does not support ACPI, for it is only on MIPS-based system;
> >
> > This is not a good justification, you have to support both in your
> > driver anyway, as shown by your CONFIG_ACPI ifdefery.
> >
> > > 2. ls1x does not support alarm function.
> >
> > It is just a matter of clearing a single bit, this is not difficult at
> > all.
> >
> > >
> > > Thanks.
> > >
> > > -------
> > > Changes since v1:
> > > 1. Rebased on top of latest loongarch-next;
> > > 2. Add interrupt descriptions to the ls2k and ls7a DTS files to avoid
> > > errors when the driver gets the IRQ number, Thanks to Qing Zhang for
> > > testing;
> > > 3. Remove some inexact CONFIG_ACPI.
> > >
> > > Binbin Zhou (4):
> > >   rtc: Add support for the Loongson-2K/LS7A RTC
> > >   LoongArch: Enable LS2X RTC in loongson3_defconfig
> > >   MIPS: Loongson64: DTS: Add RTC support to LS7A
> > >   MIPS: Loongson64: DTS: Add RTC support to Loongson-2K
> > >
> > > WANG Xuerui (3):
> > >   dt-bindings: rtc: Add Loongson LS2X RTC support
> > >   MIPS: Loongson: Enable LS2X RTC in loongson3_defconfig
> > >   MIPS: Loongson: Enable LS2X RTC in loongson2k_defconfig
> > >
> > >  .../devicetree/bindings/rtc/trivial-rtc.yaml  |   2 +
> > >  arch/loongarch/configs/loongson3_defconfig    |   1 +
> > >  .../boot/dts/loongson/loongson64-2k1000.dtsi  |   7 +
> > >  arch/mips/boot/dts/loongson/ls7a-pch.dtsi     |   7 +
> > >  arch/mips/configs/loongson2k_defconfig        |   1 +
> > >  arch/mips/configs/loongson3_defconfig         |   1 +
> > >  drivers/rtc/Kconfig                           |  11 +
> > >  drivers/rtc/Makefile                          |   1 +
> > >  drivers/rtc/rtc-ls2x.c                        | 379 ++++++++++++++++++
> > >  9 files changed, 410 insertions(+)
> > >  create mode 100644 drivers/rtc/rtc-ls2x.c
> > >
> > > --
> > > 2.31.1
> > >
> >
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> >

Re: [PATCH V2 0/7] rtc: ls2x: Add support for the Loongson-2K/LS7A RTC

[Alexandre Belloni]

On 10/02/2023 18:03:07+0800, Binbin Zhou wrote:
> On Tue, Jan 31, 2023 at 8:59 PM Binbin Zhou <zhoubb.aaron@gmail.com> wrote:
> >
> > Hi Kelvin:
> >
> > Excuse me.
> > I am submitting the Loongson-2K/LS7A RTC driver and Alexandre would
> > like me to merge the ls1x rtc driver in parallel.
> > Unfortunately I found out that the loongson-1 does not yet support DT
> > and would like to ask if you have any plans to support DT?
> >
> > I think this is the prerequisite for the merge.
> >
> > Regards.
> > Binbin
> >
> >
> 
> Hi Alexandre:
> 
> Unfortunately there has been no reply from Keguang for the past week
> or so. Can we try rtc-ls2x and rtc-ls1x to coexist until Loongson-1
> supports DT?
> Later on, if Keguang or someone else familiar with Loongson-1 adds DT
> support, I would be happy to continue trying to merge the two drivers.
> 

My point is exactly that if nobody in Loongson wants to maintain
rtc-ls1x, we may just drop it. This also probably tells a lot about how
rtc-ls2x is going to be maintained once upstreamed...

> Regards.
> Binbin
> 
> >
> >
> > On Tue, Jan 24, 2023 at 7:24 AM Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> > >
> > > On 09/01/2023 09:35:10+0800, Binbin Zhou wrote:
> > > > Hi all:
> > > >
> > > > The initial DT-base ls2x rtc driver was written by Wang Xuerui, He has
> > > > released five versions of patchset before, and all related mail records
> > > > are shown below if you are interested:
> > > >
> > > > https://lore.kernel.org/all/?q=ls2x-rtc
> > > >
> > > > In this series of patches, based on the code above, I have added the
> > > > following support:
> > > >
> > > > 1. Add ACPI-related support, as Loongson-3A5000 + LS7A is now ACPI-base
> > > >    by default under LoongArch architecture;
> > > > 2. Add rtc alarm/walarm related functions.
> > > >
> > > > I have tested on Loongson-3A5000LA+LS7A1000/LS7A2000, Loongson-2K1000LA
> > > > and Loongson-2K0500.
> > > >
> > > > BTW:
> > > > There have been discussions about merging the rtc drivers of ls1x and
> > > > ls2x, but the following reasons made the merger difficult to achieve:
> > > >
> > > > 1. ls1x does not support ACPI, for it is only on MIPS-based system;
> > >
> > > This is not a good justification, you have to support both in your
> > > driver anyway, as shown by your CONFIG_ACPI ifdefery.
> > >
> > > > 2. ls1x does not support alarm function.
> > >
> > > It is just a matter of clearing a single bit, this is not difficult at
> > > all.
> > >
> > > >
> > > > Thanks.
> > > >
> > > > -------
> > > > Changes since v1:
> > > > 1. Rebased on top of latest loongarch-next;
> > > > 2. Add interrupt descriptions to the ls2k and ls7a DTS files to avoid
> > > > errors when the driver gets the IRQ number, Thanks to Qing Zhang for
> > > > testing;
> > > > 3. Remove some inexact CONFIG_ACPI.
> > > >
> > > > Binbin Zhou (4):
> > > >   rtc: Add support for the Loongson-2K/LS7A RTC
> > > >   LoongArch: Enable LS2X RTC in loongson3_defconfig
> > > >   MIPS: Loongson64: DTS: Add RTC support to LS7A
> > > >   MIPS: Loongson64: DTS: Add RTC support to Loongson-2K
> > > >
> > > > WANG Xuerui (3):
> > > >   dt-bindings: rtc: Add Loongson LS2X RTC support
> > > >   MIPS: Loongson: Enable LS2X RTC in loongson3_defconfig
> > > >   MIPS: Loongson: Enable LS2X RTC in loongson2k_defconfig
> > > >
> > > >  .../devicetree/bindings/rtc/trivial-rtc.yaml  |   2 +
> > > >  arch/loongarch/configs/loongson3_defconfig    |   1 +
> > > >  .../boot/dts/loongson/loongson64-2k1000.dtsi  |   7 +
> > > >  arch/mips/boot/dts/loongson/ls7a-pch.dtsi     |   7 +
> > > >  arch/mips/configs/loongson2k_defconfig        |   1 +
> > > >  arch/mips/configs/loongson3_defconfig         |   1 +
> > > >  drivers/rtc/Kconfig                           |  11 +
> > > >  drivers/rtc/Makefile                          |   1 +
> > > >  drivers/rtc/rtc-ls2x.c                        | 379 ++++++++++++++++++
> > > >  9 files changed, 410 insertions(+)
> > > >  create mode 100644 drivers/rtc/rtc-ls2x.c
> > > >
> > > > --
> > > > 2.31.1
> > > >
> > >
> > > --
> > > Alexandre Belloni, co-owner and COO, Bootlin
> > > Embedded Linux and Kernel engineering
> > > https://bootlin.com
> > >

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH V2 0/7] rtc: ls2x: Add support for the Loongson-2K/LS7A RTC

[Kelvin Cheung]

Hi Alexandre,
I will maintain rtc-ls1x continuously.
And I'm adding DT support for it.

On Wed, Feb 15, 2023 at 7:18 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 10/02/2023 18:03:07+0800, Binbin Zhou wrote:
> > On Tue, Jan 31, 2023 at 8:59 PM Binbin Zhou <zhoubb.aaron@gmail.com> wrote:
> > >
> > > Hi Kelvin:
> > >
> > > Excuse me.
> > > I am submitting the Loongson-2K/LS7A RTC driver and Alexandre would
> > > like me to merge the ls1x rtc driver in parallel.
> > > Unfortunately I found out that the loongson-1 does not yet support DT
> > > and would like to ask if you have any plans to support DT?
> > >
> > > I think this is the prerequisite for the merge.
> > >
> > > Regards.
> > > Binbin
> > >
> > >
> >
> > Hi Alexandre:
> >
> > Unfortunately there has been no reply from Keguang for the past week
> > or so. Can we try rtc-ls2x and rtc-ls1x to coexist until Loongson-1
> > supports DT?
> > Later on, if Keguang or someone else familiar with Loongson-1 adds DT
> > support, I would be happy to continue trying to merge the two drivers.
> >
>
> My point is exactly that if nobody in Loongson wants to maintain
> rtc-ls1x, we may just drop it. This also probably tells a lot about how
> rtc-ls2x is going to be maintained once upstreamed...
>
> > Regards.
> > Binbin
> >
> > >
> > >
> > > On Tue, Jan 24, 2023 at 7:24 AM Alexandre Belloni
> > > <alexandre.belloni@bootlin.com> wrote:
> > > >
> > > > On 09/01/2023 09:35:10+0800, Binbin Zhou wrote:
> > > > > Hi all:
> > > > >
> > > > > The initial DT-base ls2x rtc driver was written by Wang Xuerui, He has
> > > > > released five versions of patchset before, and all related mail records
> > > > > are shown below if you are interested:
> > > > >
> > > > > https://lore.kernel.org/all/?q=ls2x-rtc
> > > > >
> > > > > In this series of patches, based on the code above, I have added the
> > > > > following support:
> > > > >
> > > > > 1. Add ACPI-related support, as Loongson-3A5000 + LS7A is now ACPI-base
> > > > >    by default under LoongArch architecture;
> > > > > 2. Add rtc alarm/walarm related functions.
> > > > >
> > > > > I have tested on Loongson-3A5000LA+LS7A1000/LS7A2000, Loongson-2K1000LA
> > > > > and Loongson-2K0500.
> > > > >
> > > > > BTW:
> > > > > There have been discussions about merging the rtc drivers of ls1x and
> > > > > ls2x, but the following reasons made the merger difficult to achieve:
> > > > >
> > > > > 1. ls1x does not support ACPI, for it is only on MIPS-based system;
> > > >
> > > > This is not a good justification, you have to support both in your
> > > > driver anyway, as shown by your CONFIG_ACPI ifdefery.
> > > >
> > > > > 2. ls1x does not support alarm function.
> > > >
> > > > It is just a matter of clearing a single bit, this is not difficult at
> > > > all.
> > > >
> > > > >
> > > > > Thanks.
> > > > >
> > > > > -------
> > > > > Changes since v1:
> > > > > 1. Rebased on top of latest loongarch-next;
> > > > > 2. Add interrupt descriptions to the ls2k and ls7a DTS files to avoid
> > > > > errors when the driver gets the IRQ number, Thanks to Qing Zhang for
> > > > > testing;
> > > > > 3. Remove some inexact CONFIG_ACPI.
> > > > >
> > > > > Binbin Zhou (4):
> > > > >   rtc: Add support for the Loongson-2K/LS7A RTC
> > > > >   LoongArch: Enable LS2X RTC in loongson3_defconfig
> > > > >   MIPS: Loongson64: DTS: Add RTC support to LS7A
> > > > >   MIPS: Loongson64: DTS: Add RTC support to Loongson-2K
> > > > >
> > > > > WANG Xuerui (3):
> > > > >   dt-bindings: rtc: Add Loongson LS2X RTC support
> > > > >   MIPS: Loongson: Enable LS2X RTC in loongson3_defconfig
> > > > >   MIPS: Loongson: Enable LS2X RTC in loongson2k_defconfig
> > > > >
> > > > >  .../devicetree/bindings/rtc/trivial-rtc.yaml  |   2 +
> > > > >  arch/loongarch/configs/loongson3_defconfig    |   1 +
> > > > >  .../boot/dts/loongson/loongson64-2k1000.dtsi  |   7 +
> > > > >  arch/mips/boot/dts/loongson/ls7a-pch.dtsi     |   7 +
> > > > >  arch/mips/configs/loongson2k_defconfig        |   1 +
> > > > >  arch/mips/configs/loongson3_defconfig         |   1 +
> > > > >  drivers/rtc/Kconfig                           |  11 +
> > > > >  drivers/rtc/Makefile                          |   1 +
> > > > >  drivers/rtc/rtc-ls2x.c                        | 379 ++++++++++++++++++
> > > > >  9 files changed, 410 insertions(+)
> > > > >  create mode 100644 drivers/rtc/rtc-ls2x.c
> > > > >
> > > > > --
> > > > > 2.31.1
> > > > >
> > > >
> > > > --
> > > > Alexandre Belloni, co-owner and COO, Bootlin
> > > > Embedded Linux and Kernel engineering
> > > > https://bootlin.com
> > > >
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



-- 
Best regards,

Kelvin Cheung