Skip to contentSkip to conversation

ARM: qemu-arm: enable RTC

PATCH New Heinrich Schuchardt 4 participants 8 messages Jun 28, 2018 – Jul 20, 2018 Real time clock (RTC)

lore
Share & tools
lore

Heinrich Schuchardt

author
#0 lore raw

PATCH 1/1 ARM: qemu-arm: enable RTC

QEMU provides an emulated ARM AMBA PrimeCell PL031 RTC.

The patch sets the base address in the board include file according to the definition in hw/arm/virt.c of the QEMU source. It defines the Kconfig option for the existing driver, and enables the RTC driver in qemu_arm64_defconfig and qemu_arm_defconfig as well as the date command.

We need an RTC to provide the GetTime() runtime service in the UEFI subsystem.

Signed-off-by Heinrich Schuchardt <[email protected]>
---
 configs/qemu_arm64_defconfig | 2 ++
 configs/qemu_arm_defconfig   | 2 ++
 drivers/rtc/Kconfig          | 7 +++++++
 include/configs/qemu-arm.h   | 3 +++
 4 files changed, 14 insertions(+)
configs/qemu_arm64_defconfig +2 -0
index cdf5072fe4..e9a35d010f 100644
@@ -9,6 +9,7 @@ CONFIG_DISTRO_DEFAULTS=y
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_CMD_PCI=y
 CONFIG_CMD_USB=y
+CONFIG_CMD_DATE=y
 CONFIG_OF_BOARD=y
 CONFIG_SCSI_AHCI=y
 CONFIG_AHCI_PCI=y

@@ -20,6 +21,7 @@ CONFIG_NVME=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_PCIE_ECAM_GENERIC=y
+CONFIG_RTC_PL031=y
 CONFIG_SCSI=y
 CONFIG_DM_SCSI=y
 CONFIG_SYSRESET=y
configs/qemu_arm_defconfig +2 -0
index bbce6cd719..9d30dc31a5 100644
@@ -9,6 +9,7 @@ CONFIG_DISTRO_DEFAULTS=y
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_CMD_PCI=y
 CONFIG_CMD_USB=y
+CONFIG_CMD_DATE=y
 CONFIG_OF_BOARD=y
 CONFIG_SCSI_AHCI=y
 CONFIG_AHCI_PCI=y

@@ -20,6 +21,7 @@ CONFIG_NVME=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_PCIE_ECAM_GENERIC=y
+CONFIG_RTC_PL031=y
 CONFIG_SCSI=y
 CONFIG_DM_SCSI=y
 CONFIG_SYSRESET=y
drivers/rtc/Kconfig +7 -0
index a3f8c8aecc..54365092ec 100644
@@ -48,6 +48,13 @@ config RTC_RX8010SJ
 	help
 	  Support for Epson RX8010SJ Real Time Clock devices.
 
+config RTC_PL031
+	bool "Enable ARM AMBA PL031 RTC driver"
+	help
+	  The ARM PrimeCell Real Time Clock (PL031) is an optional SoC
+	  peripheral based on the Advanced Microcontroller Bus Architecture
+	  (AMBA). It is emulated in QEMU virtual ARM machines.
+
 config RTC_MV
 	bool "Enable Marvell RTC driver"
 	depends on DM_RTC
include/configs/qemu-arm.h +3 -0
index 095aec28b1..be516bac0c 100644
@@ -24,6 +24,9 @@
 /* For block devices, QEMU emulates an ICH9 AHCI controller over PCI */
 #define CONFIG_SYS_SCSI_MAX_SCSI_ID 6
 
+/* QEMU emulates the ARM AMBA PL031 RTC */
+#define CONFIG_SYS_RTC_PL031_BASE	0x09010000
+
 /* Environment options */
 #define CONFIG_ENV_SIZE				SZ_64K
Signature · 1 line
2.17.1
#1 lore raw

PATCH 1/1 ARM: qemu-arm: enable RTC

Hi Heinrich,

Quoted 10 lines · Heinrich Schuchardt

QEMU provides an emulated ARM AMBA PrimeCell PL031 RTC.

The patch sets the base address in the board include file according to the definition in hw/arm/virt.c of the QEMU source. It defines the Kconfig option for the existing driver, and enables the RTC driver in qemu_arm64_defconfig and qemu_arm_defconfig as well as the date command.

We need an RTC to provide the GetTime() runtime service in the UEFI subsystem.

Quoted 8 lines
Signed-off-by Heinrich Schuchardt <[email protected]>
---
  configs/qemu_arm64_defconfig | 2 ++
  configs/qemu_arm_defconfig   | 2 ++
  drivers/rtc/Kconfig          | 7 +++++++
  include/configs/qemu-arm.h   | 3 +++
  4 files changed, 14 insertions(+)
Reviewed-by Tuomas Tynkkynen <[email protected]>
Tested-by Tuomas Tynkkynen <[email protected]>

- Tuomas

#2 lore raw

PATCH 1/1 ARM: qemu-arm: enable RTC

Quoted 22 lines · Tuomas Tynkkynen

Hi Heinrich,

Quoted 10 lines · Heinrich Schuchardt

QEMU provides an emulated ARM AMBA PrimeCell PL031 RTC.

The patch sets the base address in the board include file according to the definition in hw/arm/virt.c of the QEMU source. It defines the Kconfig option for the existing driver, and enables the RTC driver in qemu_arm64_defconfig and qemu_arm_defconfig as well as the date command.

We need an RTC to provide the GetTime() runtime service in the UEFI subsystem.

Quoted 8 lines
Signed-off-by Heinrich Schuchardt <[email protected]>
---
 configs/qemu_arm64_defconfig | 2 ++
 configs/qemu_arm_defconfig   | 2 ++
 drivers/rtc/Kconfig          | 7 +++++++
 include/configs/qemu-arm.h   | 3 +++
 4 files changed, 14 insertions(+)
Quoted 4 lines
Reviewed-by Tuomas Tynkkynen <[email protected]>
Tested-by Tuomas Tynkkynen <[email protected]>

- Tuomas

Well, it is a good time. Why not change the driver to driver model like below:
* I intentionally leave CONFIG_DM_RTC not mandatory here
* The patch may be split into two parts; one for rtc, the other for qemu-arm

---8<---

author
#3 lore raw

PATCH 1/1 ARM: qemu-arm: enable RTC

Quoted 26 lines · Takahiro AKASHI
Quoted 25 lines · Tuomas Tynkkynen

Hi Heinrich,

Quoted 10 lines · Heinrich Schuchardt

QEMU provides an emulated ARM AMBA PrimeCell PL031 RTC.

The patch sets the base address in the board include file according to the definition in hw/arm/virt.c of the QEMU source. It defines the Kconfig option for the existing driver, and enables the RTC driver in qemu_arm64_defconfig and qemu_arm_defconfig as well as the date command.

We need an RTC to provide the GetTime() runtime service in the UEFI subsystem.

Quoted 8 lines
Signed-off-by Heinrich Schuchardt <[email protected]>
---
 configs/qemu_arm64_defconfig | 2 ++
 configs/qemu_arm_defconfig   | 2 ++
 drivers/rtc/Kconfig          | 7 +++++++
 include/configs/qemu-arm.h   | 3 +++
 4 files changed, 14 insertions(+)
Reviewed-by Tuomas Tynkkynen <[email protected]>
Tested-by Tuomas Tynkkynen <[email protected]>
Quoted 6 lines
Quoted 1 line

- Tuomas

Well, it is a good time. Why not change the driver to driver model like below:
* I intentionally leave CONFIG_DM_RTC not mandatory here
* The patch may be split into two parts; one for rtc, the other for qemu-arm

Hello Takahiro,

thank you for your suggestion. Yes we should convert drivers to the driver model. Unfortunately the patch that you appended below is not applicable to u-boot/master.

Error: patch failed: include/configs/qemu-arm.h:36
error: include/configs/qemu-arm.h: patch does not apply Patch failed at 0001 ARM: qemu-arm: enable RTC

So I copied the changes to qemu-arm.h manually. Instead of defining the base address here it would be preferable to read the address from the device tree. This will become important if we get

Compiling with CONFIG_DM_RTC and CONFIG_RTC_PL031 resulted in warnings:

CC drivers/rtc/pl031.o
drivers/rtc/pl031.c: In function ‘pl031_rtc_set’:
drivers/rtc/pl031.c:141:17: warning: passing argument 1 of ‘rtc_set’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

  return rtc_set(tm);
                 ^~
drivers/rtc/pl031.c:72:5: note: expected ‘struct rtc_time *’ but
argument is of type ‘const struct rtc_time *’
 int rtc_set(struct rtc_time *tmp)
     ^~~~~~~

I tested both with qemu-arm64_defconfig and qemu-arm_defconfig. The date command is running fine in both cases.

The driver with your patch cannot be compiled without DM_RTC (due to missing symbol CONFIG_SYS_RTC_PL031_BASE) so in Kconfig it should depend on DM_RTC.

There is no need to keep any old style stuff. I suggest to drop legacy functions and #ifdef CONFIG_DM_RTC. Also the following line can be removed:

scripts/config_whitelist.txt:4118:CONFIG_SYS_RTC_PL031_BASE

In qemu/hw/arm/virt.c, function create_rtc() a device tree node is created which describes the pl031 RTC including the base address. From what I read in the code the node should look like this:

/{

	pl011 at 09010000 {
		compatible = "arm,pl011", "arm,primecell";
		#address-cells = <2>;
		#size-cells = <2>;
		reg = reg = <0x09010000 0x00001000>;
		...
	};

};

So there is no need to define SYS_RTC_BASE and using the U_BOOT_DEVICE macro. We can set up the platform data in the probe function by reading the reg property of the device node.

Should we also add .compatible="arm,primecell" to the list of IDs?

I would prefer enabling the RTC and the date command by default for qemu_arm64_defconfig and qemu_arm_defconfig as in my original patch.

If you could, please, send a rebased patch-set, I would be fine with it replacing my patch.

Best regards

Heinrich

Quoted 210 lines

---8<---

Quoted 1 line

From c2e701dfb8ca48025e8266035cd455287178f85b Mon Sep 17 00:00:00 2001

From: AKASHI Takahiro <[email protected]>
Date: Tue, 3 Jul 2018 08:32:16 +0900
Subject: [PATCH] rtc: pl031: convert the driver to driver model

Signed-off-by AKASHI Takahiro <[email protected]>
---
 board/emulation/qemu-arm/qemu-arm.c  | 13 ++++
 drivers/rtc/Kconfig                  |  7 +++
 drivers/rtc/pl031.c                  | 91 +++++++++++++++++++++++++---
 include/configs/qemu-arm.h           |  4 ++
 include/dm/platform_data/rtc_pl031.h | 10 +++
 5 files changed, 118 insertions(+), 7 deletions(-)
 create mode 100644 include/dm/platform_data/rtc_pl031.h
board/emulation/qemu-arm/qemu-arm.c +13 -0
index 085cbbef99..3084f2aa71 100644
@@ -3,8 +3,21 @@
  * Copyright (c) 2017 Tuomas Tynkkynen
  */
 #include <common.h>
+#include <dm.h>
+#include <dm/platform_data/rtc_pl031.h>
 #include <fdtdec.h>
 
+#ifdef CONFIG_DM_RTC
+static const struct pl031_rtc_platdata pl031_pdata = {
+	.base = SYS_RTC_BASE,
+};
+
+U_BOOT_DEVICE(qemu_arm_rtc) = {
+	.name = "rtc-pl031",
+	.platdata = &pl031_pdata,
+};
+#endif
+
 #ifdef CONFIG_ARM64
 #include <asm/armv8/mmu.h>
drivers/rtc/Kconfig +7 -0
index a3f8c8aecc..50d9236601 100644
@@ -55,6 +55,13 @@ config RTC_MV
 	  Enable Marvell RTC driver. This driver supports the rtc that is present
 	  on some Marvell SoCs.
 
+config RTC_PL031
+	bool "Enable ARM PL031 RTC driver"
+	imply DM_RTC
+	imply CMD_DATE
+	help
+	  Enable ARM PL031 RTC driver.
+
 config RTC_S35392A
 	bool "Enable S35392A driver"
 	select BITREVERSE
drivers/rtc/pl031.c +84 -7
index 8955805e3b..884e3a13fe 100644
@@ -8,12 +8,25 @@
 
 #include <common.h>
 #include <command.h>
+#include <dm.h>
+#include <dm/platform_data/rtc_pl031.h>
 #include <rtc.h>
 
 #if defined(CONFIG_CMD_DATE)
 
-#ifndef CONFIG_SYS_RTC_PL031_BASE
-#error CONFIG_SYS_RTC_PL031_BASE is not defined!
+#define	__RTC_WRITE_REG(base, addr, val) \
+			(*(volatile unsigned int *)((base) + (addr)) = (val))
+#define	__RTC_READ_REG(base, addr)	\
+			(*(volatile unsigned int *)((base) + (addr)))
+
+#ifdef CONFIG_DM_RTC
+phys_addr_t pl031_base;
+#else
+# ifndef CONFIG_SYS_RTC_PL031_BASE
+# error CONFIG_SYS_RTC_PL031_BASE is not defined!
+# endif
+
+phys_addr_t pl031_base = CONFIG_SYS_RTC_PL031_BASE;
 #endif
 
 /*

@@ -30,10 +43,8 @@
 
 #define RTC_CR_START	(1 << 0)
 
-#define	RTC_WRITE_REG(addr, val) \
-			(*(volatile unsigned int *)(CONFIG_SYS_RTC_PL031_BASE + (addr)) = (val))
-#define	RTC_READ_REG(addr)	\
-			(*(volatile unsigned int *)(CONFIG_SYS_RTC_PL031_BASE + (addr)))
+#define	RTC_WRITE_REG(addr, val)	__RTC_WRITE_REG(pl031_base, addr, val)
+#define	RTC_READ_REG(addr)		__RTC_READ_REG(pl031_base, addr)
 
 static int pl031_initted = 0;
 

@@ -104,4 +115,70 @@ int rtc_get(struct rtc_time *tmp)
 	return 0;
 }
 
-#endif
+#ifdef CONFIG_DM_RTC
+void pl031_rtc_init(struct pl031_rtc_platdata *pdata)
+{
+	pl031_base = pdata->base;
+}
+
+static int pl031_rtc_get(struct udevice *dev, struct rtc_time *tm)
+{
+	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
+
+	if (!pl031_initted)
+		pl031_rtc_init(pdata);
+
+	return rtc_get(tm);
+}
+
+static int pl031_rtc_set(struct udevice *dev, const struct rtc_time *tm)
+{
+	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
+
+	if (!pl031_initted)
+		pl031_rtc_init(pdata);
+
+	return rtc_set(tm);
+}
+
+static int pl031_rtc_reset(struct udevice *dev)
+{
+	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
+
+	if (!pl031_initted)
+		pl031_rtc_init(pdata);
+
+	rtc_reset();
+
+	return 0;
+}
+
+static const struct rtc_ops pl031_rtc_ops = {
+	.get = pl031_rtc_get,
+	.set = pl031_rtc_set,
+	.reset = pl031_rtc_reset,
+};
+
+static const struct udevice_id pl031_rtc_ids[] = {
+	{ .compatible = "arm,pl031" },
+	{ }
+};
+
+static int pl031_rtc_ofdata_to_platdata(struct udevice *dev)
+{
+	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
+
+	pdata->base = devfdt_get_addr(dev);
+	return 0;
+}
+
+U_BOOT_DRIVER(rtc_pl031) = {
+	.name	= "rtc-pl031",
+	.id	= UCLASS_RTC,
+	.ofdata_to_platdata = pl031_rtc_ofdata_to_platdata,
+	.of_match = pl031_rtc_ids,
+	.ops	= &pl031_rtc_ops,
+};
+#endif /* CONFIG_DM_RTC */
+
+#endif /* CONFIG_CMD_DATE */
include/configs/qemu-arm.h +4 -0
index b7debb9cc7..569fa729a9 100644
@@ -36,6 +36,10 @@
 #define CONFIG_SYS_MONITOR_BASE		CONFIG_SYS_FLASH_BASE0
 #define CONFIG_SYS_FLASH_BASE		CONFIG_SYS_FLASH_BASE0
 
+#define SYS_PERI_BASE			0x9000000
+#define SYS_UART_BASE			SYS_PERI_BASE
+#define SYS_RTC_BASE			(SYS_PERI_BASE + 0x10000)
+
 /* PCI */
 /*
  * #define CONFIG_SYS_PCI_64BIT		1
include/dm/platform_data/rtc_pl031.h new file +10 -0
new file mode 100644
index 0000000000..b35642b15d
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __rtc_pl031_h
+#define __rtc_pl031_h
+
+struct pl031_rtc_platdata {
+	phys_addr_t base;
+};
+
+#endif
author
#4 lore raw

PATCH 1/1 ARM: qemu-arm: enable RTC

Quoted 87 lines · Heinrich Schuchardt
Quoted 26 lines · Takahiro AKASHI
Quoted 25 lines · Tuomas Tynkkynen

Hi Heinrich,

Quoted 10 lines · Heinrich Schuchardt

QEMU provides an emulated ARM AMBA PrimeCell PL031 RTC.

The patch sets the base address in the board include file according to the definition in hw/arm/virt.c of the QEMU source. It defines the Kconfig option for the existing driver, and enables the RTC driver in qemu_arm64_defconfig and qemu_arm_defconfig as well as the date command.

We need an RTC to provide the GetTime() runtime service in the UEFI subsystem.

Quoted 8 lines
Signed-off-by Heinrich Schuchardt <[email protected]>
---
 configs/qemu_arm64_defconfig | 2 ++
 configs/qemu_arm_defconfig   | 2 ++
 drivers/rtc/Kconfig          | 7 +++++++
 include/configs/qemu-arm.h   | 3 +++
 4 files changed, 14 insertions(+)
Reviewed-by Tuomas Tynkkynen <[email protected]>
Tested-by Tuomas Tynkkynen <[email protected]>
Quoted 6 lines
Quoted 1 line

- Tuomas

Well, it is a good time. Why not change the driver to driver model like below:
* I intentionally leave CONFIG_DM_RTC not mandatory here
* The patch may be split into two parts; one for rtc, the other for qemu-arm

Hello Takahiro,

thank you for your suggestion. Yes we should convert drivers to the driver model. Unfortunately the patch that you appended below is not applicable to u-boot/master.

Error: patch failed: include/configs/qemu-arm.h:36
error: include/configs/qemu-arm.h: patch does not apply Patch failed at 0001 ARM: qemu-arm: enable RTC

So I copied the changes to qemu-arm.h manually. Instead of defining the base address here it would be preferable to read the address from the device tree. This will become important if we get

Compiling with CONFIG_DM_RTC and CONFIG_RTC_PL031 resulted in warnings:

CC drivers/rtc/pl031.o
drivers/rtc/pl031.c: In function ‘pl031_rtc_set’:
drivers/rtc/pl031.c:141:17: warning: passing argument 1 of ‘rtc_set’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

  return rtc_set(tm);
                 ^~
drivers/rtc/pl031.c:72:5: note: expected ‘struct rtc_time *’ but
argument is of type ‘const struct rtc_time *’
 int rtc_set(struct rtc_time *tmp)
     ^~~~~~~

I tested both with qemu-arm64_defconfig and qemu-arm_defconfig. The date command is running fine in both cases.

The driver with your patch cannot be compiled without DM_RTC (due to missing symbol CONFIG_SYS_RTC_PL031_BASE) so in Kconfig it should depend on DM_RTC.

There is no need to keep any old style stuff. I suggest to drop legacy functions and #ifdef CONFIG_DM_RTC. Also the following line can be removed:

scripts/config_whitelist.txt:4118:CONFIG_SYS_RTC_PL031_BASE

In qemu/hw/arm/virt.c, function create_rtc() a device tree node is created which describes the pl031 RTC including the base address. From what I read in the code the node should look like this:

/{

	pl011 at 09010000 {
		compatible = "arm,pl011", "arm,primecell";
		#address-cells = <2>;
		#size-cells = <2>;
		reg = reg = <0x09010000 0x00001000>;
		...
	};
Quoted 1 line

};

You can print the actual device tree within U-Boot using:

	fdt addr ${fdtcontroladdr}
	fdt print /

This is the relevant part of the output:

/ {
        interrupt-parent = <0x00008001>;
        #size-cells = <0x00000002>;
        #address-cells = <0x00000002>;
        compatible = "linux,dummy-virt";

        pl031 at 9010000 {
                clock-names = "apb_pclk";
                clocks = <0x00008000>;
                interrupts = <0x00000000 0x00000002 0x00000004>;
                reg = <0x00000000 0x09010000 0x00000000 0x00001000>;
                compatible = "arm,pl031", "arm,primecell";
        };

};

Regards

Heinrich

#5 lore raw

PATCH 1/1 ARM: qemu-arm: enable RTC

In reply to Heinrich Schuchardt
Quoted 34 lines · Heinrich Schuchardt
Quoted 26 lines · Takahiro AKASHI
Quoted 25 lines · Tuomas Tynkkynen

Hi Heinrich,

Quoted 10 lines · Heinrich Schuchardt

QEMU provides an emulated ARM AMBA PrimeCell PL031 RTC.

The patch sets the base address in the board include file according to the definition in hw/arm/virt.c of the QEMU source. It defines the Kconfig option for the existing driver, and enables the RTC driver in qemu_arm64_defconfig and qemu_arm_defconfig as well as the date command.

We need an RTC to provide the GetTime() runtime service in the UEFI subsystem.

Quoted 8 lines
Signed-off-by Heinrich Schuchardt <[email protected]>
---
 configs/qemu_arm64_defconfig | 2 ++
 configs/qemu_arm_defconfig   | 2 ++
 drivers/rtc/Kconfig          | 7 +++++++
 include/configs/qemu-arm.h   | 3 +++
 4 files changed, 14 insertions(+)
Reviewed-by Tuomas Tynkkynen <[email protected]>
Tested-by Tuomas Tynkkynen <[email protected]>
Quoted 6 lines
Quoted 1 line

- Tuomas

Well, it is a good time. Why not change the driver to driver model like below:
* I intentionally leave CONFIG_DM_RTC not mandatory here
* The patch may be split into two parts; one for rtc, the other for qemu-arm

Quoted 5 lines

Hello Takahiro,

thank you for your suggestion. Yes we should convert drivers to the driver model. Unfortunately the patch that you appended below is not applicable to u-boot/master.

Thank you for your review. It is very helpful as I have not fully tested my change.

Quoted 21 lines

Error: patch failed: include/configs/qemu-arm.h:36
error: include/configs/qemu-arm.h: patch does not apply Patch failed at 0001 ARM: qemu-arm: enable RTC

So I copied the changes to qemu-arm.h manually. Instead of defining the base address here it would be preferable to read the address from the device tree. This will become important if we get

Compiling with CONFIG_DM_RTC and CONFIG_RTC_PL031 resulted in warnings:

CC drivers/rtc/pl031.o
drivers/rtc/pl031.c: In function ‘pl031_rtc_set’:
drivers/rtc/pl031.c:141:17: warning: passing argument 1 of ‘rtc_set’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

  return rtc_set(tm);
                 ^~
drivers/rtc/pl031.c:72:5: note: expected ‘struct rtc_time *’ but
argument is of type ‘const struct rtc_time *’
 int rtc_set(struct rtc_time *tmp)
     ^~~~~~~
Quoted 6 lines

I tested both with qemu-arm64_defconfig and qemu-arm_defconfig. The date command is running fine in both cases.

The driver with your patch cannot be compiled without DM_RTC (due to missing symbol CONFIG_SYS_RTC_PL031_BASE) so in Kconfig it should depend on DM_RTC.

Ouch.

Quoted 2 lines

There is no need to keep any old style stuff. I suggest to drop legacy functions and #ifdef CONFIG_DM_RTC. Also the following line can be removed:

My concern was that it would break any downstream code.

Quoted 16 lines

scripts/config_whitelist.txt:4118:CONFIG_SYS_RTC_PL031_BASE

In qemu/hw/arm/virt.c, function create_rtc() a device tree node is created which describes the pl031 RTC including the base address. From what I read in the code the node should look like this:

/{

	pl011 at 09010000 {
		compatible = "arm,pl011", "arm,primecell";
		#address-cells = <2>;
		#size-cells = <2>;
		reg = reg = <0x09010000 0x00001000>;
		...
	};
Quoted 5 lines

};

So there is no need to define SYS_RTC_BASE and using the U_BOOT_DEVICE macro. We can set up the platform data in the probe function by reading the reg property of the device node.

Since no dts for qemu-arm is provided in u-boot tree, I'm not sure that this change makes sense.

Quoted 1 line

Should we also add .compatible="arm,primecell" to the list of IDs?

Yeah, I know "arm,primecell" is also added for other arm IPs, but think that it is too vague in contrast to pl031, isn't it?

Quoted 2 lines

I would prefer enabling the RTC and the date command by default for qemu_arm64_defconfig and qemu_arm_defconfig as in my original patch.

OK.
At least, CMD_DATE will be enabled automatically by "imply."

Quoted 2 lines

If you could, please, send a rebased patch-set, I would be fine with it replacing my patch.

OK.

Thanks,
-Takahiro AKASHI

Quoted 215 lines

Best regards

Heinrich

Quoted 210 lines

---8<---

Quoted 1 line

From c2e701dfb8ca48025e8266035cd455287178f85b Mon Sep 17 00:00:00 2001

From: AKASHI Takahiro <[email protected]>
Date: Tue, 3 Jul 2018 08:32:16 +0900
Subject: [PATCH] rtc: pl031: convert the driver to driver model

Signed-off-by AKASHI Takahiro <[email protected]>
---
 board/emulation/qemu-arm/qemu-arm.c  | 13 ++++
 drivers/rtc/Kconfig                  |  7 +++
 drivers/rtc/pl031.c                  | 91 +++++++++++++++++++++++++---
 include/configs/qemu-arm.h           |  4 ++
 include/dm/platform_data/rtc_pl031.h | 10 +++
 5 files changed, 118 insertions(+), 7 deletions(-)
 create mode 100644 include/dm/platform_data/rtc_pl031.h
board/emulation/qemu-arm/qemu-arm.c +13 -0
index 085cbbef99..3084f2aa71 100644
@@ -3,8 +3,21 @@
  * Copyright (c) 2017 Tuomas Tynkkynen
  */
 #include <common.h>
+#include <dm.h>
+#include <dm/platform_data/rtc_pl031.h>
 #include <fdtdec.h>
 
+#ifdef CONFIG_DM_RTC
+static const struct pl031_rtc_platdata pl031_pdata = {
+	.base = SYS_RTC_BASE,
+};
+
+U_BOOT_DEVICE(qemu_arm_rtc) = {
+	.name = "rtc-pl031",
+	.platdata = &pl031_pdata,
+};
+#endif
+
 #ifdef CONFIG_ARM64
 #include <asm/armv8/mmu.h>
drivers/rtc/Kconfig +7 -0
index a3f8c8aecc..50d9236601 100644
@@ -55,6 +55,13 @@ config RTC_MV
 	  Enable Marvell RTC driver. This driver supports the rtc that is present
 	  on some Marvell SoCs.
 
+config RTC_PL031
+	bool "Enable ARM PL031 RTC driver"
+	imply DM_RTC
+	imply CMD_DATE
+	help
+	  Enable ARM PL031 RTC driver.
+
 config RTC_S35392A
 	bool "Enable S35392A driver"
 	select BITREVERSE
drivers/rtc/pl031.c +84 -7
index 8955805e3b..884e3a13fe 100644
@@ -8,12 +8,25 @@
 
 #include <common.h>
 #include <command.h>
+#include <dm.h>
+#include <dm/platform_data/rtc_pl031.h>
 #include <rtc.h>
 
 #if defined(CONFIG_CMD_DATE)
 
-#ifndef CONFIG_SYS_RTC_PL031_BASE
-#error CONFIG_SYS_RTC_PL031_BASE is not defined!
+#define	__RTC_WRITE_REG(base, addr, val) \
+			(*(volatile unsigned int *)((base) + (addr)) = (val))
+#define	__RTC_READ_REG(base, addr)	\
+			(*(volatile unsigned int *)((base) + (addr)))
+
+#ifdef CONFIG_DM_RTC
+phys_addr_t pl031_base;
+#else
+# ifndef CONFIG_SYS_RTC_PL031_BASE
+# error CONFIG_SYS_RTC_PL031_BASE is not defined!
+# endif
+
+phys_addr_t pl031_base = CONFIG_SYS_RTC_PL031_BASE;
 #endif
 
 /*

@@ -30,10 +43,8 @@
 
 #define RTC_CR_START	(1 << 0)
 
-#define	RTC_WRITE_REG(addr, val) \
-			(*(volatile unsigned int *)(CONFIG_SYS_RTC_PL031_BASE + (addr)) = (val))
-#define	RTC_READ_REG(addr)	\
-			(*(volatile unsigned int *)(CONFIG_SYS_RTC_PL031_BASE + (addr)))
+#define	RTC_WRITE_REG(addr, val)	__RTC_WRITE_REG(pl031_base, addr, val)
+#define	RTC_READ_REG(addr)		__RTC_READ_REG(pl031_base, addr)
 
 static int pl031_initted = 0;
 

@@ -104,4 +115,70 @@ int rtc_get(struct rtc_time *tmp)
 	return 0;
 }
 
-#endif
+#ifdef CONFIG_DM_RTC
+void pl031_rtc_init(struct pl031_rtc_platdata *pdata)
+{
+	pl031_base = pdata->base;
+}
+
+static int pl031_rtc_get(struct udevice *dev, struct rtc_time *tm)
+{
+	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
+
+	if (!pl031_initted)
+		pl031_rtc_init(pdata);
+
+	return rtc_get(tm);
+}
+
+static int pl031_rtc_set(struct udevice *dev, const struct rtc_time *tm)
+{
+	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
+
+	if (!pl031_initted)
+		pl031_rtc_init(pdata);
+
+	return rtc_set(tm);
+}
+
+static int pl031_rtc_reset(struct udevice *dev)
+{
+	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
+
+	if (!pl031_initted)
+		pl031_rtc_init(pdata);
+
+	rtc_reset();
+
+	return 0;
+}
+
+static const struct rtc_ops pl031_rtc_ops = {
+	.get = pl031_rtc_get,
+	.set = pl031_rtc_set,
+	.reset = pl031_rtc_reset,
+};
+
+static const struct udevice_id pl031_rtc_ids[] = {
+	{ .compatible = "arm,pl031" },
+	{ }
+};
+
+static int pl031_rtc_ofdata_to_platdata(struct udevice *dev)
+{
+	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
+
+	pdata->base = devfdt_get_addr(dev);
+	return 0;
+}
+
+U_BOOT_DRIVER(rtc_pl031) = {
+	.name	= "rtc-pl031",
+	.id	= UCLASS_RTC,
+	.ofdata_to_platdata = pl031_rtc_ofdata_to_platdata,
+	.of_match = pl031_rtc_ids,
+	.ops	= &pl031_rtc_ops,
+};
+#endif /* CONFIG_DM_RTC */
+
+#endif /* CONFIG_CMD_DATE */
include/configs/qemu-arm.h +4 -0
index b7debb9cc7..569fa729a9 100644
@@ -36,6 +36,10 @@
 #define CONFIG_SYS_MONITOR_BASE		CONFIG_SYS_FLASH_BASE0
 #define CONFIG_SYS_FLASH_BASE		CONFIG_SYS_FLASH_BASE0
 
+#define SYS_PERI_BASE			0x9000000
+#define SYS_UART_BASE			SYS_PERI_BASE
+#define SYS_RTC_BASE			(SYS_PERI_BASE + 0x10000)
+
 /* PCI */
 /*
  * #define CONFIG_SYS_PCI_64BIT		1
include/dm/platform_data/rtc_pl031.h new file +10 -0
new file mode 100644
index 0000000000..b35642b15d
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __rtc_pl031_h
+#define __rtc_pl031_h
+
+struct pl031_rtc_platdata {
+	phys_addr_t base;
+};
+
+#endif
author
#6 lore raw

PATCH 1/1 ARM: qemu-arm: enable RTC

Quoted 96 lines · Takahiro AKASHI
Quoted 34 lines · Heinrich Schuchardt
Quoted 26 lines · Takahiro AKASHI
Quoted 25 lines · Tuomas Tynkkynen

Hi Heinrich,

Quoted 10 lines · Heinrich Schuchardt

QEMU provides an emulated ARM AMBA PrimeCell PL031 RTC.

The patch sets the base address in the board include file according to the definition in hw/arm/virt.c of the QEMU source. It defines the Kconfig option for the existing driver, and enables the RTC driver in qemu_arm64_defconfig and qemu_arm_defconfig as well as the date command.

We need an RTC to provide the GetTime() runtime service in the UEFI subsystem.

Quoted 8 lines
Signed-off-by Heinrich Schuchardt <[email protected]>
---
 configs/qemu_arm64_defconfig | 2 ++
 configs/qemu_arm_defconfig   | 2 ++
 drivers/rtc/Kconfig          | 7 +++++++
 include/configs/qemu-arm.h   | 3 +++
 4 files changed, 14 insertions(+)
Reviewed-by Tuomas Tynkkynen <[email protected]>
Tested-by Tuomas Tynkkynen <[email protected]>
Quoted 6 lines
Quoted 1 line

- Tuomas

Well, it is a good time. Why not change the driver to driver model like below:
* I intentionally leave CONFIG_DM_RTC not mandatory here
* The patch may be split into two parts; one for rtc, the other for qemu-arm

Quoted 5 lines

Hello Takahiro,

thank you for your suggestion. Yes we should convert drivers to the driver model. Unfortunately the patch that you appended below is not applicable to u-boot/master.

Thank you for your review. It is very helpful as I have not fully tested my change.

Quoted 21 lines

Error: patch failed: include/configs/qemu-arm.h:36
error: include/configs/qemu-arm.h: patch does not apply Patch failed at 0001 ARM: qemu-arm: enable RTC

So I copied the changes to qemu-arm.h manually. Instead of defining the base address here it would be preferable to read the address from the device tree. This will become important if we get

Compiling with CONFIG_DM_RTC and CONFIG_RTC_PL031 resulted in warnings:

CC drivers/rtc/pl031.o
drivers/rtc/pl031.c: In function ‘pl031_rtc_set’:
drivers/rtc/pl031.c:141:17: warning: passing argument 1 of ‘rtc_set’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

  return rtc_set(tm);
                 ^~
drivers/rtc/pl031.c:72:5: note: expected ‘struct rtc_time *’ but
argument is of type ‘const struct rtc_time *’
 int rtc_set(struct rtc_time *tmp)
     ^~~~~~~
Quoted 6 lines

I tested both with qemu-arm64_defconfig and qemu-arm_defconfig. The date command is running fine in both cases.

The driver with your patch cannot be compiled without DM_RTC (due to missing symbol CONFIG_SYS_RTC_PL031_BASE) so in Kconfig it should depend on DM_RTC.

Ouch.

Quoted 2 lines

There is no need to keep any old style stuff. I suggest to drop legacy functions and #ifdef CONFIG_DM_RTC. Also the following line can be removed:

My concern was that it would break any downstream code.

Quoted 16 lines

scripts/config_whitelist.txt:4118:CONFIG_SYS_RTC_PL031_BASE

In qemu/hw/arm/virt.c, function create_rtc() a device tree node is created which describes the pl031 RTC including the base address. From what I read in the code the node should look like this:

/{

	pl011 at 09010000 {
		compatible = "arm,pl011", "arm,primecell";
		#address-cells = <2>;
		#size-cells = <2>;
		reg = reg = <0x09010000 0x00001000>;
		...
	};
Quoted 8 lines
Quoted 5 lines

};

So there is no need to define SYS_RTC_BASE and using the U_BOOT_DEVICE macro. We can set up the platform data in the probe function by reading the reg property of the device node.

Since no dts for qemu-arm is provided in u-boot tree, I'm not sure that this change makes sense.

There is no device-tree provided by U-Boot for qemu-arm because it is already provided by QEMU itself. You can verify with U-Boot command 'fdt print' that this device tree provides a reg property for the clock.

	fdt addr ${fdtcontroladdr}
	fdt print /

Cf. function board_fdt_blob_setup() in board/emulation/qemu-arm/qemu-arm.c.

Looking at the device trees in the Linux kernel for boards that we do not currently support like linux/arch/arm/boot/dts/arm-realview-eb.dtsi
you will find that all boards but one provide a reg property for the PL031 with the address of the clock registers. And this address depends on the board.

So if you rely on the QEMU delivered device tree to provide the address of the RTC registers this will enable us to support the PL031 RTC on other boards like the HiKey 960 in the future.

Quoted 5 lines
Quoted 2 lines

Should we also add .compatible="arm,primecell" to the list of IDs?

Yeah, I know "arm,primecell" is also added for other arm IPs, but think that it is too vague in contrast to pl031, isn't it?

You are right. In the qemu-arm device tree that string is also used for other items.

Regards

Heinrich

#7 lore raw

PATCH 1/1 ARM: qemu-arm: enable RTC

In reply to Heinrich Schuchardt
Quoted 7 lines · Heinrich Schuchardt

QEMU provides an emulated ARM AMBA PrimeCell PL031 RTC.

The patch sets the base address in the board include file according to the definition in hw/arm/virt.c of the QEMU source. It defines the Kconfig option for the existing driver, and enables the RTC driver in qemu_arm64_defconfig and qemu_arm_defconfig as well as the date command.

Quoted 7 lines

We need an RTC to provide the GetTime() runtime service in the UEFI subsystem.

Signed-off-by Heinrich Schuchardt <[email protected]>
Reviewed-by Tuomas Tynkkynen <[email protected]>
Tested-by Tuomas Tynkkynen <[email protected]>
Signed-off-by AKASHI Takahiro <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/045e9750/attachment.sig>