[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 at 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