[PATCH 0/5] RTC: update APM X-Gene RTC driver by adding the pre-scaler support

[PATCH 0/5] RTC: update APM X-Gene RTC driver by adding the pre-scaler support

[wefu]

From: Wei Fu <wefu@redhat.com>

This patch add Counter Prescaler support in APM X-Gene RTC driver by
getting prescaler (Optional) property value from dtb. 

This hardware support can only be found in DW_apb_rtc after v2.06a.

And adds the optional property "prescaler" in bindings for APM X-Gene RTC.
Because the clock source on some platform to RTC is NOT 1HZ,
so we need to prescale the clock to make the input clock become 1HZ,
like (32K/prescaler) = 1HZ on the XuanTie TH1520 AP sub-system RTC.

This patch also add "snps,dw-apb-rtc" into the "compatible".

Since XuanTie TH1520 is using the same RTC, so this patch enables 
APM X-Gene RTC driver for XuanTie TH1520, and add a device node in 
the XuanTie TH1520 Soc dts: th1520.dtsi.

Wei Fu (5):
  drivers/rtc/rtc-xgene: Add prescaler support in APM X-Gene RTC driver
  dt-bindings: rtc: Add optional property "prescaler" in APM X-Gene RTC
    Document
  drivers/rtc/rtc-xgene: Add "snps,dw-apb-rtc" into the "compatible"
  Kconfig: Enable APM X-Gene RTC for XuanTie TH1520
  riscv: dts: thead: Add XuanTie TH1520 RTC device node

 .../devicetree/bindings/rtc/xgene-rtc.txt     | 16 ++++++++++
 arch/riscv/boot/dts/thead/th1520.dtsi         | 11 +++++++
 drivers/rtc/Kconfig                           | 12 ++++++-
 drivers/rtc/rtc-xgene.c                       | 32 +++++++++++++++++++
 4 files changed, 70 insertions(+), 1 deletion(-)

-- 
2.44.0

[PATCH 1/5] drivers/rtc/rtc-xgene: Add prescaler support in APM X-Gene RTC driver

[wefu]

From: Wei Fu <wefu@redhat.com>

This patch add Counter Prescaler support in APM X-Gene RTC driver by
getting prescaler (Optional) property value from dtb.

Signed-off-by: Wei Fu <wefu@redhat.com>
---
 drivers/rtc/Kconfig     | 10 ++++++++++
 drivers/rtc/rtc-xgene.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index c63e32d012f2..3a89f1e6095d 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1888,6 +1888,16 @@ config RTC_DRV_XGENE
 	  This driver can also be built as a module, if so, the module
 	  will be called "rtc-xgene".
 
+config RTC_DRV_XGENE_PRESCALER
+	bool "Pre-scaler Counter support for APM X-Gene RTC driver"
+	depends on RTC_DRV_XGENE
+	depends on ARCH_THEAD
+	default y
+	help
+	  Say Y here if your Soc has Pre-scaler Counter support on rtc-xgene.
+
+	  This hardware support can only be found in DW_apb_rtc after v2.06a.
+
 config RTC_DRV_PIC32
 	tristate "Microchip PIC32 RTC"
 	depends on MACH_PIC32
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index f78efc9760c0..4d6f1629b973 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -26,11 +26,13 @@
 #define  RTC_CCR_MASK		BIT(1)
 #define  RTC_CCR_EN		BIT(2)
 #define  RTC_CCR_WEN		BIT(3)
+#define  RTC_CCR_PSCLR_EN	BIT(4)
 #define RTC_STAT		0x10
 #define  RTC_STAT_BIT		BIT(0)
 #define RTC_RSTAT		0x14
 #define RTC_EOI			0x18
 #define RTC_VER			0x1C
+#define RTC_CPSR		0x20
 
 struct xgene_rtc_dev {
 	struct rtc_device *rtc;
@@ -40,6 +42,33 @@ struct xgene_rtc_dev {
 	unsigned int irq_enabled;
 };
 
+static void xgene_rtc_set_prescaler(struct device *dev)
+{
+#ifdef CONFIG_RTC_DRV_XGENE_PRESCALER
+	u32 ccr;
+	u32 prescaler;
+	struct xgene_rtc_dev *pdata = dev_get_drvdata(dev);
+
+	if (device_property_read_u32(dev, "prescaler", &prescaler)) {
+		dev_warn(dev, "Missing the pre-scaler config for RTC.\n");
+		dev_warn(dev, "The current pre-scaler config is 0x%x.\n",
+			 readl(pdata->csr_base + RTC_CPSR));
+		return;
+	}
+
+	/* The clock source on some platform to RTC is NOT 1HZ,
+	 * so we need to prescale the clock to make the input clock become 1HZ,
+	 * like (clock_source/prescaler) = 1HZ
+	 */
+	writel(prescaler, pdata->csr_base + RTC_CPSR);
+
+	/* enable RTC Prescaler feature in CCR register */
+	ccr = readl(pdata->csr_base + RTC_CCR);
+	ccr |= RTC_CCR_PSCLR_EN;
+	writel(ccr, pdata->csr_base + RTC_CCR);
+#endif /* CONFIG_RTC_DRV_XGENE_PRESCALER */
+}
+
 static int xgene_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct xgene_rtc_dev *pdata = dev_get_drvdata(dev);
@@ -174,6 +203,8 @@ static int xgene_rtc_probe(struct platform_device *pdev)
 	/* Turn on the clock and the crystal */
 	writel(RTC_CCR_EN, pdata->csr_base + RTC_CCR);
 
+	xgene_rtc_set_prescaler(&pdev->dev);
+
 	ret = device_init_wakeup(&pdev->dev, 1);
 	if (ret) {
 		clk_disable_unprepare(pdata->clk);
-- 
2.44.0

Re: [PATCH 1/5] drivers/rtc/rtc-xgene: Add prescaler support in APM X-Gene RTC driver

[Alexandre Belloni]

On 12/04/2024 16:01:43+0800, wefu@redhat.com wrote:
> From: Wei Fu <wefu@redhat.com>
> 
> This patch add Counter Prescaler support in APM X-Gene RTC driver by
> getting prescaler (Optional) property value from dtb.
> 
> Signed-off-by: Wei Fu <wefu@redhat.com>
> ---
>  drivers/rtc/Kconfig     | 10 ++++++++++
>  drivers/rtc/rtc-xgene.c | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index c63e32d012f2..3a89f1e6095d 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1888,6 +1888,16 @@ config RTC_DRV_XGENE
>  	  This driver can also be built as a module, if so, the module
>  	  will be called "rtc-xgene".
>  
> +config RTC_DRV_XGENE_PRESCALER
> +	bool "Pre-scaler Counter support for APM X-Gene RTC driver"
> +	depends on RTC_DRV_XGENE
> +	depends on ARCH_THEAD
> +	default y
> +	help
> +	  Say Y here if your Soc has Pre-scaler Counter support on rtc-xgene.
> +
> +	  This hardware support can only be found in DW_apb_rtc after v2.06a.
> +

I don't feel like this needs a config option, rather you should have a
proper compatible string that will tell the driver whether the feature
is available.


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

[PATCH 2/5] dt-bindings: rtc: Add optional property "prescaler" in APM X-Gene RTC Document

[wefu]

From: Wei Fu <wefu@redhat.com>

Add optional property "prescaler" for APM X-Gene RTC.
The clock source on some platform to RTC is NOT 1HZ,
so we need to prescale the clock to make the input clock become 1HZ,
like (32K/prescaler) = 1HZ on the XuanTie TH1520 AP sub-system RTC.

Signed-off-by: Wei Fu <wefu@redhat.com>
---
 .../devicetree/bindings/rtc/xgene-rtc.txt        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/xgene-rtc.txt b/Documentation/devicetree/bindings/rtc/xgene-rtc.txt
index fd195c358446..25ba8cf0cc31 100644
--- a/Documentation/devicetree/bindings/rtc/xgene-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/xgene-rtc.txt
@@ -10,6 +10,9 @@ Required properties:
 - #clock-cells: Should be 1.
 - clocks: Reference to the clock entry.
 
+Optional properties:
+- prescaler: Reference to the Value of Counter Prescaler.
+
 Example:
 
 rtcclk: rtcclk {
@@ -26,3 +29,16 @@ rtc: rtc@10510000 {
 	#clock-cells = <1>;
 	clocks = <&rtcclk 0>;
 };
+
+Example XuanTie TH1520 RTC node with Counter Prescaler(prescaler):
+
+rtc: rtc@fffff40000 {
+	compatible = "snps,dw-apb-rtc";
+	reg = <0xff 0xfff40000 0x0 0x1000>;
+	interrupts = <74 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&osc_32k>;
+	clock-names = "osc_32k";
+	wakeup-source;
+	prescaler = <0x8000>;
+	status = "okay";
+};
-- 
2.44.0

Re: [PATCH 2/5] dt-bindings: rtc: Add optional property "prescaler" in APM X-Gene RTC Document

[Alexandre Belloni]

On 12/04/2024 16:01:44+0800, wefu@redhat.com wrote:
> From: Wei Fu <wefu@redhat.com>
> 
> Add optional property "prescaler" for APM X-Gene RTC.
> The clock source on some platform to RTC is NOT 1HZ,
> so we need to prescale the clock to make the input clock become 1HZ,
> like (32K/prescaler) = 1HZ on the XuanTie TH1520 AP sub-system RTC.
> 
> Signed-off-by: Wei Fu <wefu@redhat.com>
> ---
>  .../devicetree/bindings/rtc/xgene-rtc.txt        | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/xgene-rtc.txt b/Documentation/devicetree/bindings/rtc/xgene-rtc.txt
> index fd195c358446..25ba8cf0cc31 100644
> --- a/Documentation/devicetree/bindings/rtc/xgene-rtc.txt
> +++ b/Documentation/devicetree/bindings/rtc/xgene-rtc.txt
> @@ -10,6 +10,9 @@ Required properties:
>  - #clock-cells: Should be 1.
>  - clocks: Reference to the clock entry.
>  
> +Optional properties:
> +- prescaler: Reference to the Value of Counter Prescaler.

What about getting the input clock rate and calculate the needed
prescaler instead of having it in the device tree?

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

[PATCH 3/5] drivers/rtc/rtc-xgene: Add "snps,dw-apb-rtc" into the "compatible"

[wefu]

From: Wei Fu <wefu@redhat.com>

This patch add "snps,dw-apb-rtc" into the "compatible".

Signed-off-by: Wei Fu <wefu@redhat.com>
---
 drivers/rtc/rtc-xgene.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index 4d6f1629b973..5163682f9816 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -287,6 +287,7 @@ static SIMPLE_DEV_PM_OPS(xgene_rtc_pm_ops, xgene_rtc_suspend, xgene_rtc_resume);
 #ifdef CONFIG_OF
 static const struct of_device_id xgene_rtc_of_match[] = {
 	{.compatible = "apm,xgene-rtc" },
+	{.compatible = "snps,dw-apb-rtc" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, xgene_rtc_of_match);
-- 
2.44.0

Re: [PATCH 3/5] drivers/rtc/rtc-xgene: Add "snps,dw-apb-rtc" into the "compatible"

[Krzysztof Kozlowski]

On 12/04/2024 10:01, wefu@redhat.com wrote:
> From: Wei Fu <wefu@redhat.com>
> 
> This patch add "snps,dw-apb-rtc" into the "compatible".


Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

This we see from the patch. Say what hardware are you adding?

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.


Best regards,
Krzysztof

Re: [PATCH 3/5] drivers/rtc/rtc-xgene: Add "snps,dw-apb-rtc" into the "compatible"

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

On Fri, Apr 12, 2024 at 11:17:38AM +0200, Krzysztof Kozlowski wrote:
> On 12/04/2024 10:01, wefu@redhat.com wrote:
> > From: Wei Fu <wefu@redhat.com>
> > 
> > This patch add "snps,dw-apb-rtc" into the "compatible".
> 
> 
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> 
> This we see from the patch. Say what hardware are you adding?

The compatible, unless I misread the patchset, isn't actually documented
either, only the new property is.

> 
> Please run scripts/checkpatch.pl and fix reported warnings. Then please
> run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
> Some warnings can be ignored, especially from --strict run, but the code
> here looks like it needs a fix. Feel free to get in touch if the warning
> is not clear.
> 
> 
> Best regards,
> Krzysztof
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 4/5] Kconfig: Enable APM X-Gene RTC for XuanTie TH1520

[wefu]

From: Wei Fu <wefu@redhat.com>

This patch enables APM X-Gene RTC for XuanTie TH1520.

Signed-off-by: Wei Fu <wefu@redhat.com>
---
 drivers/rtc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 3a89f1e6095d..b219aeef4ce9 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1880,7 +1880,7 @@ config RTC_DRV_MT7622
 config RTC_DRV_XGENE
 	tristate "APM X-Gene RTC"
 	depends on HAS_IOMEM
-	depends on ARCH_XGENE || COMPILE_TEST
+	depends on ARCH_XGENE || ARCH_THEAD || COMPILE_TEST
 	help
 	  If you say yes here you get support for the APM X-Gene SoC real time
 	  clock.
-- 
2.44.0

Re: [PATCH 4/5] Kconfig: Enable APM X-Gene RTC for XuanTie TH1520

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

On Fri, Apr 12, 2024 at 04:01:46PM +0800, wefu@redhat.com wrote:
> From: Wei Fu <wefu@redhat.com>
> 
> This patch enables APM X-Gene RTC for XuanTie TH1520.
> 
> Signed-off-by: Wei Fu <wefu@redhat.com>
> ---
>  drivers/rtc/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 3a89f1e6095d..b219aeef4ce9 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1880,7 +1880,7 @@ config RTC_DRV_MT7622
>  config RTC_DRV_XGENE
>  	tristate "APM X-Gene RTC"
>  	depends on HAS_IOMEM
> -	depends on ARCH_XGENE || COMPILE_TEST
> +	depends on ARCH_XGENE || ARCH_THEAD || COMPILE_TEST
>  	help
>  	  If you say yes here you get support for the APM X-Gene SoC real time
>  	  clock.

If I was configuring my system by reading menuconfig, I would have
absolutely no idea that this driver supports platforms other than the
X-Gene one. I think the Kconfig stuff for this likely needs an update to
convey that it's no longer just one SoC family that's supported here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 5/5] riscv: dts: thead: Add XuanTie TH1520 RTC device node

[wefu]

From: Wei Fu <wefu@redhat.com>

Add nodes for the XuanTie TH1520 RTC device node on the XuanTie TH1520 Soc.

Signed-off-by: Wei Fu <wefu@redhat.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index cd6bc89a240c..62e588dbc942 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -463,6 +463,17 @@ timer7: timer@ffffc3303c {
 			status = "disabled";
 		};
 
+		rtc: rtc@fffff40000 {
+			compatible = "snps,dw-apb-rtc";
+			reg = <0xff 0xfff40000 0x0 0x1000>;
+			interrupts = <74 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&osc_32k>;
+			clock-names = "osc_32k";
+			wakeup-source;
+			prescaler = <0x8000>;
+			status = "okay";
+		};
+
 		gpio@fffff41000 {
 			compatible = "snps,dw-apb-gpio";
 			reg = <0xff 0xfff41000 0x0 0x1000>;
-- 
2.44.0

Re: [PATCH 5/5] riscv: dts: thead: Add XuanTie TH1520 RTC device node

[Alexandre Belloni]

On 12/04/2024 16:01:47+0800, wefu@redhat.com wrote:
> From: Wei Fu <wefu@redhat.com>
> 
> Add nodes for the XuanTie TH1520 RTC device node on the XuanTie TH1520 Soc.
> 
> Signed-off-by: Wei Fu <wefu@redhat.com>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index cd6bc89a240c..62e588dbc942 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -463,6 +463,17 @@ timer7: timer@ffffc3303c {
>  			status = "disabled";
>  		};
>  
> +		rtc: rtc@fffff40000 {
> +			compatible = "snps,dw-apb-rtc";
> +			reg = <0xff 0xfff40000 0x0 0x1000>;
> +			interrupts = <74 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&osc_32k>;
> +			clock-names = "osc_32k";
> +			wakeup-source;

The wakeup-source and interrupts properties are mutually exclusive.

> +			prescaler = <0x8000>;
> +			status = "okay";
> +		};
> +
>  		gpio@fffff41000 {
>  			compatible = "snps,dw-apb-gpio";
>  			reg = <0xff 0xfff41000 0x0 0x1000>;
> -- 
> 2.44.0
> 

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

Re: [PATCH 5/5] riscv: dts: thead: Add XuanTie TH1520 RTC device node

[Krzysztof Kozlowski]

On 12/04/2024 10:01, wefu@redhat.com wrote:
> From: Wei Fu <wefu@redhat.com>
> 
> Add nodes for the XuanTie TH1520 RTC device node on the XuanTie TH1520 Soc.
> 
> Signed-off-by: Wei Fu <wefu@redhat.com>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index cd6bc89a240c..62e588dbc942 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -463,6 +463,17 @@ timer7: timer@ffffc3303c {
>  			status = "disabled";
>  		};
>  
> +		rtc: rtc@fffff40000 {
> +			compatible = "snps,dw-apb-rtc";

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

> +			reg = <0xff 0xfff40000 0x0 0x1000>;
> +			interrupts = <74 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&osc_32k>;
> +			clock-names = "osc_32k";
> +			wakeup-source;
> +			prescaler = <0x8000>;

What is 0x8000? Why is it in hex if this divides the clock? Clock is in
Hz and we all operate on decimal units.

> +			status = "okay";

Drop, please don't upstream directly downstream code.

> +		};
> +
>  		gpio@fffff41000 {
>  			compatible = "snps,dw-apb-gpio";
>  			reg = <0xff 0xfff41000 0x0 0x1000>;

Best regards,
Krzysztof