[U-Boot] [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 xypron.glpk@gmx.de --- 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(+)
diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig index cdf5072fe4..e9a35d010f 100644 --- a/configs/qemu_arm64_defconfig +++ b/configs/qemu_arm64_defconfig @@ -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 diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig index bbce6cd719..9d30dc31a5 100644 --- a/configs/qemu_arm_defconfig +++ b/configs/qemu_arm_defconfig @@ -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 diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index a3f8c8aecc..54365092ec 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -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 diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h index 095aec28b1..be516bac0c 100644 --- a/include/configs/qemu-arm.h +++ b/include/configs/qemu-arm.h @@ -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
Hi Heinrich,
On 06/29/2018 01:34 AM, Heinrich Schuchardt wrote:
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 xypron.glpk@gmx.de
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 tuomas.tynkkynen@iki.fi Tested-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
- Tuomas
On Mon, Jul 02, 2018 at 07:07:55PM +0300, Tuomas Tynkkynen wrote:
Hi Heinrich,
On 06/29/2018 01:34 AM, Heinrich Schuchardt wrote:
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 xypron.glpk@gmx.de
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 tuomas.tynkkynen@iki.fi Tested-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
- 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<---
From c2e701dfb8ca48025e8266035cd455287178f85b Mon Sep 17 00:00:00 2001
From: AKASHI Takahiro takahiro.akashi@linaro.org Date: Tue, 3 Jul 2018 08:32:16 +0900 Subject: [PATCH] rtc: pl031: convert the driver to driver model
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- 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
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c index 085cbbef99..3084f2aa71 100644 --- a/board/emulation/qemu-arm/qemu-arm.c +++ b/board/emulation/qemu-arm/qemu-arm.c @@ -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>
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index a3f8c8aecc..50d9236601 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -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 diff --git a/drivers/rtc/pl031.c b/drivers/rtc/pl031.c index 8955805e3b..884e3a13fe 100644 --- a/drivers/rtc/pl031.c +++ b/drivers/rtc/pl031.c @@ -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 */ diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h index b7debb9cc7..569fa729a9 100644 --- a/include/configs/qemu-arm.h +++ b/include/configs/qemu-arm.h @@ -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 diff --git a/include/dm/platform_data/rtc_pl031.h b/include/dm/platform_data/rtc_pl031.h new file mode 100644 index 0000000000..b35642b15d --- /dev/null +++ b/include/dm/platform_data/rtc_pl031.h @@ -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
On 07/03/2018 01:51 AM, Takahiro AKASHI wrote:
On Mon, Jul 02, 2018 at 07:07:55PM +0300, Tuomas Tynkkynen wrote:
Hi Heinrich,
On 06/29/2018 01:34 AM, Heinrich Schuchardt wrote:
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 xypron.glpk@gmx.de
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 tuomas.tynkkynen@iki.fi Tested-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
- 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@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
---8<---
From c2e701dfb8ca48025e8266035cd455287178f85b Mon Sep 17 00:00:00 2001
From: AKASHI Takahiro takahiro.akashi@linaro.org Date: Tue, 3 Jul 2018 08:32:16 +0900 Subject: [PATCH] rtc: pl031: convert the driver to driver model
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
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
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c index 085cbbef99..3084f2aa71 100644 --- a/board/emulation/qemu-arm/qemu-arm.c +++ b/board/emulation/qemu-arm/qemu-arm.c @@ -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>
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index a3f8c8aecc..50d9236601 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -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 diff --git a/drivers/rtc/pl031.c b/drivers/rtc/pl031.c index 8955805e3b..884e3a13fe 100644 --- a/drivers/rtc/pl031.c +++ b/drivers/rtc/pl031.c @@ -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 */ diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h index b7debb9cc7..569fa729a9 100644 --- a/include/configs/qemu-arm.h +++ b/include/configs/qemu-arm.h @@ -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
diff --git a/include/dm/platform_data/rtc_pl031.h b/include/dm/platform_data/rtc_pl031.h new file mode 100644 index 0000000000..b35642b15d --- /dev/null +++ b/include/dm/platform_data/rtc_pl031.h @@ -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
On 07/03/2018 04:07 AM, Heinrich Schuchardt wrote:
On 07/03/2018 01:51 AM, Takahiro AKASHI wrote:
On Mon, Jul 02, 2018 at 07:07:55PM +0300, Tuomas Tynkkynen wrote:
Hi Heinrich,
On 06/29/2018 01:34 AM, Heinrich Schuchardt wrote:
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 xypron.glpk@gmx.de
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 tuomas.tynkkynen@iki.fi Tested-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
- 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@09010000 { compatible = "arm,pl011", "arm,primecell"; #address-cells = <2>; #size-cells = <2>; reg = reg = <0x09010000 0x00001000>; ... };
};
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@9010000 { clock-names = "apb_pclk"; clocks = <0x00008000>; interrupts = <0x00000000 0x00000002 0x00000004>; reg = <0x00000000 0x09010000 0x00000000 0x00001000>; compatible = "arm,pl031", "arm,primecell"; };
};
Regards
Heinrich
On Tue, Jul 03, 2018 at 04:07:31AM +0200, Heinrich Schuchardt wrote:
On 07/03/2018 01:51 AM, Takahiro AKASHI wrote:
On Mon, Jul 02, 2018 at 07:07:55PM +0300, Tuomas Tynkkynen wrote:
Hi Heinrich,
On 06/29/2018 01:34 AM, Heinrich Schuchardt wrote:
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 xypron.glpk@gmx.de
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 tuomas.tynkkynen@iki.fi Tested-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
- 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.
Thank you for your review. It is very helpful as I have not fully tested my change.
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.
Ouch.
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.
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@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.
Since no dts for qemu-arm is provided in u-boot tree, I'm not sure that this change makes sense.
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?
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."
If you could, please, send a rebased patch-set, I would be fine with it replacing my patch.
OK.
Thanks, -Takahiro AKASHI
Best regards
Heinrich
---8<---
From c2e701dfb8ca48025e8266035cd455287178f85b Mon Sep 17 00:00:00 2001
From: AKASHI Takahiro takahiro.akashi@linaro.org Date: Tue, 3 Jul 2018 08:32:16 +0900 Subject: [PATCH] rtc: pl031: convert the driver to driver model
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
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
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c index 085cbbef99..3084f2aa71 100644 --- a/board/emulation/qemu-arm/qemu-arm.c +++ b/board/emulation/qemu-arm/qemu-arm.c @@ -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>
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index a3f8c8aecc..50d9236601 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -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 diff --git a/drivers/rtc/pl031.c b/drivers/rtc/pl031.c index 8955805e3b..884e3a13fe 100644 --- a/drivers/rtc/pl031.c +++ b/drivers/rtc/pl031.c @@ -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 */ diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h index b7debb9cc7..569fa729a9 100644 --- a/include/configs/qemu-arm.h +++ b/include/configs/qemu-arm.h @@ -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
diff --git a/include/dm/platform_data/rtc_pl031.h b/include/dm/platform_data/rtc_pl031.h new file mode 100644 index 0000000000..b35642b15d --- /dev/null +++ b/include/dm/platform_data/rtc_pl031.h @@ -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
On 07/03/2018 07:29 AM, Takahiro AKASHI wrote:
On Tue, Jul 03, 2018 at 04:07:31AM +0200, Heinrich Schuchardt wrote:
On 07/03/2018 01:51 AM, Takahiro AKASHI wrote:
On Mon, Jul 02, 2018 at 07:07:55PM +0300, Tuomas Tynkkynen wrote:
Hi Heinrich,
On 06/29/2018 01:34 AM, Heinrich Schuchardt wrote:
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 xypron.glpk@gmx.de
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 tuomas.tynkkynen@iki.fi Tested-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
- 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.
Thank you for your review. It is very helpful as I have not fully tested my change.
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.
Ouch.
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.
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@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.
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.
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
On Fri, Jun 29, 2018 at 12:34:16AM +0200, Heinrich Schuchardt wrote:
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 xypron.glpk@gmx.de Reviewed-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Tested-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
Applied to u-boot/master, thanks!
participants (4)
-
Heinrich Schuchardt -
Takahiro AKASHI -
Tom Rini -
Tuomas Tynkkynen