linux-kernel - RE: [PATCH v2 01/10] ARC: timer: rtc: implement read loop in "C" vs. inline asm

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]

Date:   Fri, 4 Nov 2016 17:58:11 +0000
From:   Alexey Brodkin <Alexey.Brodkin@...opsys.com>
To:     Vineet Gupta <Vineet.Gupta1@...opsys.com>
CC:     Noam Camus <noamca@...lanox.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-snps-arc@...ts.infradead.org" 
        <linux-snps-arc@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>
Subject: RE: [PATCH v2 01/10] ARC: timer: rtc: implement read loop in "C"
 vs. inline asm

Hi Vineet,

> -----Original Message-----
> From: Vineet Gupta [mailto:vgupta@...opsys.com]
> Sent: Friday, November 04, 2016 12:32 AM
> To: Daniel Lezcano <daniel.lezcano@...aro.org>
> Cc: Noam Camus <noamca@...lanox.com>; tglx@...utronix.de; linux-snps-arc@...ts.infradead.org; linux-kernel@...r.kernel.org;
> Alexey.Brodkin@...opsys.com; Vineet Gupta <vgupta@...opsys.com>; stable@...r.kernel.org
> Subject: [PATCH v2 01/10] ARC: timer: rtc: implement read loop in "C" vs. inline asm
> 
> The current code doesn't even compile ....
> 
> CC: stable@...r.kernel.org
> Signed-off-by: Vineet Gupta <vgupta@...opsys.com>
> ---
>  arch/arc/kernel/time.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
> index f927b8dc6edd..1a117b999c0c 100644
> --- a/arch/arc/kernel/time.c
> +++ b/arch/arc/kernel/time.c
> @@ -152,14 +152,17 @@ static cycle_t arc_read_rtc(struct clocksource *cs)
>  		cycle_t  full;
>  	} stamp;
> 
> -
> -	__asm__ __volatile(
> -	"1:						\n"
> -	"	lr		%0, [AUX_RTC_LOW]	\n"
> -	"	lr		%1, [AUX_RTC_HIGH]	\n"
> -	"	lr		%2, [AUX_RTC_CTRL]	\n"
> -	"	bbit0.nt	%2, 31, 1b		\n"
> -	: "=r" (stamp.low), "=r" (stamp.high), "=r" (status));
> +        /*
> +         * hardware has an internal state machine which tracks readout of
> +         * low/high and updates the CTRL.status if
> +         *  - interrupt/exception taken between the two reads
> +         *  - high increments after low has been read
> +         */
> +	do {
> +		stamp.low = read_aux_reg(AUX_RTC_LOW);
> +		stamp.high = read_aux_reg(AUX_RTC_HIGH);
> +		status = read_aux_reg(AUX_RTC_CTRL);
> +	} while (!(status & _BITUL(31)));

I think original Daniel's comment was about constant value used here.
Now with "_BITUL" it already looks much better but IMHO it would be even better if
31 gets converted here to something like:
------------------------>8--------------------
#define RTC_CTRL_A1_OFFSET 31
------------------------>8--------------------

-Alexey