Message-ID: <1b217171-b301-f2fe-f455-9bddabe2e115@o2.pl>
Date:   Thu, 25 Nov 2021 06:28:45 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.2
Subject: Re: [PATCH RESEND v3 3/7] rtc-mc146818-lib: extract
 mc146818_do_avoiding_UIP
Content-Language: en-GB
To:     Alexandre Belloni <alexandre.belloni@bootlin.com>
References: <20211119204221.66918-1-mat.jonczyk@o2.pl>
 <20211119204221.66918-4-mat.jonczyk@o2.pl> <YZ6/GC3xouzEZmEh@piout.net>
From:   =?UTF-8?Q?Mateusz_Jo=c5=84czyk?= <mat.jonczyk@o2.pl>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        linux-rtc@vger.kernel.org, Alessandro Zummo <a.zummo@towertech.it>
In-Reply-To: <YZ6/GC3xouzEZmEh@piout.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@o2.pl>
>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>
>> ---
>>  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]