From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 28F9DAC0E6F for ; Fri, 5 Jan 2024 08:26:56 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=qHCIRUX2JLFppyzNihNpdIVikvs6o251Nj4OQPRJKEQ=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1704443215; v=1; b=i6/qMaGHLXOdRYVsnZhzHvPhf+QuKToOqoqyhRovQqDA/G+wWjO7pvjYL3WmnsyX1GDPd/VV xHvEz82dohVcd/1+bvzfBqDvMXkyTPARUJ11nmqhamuMCVMt5xHJk3v9NUj/29/jDzIsQ/mFlkZ 4f7201yo+/ztHhCzyepD+V4I= X-Received: by 127.0.0.2 with SMTP id TgbUYY7687511xven9UAwxSU; Fri, 05 Jan 2024 00:26:55 -0800 X-Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by mx.groups.io with SMTP id smtpd.web11.19484.1704443214626383067 for ; Fri, 05 Jan 2024 00:26:55 -0800 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 90C17CE1D22 for ; Fri, 5 Jan 2024 08:26:51 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id CC3AEC433CA for ; Fri, 5 Jan 2024 08:26:50 +0000 (UTC) X-Received: by mail-lf1-f53.google.com with SMTP id 2adb3069b0e04-50e7d6565b5so1473958e87.0 for ; Fri, 05 Jan 2024 00:26:50 -0800 (PST) X-Gm-Message-State: Vlng7TF19U4pPy7yhSYT7iMmx7686176AA= X-Google-Smtp-Source: AGHT+IGIzqqX95+WS1cslujuVdRG6kmJk1EDNhWfgmvo5q+xTGzD+xSFTiWMz2V82goD0GdhgZqfFzZgQszuIDIuUc0= X-Received: by 2002:a05:6512:314f:b0:50e:84a3:d74e with SMTP id s15-20020a056512314f00b0050e84a3d74emr715749lfi.73.1704443209002; Fri, 05 Jan 2024 00:26:49 -0800 (PST) MIME-Version: 1.0 References: <20240105051430.465510-1-rebecca@os.amperecomputing.com> <20240105051430.465510-3-rebecca@os.amperecomputing.com> In-Reply-To: <20240105051430.465510-3-rebecca@os.amperecomputing.com> From: "Ard Biesheuvel" Date: Fri, 5 Jan 2024 09:26:37 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v3 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation To: Rebecca Cran Cc: Leif Lindholm , Ard Biesheuvel , Sami Mujawar , devel@edk2.groups.io Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b="i6/qMaGH"; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Fri, 5 Jan 2024 at 06:15, Rebecca Cran wrote: > > The calculation of the timer period was broken. Introduce a global > mTimerPeriod so the calculation can be removed. Since mTimerFrequencyHz > is only used in one place, remove the global and make it a local > variable. Do the same with mNumTimerTicks. > > Signed-off-by: Rebecca Cran > --- > ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 36 ++++++++++++++---------------- > 1 file changed, 17 insertions(+), 19 deletions(-) > > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > index f8c39458a53a..78cee62a19d6 100644 > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c > @@ -28,13 +28,10 @@ > in a second */ > #define TIME_UNITS_PER_SECOND 10000000 > > -// Tick frequency of the generic timer basis of the generic watchdog. > -STATIC UINTN mTimerFrequencyHz = 0; > - > /* In cases where the compare register was set manually, information about > how long the watchdog was asked to wait cannot be retrieved from hardware. > It is therefore stored here. 0 means the timer is not running. */ > -STATIC UINT64 mNumTimerTicks = 0; > +STATIC UINT64 mTimerPeriod = 0; > > #define MAX_UINT48 0xFFFFFFFFFFFFULL > > @@ -91,7 +88,8 @@ WatchdogExitBootServicesEvent ( > ) > { > WatchdogDisable (); > - mNumTimerTicks = 0; > + mTimerPeriod = 0; > + mExitedBootServices = TRUE; Where is this declared/defined? > } > > /* This function is called when the watchdog's first signal (WS0) goes high. > @@ -106,7 +104,6 @@ WatchdogInterruptHandler ( > ) > { > STATIC CONST CHAR16 ResetString[] = L"The generic watchdog timer ran out."; > - UINT64 TimerPeriod; > > WatchdogDisable (); > > @@ -119,8 +116,7 @@ WatchdogInterruptHandler ( > // the timer period plus 1. > // > if (mWatchdogNotify != NULL) { > - TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); > - mWatchdogNotify (TimerPeriod + 1); > + mWatchdogNotify (mTimerPeriod + 1); > } > > gRT->ResetSystem ( > @@ -200,22 +196,27 @@ WatchdogSetTimerPeriod ( > IN UINT64 TimerPeriod // In 100ns units > ) > { > - UINTN SystemCount; > + UINTN SystemCount; > + UINT64 TimerFrequencyHz; > + UINT64 NumTimerTicks; > > // if TimerPeriod is 0, this is a request to stop the watchdog. > if (TimerPeriod == 0) { > - mNumTimerTicks = 0; > + mTimerPeriod = 0; > WatchdogDisable (); > return EFI_SUCCESS; > } > > // Work out how many timer ticks will equate to TimerPeriod > - mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND; > + TimerFrequencyHz = ArmGenericTimerGetTimerFreq (); > + ASSERT (TimerFrequencyHz != 0); > + mTimerPeriod = TimerPeriod; > + NumTimerTicks = (TimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND; > > /* If the number of required ticks is greater than the max the watchdog's > offset register (WOR) can hold, we need to manually compute and set > the compare register (WCV) */ > - if (mNumTimerTicks > MAX_UINT48) { > + if (NumTimerTicks > MAX_UINT48) { > /* We need to enable the watchdog *before* writing to the compare register, > because enabling the watchdog causes an "explicit refresh", which > clobbers the compare register (WCV). In order to make sure this doesn't > @@ -223,9 +224,9 @@ WatchdogSetTimerPeriod ( > WatchdogWriteOffsetRegister (MAX_UINT48); > WatchdogEnable (); > SystemCount = ArmGenericTimerGetSystemCount (); > - WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); > + WatchdogWriteCompareRegister (SystemCount + NumTimerTicks); > } else { > - WatchdogWriteOffsetRegister (mNumTimerTicks); > + WatchdogWriteOffsetRegister (NumTimerTicks); > WatchdogEnable (); > } > > @@ -260,7 +261,7 @@ WatchdogGetTimerPeriod ( > return EFI_INVALID_PARAMETER; > } > > - *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); > + *TimerPeriod = mTimerPeriod; > > return EFI_SUCCESS; > } > @@ -327,9 +328,6 @@ GenericWatchdogEntry ( > This will avoid conflicts with the universal watchdog */ > ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEfiWatchdogTimerArchProtocolGuid); > > - mTimerFrequencyHz = ArmGenericTimerGetTimerFreq (); > - ASSERT (mTimerFrequencyHz != 0); > - > // Install interrupt handler > Status = mInterruptProtocol->RegisterInterruptSource ( > mInterruptProtocol, > @@ -371,7 +369,7 @@ GenericWatchdogEntry ( > ); > ASSERT_EFI_ERROR (Status); > > - mNumTimerTicks = 0; > + mTimerPeriod = 0; > WatchdogDisable (); > > return EFI_SUCCESS; > -- > 2.34.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113222): https://edk2.groups.io/g/devel/message/113222 Mute This Topic: https://groups.io/mt/103538116/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-