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.g...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot