rtc: ls2x: Add support for the Loongson-2K/LS7A RTC

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

Signed-off-by WANG Xuerui <[email protected]>

Signed-off-by Binbin Zhou <[email protected]>

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

drivers/rtc/Kconfig +11 -0

index bb63edb507da..f8586aa00fce 100644
@@ -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

drivers/rtc/Makefile +1 -0

index aab22bc63432..d5a467e9eec8 100644
@@ -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

drivers/rtc/rtc-ls2x.c new file +379 -0

new file mode 100644
index 000000000000..06ef249a9485
@@ -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 <[email protected]>.
+ *
+ * Maintained out-of-tree by Huacai Chen <[email protected]>.
+ * Rewritten for mainline by WANG Xuerui <[email protected]>.
+ *                           Binbin Zhou <[email protected]>
+ */
+
+#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 <[email protected]>");
+MODULE_AUTHOR("Huacai Chen <[email protected]>");
+MODULE_AUTHOR("Binbin Zhou <[email protected]>");
+MODULE_LICENSE("GPL");

Signature · 1 line

2.31.1

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

drivers/rtc/rtc-ls2x.c new file +15 -0

+#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);
+

drivers/rtc/rtc-ls2x.c new file +28 -0

+					 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);

drivers/rtc/rtc-ls2x.c new file +17 -0

+}
+
+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;

drivers/rtc/rtc-ls2x.c new file +34 -0

+	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;
+}

drivers/rtc/rtc-ls2x.c new file +79 -0

+	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.
+	 *

drivers/rtc/rtc-ls2x.c new file +42 -0

+
+	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 <[email protected]>");
+MODULE_AUTHOR("Huacai Chen <[email protected]>");
+MODULE_AUTHOR("Binbin Zhou <[email protected]>");
+MODULE_LICENSE("GPL");

Signature · 1 line

2.31.1

Hello,

Quoted 4 lines · 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.

Quoted 3 lines

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

Quoted 22 lines

drivers/rtc/rtc-ls2x.c new file +20 -0

+#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.

drivers/rtc/rtc-ls2x.c line 122

Quoted 5 lines

drivers/rtc/rtc-ls2x.c new file +5 -0

+      * 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.

Review

Quoted 3 lines

(quoted diff) +2 -0

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

drivers/rtc/rtc-ls2x.c line 129

Quoted 5 lines

drivers/rtc/rtc-ls2x.c new file +5 -0

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

Quoted 33 lines

drivers/rtc/rtc-ls2x.c new file +32 -0

+                                      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

drivers/rtc/rtc-ls2x.c line 167

Quoted 6 lines

drivers/rtc/rtc-ls2x.c new file +6 -0

+      * 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?

Quoted 18 lines

drivers/rtc/rtc-ls2x.c new file +17 -0

+}
+
+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;

drivers/rtc/rtc-ls2x.c line 194

Quoted 10 lines

drivers/rtc/rtc-ls2x.c new file +10 -0

+     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.
"

Quoted 35 lines

drivers/rtc/rtc-ls2x.c new file +34 -0

+     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;
+}

drivers/rtc/rtc-ls2x.c line 238

Quoted 10 lines

drivers/rtc/rtc-ls2x.c new file +10 -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

Quoted 83 lines

drivers/rtc/rtc-ls2x.c new file +83 -0

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

drivers/rtc/rtc-ls2x.c line 327

Quoted 6 lines

drivers/rtc/rtc-ls2x.c new file +6 -0

+     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?

drivers/rtc/rtc-ls2x.c line 337

Quoted 11 lines

drivers/rtc/rtc-ls2x.c new file +10 -0

+
+#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.

Quoted 44 lines

drivers/rtc/rtc-ls2x.c new file +42 -1

+
+     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 <[email protected]>");
+MODULE_AUTHOR("Huacai Chen <[email protected]>");
+MODULE_AUTHOR("Binbin Zhou <[email protected]>");
+MODULE_LICENSE("GPL");
--

Quoted 2 lines

2.31.1

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

Hi Alexandre:

Sorry for the late reply.

Quoted 9 lines · Alexandre Belloni

Hello,

Quoted 4 lines · 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.

Quoted 5 lines

Quoted 3 lines

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.

Quoted 22 lines

Quoted 22 lines

drivers/rtc/rtc-ls2x.c new file +20 -0

+#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.

Quoted 8 lines

drivers/rtc/rtc-ls2x.c line 122

Quoted 5 lines

drivers/rtc/rtc-ls2x.c new file +5 -0

+      * 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.

Quoted 7 lines

Review

Quoted 3 lines

(quoted diff) +2 -0

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

Quoted 8 lines

drivers/rtc/rtc-ls2x.c line 129

Quoted 5 lines

drivers/rtc/rtc-ls2x.c new file +5 -0

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

Quoted 33 lines

Quoted 33 lines

drivers/rtc/rtc-ls2x.c new file +32 -0

+                                      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

Quoted 8 lines

drivers/rtc/rtc-ls2x.c line 167

Quoted 6 lines

drivers/rtc/rtc-ls2x.c new file +6 -0

+      * 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?

Quoted 31 lines

Quoted 18 lines

drivers/rtc/rtc-ls2x.c new file +17 -0

+}
+
+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;

drivers/rtc/rtc-ls2x.c line 194

Quoted 10 lines

drivers/rtc/rtc-ls2x.c new file +10 -0

+     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:

Quoted 11 lines

"
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);

Quoted 131 lines

Quoted 36 lines

drivers/rtc/rtc-ls2x.c new file +34 -0

+     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;
+}

drivers/rtc/rtc-ls2x.c line 238

Quoted 10 lines

drivers/rtc/rtc-ls2x.c new file +10 -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

Quoted 83 lines

drivers/rtc/rtc-ls2x.c new file +83 -0

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

Quoted 8 lines

drivers/rtc/rtc-ls2x.c line 327

Quoted 6 lines

drivers/rtc/rtc-ls2x.c new file +6 -0

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

Quoted 15 lines

drivers/rtc/rtc-ls2x.c line 337

Quoted 11 lines

drivers/rtc/rtc-ls2x.c new file +10 -0

+
+#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?

Quoted 113 lines · Binbin Zhou

Hi Alexandre:

Sorry for the late reply.

Quoted 9 lines · Alexandre Belloni

Hello,

Quoted 4 lines · 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.

Quoted 5 lines

Quoted 3 lines

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.

Quoted 22 lines

Quoted 22 lines

drivers/rtc/rtc-ls2x.c new file +20 -0

+#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.

Quoted 8 lines

drivers/rtc/rtc-ls2x.c line 122

Quoted 5 lines

drivers/rtc/rtc-ls2x.c new file +5 -0

+      * 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.

Quoted 7 lines

Review

Quoted 3 lines

(quoted diff) +2 -0

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

Quoted 8 lines

drivers/rtc/rtc-ls2x.c line 129

Quoted 5 lines

drivers/rtc/rtc-ls2x.c new file +5 -0

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

Quoted 41 lines

Quoted 29 lines

drivers/rtc/rtc-ls2x.c new file +28 -0

+                                      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);

drivers/rtc/rtc-ls2x.c line 167

Quoted 10 lines

drivers/rtc/rtc-ls2x.c new file +10 -0

+     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?

Quoted 4 lines

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.

Quoted 59 lines

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

Quoted 31 lines

Quoted 18 lines

drivers/rtc/rtc-ls2x.c new file +17 -0

+}
+
+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;

drivers/rtc/rtc-ls2x.c line 194

Quoted 10 lines

drivers/rtc/rtc-ls2x.c new file +10 -0

+     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:

Quoted 11 lines

"
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);

Quoted 3 lines

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.

Quoted 140 lines

Quoted 139 lines

Quoted 36 lines

drivers/rtc/rtc-ls2x.c new file +34 -0

+     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;
+}

drivers/rtc/rtc-ls2x.c line 238

Quoted 10 lines

drivers/rtc/rtc-ls2x.c new file +10 -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

Quoted 79 lines

drivers/rtc/rtc-ls2x.c new file +79 -0

+     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.
+      *

drivers/rtc/rtc-ls2x.c line 327

Quoted 10 lines

drivers/rtc/rtc-ls2x.c new file +10 -0

+      * 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?

Quoted 3 lines

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.

Quoted 16 lines

Quoted 15 lines

drivers/rtc/rtc-ls2x.c line 337

Quoted 11 lines

drivers/rtc/rtc-ls2x.c new file +10 -0

+
+#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.

Quoted 4 lines

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

Quoted 20 lines

Quoted 20 lines

Quoted 19 lines

Quoted 7 lines

drivers/rtc/rtc-ls2x.c new file +7 -0

+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);

drivers/rtc/rtc-ls2x.c line 167

Quoted 10 lines

drivers/rtc/rtc-ls2x.c new file +10 -0

+     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?

Quoted 9 lines

Quoted 4 lines

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.

Quoted 17 lines

Quoted 16 lines

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);

Quoted 4 lines

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?

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

Signed-off-by Binbin Zhou <[email protected]>

---
 arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi +7 -0

index 8143a61111e3..c22414595140 100644
@@ -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>,

Signature · 1 line

2.31.1

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

Signature · 1 line

2.31.1

Quoted 15 lines · 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.

Quoted 8 lines

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.

Quoted 1 line

2. ls1x does not support alarm function.

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

Quoted 34 lines

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

Quoted 3 lines

--
2.31.1

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

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

Quoted 77 lines · Alexandre Belloni

Quoted 15 lines · 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.

Quoted 8 lines

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.

Quoted 1 line

2. ls1x does not support alarm function.

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

Quoted 34 lines

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

Quoted 3 lines

--
2.31.1

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

Quoted 9 lines · 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?

Quoted 6 lines

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

Quoted 81 lines

Quoted 77 lines · Alexandre Belloni

Quoted 15 lines · 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.

Quoted 8 lines

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.

Quoted 1 line

2. ls1x does not support alarm function.

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

Quoted 34 lines

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

Quoted 3 lines

--
2.31.1

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

Quoted 17 lines · Binbin Zhou

Quoted 9 lines · 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?

Quoted 6 lines

I think this is the prerequisite for the merge.

Regards.
Binbin

Quoted 8 lines

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

Quoted 84 lines

Regards.
Binbin

Quoted 81 lines

Quoted 77 lines · Alexandre Belloni

Quoted 15 lines · 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.

Quoted 8 lines

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.

Quoted 1 line

2. ls1x does not support alarm function.

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

Quoted 34 lines

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

Quoted 3 lines

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