* [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC @ 2018-06-28 22:34 Heinrich Schuchardt 2018-07-02 16:07 ` Tuomas Tynkkynen 2018-07-20 12:36 ` [U-Boot] [U-Boot,1/1] " Tom Rini 0 siblings, 2 replies; 8+ messages in thread From: Heinrich Schuchardt @ 2018-06-28 22:34 UTC (permalink / raw) To: u-boot 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 -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC 2018-06-28 22:34 [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC Heinrich Schuchardt @ 2018-07-02 16:07 ` Tuomas Tynkkynen 2018-07-02 23:51 ` Takahiro AKASHI 2018-07-20 12:36 ` [U-Boot] [U-Boot,1/1] " Tom Rini 1 sibling, 1 reply; 8+ messages in thread From: Tuomas Tynkkynen @ 2018-07-02 16:07 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC 2018-07-02 16:07 ` Tuomas Tynkkynen @ 2018-07-02 23:51 ` Takahiro AKASHI 2018-07-03 2:07 ` Heinrich Schuchardt 0 siblings, 1 reply; 8+ messages in thread From: Takahiro AKASHI @ 2018-07-02 23:51 UTC (permalink / raw) To: u-boot 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<--- ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC 2018-07-02 23:51 ` Takahiro AKASHI @ 2018-07-03 2:07 ` Heinrich Schuchardt 2018-07-03 3:30 ` Heinrich Schuchardt 2018-07-03 5:29 ` Takahiro AKASHI 0 siblings, 2 replies; 8+ messages in thread From: Heinrich Schuchardt @ 2018-07-03 2:07 UTC (permalink / raw) To: u-boot 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 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 > > ---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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC 2018-07-03 2:07 ` Heinrich Schuchardt @ 2018-07-03 3:30 ` Heinrich Schuchardt 2018-07-03 5:29 ` Takahiro AKASHI 1 sibling, 0 replies; 8+ messages in thread From: Heinrich Schuchardt @ 2018-07-03 3:30 UTC (permalink / raw) To: u-boot 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 at 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 at 9010000 { clock-names = "apb_pclk"; clocks = <0x00008000>; interrupts = <0x00000000 0x00000002 0x00000004>; reg = <0x00000000 0x09010000 0x00000000 0x00001000>; compatible = "arm,pl031", "arm,primecell"; }; }; Regards Heinrich ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC 2018-07-03 2:07 ` Heinrich Schuchardt 2018-07-03 3:30 ` Heinrich Schuchardt @ 2018-07-03 5:29 ` Takahiro AKASHI 2018-07-03 6:00 ` Heinrich Schuchardt 1 sibling, 1 reply; 8+ messages in thread From: Takahiro AKASHI @ 2018-07-03 5:29 UTC (permalink / raw) To: u-boot 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 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. 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 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC 2018-07-03 5:29 ` Takahiro AKASHI @ 2018-07-03 6:00 ` Heinrich Schuchardt 0 siblings, 0 replies; 8+ messages in thread From: Heinrich Schuchardt @ 2018-07-03 6:00 UTC (permalink / raw) To: u-boot 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 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. > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [U-Boot,1/1] ARM: qemu-arm: enable RTC 2018-06-28 22:34 [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC Heinrich Schuchardt 2018-07-02 16:07 ` Tuomas Tynkkynen @ 2018-07-20 12:36 ` Tom Rini 1 sibling, 0 replies; 8+ messages in thread From: Tom Rini @ 2018-07-20 12:36 UTC (permalink / raw) To: u-boot 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! -- 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-07-20 12:36 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-28 22:34 [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC Heinrich Schuchardt 2018-07-02 16:07 ` Tuomas Tynkkynen 2018-07-02 23:51 ` Takahiro AKASHI 2018-07-03 2:07 ` Heinrich Schuchardt 2018-07-03 3:30 ` Heinrich Schuchardt 2018-07-03 5:29 ` Takahiro AKASHI 2018-07-03 6:00 ` Heinrich Schuchardt 2018-07-20 12:36 ` [U-Boot] [U-Boot,1/1] " Tom Rini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.