Re: [PATCH RESEND v3 3/7] rtc-mc146818-lib: extract mc146818_do_avoiding_UIP

From: Mateusz Jończyk
Date: Thu Nov 25 2021 - 00:30:57 EST


W dniu 24.11.2021 o 23:39, Alexandre Belloni pisze:
> On 19/11/2021 21:42:17+0100, Mateusz Jończyk wrote:
>> Function mc146818_get_time() contains an elaborate mechanism of reading
>> the RTC time while no RTC update is in progress. It turns out that
>> reading the RTC alarm clock also requires avoiding the RTC update (see
>> following patches). Therefore, the mechanism in mc146818_get_time()
>> should be reused - so extract it into a separate function.
>>
>> The logic in mc146818_do_avoiding_UIP() is same as in
>> mc146818_get_time() except that after every
>>
>> if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) {
>>
>> there is now "mdelay(1)".
>>
>> To avoid producing an unreadable diff, mc146818_get_time() will be
>> refactored to use mc146818_do_avoiding_UIP() in the next patch.
>>
>> Signed-off-by: Mateusz Jończyk <mat.jonczyk@xxxxx>
>> Cc: Alessandro Zummo <a.zummo@xxxxxxxxxxxx>
>> Cc: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
>>
>> ---
>> drivers/rtc/rtc-mc146818-lib.c | 69 ++++++++++++++++++++++++++++++++++
>> include/linux/mc146818rtc.h | 3 ++
>> 2 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c
>> index b50612ce1a6d..946ad43a512c 100644
>> --- a/drivers/rtc/rtc-mc146818-lib.c
>> +++ b/drivers/rtc/rtc-mc146818-lib.c
>> @@ -8,6 +8,75 @@
>> #include <linux/acpi.h>
>> #endif
>>
>> +/*
>> + * Execute a function while the UIP (Update-in-progress) bit of the RTC is
>> + * unset.
>> + *
>> + * Warning: callback may be executed more then once.
>> + */
>> +bool mc146818_do_avoiding_UIP(mc146818_callback_t callback, void *param)
> mc146818_avoid_UIP would be a simpler name.
Right
> Also, I'm pretty sure we can
> avoid the mc146818_callback_t typedef

Do you mean doing something like:

bool mc146818_avoid_UIP(
void (*callback)(unsigned char seconds, void *param), void *param);

Thanks for reviewing.

Greetings,
Mateusz

>> +{
>> + int i;
>> + unsigned long flags;
[snip]