Skip to contentSkip to conversation

RTC: Add Loongson-2K0300 support

PATCH Superseded Binbin Zhou 4 participants 11 messages Jan 6 – Jan 16

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover RTC: Add Loongson-2K0300 support

Hi all:

This patch set introduces the Loongson-2K0300 RTC, which has a similar hardware design to the Loongson-1B, but without the alarm feature.

Thanks.
Binbin

==========
V2:
Patch (1/3):
- New patch, correct Loongson-1C interrupts property;

Patch (2/3):
- Drop Loongson-1C changes;

Patch (3/3):
- Rename LS1C_RTC_CTRL_WORKAROUND to LOONGSON_RTC_CTRL_WORKAROUND for consistency.

Link to V1:
https://lore.kernel.org/all/[email protected]/

Binbin Zhou (3):
dt-binding: rtc: loongson: Correct Loongson-1C interrupts property
dt-binding: rtc: loongson: Document Loongson-2K0300 compatible
rtc: loongson: Add Loongson-2K0300 support

 .../devicetree/bindings/rtc/loongson,rtc.yaml | 14 ++++
 drivers/rtc/rtc-loongson.c                    | 71 ++++++++++++-------
 2 files changed, 61 insertions(+), 24 deletions(-)

base-commit: 16bd954c93360145bc77cc601e350913fc28182d

Signature · 1 line
2.47.3
#1 lore raw

PATCH v2 1/3 dt-binding: rtc: loongson: Correct Loongson-1C interrupts property

The interrupts property indicates an RTC alarm interrupt, which is only required for RTCs that support the alarm feature.

As we know, the Loongson-1C RTC does not support the alarm feature, so it needs to be excluded.

Fixes 487ef32caebe ("dt-bindings: rtc: Split loongson,ls2x-rtc into SoC-based compatibles")
---
 .../devicetree/bindings/rtc/loongson,rtc.yaml        | 12 ++++++++++++
 1 file changed, 12 insertions(+)
Documentation/devicetree/bindings/rtc/loongson,rtc.yaml +12 -0
index f89c1f660aee..8a2520f963d8 100644
@@ -42,6 +42,18 @@ required:
 
 unevaluatedProperties: false
 
+if:
+  properties:
+    compatible:
+      not:
+        contains:
+          enum:
+            - loongson,ls1c-rtc
+
+then:
+  required:
+    - interrupts
+
 examples:
   - |
     #include <dt-bindings/interrupt-controller/irq.h>
Signature · 1 line
2.47.3
#2 lore raw

PATCH v2 2/3 dt-binding: rtc: loongson: Document Loongson-2K0300 compatible

In reply to Binbin Zhou

Add "loongson,ls2k0300-rtc" dedicated compatible to represent the RTC interface of the Loongson-2K0300 chip.

Its hardware design is similar to that of the Loongson-1B, but it does not support the alarm feature.

---
 Documentation/devicetree/bindings/rtc/loongson,rtc.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/rtc/loongson,rtc.yaml +2 -0
index 8a2520f963d8..b62419c33fd5 100644
@@ -23,6 +23,7 @@ properties:
           - loongson,ls1b-rtc
           - loongson,ls1c-rtc
           - loongson,ls7a-rtc
+          - loongson,ls2k0300-rtc
           - loongson,ls2k1000-rtc
       - items:
           - enum:
@@ -49,6 +50,7 @@ if:
         contains:
           enum:
             - loongson,ls1c-rtc
+            - loongson,ls2k0300-rtc
 
 then:
   required:
Signature · 1 line
2.47.3
maintainer
#3 lore raw

Re: [PATCH v2 2/3] dt-binding: rtc: loongson: Document Loongson-2K0300 compatible

Quoted 5 lines · Binbin Zhou

Add "loongson,ls2k0300-rtc" dedicated compatible to represent the RTC interface of the Loongson-2K0300 chip.

Its hardware design is similar to that of the Loongson-1B, but it does not support the alarm feature.

But you are requiring the interrupt property for it? Isn't it no alarm feature means no interrupt?

Quoted 29 lines
---
 Documentation/devicetree/bindings/rtc/loongson,rtc.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/rtc/loongson,rtc.yaml +2 -0
index 8a2520f963d8..b62419c33fd5 100644
@@ -23,6 +23,7 @@ properties:
           - loongson,ls1b-rtc
           - loongson,ls1c-rtc
           - loongson,ls7a-rtc
+          - loongson,ls2k0300-rtc
           - loongson,ls2k1000-rtc
       - items:
           - enum:
@@ -49,6 +50,7 @@ if:
         contains:
           enum:
             - loongson,ls1c-rtc
+            - loongson,ls2k0300-rtc
 
 then:
   required:
Signature · 1 line
2.47.3
#4 lore raw

Re: [PATCH v2 2/3] dt-binding: rtc: loongson: Document Loongson-2K0300 compatible

Hi Rob:

Thanks for your review.

Quoted 10 lines · Rob Herring
Quoted 5 lines · Binbin Zhou

Add "loongson,ls2k0300-rtc" dedicated compatible to represent the RTC interface of the Loongson-2K0300 chip.

Its hardware design is similar to that of the Loongson-1B, but it does not support the alarm feature.

But you are requiring the interrupt property for it? Isn't it no alarm feature means no interrupt?

Yes, the interrupts attribute is not required without the alarm feature.

But my judgment condition is not contains (added in patch-1[1]). There are only a few SoCs on the Loongson platform that don't support the RTC alarm feature, so I think not contains looks cleaner and simpler.

[1]: https://lore.kernel.org/all/8876bebaf08121bb5edd2500f5289284b75df011.1767663073.git.zhoubinbin@loongson.cn/

Quoted 30 lines
Quoted 30 lines
---
 Documentation/devicetree/bindings/rtc/loongson,rtc.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/rtc/loongson,rtc.yaml +2 -1
index 8a2520f963d8..b62419c33fd5 100644
@@ -23,6 +23,7 @@ properties:
           - loongson,ls1b-rtc
           - loongson,ls1c-rtc
           - loongson,ls7a-rtc
+          - loongson,ls2k0300-rtc
           - loongson,ls2k1000-rtc
       - items:
           - enum:
@@ -49,6 +50,7 @@ if:
         contains:
           enum:
             - loongson,ls1c-rtc
+            - loongson,ls2k0300-rtc
 
 then:
   required:
--

2.47.3

Signature · 2 lines
Thanks.
Binbin
maintainer
#5 lore raw

Re: [PATCH v2 2/3] dt-binding: rtc: loongson: Document Loongson-2K0300 compatible

Quoted 16 lines · Binbin Zhou

Hi Rob:

Thanks for your review.

Quoted 10 lines · Rob Herring
Quoted 5 lines · Binbin Zhou

Add "loongson,ls2k0300-rtc" dedicated compatible to represent the RTC interface of the Loongson-2K0300 chip.

Its hardware design is similar to that of the Loongson-1B, but it does not support the alarm feature.

But you are requiring the interrupt property for it? Isn't it no alarm feature means no interrupt?

Quoted 6 lines

Yes, the interrupts attribute is not required without the alarm feature.

But my judgment condition is not contains (added in patch-1[1]). There are only a few SoCs on the Loongson platform that don't support the RTC alarm feature, so I think not contains looks cleaner and simpler.

I should have said allowing rather than requiring.

You are allowing (though not requiring) 'interrupts' for Loongson-1B and Loongson-2K0300. In patch 1, you made it required for other platforms which is an ABI change. That's fine if it was a mistake and is truly required.

Rob

#6 lore raw

Re: [PATCH v2 2/3] dt-binding: rtc: loongson: Document Loongson-2K0300 compatible

Hi Rob:

Thanks for your reply.

Quoted 25 lines · Rob Herring
Quoted 16 lines · Binbin Zhou

Hi Rob:

Thanks for your review.

Quoted 10 lines · Rob Herring
Quoted 5 lines · Binbin Zhou

Add "loongson,ls2k0300-rtc" dedicated compatible to represent the RTC interface of the Loongson-2K0300 chip.

Its hardware design is similar to that of the Loongson-1B, but it does not support the alarm feature.

But you are requiring the interrupt property for it? Isn't it no alarm feature means no interrupt?

Quoted 6 lines

Yes, the interrupts attribute is not required without the alarm feature.

But my judgment condition is not contains (added in patch-1[1]). There are only a few SoCs on the Loongson platform that don't support the RTC alarm feature, so I think not contains looks cleaner and simpler.

Quoted 6 lines

I should have said allowing rather than requiring.

You are allowing (though not requiring) 'interrupts' for Loongson-1B and Loongson-2K0300. In patch 1, you made it required for other platforms which is an ABI change. That's fine if it was a mistake and is truly required.

Emm, it's true that for the binding interface, Patch-1 is indeed an ABI change, but it's more of a fixed patch.

Throughout all existing Loongson DTS{i}, RTC nodes decide whether to include the interrupts property or not based on the alarm feature. Loongson-1c rtc nodes do not include the interrupts attribute [1], while all other Loongson chip rtc nodes do [2].

So, while this is an ABI change, I don't think it affects existing Loongson DTS{i} rtc nodes. Also, it more accurately describes the features of the corresponding RTC device.

Therefore, I would like to clarify it in the Patch-1 commit message of the next patch version and fix the error in the commit title:
dt-binding -> dt-bindings.

How do you feel about that?

[1]: https://elixir.bootlin.com/linux/v6.18/source/arch/mips/boot/dts/loongson/loongson1c.dtsi#L98
[2]: https://elixir.bootlin.com/linux/v6.18/source/arch/loongarch/boot/dts/loongson-2k0500.dtsi#L486

Quoted 2 lines

Rob

Signature · 2 lines
Thanks.
Binbin
maintainer
#7 lore raw

Re: [PATCH v2 2/3] dt-binding: rtc: loongson: Document Loongson-2K0300 compatible

Quoted 50 lines · Binbin Zhou

Hi Rob:

Thanks for your reply.

Quoted 25 lines · Rob Herring
Quoted 16 lines · Binbin Zhou

Hi Rob:

Thanks for your review.

Quoted 10 lines · Rob Herring
Quoted 5 lines · Binbin Zhou

Add "loongson,ls2k0300-rtc" dedicated compatible to represent the RTC interface of the Loongson-2K0300 chip.

Its hardware design is similar to that of the Loongson-1B, but it does not support the alarm feature.

But you are requiring the interrupt property for it? Isn't it no alarm feature means no interrupt?

Quoted 6 lines

Yes, the interrupts attribute is not required without the alarm feature.

But my judgment condition is not contains (added in patch-1[1]). There are only a few SoCs on the Loongson platform that don't support the RTC alarm feature, so I think not contains looks cleaner and simpler.

Quoted 6 lines

I should have said allowing rather than requiring.

You are allowing (though not requiring) 'interrupts' for Loongson-1B and Loongson-2K0300. In patch 1, you made it required for other platforms which is an ABI change. That's fine if it was a mistake and is truly required.

Emm, it's true that for the binding interface, Patch-1 is indeed an ABI change, but it's more of a fixed patch.

Throughout all existing Loongson DTS{i}, RTC nodes decide whether to include the interrupts property or not based on the alarm feature. Loongson-1c rtc nodes do not include the interrupts attribute [1], while all other Loongson chip rtc nodes do [2].

So, while this is an ABI change, I don't think it affects existing Loongson DTS{i} rtc nodes. Also, it more accurately describes the features of the corresponding RTC device.

Quoted 5 lines

Therefore, I would like to clarify it in the Patch-1 commit message of the next patch version and fix the error in the commit title:
dt-binding -> dt-bindings.

How do you feel about that?

That's fine, but you also need:

else:
properties:
interrupts: false

So that on the 2 platforms without an interrupt(alarm), 'interrupts' is not allowed.

With that, you might as well just drop the 'not' and flip the 'then' and 'else' schemas around.

Rob

#8 lore raw

Re: [PATCH v2 2/3] dt-binding: rtc: loongson: Document Loongson-2K0300 compatible

Hi Rob:

Thanks for your reply.

Quoted 64 lines · Rob Herring
Quoted 50 lines · Binbin Zhou

Hi Rob:

Thanks for your reply.

Quoted 25 lines · Rob Herring
Quoted 16 lines · Binbin Zhou

Hi Rob:

Thanks for your review.

Quoted 10 lines · Rob Herring
Quoted 5 lines · Binbin Zhou

Add "loongson,ls2k0300-rtc" dedicated compatible to represent the RTC interface of the Loongson-2K0300 chip.

Its hardware design is similar to that of the Loongson-1B, but it does not support the alarm feature.

But you are requiring the interrupt property for it? Isn't it no alarm feature means no interrupt?

Quoted 6 lines

Yes, the interrupts attribute is not required without the alarm feature.

But my judgment condition is not contains (added in patch-1[1]). There are only a few SoCs on the Loongson platform that don't support the RTC alarm feature, so I think not contains looks cleaner and simpler.

Quoted 6 lines

I should have said allowing rather than requiring.

You are allowing (though not requiring) 'interrupts' for Loongson-1B and Loongson-2K0300. In patch 1, you made it required for other platforms which is an ABI change. That's fine if it was a mistake and is truly required.

Emm, it's true that for the binding interface, Patch-1 is indeed an ABI change, but it's more of a fixed patch.

Throughout all existing Loongson DTS{i}, RTC nodes decide whether to include the interrupts property or not based on the alarm feature. Loongson-1c rtc nodes do not include the interrupts attribute [1], while all other Loongson chip rtc nodes do [2].

So, while this is an ABI change, I don't think it affects existing Loongson DTS{i} rtc nodes. Also, it more accurately describes the features of the corresponding RTC device.

Quoted 5 lines

Therefore, I would like to clarify it in the Patch-1 commit message of the next patch version and fix the error in the commit title:
dt-binding -> dt-bindings.

How do you feel about that?

That's fine, but you also need:

else:
properties:
interrupts: false

Quoted 5 lines

So that on the 2 platforms without an interrupt(alarm), 'interrupts' is not allowed.

With that, you might as well just drop the 'not' and flip the 'then' and 'else' schemas around.

OK, I'll fall back to the writeup in the v1 patchset as follows:

if:
properties:
compatible:
contains:
enum:
- loongson,ls1c-rtc
- loongson,ls2k0300-rtc

then:
properties:
interrupts: false

Quoted 2 lines

Rob

Signature · 2 lines
Thanks.
Binbin
#9 lore raw

PATCH v2 3/3 rtc: loongson: Add Loongson-2K0300 support

In reply to Binbin Zhou

The Loongson-2K0300's rtc hardware design is similar to that of the Loongson-1B, but it does not support the alarm feature.

Introduce LOONGSON_RTC_ALARM_WORKAROUND, which indicates a chip that does not support the alarm feature, and rewrite the related logic in loongson_rtc_alarm_setting().

---
 drivers/rtc/rtc-loongson.c | 71 +++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 24 deletions(-)
drivers/rtc/rtc-loongson.c +47 -24
index 2ca7ffd5d7a9..066f0644d1c3 100644
@@ -66,7 +66,8 @@
  * According to the LS1C manual, RTC_CTRL and alarm-related registers are not defined.
  * Accessing the relevant registers will cause the system to hang.
  */
-#define LS1C_RTC_CTRL_WORKAROUND	BIT(0)
+#define LOONGSON_RTC_CTRL_WORKAROUND	BIT(0)
+#define LOONGSON_RTC_ALARM_WORKAROUND	BIT(1)
 
 struct loongson_rtc_config {
 	u32 pm_offset;	/* Offset of PM domain, for RTC alarm wakeup */
@@ -89,7 +90,7 @@ static const struct loongson_rtc_config ls1b_rtc_config = {
 
 static const struct loongson_rtc_config ls1c_rtc_config = {
 	.pm_offset = 0,
-	.flags = LS1C_RTC_CTRL_WORKAROUND,
+	.flags = LOONGSON_RTC_CTRL_WORKAROUND | LOONGSON_RTC_ALARM_WORKAROUND,
 };
 
 static const struct loongson_rtc_config generic_rtc_config = {
@@ -97,6 +98,11 @@ static const struct loongson_rtc_config generic_rtc_config = {
 	.flags = 0,
 };
 
+static const struct loongson_rtc_config ls2k0300_rtc_config = {
+	.pm_offset = 0x0,
+	.flags = LOONGSON_RTC_ALARM_WORKAROUND,
+};
+
 static const struct loongson_rtc_config ls2k1000_rtc_config = {
 	.pm_offset = 0x800,
 	.flags = 0,
@@ -153,7 +159,7 @@ static int loongson_rtc_set_enabled(struct device *dev)
 {
 	struct loongson_rtc_priv *priv = dev_get_drvdata(dev);
 
-	if (priv->config->flags & LS1C_RTC_CTRL_WORKAROUND)
+	if (priv->config->flags & LOONGSON_RTC_CTRL_WORKAROUND)
 		return 0;
 
 	/* Enable RTC TOY counters and crystal */
@@ -167,7 +173,7 @@ static bool loongson_rtc_get_enabled(struct device *dev)
 	u32 ctrl_data;
 	struct loongson_rtc_priv *priv = dev_get_drvdata(dev);
 
-	if (priv->config->flags & LS1C_RTC_CTRL_WORKAROUND)
+	if (priv->config->flags & LOONGSON_RTC_CTRL_WORKAROUND)
 		return true;
 
 	ret = regmap_read(priv->regmap, RTC_CTRL_REG, &ctrl_data);
@@ -299,9 +305,41 @@ static const struct rtc_class_ops loongson_rtc_ops = {
 	.alarm_irq_enable = loongson_rtc_alarm_irq_enable,
 };
 
+static int loongson_rtc_alarm_setting(struct platform_device *pdev, void __iomem *regs)
+{
+	int ret = 0, alarm_irq;
+	struct device *dev = &pdev->dev;
+	struct loongson_rtc_priv *priv = dev_get_drvdata(dev);
+
+	if (priv->config->flags & LOONGSON_RTC_ALARM_WORKAROUND) {
+		/* Loongson-1C/Loongson-2K0300 RTC does not support alarm */
+		clear_bit(RTC_FEATURE_ALARM, priv->rtcdev->features);
+		return 0;
+	}
+
+	/* Get RTC alarm irq */
+	alarm_irq = platform_get_irq(pdev, 0);
+	if (alarm_irq < 0)
+		return alarm_irq;
+
+	ret = devm_request_irq(dev, alarm_irq, loongson_rtc_isr, 0, "loongson-alarm",
+			       priv);
+	if (ret < 0)
+		return ret;
+
+	priv->pm_base = regs - priv->config->pm_offset;
+	device_init_wakeup(dev, true);
+
+	if (has_acpi_companion(dev))
+		acpi_install_fixed_event_handler(ACPI_EVENT_RTC,
+						 loongson_rtc_handler, priv);
+
+	return ret;
+}
+
 static int loongson_rtc_probe(struct platform_device *pdev)
 {
-	int ret, alarm_irq;
+	int ret;
 	void __iomem *regs;
 	struct loongson_rtc_priv *priv;
 	struct device *dev = &pdev->dev;
@@ -330,25 +368,9 @@ static int loongson_rtc_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(priv->rtcdev),
 				     "devm_rtc_allocate_device failed\n");
 
-	/* Get RTC alarm irq */
-	alarm_irq = platform_get_irq(pdev, 0);
-	if (alarm_irq > 0) {
-		ret = devm_request_irq(dev, alarm_irq, loongson_rtc_isr,
-				       0, "loongson-alarm", priv);
-		if (ret < 0)
-			return dev_err_probe(dev, ret, "Unable to request irq %d\n",
-					     alarm_irq);
-
-		priv->pm_base = regs - priv->config->pm_offset;
-		device_init_wakeup(dev, true);
-
-		if (has_acpi_companion(dev))
-			acpi_install_fixed_event_handler(ACPI_EVENT_RTC,
-							 loongson_rtc_handler, priv);
-	} else {
-		/* Loongson-1C RTC does not support alarm */
-		clear_bit(RTC_FEATURE_ALARM, priv->rtcdev->features);
-	}
+	ret = loongson_rtc_alarm_setting(pdev, regs);
+	if (ret)
+		return ret;
 
 	/* Loongson RTC does not support UIE */
 	clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, priv->rtcdev->features);
@@ -379,6 +401,7 @@ static const struct of_device_id loongson_rtc_of_match[] = {
 	{ .compatible = "loongson,ls1b-rtc", .data = &ls1b_rtc_config },
 	{ .compatible = "loongson,ls1c-rtc", .data = &ls1c_rtc_config },
 	{ .compatible = "loongson,ls7a-rtc", .data = &generic_rtc_config },
+	{ .compatible = "loongson,ls2k0300-rtc", .data = &ls2k0300_rtc_config },
 	{ .compatible = "loongson,ls2k1000-rtc", .data = &ls2k1000_rtc_config },
 	{ /* sentinel */ }
 };
Signature · 1 line
2.47.3
maintainer Reviewed-by on the v2 series
#10 lore raw
In reply to Binbin Zhou

For the whole series:

Quoted 38 lines · Binbin Zhou

Hi all:

This patch set introduces the Loongson-2K0300 RTC, which has a similar hardware design to the Loongson-1B, but without the alarm feature.

Thanks.
Binbin

==========
V2:
Patch (1/3):
- New patch, correct Loongson-1C interrupts property;

Patch (2/3):
- Drop Loongson-1C changes;

Patch (3/3):
- Rename LS1C_RTC_CTRL_WORKAROUND to LOONGSON_RTC_CTRL_WORKAROUND for consistency.

Link to V1:
https://lore.kernel.org/all/[email protected]/

Binbin Zhou (3):
dt-binding: rtc: loongson: Correct Loongson-1C interrupts property
dt-binding: rtc: loongson: Document Loongson-2K0300 compatible
rtc: loongson: Add Loongson-2K0300 support

 .../devicetree/bindings/rtc/loongson,rtc.yaml | 14 ++++
 drivers/rtc/rtc-loongson.c                    | 71 ++++++++++++-------
 2 files changed, 61 insertions(+), 24 deletions(-)

base-commit: 16bd954c93360145bc77cc601e350913fc28182d --
2.47.3