linux-kernel - Re: [PATCH RESEND v3 3/7] rtc-mc146818-lib: extract mc146818_do_avoiding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YZ9DoQYl0JUDq7pt@piout.net>
Date:   Thu, 25 Nov 2021 09:04:49 +0100
From:   Alexandre Belloni <alexandre.belloni@...tlin.com>
To:     Mateusz Jończyk <mat.jonczyk@...pl>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-rtc@...r.kernel.org, Alessandro Zummo <a.zummo@...ertech.it>
Subject: Re: [PATCH RESEND v3 3/7] rtc-mc146818-lib: extract
 mc146818_do_avoiding_UIP

On 25/11/2021 06:28:45+0100, Mateusz Jończyk wrote:
> 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@...pl>
> >> Cc: Alessandro Zummo <a.zummo@...ertech.it>
> >> Cc: Alexandre Belloni <alexandre.belloni@...tlin.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);
> 

yes!

> Thanks for reviewing.
> 
> Greetings,
> Mateusz
> 
> >> +{
> >> +	int i;
> >> +	unsigned long flags;
> [snip]

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com