[U-Boot] [PATCH 0/5] efi_loader: complete implementation of GetTime()

[U-Boot] [PATCH 0/5] efi_loader: complete implementation of GetTime()

[Heinrich Schuchardt]

The EFI runtime service GetTime() is used to read the time form the real
time clock.

The RTC drivers are made usable even if U-Boot is built without
CONFIG_CMD_DATE.

RTC drivers not complying to the driver model are now supported by the EFI
subsystem.

The implementation of the GetTime() runtime service is corrected.

A unit test is provided.

Heinrich Schuchardt (5):
  rtc: remove CONFIG_CMD_DATE dependency
  efi_loader: remove unused efi_get_time_init()
  efi_loader: complete implementation of GetTime()
  efi_selftest: support printing leading zeroes
  efi_selftest: unit test for GetTime()

 cmd/bootefi.c                           |   3 -
 drivers/rtc/at91sam9_rtt.c              |   4 -
 drivers/rtc/davinci.c                   |   2 -
 drivers/rtc/ds1302.c                    |   4 -
 drivers/rtc/ds1306.c                    |   4 -
 drivers/rtc/ds1307.c                    |   4 -
 drivers/rtc/ds1337.c                    |   4 -
 drivers/rtc/ds1374.c                    |   3 -
 drivers/rtc/ds164x.c                    |   4 -
 drivers/rtc/ds174x.c                    |   4 -
 drivers/rtc/ds3231.c                    |   4 -
 drivers/rtc/imxdi.c                     |   4 -
 drivers/rtc/m41t11.c                    |   3 -
 drivers/rtc/m41t60.c                    |   3 -
 drivers/rtc/m41t62.c                    |   4 -
 drivers/rtc/m48t35ax.c                  |   4 -
 drivers/rtc/max6900.c                   |   4 -
 drivers/rtc/mc146818.c                  |   3 -
 drivers/rtc/mcfrtc.c                    |   4 -
 drivers/rtc/mk48t59.c                   |   4 -
 drivers/rtc/pcf8563.c                   |   4 -
 drivers/rtc/pl031.c                     |   4 -
 drivers/rtc/rs5c372.c                   |   3 -
 drivers/rtc/rx8025.c                    |   4 -
 drivers/rtc/s3c24x0_rtc.c               |   4 -
 drivers/rtc/x1205.c                     |   4 -
 include/efi_loader.h                    |   1 -
 lib/efi_loader/efi_runtime.c            | 106 +++++++++++++++++++-----
 lib/efi_selftest/Makefile               |   1 +
 lib/efi_selftest/efi_selftest_console.c |  33 +++++---
 lib/efi_selftest/efi_selftest_rtc.c     |  67 +++++++++++++++
 31 files changed, 176 insertions(+), 128 deletions(-)
 create mode 100644 lib/efi_selftest/efi_selftest_rtc.c

-- 
2.18.0

[U-Boot] [PATCH 1/5] rtc: remove CONFIG_CMD_DATE dependency

[Heinrich Schuchardt]

The EFI subsystem accesses the real time clock and is enabled by default.
So we should drop any CONFIG_CMD_DATE dependency from the real time clock
drivers.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 drivers/rtc/at91sam9_rtt.c | 4 ----
 drivers/rtc/davinci.c      | 2 --
 drivers/rtc/ds1302.c       | 4 ----
 drivers/rtc/ds1306.c       | 4 ----
 drivers/rtc/ds1307.c       | 4 ----
 drivers/rtc/ds1337.c       | 4 ----
 drivers/rtc/ds1374.c       | 3 ---
 drivers/rtc/ds164x.c       | 4 ----
 drivers/rtc/ds174x.c       | 4 ----
 drivers/rtc/ds3231.c       | 4 ----
 drivers/rtc/imxdi.c        | 4 ----
 drivers/rtc/m41t11.c       | 3 ---
 drivers/rtc/m41t60.c       | 3 ---
 drivers/rtc/m41t62.c       | 4 ----
 drivers/rtc/m48t35ax.c     | 4 ----
 drivers/rtc/max6900.c      | 4 ----
 drivers/rtc/mc146818.c     | 3 ---
 drivers/rtc/mcfrtc.c       | 4 ----
 drivers/rtc/mk48t59.c      | 4 ----
 drivers/rtc/pcf8563.c      | 4 ----
 drivers/rtc/pl031.c        | 4 ----
 drivers/rtc/rs5c372.c      | 3 ---
 drivers/rtc/rx8025.c       | 4 ----
 drivers/rtc/s3c24x0_rtc.c  | 4 ----
 drivers/rtc/x1205.c        | 4 ----
 25 files changed, 93 deletions(-)

diff --git a/drivers/rtc/at91sam9_rtt.c b/drivers/rtc/at91sam9_rtt.c
index da5bb3e2f7..6f92660ef1 100644
--- a/drivers/rtc/at91sam9_rtt.c
+++ b/drivers/rtc/at91sam9_rtt.c
@@ -27,8 +27,6 @@
 #include <asm/arch/at91_rtt.h>
 #include <asm/arch/at91_gpbr.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 int rtc_get (struct rtc_time *tmp)
 {
 	at91_rtt_t *rtt = (at91_rtt_t *) ATMEL_BASE_RTT;
@@ -78,5 +76,3 @@ void rtc_reset (void)
 	while (readl(&rtt->vr) != 0)
 		;
 }
-
-#endif
diff --git a/drivers/rtc/davinci.c b/drivers/rtc/davinci.c
index f03dc56069..b6930bd839 100644
--- a/drivers/rtc/davinci.c
+++ b/drivers/rtc/davinci.c
@@ -9,7 +9,6 @@
 #include <asm/io.h>
 #include <asm/davinci_rtc.h>
 
-#if defined(CONFIG_CMD_DATE)
 int rtc_get(struct rtc_time *tmp)
 {
 	struct davinci_rtc *rtc = (struct davinci_rtc *)DAVINCI_RTC_BASE;
@@ -79,4 +78,3 @@ void rtc_reset(void)
 	/* run RTC counter */
 	writel(0x01, &rtc->ctrl);
 }
-#endif
diff --git a/drivers/rtc/ds1302.c b/drivers/rtc/ds1302.c
index 87ddd01916..b94163f748 100644
--- a/drivers/rtc/ds1302.c
+++ b/drivers/rtc/ds1302.c
@@ -9,8 +9,6 @@
 #include <command.h>
 #include <rtc.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 /* GPP Pins */
 #define DATA		0x200
 #define SCLK		0x400
@@ -328,5 +326,3 @@ int rtc_set(struct rtc_time *tmp)
 
 	return 0;
 }
-
-#endif
diff --git a/drivers/rtc/ds1306.c b/drivers/rtc/ds1306.c
index bd1e0845aa..02d617e09c 100644
--- a/drivers/rtc/ds1306.c
+++ b/drivers/rtc/ds1306.c
@@ -19,8 +19,6 @@
 #include <rtc.h>
 #include <spi.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 #define	RTC_SECONDS		0x00
 #define	RTC_MINUTES		0x01
 #define	RTC_HOURS		0x02
@@ -437,5 +435,3 @@ static void rtc_write (unsigned char reg, unsigned char val)
 }
 
 #endif /* end of code exclusion (see #ifdef CONFIG_SXNI855T above) */
-
-#endif
diff --git a/drivers/rtc/ds1307.c b/drivers/rtc/ds1307.c
index cdb088c9b1..48220b45db 100644
--- a/drivers/rtc/ds1307.c
+++ b/drivers/rtc/ds1307.c
@@ -51,8 +51,6 @@ enum ds_type {
 
 #ifndef CONFIG_DM_RTC
 
-#if defined(CONFIG_CMD_DATE)
-
 /*---------------------------------------------------------------------*/
 #undef DEBUG_RTC
 
@@ -204,8 +202,6 @@ static void rtc_write (uchar reg, uchar val)
 	i2c_reg_write (CONFIG_SYS_I2C_RTC_ADDR, reg, val);
 }
 
-#endif /* CONFIG_CMD_DATE*/
-
 #endif /* !CONFIG_DM_RTC */
 
 #ifdef CONFIG_DM_RTC
diff --git a/drivers/rtc/ds1337.c b/drivers/rtc/ds1337.c
index 9a0a214386..9b31048e97 100644
--- a/drivers/rtc/ds1337.c
+++ b/drivers/rtc/ds1337.c
@@ -15,8 +15,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 /*
  * RTC register addresses
  */
@@ -190,5 +188,3 @@ static void rtc_write (uchar reg, uchar val)
 {
 	i2c_reg_write (CONFIG_SYS_I2C_RTC_ADDR, reg, val);
 }
-
-#endif
diff --git a/drivers/rtc/ds1374.c b/drivers/rtc/ds1374.c
index bc27f61c72..5a2060fe75 100644
--- a/drivers/rtc/ds1374.c
+++ b/drivers/rtc/ds1374.c
@@ -18,8 +18,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 /*---------------------------------------------------------------------*/
 #undef DEBUG_RTC
 #define DEBUG_RTC
@@ -214,4 +212,3 @@ static void rtc_write_raw (uchar reg, uchar val)
 {
 		i2c_reg_write (CONFIG_SYS_I2C_RTC_ADDR, reg, val);
 }
-#endif
diff --git a/drivers/rtc/ds164x.c b/drivers/rtc/ds164x.c
index 7ee6214b2b..f8707892e7 100644
--- a/drivers/rtc/ds164x.c
+++ b/drivers/rtc/ds164x.c
@@ -20,8 +20,6 @@
 #include <rtc.h>
 
 
-#if defined(CONFIG_CMD_DATE)
-
 static uchar    rtc_read(unsigned int addr );
 static void     rtc_write(unsigned int addr, uchar val);
 
@@ -171,5 +169,3 @@ static void rtc_write( unsigned int addr, uchar val )
 #endif
 	*(volatile unsigned char*)(addr) = val;
 }
-
-#endif
diff --git a/drivers/rtc/ds174x.c b/drivers/rtc/ds174x.c
index b6daf59e80..94f943d97a 100644
--- a/drivers/rtc/ds174x.c
+++ b/drivers/rtc/ds174x.c
@@ -16,8 +16,6 @@
 #include <command.h>
 #include <rtc.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 static uchar rtc_read( unsigned int addr );
 static void  rtc_write( unsigned int addr, uchar val);
 
@@ -172,5 +170,3 @@ static void rtc_write( unsigned int addr, uchar val )
 #endif
 	out8( addr, val );
 }
-
-#endif
diff --git a/drivers/rtc/ds3231.c b/drivers/rtc/ds3231.c
index 0e66f6ea0f..9352ff87a2 100644
--- a/drivers/rtc/ds3231.c
+++ b/drivers/rtc/ds3231.c
@@ -16,8 +16,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 /*
  * RTC register addresses
  */
@@ -166,5 +164,3 @@ static void rtc_write (uchar reg, uchar val)
 {
 	i2c_reg_write (CONFIG_SYS_I2C_RTC_ADDR, reg, val);
 }
-
-#endif
diff --git a/drivers/rtc/imxdi.c b/drivers/rtc/imxdi.c
index 3c4797f4c5..39920f1a7a 100644
--- a/drivers/rtc/imxdi.c
+++ b/drivers/rtc/imxdi.c
@@ -17,8 +17,6 @@
 #include <linux/compat.h>
 #include <rtc.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 #include <asm/io.h>
 #include <asm/arch/imx-regs.h>
 
@@ -222,5 +220,3 @@ void rtc_reset(void)
 {
 	di_init();
 }
-
-#endif
diff --git a/drivers/rtc/m41t11.c b/drivers/rtc/m41t11.c
index e997152098..960348bb3e 100644
--- a/drivers/rtc/m41t11.c
+++ b/drivers/rtc/m41t11.c
@@ -29,8 +29,6 @@
 #endif
 */
 
-#if defined(CONFIG_SYS_I2C_RTC_ADDR) && defined(CONFIG_CMD_DATE)
-
 /* ------------------------------------------------------------------------- */
 /*
   these are simple defines for the chip local to here so they aren't too
@@ -167,4 +165,3 @@ void rtc_reset (void)
 	val = val & 0x3F;/*turn off freq test keep calibration*/
 	i2c_write(CONFIG_SYS_I2C_RTC_ADDR, RTC_CONTROL_ADDR, 1, &val, 1);
 }
-#endif
diff --git a/drivers/rtc/m41t60.c b/drivers/rtc/m41t60.c
index 7846193e26..c84c8e11b3 100644
--- a/drivers/rtc/m41t60.c
+++ b/drivers/rtc/m41t60.c
@@ -20,8 +20,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_SYS_I2C_RTC_ADDR) && defined(CONFIG_CMD_DATE)
-
 /*
  * Convert between century and "century bits" (CB1 and CB0).  These routines
  * assume years are in the range 1900 - 2299.
@@ -237,4 +235,3 @@ void rtc_reset(void)
 	}
 	rtc_dump("end reset");
 }
-#endif /* CONFIG_RTC_M41T60 && CONFIG_SYS_I2C_RTC_ADDR && CONFIG_CMD_DATE */
diff --git a/drivers/rtc/m41t62.c b/drivers/rtc/m41t62.c
index d53ef7d862..137438389d 100644
--- a/drivers/rtc/m41t62.c
+++ b/drivers/rtc/m41t62.c
@@ -18,8 +18,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 #define M41T62_REG_SSEC	0
 #define M41T62_REG_SEC	1
 #define M41T62_REG_MIN	2
@@ -130,5 +128,3 @@ void rtc_reset(void)
 	val &= ~M41T80_ALHOUR_HT;
 	i2c_write(CONFIG_SYS_I2C_RTC_ADDR, M41T62_REG_ALARM_HOUR, 1, &val, 1);
 }
-
-#endif
diff --git a/drivers/rtc/m48t35ax.c b/drivers/rtc/m48t35ax.c
index 0b035ced26..1cc24cccae 100644
--- a/drivers/rtc/m48t35ax.c
+++ b/drivers/rtc/m48t35ax.c
@@ -16,8 +16,6 @@
 #include <rtc.h>
 #include <config.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 static uchar rtc_read  (uchar reg);
 static void  rtc_write (uchar reg, uchar val);
 
@@ -135,5 +133,3 @@ static void rtc_write (uchar reg, uchar val)
 	*(unsigned char *)
 		((CONFIG_SYS_NVRAM_BASE_ADDR + CONFIG_SYS_NVRAM_SIZE - 8) + reg) = val;
 }
-
-#endif
diff --git a/drivers/rtc/max6900.c b/drivers/rtc/max6900.c
index f5c651bad0..b35186579a 100644
--- a/drivers/rtc/max6900.c
+++ b/drivers/rtc/max6900.c
@@ -15,8 +15,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 #ifndef	CONFIG_SYS_I2C_RTC_ADDR
 #define	CONFIG_SYS_I2C_RTC_ADDR	0x50
 #endif
@@ -104,5 +102,3 @@ int rtc_set (struct rtc_time *tmp)
 void rtc_reset (void)
 {
 }
-
-#endif
diff --git a/drivers/rtc/mc146818.c b/drivers/rtc/mc146818.c
index 500eef2ee0..444d4baf0b 100644
--- a/drivers/rtc/mc146818.c
+++ b/drivers/rtc/mc146818.c
@@ -19,8 +19,6 @@
 #define out8(p, v) outb(v, p)
 #endif
 
-#if defined(CONFIG_CMD_DATE)
-
 /* Set this to 1 to clear the CMOS RAM */
 #define CLEAR_CMOS		0
 
@@ -191,7 +189,6 @@ static void mc146818_init(void)
 	/* Clear any pending interrupts */
 	mc146818_read8(RTC_CONFIG_C);
 }
-#endif /* CONFIG_CMD_DATE */
 
 #ifdef CONFIG_DM_RTC
 
diff --git a/drivers/rtc/mcfrtc.c b/drivers/rtc/mcfrtc.c
index 0ac8e4dbc5..e10638ec7d 100644
--- a/drivers/rtc/mcfrtc.c
+++ b/drivers/rtc/mcfrtc.c
@@ -6,8 +6,6 @@
 
 #include <common.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 #include <command.h>
 #include <rtc.h>
 #include <asm/immap.h>
@@ -104,5 +102,3 @@ void rtc_reset(void)
 
 	rtc->cr |= RTC_CR_SWR;
 }
-
-#endif				/* CONFIG_MCFRTC && CONFIG_CMD_DATE */
diff --git a/drivers/rtc/mk48t59.c b/drivers/rtc/mk48t59.c
index d29d5ce600..8c90a70407 100644
--- a/drivers/rtc/mk48t59.c
+++ b/drivers/rtc/mk48t59.c
@@ -70,8 +70,6 @@ void nvram_write(short dest, const void *src, size_t count)
 		rtc_write(d++, *s++);
 }
 
-#if defined(CONFIG_CMD_DATE)
-
 /* ------------------------------------------------------------------------- */
 
 int rtc_get (struct rtc_time *tmp)
@@ -175,5 +173,3 @@ void rtc_set_watchdog(short multi, short res)
 	wd_value = RTC_WDS | ((multi & 0x1F) << 2) | (res & 0x3);
 	rtc_write(RTC_WATCHDOG, wd_value);
 }
-
-#endif
diff --git a/drivers/rtc/pcf8563.c b/drivers/rtc/pcf8563.c
index e2fa6b6aab..a839d6cc98 100644
--- a/drivers/rtc/pcf8563.c
+++ b/drivers/rtc/pcf8563.c
@@ -15,8 +15,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 static uchar rtc_read  (uchar reg);
 static void  rtc_write (uchar reg, uchar val);
 
@@ -117,5 +115,3 @@ static void rtc_write (uchar reg, uchar val)
 {
 	i2c_reg_write (CONFIG_SYS_I2C_RTC_ADDR, reg, val);
 }
-
-#endif
diff --git a/drivers/rtc/pl031.c b/drivers/rtc/pl031.c
index 8955805e3b..4a1044ec98 100644
--- a/drivers/rtc/pl031.c
+++ b/drivers/rtc/pl031.c
@@ -10,8 +10,6 @@
 #include <command.h>
 #include <rtc.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 #ifndef CONFIG_SYS_RTC_PL031_BASE
 #error CONFIG_SYS_RTC_PL031_BASE is not defined!
 #endif
@@ -103,5 +101,3 @@ int rtc_get(struct rtc_time *tmp)
 
 	return 0;
 }
-
-#endif
diff --git a/drivers/rtc/rs5c372.c b/drivers/rtc/rs5c372.c
index c815c915d5..97ec001aef 100644
--- a/drivers/rtc/rs5c372.c
+++ b/drivers/rtc/rs5c372.c
@@ -24,7 +24,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_CMD_DATE)
 /*
  * Reads are always done starting with register 15, which requires some
  * jumping-through-hoops to access the data correctly.
@@ -255,5 +254,3 @@ rtc_reset (void)
 	if (!setup_done)
 		rs5c372_enable();
 }
-
-#endif
diff --git a/drivers/rtc/rx8025.c b/drivers/rtc/rx8025.c
index 1c2b92fe00..7bd9f8b42a 100644
--- a/drivers/rtc/rx8025.c
+++ b/drivers/rtc/rx8025.c
@@ -13,8 +13,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 /*---------------------------------------------------------------------*/
 #undef DEBUG_RTC
 
@@ -190,5 +188,3 @@ static void rtc_write (uchar reg, uchar val)
 		printf("Error writing to RTC\n");
 
 }
-
-#endif /* CONFIG_RTC_RX8025 && CONFIG_CMD_DATE */
diff --git a/drivers/rtc/s3c24x0_rtc.c b/drivers/rtc/s3c24x0_rtc.c
index 1253085f53..96ea3cf878 100644
--- a/drivers/rtc/s3c24x0_rtc.c
+++ b/drivers/rtc/s3c24x0_rtc.c
@@ -11,8 +11,6 @@
 #include <common.h>
 #include <command.h>
 
-#if (defined(CONFIG_CMD_DATE))
-
 #include <asm/arch/s3c24x0_cpu.h>
 
 #include <rtc.h>
@@ -149,5 +147,3 @@ void rtc_reset(void)
 	writeb((readb(&rtc->rtccon) & ~0x06) | 0x08, &rtc->rtccon);
 	writeb(readb(&rtc->rtccon) & ~(0x08 | 0x01), &rtc->rtccon);
 }
-
-#endif
diff --git a/drivers/rtc/x1205.c b/drivers/rtc/x1205.c
index e9d8390f39..ed6aaa5237 100644
--- a/drivers/rtc/x1205.c
+++ b/drivers/rtc/x1205.c
@@ -22,8 +22,6 @@
 #include <rtc.h>
 #include <i2c.h>
 
-#if defined(CONFIG_CMD_DATE)
-
 #define CCR_SEC			0
 #define CCR_MIN			1
 #define CCR_HOUR		2
@@ -160,5 +158,3 @@ void rtc_reset(void)
 	 * Nothing to do
 	 */
 }
-
-#endif
-- 
2.18.0

[U-Boot] [PATCH 2/5] efi_loader: remove unused efi_get_time_init()

[Heinrich Schuchardt]

Remove unused function efi_get_time_init().

Initialization of the RTC has to be done in board bring up not in the EFI
subsystem.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 cmd/bootefi.c                | 3 ---
 include/efi_loader.h         | 1 -
 lib/efi_loader/efi_runtime.c | 5 -----
 3 files changed, 9 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 91277106a2..75d3eb967a 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -80,9 +80,6 @@ efi_status_t efi_init_obj_list(void)
 	ret = efi_reset_system_init();
 	if (ret != EFI_SUCCESS)
 		goto out;
-	ret = efi_get_time_init();
-	if (ret != EFI_SUCCESS)
-		goto out;
 
 out:
 	efi_obj_list_initialized = ret;
diff --git a/include/efi_loader.h b/include/efi_loader.h
index d6e1f50e22..cd6c722e00 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -418,7 +418,6 @@ efi_status_t efi_reset_system_init(void);
 efi_status_t __efi_runtime EFIAPI efi_get_time(
 			struct efi_time *time,
 			struct efi_time_cap *capabilities);
-efi_status_t efi_get_time_init(void);
 
 #ifdef CONFIG_CMD_BOOTEFI_SELFTEST
 /*
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index dd3ff8ad23..5ec17867fb 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -174,11 +174,6 @@ efi_status_t __weak __efi_runtime EFIAPI efi_get_time(
 	return EFI_DEVICE_ERROR;
 }
 
-efi_status_t __weak efi_get_time_init(void)
-{
-	return EFI_SUCCESS;
-}
-
 struct efi_runtime_detach_list_struct {
 	void *ptr;
 	void *patchto;
-- 
2.18.0

[U-Boot] [PATCH 3/5] efi_loader: complete implementation of GetTime()

[Heinrich Schuchardt]

Implement the missing parts of the GetTime() runtime service.

Support CONFIG_DM_RTC=n.
Fill seconds.
Fill daylight saving time flag correctly.
Provide dummy values for capabilities.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_runtime.c | 101 +++++++++++++++++++++++++++++------
 1 file changed, 86 insertions(+), 15 deletions(-)

diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 5ec17867fb..20eb3f373d 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -10,6 +10,7 @@
 #include <dm.h>
 #include <elf.h>
 #include <efi_loader.h>
+#include <i2c.h>
 #include <rtc.h>
 
 /* For manual relocation support */
@@ -117,24 +118,86 @@ static void EFIAPI efi_reset_system_boottime(
 	while (1) { }
 }
 
+int __weak rtc_get(struct rtc_time *tm)
+{
+	return 1;
+}
+
+#if defined(CONFIG_SYS_RTC_BUS_NUM) && !defined(CONFIG_DM_RTC)
+/**
+ * efi_set_rtc_i2c_bus - select I2C bus for real time clock
+ *
+ * @bus:		bus to select, -1 for default
+ * Return Value:	previously selected bus
+ */
+static int efi_set_rtc_i2c_bus(int bus)
+{
+	int old_bus;
+
+	if (bus < 0)
+		bus = CONFIG_SYS_RTC_BUS_NUM;
+
+#ifdef CONFIG_SYS_I2C
+	old_bus = i2c_get_bus_num();
+	i2c_set_bus_num(bus);
+#else
+	old_bus = I2C_GET_BUS();
+	I2C_SET_BUS(bus);
+#endif
+	return old_bus;
+}
+#endif /* CONFIG_SYS_RTC_BUS_NUM && !CONFIG_DM_RTC */
+
+/**
+ * efi_get_time_boottime - get current time
+ *
+ * This function implements the GetTime runtime service.
+ * See the Unified Extensible Firmware Interface (UEFI) specification
+ * for details.
+ *
+ * @time:		pointer to structure to receive current time
+ * @capabilities:	pointer to structure to receive RTC properties
+ * Return Value:	status code
+ */
 static efi_status_t EFIAPI efi_get_time_boottime(
 			struct efi_time *time,
 			struct efi_time_cap *capabilities)
 {
-#if defined(CONFIG_CMD_DATE) && defined(CONFIG_DM_RTC)
-	struct rtc_time tm;
+	efi_status_t ret = EFI_SUCCESS;
 	int r;
-	struct udevice *dev;
+	struct rtc_time tm;
 
 	EFI_ENTRY("%p %p", time, capabilities);
 
-	r = uclass_get_device(UCLASS_RTC, 0, &dev);
-	if (r)
-		return EFI_EXIT(EFI_DEVICE_ERROR);
+	if (!time) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
 
-	r = dm_rtc_get(dev, &tm);
-	if (r)
-		return EFI_EXIT(EFI_DEVICE_ERROR);
+#ifdef CONFIG_DM_RTC
+	{
+		struct udevice *dev;
+
+		r = uclass_get_device(UCLASS_RTC, 0, &dev);
+		if (!r)
+			r = dm_rtc_get(dev, &tm);
+	}
+#else
+	{
+#ifdef CONFIG_SYS_RTC_BUS_NUM
+		int oldbus = efi_set_rtc_i2c_bus(-1);
+
+#endif
+		r = rtc_get(&tm);
+#ifdef CONFIG_SYS_RTC_BUS_NUM
+		efi_set_rtc_i2c_bus(oldbus);
+#endif
+	}
+#endif
+	if (r) {
+		ret = EFI_DEVICE_ERROR;
+		goto out;
+	}
 
 	memset(time, 0, sizeof(*time));
 	time->year = tm.tm_year;
@@ -142,12 +205,20 @@ static efi_status_t EFIAPI efi_get_time_boottime(
 	time->day = tm.tm_mday;
 	time->hour = tm.tm_hour;
 	time->minute = tm.tm_min;
-	time->daylight = tm.tm_isdst;
-
-	return EFI_EXIT(EFI_SUCCESS);
-#else
-	return EFI_DEVICE_ERROR;
-#endif
+	time->second = tm.tm_sec;
+	time->daylight = EFI_TIME_ADJUST_DAYLIGHT;
+	if (tm.tm_isdst > 0)
+		time->daylight |= EFI_TIME_IN_DAYLIGHT;
+	time->timezone = EFI_UNSPECIFIED_TIMEZONE;
+
+	if (capabilities) {
+		/* Set reasonable dummy values */
+		capabilities->resolution = 1;		/* 1 Hz */
+		capabilities->accuracy = 100000000;	/* 100 ppm */
+		capabilities->sets_to_zero = false;
+	}
+out:
+	return EFI_EXIT(ret);
 }
 
 /* Boards may override the helpers below to implement RTS functionality */
-- 
2.18.0

[U-Boot] [PATCH 4/5] efi_selftest: support printing leading zeroes

[Heinrich Schuchardt]

Allow specifying the precision when printing integers, e.g.

efi_st_printf("%.4u-%.2u-%.2u\n", year, month, day);

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_selftest/efi_selftest_console.c | 33 ++++++++++++++++---------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_console.c b/lib/efi_selftest/efi_selftest_console.c
index c3655a1fcc..eb139c127c 100644
--- a/lib/efi_selftest/efi_selftest_console.c
+++ b/lib/efi_selftest/efi_selftest_console.c
@@ -70,11 +70,12 @@ static void pointer(void *pointer, u16 **buf)
 /*
  * Print an unsigned 32bit value as decimal number to an u16 string
  *
- * @value: value to be printed
- * @buf: pointer to buffer address
- * on return position of terminating zero word
+ * @value:	value to be printed
+ * @prec:	minimum number of digits to display
+ * @buf:	pointer to buffer address
+ *		on return position of terminating zero word
  */
-static void uint2dec(u32 value, u16 **buf)
+static void uint2dec(u32 value, int prec, u16 **buf)
 {
 	u16 *pos = *buf;
 	int i;
@@ -93,7 +94,7 @@ static void uint2dec(u32 value, u16 **buf)
 	for (i = 0; i < 10; ++i) {
 		/* Write current digit */
 		c = f >> 60;
-		if (c || pos != *buf)
+		if (c || pos != *buf || 10 - i <= prec)
 			*pos++ = c + '0';
 		/* Eliminate current digit */
 		f &= 0xfffffffffffffff;
@@ -109,11 +110,12 @@ static void uint2dec(u32 value, u16 **buf)
 /*
  * Print a signed 32bit value as decimal number to an u16 string
  *
- * @value: value to be printed
- * @buf: pointer to buffer address
+ * @value:	value to be printed
+ * @prec:	minimum number of digits to display
+ * @buf:	pointer to buffer address
  * on return position of terminating zero word
  */
-static void int2dec(s32 value, u16 **buf)
+static void int2dec(s32 value, int prec, u16 **buf)
 {
 	u32 u;
 	u16 *pos = *buf;
@@ -124,7 +126,7 @@ static void int2dec(s32 value, u16 **buf)
 	} else {
 		u = value;
 	}
-	uint2dec(u, &pos);
+	uint2dec(u, prec, &pos);
 	*buf = pos;
 }
 
@@ -143,6 +145,7 @@ void efi_st_printc(int color, const char *fmt, ...)
 	u16 *pos = buf;
 	const char *s;
 	u16 *u;
+	int prec;
 
 	va_start(args, fmt);
 
@@ -172,12 +175,20 @@ void efi_st_printc(int color, const char *fmt, ...)
 			break;
 		case '%':
 			++c;
+			/* Parse precision */
+			if (*c == '.') {
+				++c;
+				prec = *c - '0';
+				++c;
+			} else {
+				prec = 0;
+			}
 			switch (*c) {
 			case '\0':
 				--c;
 				break;
 			case 'd':
-				int2dec(va_arg(args, s32), &pos);
+				int2dec(va_arg(args, s32), prec, &pos);
 				break;
 			case 'p':
 				++c;
@@ -209,7 +220,7 @@ void efi_st_printc(int color, const char *fmt, ...)
 					*pos++ = *s;
 				break;
 			case 'u':
-				uint2dec(va_arg(args, u32), &pos);
+				uint2dec(va_arg(args, u32), prec, &pos);
 				break;
 			default:
 				break;
-- 
2.18.0

[U-Boot] [PATCH 5/5] efi_selftest: unit test for GetTime()

[Heinrich Schuchardt]

Provide a unit test for the GetTime() runtime service.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_selftest/Makefile           |  1 +
 lib/efi_selftest/efi_selftest_rtc.c | 67 +++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)
 create mode 100644 lib/efi_selftest/efi_selftest_rtc.c

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index bf5c8199cb..06b102bc30 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -23,6 +23,7 @@ efi_selftest_exitbootservices.o \
 efi_selftest_fdt.o \
 efi_selftest_gop.o \
 efi_selftest_manageprotocols.o \
+efi_selftest_rtc.o \
 efi_selftest_snp.o \
 efi_selftest_textinput.o \
 efi_selftest_textoutput.o \
diff --git a/lib/efi_selftest/efi_selftest_rtc.c b/lib/efi_selftest/efi_selftest_rtc.c
new file mode 100644
index 0000000000..8d440dc0b3
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_rtc.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * efi_selftest_rtc
+ *
+ * Copyright (c) 2018 Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * Test the real time clock runtime services.
+ */
+
+#include <efi_selftest.h>
+
+#define EFI_ST_NO_RTC "Could not read real time clock\n"
+
+static struct efi_runtime_services *runtime;
+
+/*
+ * Setup unit test.
+ *
+ * @handle:	handle of the loaded image
+ * @systable:	system table
+ * @return:	EFI_ST_SUCCESS for success
+ */
+static int setup(const efi_handle_t handle,
+		 const struct efi_system_table *systable)
+{
+	runtime = systable->runtime;
+	return EFI_ST_SUCCESS;
+}
+
+/*
+ * Execute unit test.
+ *
+ * Display current time.
+ *
+ * @return:	EFI_ST_SUCCESS for success
+ */
+static int execute(void)
+{
+	efi_status_t ret;
+	struct efi_time tm;
+
+	/* Display current time */
+	ret = runtime->get_time(&tm, NULL);
+	if (ret != EFI_SUCCESS) {
+#ifdef CONFIG_CMD_DATE
+		efi_st_error(EFI_ST_NO_RTC);
+		return EFI_ST_FAILURE;
+#else
+		efi_st_todo(EFI_ST_NO_RTC);
+		return EFI_ST_SUCCESS;
+#endif
+	} else {
+		efi_st_printf("Time according to real time clock: "
+			      "%.4u-%.2u-%.2u %.2u:%.2u:%.2u\n",
+			      tm.year, tm.month, tm.day,
+			      tm.hour, tm.minute, tm.second);
+	}
+
+	return EFI_ST_SUCCESS;
+}
+
+EFI_UNIT_TEST(rtc) = {
+	.name = "real time clock",
+	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+	.setup = setup,
+	.execute = execute,
+};
-- 
2.18.0

[U-Boot] [PATCH 2/5] efi_loader: remove unused efi_get_time_init()

[Alexander Graf]

On 06/30/2018 04:52 AM, Heinrich Schuchardt wrote:
> Remove unused function efi_get_time_init().
>
> Initialization of the RTC has to be done in board bring up not in the EFI
> subsystem.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Are there no EFI objects for RTC devices? These would have to get 
initialized from efi_init_obj_list().


Alex

[U-Boot] [PATCH 3/5] efi_loader: complete implementation of GetTime()

[Alexander Graf]

On 06/30/2018 04:52 AM, Heinrich Schuchardt wrote:
> Implement the missing parts of the GetTime() runtime service.
>
> Support CONFIG_DM_RTC=n.
> Fill seconds.
> Fill daylight saving time flag correctly.
> Provide dummy values for capabilities.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>   lib/efi_loader/efi_runtime.c | 101 +++++++++++++++++++++++++++++------
>   1 file changed, 86 insertions(+), 15 deletions(-)
>
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 5ec17867fb..20eb3f373d 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -10,6 +10,7 @@
>   #include <dm.h>
>   #include <elf.h>
>   #include <efi_loader.h>
> +#include <i2c.h>
>   #include <rtc.h>
>   
>   /* For manual relocation support */
> @@ -117,24 +118,86 @@ static void EFIAPI efi_reset_system_boottime(
>   	while (1) { }
>   }
>   
> +int __weak rtc_get(struct rtc_time *tm)
> +{
> +	return 1;
> +}
> +
> +#if defined(CONFIG_SYS_RTC_BUS_NUM) && !defined(CONFIG_DM_RTC)
> +/**
> + * efi_set_rtc_i2c_bus - select I2C bus for real time clock
> + *
> + * @bus:		bus to select, -1 for default
> + * Return Value:	previously selected bus
> + */
> +static int efi_set_rtc_i2c_bus(int bus)
> +{
> +	int old_bus;
> +
> +	if (bus < 0)
> +		bus = CONFIG_SYS_RTC_BUS_NUM;
> +
> +#ifdef CONFIG_SYS_I2C
> +	old_bus = i2c_get_bus_num();
> +	i2c_set_bus_num(bus);
> +#else
> +	old_bus = I2C_GET_BUS();
> +	I2C_SET_BUS(bus);
> +#endif
> +	return old_bus;
> +}
> +#endif /* CONFIG_SYS_RTC_BUS_NUM && !CONFIG_DM_RTC */
> +
> +/**
> + * efi_get_time_boottime - get current time
> + *
> + * This function implements the GetTime runtime service.
> + * See the Unified Extensible Firmware Interface (UEFI) specification
> + * for details.
> + *
> + * @time:		pointer to structure to receive current time
> + * @capabilities:	pointer to structure to receive RTC properties
> + * Return Value:	status code
> + */
>   static efi_status_t EFIAPI efi_get_time_boottime(
>   			struct efi_time *time,
>   			struct efi_time_cap *capabilities)
>   {
> -#if defined(CONFIG_CMD_DATE) && defined(CONFIG_DM_RTC)
> -	struct rtc_time tm;
> +	efi_status_t ret = EFI_SUCCESS;
>   	int r;
> -	struct udevice *dev;
> +	struct rtc_time tm;
>   
>   	EFI_ENTRY("%p %p", time, capabilities);
>   
> -	r = uclass_get_device(UCLASS_RTC, 0, &dev);
> -	if (r)
> -		return EFI_EXIT(EFI_DEVICE_ERROR);
> +	if (!time) {
> +		ret = EFI_INVALID_PARAMETER;
> +		goto out;
> +	}
>   
> -	r = dm_rtc_get(dev, &tm);
> -	if (r)
> -		return EFI_EXIT(EFI_DEVICE_ERROR);
> +#ifdef CONFIG_DM_RTC
> +	{
> +		struct udevice *dev;
> +
> +		r = uclass_get_device(UCLASS_RTC, 0, &dev);
> +		if (!r)
> +			r = dm_rtc_get(dev, &tm);
> +	}
> +#else
> +	{
> +#ifdef CONFIG_SYS_RTC_BUS_NUM
> +		int oldbus = efi_set_rtc_i2c_bus(-1);

Please make up your mind whether you want an #ifdef in this code path or 
not :). So IMHO you should either do the bus setting with ifdefs, but 
then explicitly pass CONFIG_SYS_RTC_BUS_NUM as parameter or do it all 
without ifdefs and just #ifdef out the body of efi_set_rtc_i2c_bus().


Alex

> +
> +#endif
> +		r = rtc_get(&tm);
> +#ifdef CONFIG_SYS_RTC_BUS_NUM
> +		efi_set_rtc_i2c_bus(oldbus);
> +#endif
> +	}
> +#endif
> +	if (r) {
> +		ret = EFI_DEVICE_ERROR;
> +		goto out;
> +	}
>   
>   	memset(time, 0, sizeof(*time));
>   	time->year = tm.tm_year;
> @@ -142,12 +205,20 @@ static efi_status_t EFIAPI efi_get_time_boottime(
>   	time->day = tm.tm_mday;
>   	time->hour = tm.tm_hour;
>   	time->minute = tm.tm_min;
> -	time->daylight = tm.tm_isdst;
> -
> -	return EFI_EXIT(EFI_SUCCESS);
> -#else
> -	return EFI_DEVICE_ERROR;
> -#endif
> +	time->second = tm.tm_sec;
> +	time->daylight = EFI_TIME_ADJUST_DAYLIGHT;
> +	if (tm.tm_isdst > 0)
> +		time->daylight |= EFI_TIME_IN_DAYLIGHT;
> +	time->timezone = EFI_UNSPECIFIED_TIMEZONE;
> +
> +	if (capabilities) {
> +		/* Set reasonable dummy values */
> +		capabilities->resolution = 1;		/* 1 Hz */
> +		capabilities->accuracy = 100000000;	/* 100 ppm */
> +		capabilities->sets_to_zero = false;
> +	}
> +out:
> +	return EFI_EXIT(ret);
>   }
>   
>   /* Boards may override the helpers below to implement RTS functionality */

[U-Boot] [PATCH 2/5] efi_loader: remove unused efi_get_time_init()

[Heinrich Schuchardt]

On 07/04/2018 05:15 PM, Alexander Graf wrote:
> On 06/30/2018 04:52 AM, Heinrich Schuchardt wrote:
>> Remove unused function efi_get_time_init().
>>
>> Initialization of the RTC has to be done in board bring up not in the EFI
>> subsystem.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> Are there no EFI objects for RTC devices? These would have to get
> initialized from efi_init_obj_list().

I am not aware of any EFI object relating to real time clocks. The RTC
is only accessed via the runtime services. See the UEFI spec.

Best regards

Heinrich

[U-Boot] [PATCH 3/5] efi_loader: complete implementation of GetTime()

[Heinrich Schuchardt]

On 07/04/2018 05:46 PM, Alexander Graf wrote:
> On 06/30/2018 04:52 AM, Heinrich Schuchardt wrote:
>> Implement the missing parts of the GetTime() runtime service.
>>
>> Support CONFIG_DM_RTC=n.
>> Fill seconds.
>> Fill daylight saving time flag correctly.
>> Provide dummy values for capabilities.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   lib/efi_loader/efi_runtime.c | 101 +++++++++++++++++++++++++++++------
>>   1 file changed, 86 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>> index 5ec17867fb..20eb3f373d 100644
>> --- a/lib/efi_loader/efi_runtime.c
>> +++ b/lib/efi_loader/efi_runtime.c
>> @@ -10,6 +10,7 @@
>>   #include <dm.h>
>>   #include <elf.h>
>>   #include <efi_loader.h>
>> +#include <i2c.h>
>>   #include <rtc.h>
>>     /* For manual relocation support */
>> @@ -117,24 +118,86 @@ static void EFIAPI efi_reset_system_boottime(
>>       while (1) { }
>>   }
>>   +int __weak rtc_get(struct rtc_time *tm)
>> +{
>> +    return 1;
>> +}
>> +
>> +#if defined(CONFIG_SYS_RTC_BUS_NUM) && !defined(CONFIG_DM_RTC)
>> +/**
>> + * efi_set_rtc_i2c_bus - select I2C bus for real time clock
>> + *
>> + * @bus:        bus to select, -1 for default
>> + * Return Value:    previously selected bus
>> + */
>> +static int efi_set_rtc_i2c_bus(int bus)
>> +{
>> +    int old_bus;
>> +
>> +    if (bus < 0)
>> +        bus = CONFIG_SYS_RTC_BUS_NUM;
>> +
>> +#ifdef CONFIG_SYS_I2C
>> +    old_bus = i2c_get_bus_num();
>> +    i2c_set_bus_num(bus);
>> +#else
>> +    old_bus = I2C_GET_BUS();
>> +    I2C_SET_BUS(bus);
>> +#endif
>> +    return old_bus;
>> +}
>> +#endif /* CONFIG_SYS_RTC_BUS_NUM && !CONFIG_DM_RTC */
>> +
>> +/**
>> + * efi_get_time_boottime - get current time
>> + *
>> + * This function implements the GetTime runtime service.
>> + * See the Unified Extensible Firmware Interface (UEFI) specification
>> + * for details.
>> + *
>> + * @time:        pointer to structure to receive current time
>> + * @capabilities:    pointer to structure to receive RTC properties
>> + * Return Value:    status code
>> + */
>>   static efi_status_t EFIAPI efi_get_time_boottime(
>>               struct efi_time *time,
>>               struct efi_time_cap *capabilities)
>>   {
>> -#if defined(CONFIG_CMD_DATE) && defined(CONFIG_DM_RTC)
>> -    struct rtc_time tm;
>> +    efi_status_t ret = EFI_SUCCESS;
>>       int r;
>> -    struct udevice *dev;
>> +    struct rtc_time tm;
>>         EFI_ENTRY("%p %p", time, capabilities);
>>   -    r = uclass_get_device(UCLASS_RTC, 0, &dev);
>> -    if (r)
>> -        return EFI_EXIT(EFI_DEVICE_ERROR);
>> +    if (!time) {
>> +        ret = EFI_INVALID_PARAMETER;
>> +        goto out;
>> +    }
>>   -    r = dm_rtc_get(dev, &tm);
>> -    if (r)
>> -        return EFI_EXIT(EFI_DEVICE_ERROR);
>> +#ifdef CONFIG_DM_RTC
>> +    {
>> +        struct udevice *dev;
>> +
>> +        r = uclass_get_device(UCLASS_RTC, 0, &dev);
>> +        if (!r)
>> +            r = dm_rtc_get(dev, &tm);
>> +    }
>> +#else
>> +    {
>> +#ifdef CONFIG_SYS_RTC_BUS_NUM
>> +        int oldbus = efi_set_rtc_i2c_bus(-1);
> 
> Please make up your mind whether you want an #ifdef in this code path or
> not :). So IMHO you should either do the bus setting with ifdefs, but
> then explicitly pass CONFIG_SYS_RTC_BUS_NUM as parameter or do it all
> without ifdefs and just #ifdef out the body of efi_set_rtc_i2c_bus().

The first thing to decide is if we want to support non-DM RTC at all.
What is your oppinion? - I put in the support for non-DM because I did
not see that QEMU was using a device tree. But now Takahiro has set up
the QEMU RTC driver as DM driver.

If CONFIG_SYS_RTC_BUS_NUM is defined or not is independent of
CONFIG_DM_RTC, so we cannot unconditionally pass CONFIG_SYS_RTC_BUS_NUM
here.

Yes I can move the ifdefs to the body efi_set_rtc_i2c_bus() at the
expense of some superfluous code being generated in case the function is
not needed.

But please, answer the above question first.

Best regards

Heinrich

[U-Boot] [PATCH 3/5] efi_loader: complete implementation of GetTime()

[Alexander Graf]

On 04.07.18 21:26, Heinrich Schuchardt wrote:
> On 07/04/2018 05:46 PM, Alexander Graf wrote:
>> On 06/30/2018 04:52 AM, Heinrich Schuchardt wrote:
>>> Implement the missing parts of the GetTime() runtime service.
>>>
>>> Support CONFIG_DM_RTC=n.
>>> Fill seconds.
>>> Fill daylight saving time flag correctly.
>>> Provide dummy values for capabilities.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>>   lib/efi_loader/efi_runtime.c | 101 +++++++++++++++++++++++++++++------
>>>   1 file changed, 86 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>>> index 5ec17867fb..20eb3f373d 100644
>>> --- a/lib/efi_loader/efi_runtime.c
>>> +++ b/lib/efi_loader/efi_runtime.c
>>> @@ -10,6 +10,7 @@
>>>   #include <dm.h>
>>>   #include <elf.h>
>>>   #include <efi_loader.h>
>>> +#include <i2c.h>
>>>   #include <rtc.h>
>>>     /* For manual relocation support */
>>> @@ -117,24 +118,86 @@ static void EFIAPI efi_reset_system_boottime(
>>>       while (1) { }
>>>   }
>>>   +int __weak rtc_get(struct rtc_time *tm)
>>> +{
>>> +    return 1;
>>> +}
>>> +
>>> +#if defined(CONFIG_SYS_RTC_BUS_NUM) && !defined(CONFIG_DM_RTC)
>>> +/**
>>> + * efi_set_rtc_i2c_bus - select I2C bus for real time clock
>>> + *
>>> + * @bus:        bus to select, -1 for default
>>> + * Return Value:    previously selected bus
>>> + */
>>> +static int efi_set_rtc_i2c_bus(int bus)
>>> +{
>>> +    int old_bus;
>>> +
>>> +    if (bus < 0)
>>> +        bus = CONFIG_SYS_RTC_BUS_NUM;
>>> +
>>> +#ifdef CONFIG_SYS_I2C
>>> +    old_bus = i2c_get_bus_num();
>>> +    i2c_set_bus_num(bus);
>>> +#else
>>> +    old_bus = I2C_GET_BUS();
>>> +    I2C_SET_BUS(bus);
>>> +#endif
>>> +    return old_bus;
>>> +}
>>> +#endif /* CONFIG_SYS_RTC_BUS_NUM && !CONFIG_DM_RTC */
>>> +
>>> +/**
>>> + * efi_get_time_boottime - get current time
>>> + *
>>> + * This function implements the GetTime runtime service.
>>> + * See the Unified Extensible Firmware Interface (UEFI) specification
>>> + * for details.
>>> + *
>>> + * @time:        pointer to structure to receive current time
>>> + * @capabilities:    pointer to structure to receive RTC properties
>>> + * Return Value:    status code
>>> + */
>>>   static efi_status_t EFIAPI efi_get_time_boottime(
>>>               struct efi_time *time,
>>>               struct efi_time_cap *capabilities)
>>>   {
>>> -#if defined(CONFIG_CMD_DATE) && defined(CONFIG_DM_RTC)
>>> -    struct rtc_time tm;
>>> +    efi_status_t ret = EFI_SUCCESS;
>>>       int r;
>>> -    struct udevice *dev;
>>> +    struct rtc_time tm;
>>>         EFI_ENTRY("%p %p", time, capabilities);
>>>   -    r = uclass_get_device(UCLASS_RTC, 0, &dev);
>>> -    if (r)
>>> -        return EFI_EXIT(EFI_DEVICE_ERROR);
>>> +    if (!time) {
>>> +        ret = EFI_INVALID_PARAMETER;
>>> +        goto out;
>>> +    }
>>>   -    r = dm_rtc_get(dev, &tm);
>>> -    if (r)
>>> -        return EFI_EXIT(EFI_DEVICE_ERROR);
>>> +#ifdef CONFIG_DM_RTC
>>> +    {
>>> +        struct udevice *dev;
>>> +
>>> +        r = uclass_get_device(UCLASS_RTC, 0, &dev);
>>> +        if (!r)
>>> +            r = dm_rtc_get(dev, &tm);
>>> +    }
>>> +#else
>>> +    {
>>> +#ifdef CONFIG_SYS_RTC_BUS_NUM
>>> +        int oldbus = efi_set_rtc_i2c_bus(-1);
>>
>> Please make up your mind whether you want an #ifdef in this code path or
>> not :). So IMHO you should either do the bus setting with ifdefs, but
>> then explicitly pass CONFIG_SYS_RTC_BUS_NUM as parameter or do it all
>> without ifdefs and just #ifdef out the body of efi_set_rtc_i2c_bus().
> 
> The first thing to decide is if we want to support non-DM RTC at all.
> What is your oppinion? - I put in the support for non-DM because I did
> not see that QEMU was using a device tree. But now Takahiro has set up
> the QEMU RTC driver as DM driver.

I think the RTC is a leaf enough use case to make DM mandatory on it.

> If CONFIG_SYS_RTC_BUS_NUM is defined or not is independent of
> CONFIG_DM_RTC, so we cannot unconditionally pass CONFIG_SYS_RTC_BUS_NUM
> here.

Yes, what I was trying to say is that this should either be:

	{
#ifdef CONFIG_SYS_RTC_BUS_NUM
		int oldbus = efi_set_rtc_i2c_bus(CONFIG_SYS_RTC_BUS_NUM);

#endif
		r = rtc_get(&tm);
#ifdef CONFIG_SYS_RTC_BUS_NUM
		efi_set_rtc_i2c_bus(oldbus);
#endif
	}


or

	{
		int oldbus = efi_set_rtc_i2c_bus(-1);

		r = rtc_get(&tm);
		efi_set_rtc_i2c_bus(oldbus);
	}

but passing in -1 when you are already in an #ifdef path that only
exists when CONFIG_SYS_RTC_BUS_NUM is available feels ... weird.

> Yes I can move the ifdefs to the body efi_set_rtc_i2c_bus() at the
> expense of some superfluous code being generated in case the function is
> not needed.

It will get optimized away :).

> But please, answer the above question first.

I don't mind it being DM only, as indicated above.


Alex