From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=40.107.7.80; helo=eur04-he1-obe.outbound.protection.outlook.com; envelope-from=udit.kumar@nxp.com; receiver=edk2-devel@lists.01.org Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-eopbgr70080.outbound.protection.outlook.com [40.107.7.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B992521199B0C for ; Tue, 18 Dec 2018 07:58:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=TUQVI4P/ivv34qVDJFxDkIX/F/8hFF4hqYSnCi1O4vo=; b=w0cewYGxEqcSXQIdlPMTCxSGwgfkN3Jal+SifYvjI0oTcr8JyXw1JrErH4yEp4QzroN6LNe6ewmPke69uQXJ6RmxYTnt7T87z9VOakisKzphatu3W4Uw23fOpCV16rnytpG3k/f9DUAzKcEya616KzSNgJCdc6yvwCfqZMEthck= Received: from VI1PR04MB4640.eurprd04.prod.outlook.com (20.177.56.27) by VI1PR04MB3167.eurprd04.prod.outlook.com (10.170.229.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1425.18; Tue, 18 Dec 2018 15:58:56 +0000 Received: from VI1PR04MB4640.eurprd04.prod.outlook.com ([fe80::1dd2:4456:83ee:1bd5]) by VI1PR04MB4640.eurprd04.prod.outlook.com ([fe80::1dd2:4456:83ee:1bd5%2]) with mapi id 15.20.1425.023; Tue, 18 Dec 2018 15:58:56 +0000 From: Udit Kumar To: Ard Biesheuvel CC: "edk2-devel@lists.01.org" , Leif Lindholm , Sami Mujawar , Thomas Panakamattam Abraham , Meenakshi Aggarwal , Udit Kumar , Matteo Carlini , Nariman Poushin Thread-Topic: [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode Thread-Index: AQHUltMIMyAkZgK88ESMSWnrl1Re3KWEpeEA Date: Tue, 18 Dec 2018 15:58:56 +0000 Message-ID: References: <20181218131015.20062-1-ard.biesheuvel@linaro.org> <20181218131015.20062-3-ard.biesheuvel@linaro.org> In-Reply-To: <20181218131015.20062-3-ard.biesheuvel@linaro.org> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=udit.kumar@nxp.com; x-originating-ip: [182.69.235.121] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; VI1PR04MB3167; 6:xN8DnjhXqg044DINE4rmLK31b6gg6v9ybobWLuqXScd1SMYwfbmdhGhZCExNDCorv/FLXeFlCarCZcnX6rep4COlMEb+WXPhPSiudCBbE9ZOE1vKcB5/SKOUByCiC8E/txtqIzgeNFCJqNYyzgks6HVkQ/BQQq4V2GC1F5ip/SvJw0DAa4Vw12SYuZ6rgep+ycBI/t7o47EWpCbmA93giCF/1cqH4OY8z/5eMGEMJIP1lvqCm/viwHmebXNnUhnhpfO3M5YXRc2i29X9TzWVt/InrF7RPAZOcAT2rdvxayelcG75hCC6lnm9Fs9CCSwgecwB6dMHKjLA1AmV0dvkeFYRjScpFz9x8o1FvGavXcrSKR+qLm87zGc6rDXz6bOkJ3pL67zTdf588BKGMhpi9msi2iLQFIGByoFcXc4uHoMnklc6E1Bl2Gtxf/TYLI/LuqUBdmt2lVxSfHdfyFG0Ww==; 5:Un81gCIyEMJmApPyJAPqkw0q7bmCFwLdul03MU5EsydTwGbFNDNjpiOX5Jh3lo2mx0eYOZI7C5RfmrJDSn/If74mQotDRIrLR2mwHblkGONH+cLjHknRqxLDVKNRX4NP4ebsPiNAb5zOvukX9PQ3DF2agqcZX6lS8ual9q54kiA=; 7:ypjY3X4nGk6B/LjnoReDB14qxiuUVZuK5inp6FBcBvaoOmwz2AsuK0fM+mUL6X+rou2re82Q1Ds/ggeCtywVOzGIJmZIwx/FA9keZhRUSZZzkJkg9Z+iVfhCo+2qkXfSGRg20PNPnwSLHTVd9tC5qw== x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 31a428eb-5fe9-4dd2-e566-08d66501b74c x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:VI1PR04MB3167; x-ms-traffictypediagnostic: VI1PR04MB3167: x-microsoft-antispam-prvs: x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(3230021)(999002)(6040522)(2401047)(5005006)(8121501046)(3231475)(944501520)(52105112)(93006095)(93001095)(3002001)(10201501046)(6055026)(148016)(149066)(150057)(6041310)(20161123558120)(20161123562045)(20161123560045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991095); SRVR:VI1PR04MB3167; BCL:0; PCL:0; RULEID:; SRVR:VI1PR04MB3167; x-forefront-prvs: 08902E536D x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(136003)(346002)(366004)(376002)(396003)(13464003)(199004)(189003)(256004)(14444005)(478600001)(305945005)(5660300001)(19627235002)(71190400001)(71200400001)(486006)(106356001)(476003)(446003)(78486014)(11346002)(6506007)(102836004)(25786009)(53546011)(68736007)(26005)(7696005)(186003)(229853002)(99286004)(76176011)(4326008)(86362001)(6436002)(6246003)(97736004)(33656002)(316002)(575784001)(53936002)(9686003)(55016002)(74316002)(44832011)(6916009)(105586002)(3846002)(6116002)(14454004)(66066001)(7736002)(2906002)(54906003)(8936002)(81166006)(81156014)(8676002); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR04MB3167; H:VI1PR04MB4640.eurprd04.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: wGgoU/6HWjx7IyqsDO8bHQiYwSXiJDN4IhXcbQ2yKcXoOJYqZZzcWMAF2ChFD7IJlXqn2ffJhfYV9aiU3lYmkCtGdX88rm/x84qAmF03VeZIEUc0ZrKw6sCmBrtXp2iMozhDM6eE1UPJl5rRsnGKC4IP9rMZ1e4+fvPGB1yycM6l+TiERleE4PDraEPYSZzAEnojUze9Ur2btYqWVSKyaAdaH9lZzOmWArX0EOmvUfWSaXKTikiw1w86Y+Fahf/MI+ACVsJRj4frLeOoVd/SUJqMDmjc/CxtBrewmh+JU3ufIPbYD8AhMtPuwRG7tHmg spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 31a428eb-5fe9-4dd2-e566-08d66501b74c X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Dec 2018 15:58:56.2895 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB3167 Subject: Re: [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Dec 2018 15:59:01 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Ard Biesheuvel > Sent: Tuesday, December 18, 2018 6:40 PM > To: edk2-devel@lists.01.org > Cc: Ard Biesheuvel ; Leif Lindholm > ; Sami Mujawar ; Thomas > Panakamattam Abraham ; Meenakshi Aggarwal > ; Udit Kumar ; Matteo > Carlini ; Nariman Poushin > > Subject: [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt > mode >=20 > The SP805 watchdog driver doesn't implement the PI watchdog protocol full= y, > but always simply resets the system if the watchdog time runs out. >=20 > However, the hardware does support the intended usage model, as long as t= he > SP805 is wired up correctly. So let's implement interrupt based mode invo= lving a > handler that is registered by the DXE core and invoked when the watchdog = runs > out. In the interrupt handler, we invoke the notify function if one was r= egistered, > or call the ResetSystem() runtime service otherwise (as per the UEFI spec= ) Specs, says=20 If no handler has been registered, or the registered handler returns, then = the system will be reset by calling the Runtime Service ResetSystem(). My understanding is that we have to ResetSystem() always irrespective of no= tify function =20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > ArmPlatformPkg/ArmPlatformPkg.dec | 1 + > ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c | 95 > ++++++++++++++------ > ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf | 6 +- > 3 files changed, 75 insertions(+), 27 deletions(-) >=20 > diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec > b/ArmPlatformPkg/ArmPlatformPkg.dec > index 5f67e7415469..44c00bd0c133 100644 > --- a/ArmPlatformPkg/ArmPlatformPkg.dec > +++ b/ArmPlatformPkg/ArmPlatformPkg.dec > @@ -70,6 +70,7 @@ > ## SP805 Watchdog >=20 > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x0|UINT32|0x000000 > 23 >=20 > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|32000| > UINT32|0x00000021 > + > + > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt|0|UINT32|0x0000 > 00 > + 2E >=20 > ## PL011 UART >=20 > gArmPlatformTokenSpaceGuid.PL011UartClkInHz|24000000|UINT32|0x000000 > 1F > diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c > b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c > index 12c2f0a1fe49..4f09acf1fa28 100644 > --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c > +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c > @@ -21,12 +21,17 @@ > #include > #include > #include > +#include >=20 > +#include > #include >=20 > #include "SP805Watchdog.h" >=20 > -STATIC EFI_EVENT mEfiExitBootServicesEvent; > +STATIC EFI_EVENT mEfiExitBootServicesEvent; > +STATIC EFI_HARDWARE_INTERRUPT_PROTOCOL *mInterrupt; > +STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; > +STATIC UINT32 mTimerPeriod; >=20 > /** > Make sure the SP805 registers are unlocked for writing. > @@ -65,6 +70,33 @@ SP805Lock ( > } > } >=20 > +STATIC > +VOID > +EFIAPI > +SP805InterruptHandler ( > + IN HARDWARE_INTERRUPT_SOURCE Source, > + IN EFI_SYSTEM_CONTEXT SystemContext > + ) > +{ > + // > + // The notify function should be called with the elapsed number of > +ticks > + // since the watchdog was armed, which should exceed the timer period. > + // We don't actually know the elapsed number of ticks, so let's > +return > + // the timer period plus 1. > + // > + if (mWatchdogNotify !=3D NULL) { > + mWatchdogNotify (mTimerPeriod + 1); > + } else { > + gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, 0, NULL); > + } Shouldn't we ResetSystem in all cases. =20 > + > + SP805Unlock (); > + MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears > + the irq SP805Lock (); > + > + mInterrupt->EndOfInterrupt (mInterrupt, Source); } > + IMO, this routine should be=20 1/ Handle IT 2/ Do callback mWatchdogNotify 3/ Reset the system. > /** > Stop the SP805 watchdog timer from counting down by disabling interrup= ts. > **/ > @@ -149,9 +181,16 @@ SP805RegisterHandler ( > IN EFI_WATCHDOG_TIMER_NOTIFY NotifyFunction > ) > { > - // ERROR: This function is not supported. > - // The hardware watchdog will reset the board > - return EFI_INVALID_PARAMETER; > + if (mWatchdogNotify =3D=3D NULL && NotifyFunction =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + if (mWatchdogNotify !=3D NULL && NotifyFunction !=3D NULL) { > + return EFI_ALREADY_STARTED; > + } > + > + mWatchdogNotify =3D NotifyFunction; > + return EFI_SUCCESS; > } >=20 > /** > @@ -202,19 +241,16 @@ SP805SetTimerPeriod ( > SP805Stop (); > } else { > // Calculate the Watchdog ticks required for a delay of (TimerTicks = * 100) > nanoseconds > - // The SP805 will count down to ZERO once, generate an interrupt and > - // then it will again reload the initial value and start again. > - // On the second time when it reaches ZERO, it will actually reset t= he board. > - // Therefore, we need to load half the required delay. > + // The SP805 will count down to zero and generate an interrupt. > // > - // WatchdogTicks =3D ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / > 1GHz) / 2 ; > + // WatchdogTicks =3D ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / > + 1GHz); > // > // i.e.: > // > - // WatchdogTicks =3D (TimerPeriod * SP805_CLOCK_FREQUENCY) / 20 MHz = ; > + // WatchdogTicks =3D (TimerPeriod * SP805_CLOCK_FREQUENCY) / 10 MHz = ; >=20 > Ticks64bit =3D MultU64x32 (TimerPeriod, PcdGet32 > (PcdSP805WatchdogClockFrequencyInHz)); > - Ticks64bit =3D DivU64x32 (Ticks64bit, 20000000); > + Ticks64bit =3D DivU64x32 (Ticks64bit, 10 * 1000 * 1000); >=20 > // The registers in the SP805 are only 32 bits > if (Ticks64bit > MAX_UINT32) { > @@ -233,9 +269,12 @@ SP805SetTimerPeriod ( > SP805Start (); > } >=20 > + mTimerPeriod =3D TimerPeriod; > + > EXIT: > // Ensure the watchdog is locked before exiting. > SP805Lock (); > + ASSERT_EFI_ERROR (Status); > return Status; > } >=20 > @@ -262,25 +301,11 @@ SP805GetTimerPeriod ( > OUT UINT64 *TimerPeriod > ) > { > - UINT64 ReturnValue; > - > if (TimerPeriod =3D=3D NULL) { > return EFI_INVALID_PARAMETER; > } >=20 > - // Check if the watchdog is stopped > - if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & > SP805_WDOG_CTRL_INTEN) =3D=3D 0) { > - // It is stopped, so return zero. > - ReturnValue =3D 0; > - } else { > - // Convert the Watchdog ticks into TimerPeriod > - // Ensure 64bit arithmetic throughout because the Watchdog ticks may > already > - // be at the maximum 32 bit value and we still need to multiply that= by 600. > - ReturnValue =3D MultU64x32 (MmioRead32 (SP805_WDOG_LOAD_REG), 600); > - } > - > - *TimerPeriod =3D ReturnValue; > - > + *TimerPeriod =3D mTimerPeriod; > return EFI_SUCCESS; > } >=20 > @@ -343,6 +368,11 @@ SP805Initialize ( > EFI_STATUS Status; > EFI_HANDLE Handle; >=20 > + // Find the interrupt controller protocol. ASSERT if not found. > + Status =3D gBS->LocateProtocol (&gHardwareInterruptProtocolGuid, NULL, > + (VOID **)&mInterrupt); ASSERT_EFI_ERROR (Status); > + > // Unlock access to the SP805 registers > SP805Unlock (); >=20 > @@ -350,13 +380,26 @@ SP805Initialize ( > SP805Stop (); >=20 > // Set the watchdog to reset the board when triggered > + // This is a last resort in case the interrupt handler fails > if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & > SP805_WDOG_CTRL_RESEN) =3D=3D 0) { > MmioOr32 (SP805_WDOG_CONTROL_REG, SP805_WDOG_CTRL_RESEN); > } >=20 > + // Clear any pending interrupts > + MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears > + the irq > + > // Prohibit any rogue access to SP805 registers > SP805Lock (); >=20 > + Status =3D mInterrupt->RegisterInterruptSource (mInterrupt, > + PcdGet32 (PcdSP805WatchdogInterrupt), > + SP805InterruptHandler); if (EFI_ERROR > + (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: failed to register watchdog interrupt - %r= \n", > + __FUNCTION__, Status)); > + return Status; > + } > + > // > // Make sure the Watchdog Timer Architectural Protocol has not been in= stalled > in the system yet. > // This will avoid conflicts with the universal watchdog diff --git > a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf > b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf > index 99ecab477567..1373e267612f 100644 > --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf > +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf > @@ -27,6 +27,7 @@ > [Packages] > ArmPlatformPkg/ArmPlatformPkg.dec > ArmPkg/ArmPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > MdePkg/MdePkg.dec >=20 > [LibraryClasses] > @@ -35,13 +36,16 @@ > IoLib > UefiBootServicesTableLib > UefiDriverEntryPoint > + UefiRuntimeServicesTableLib >=20 > [Pcd] > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz > + gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt >=20 > [Protocols] > + gHardwareInterruptProtocolGuid ## ALWAYS_CONSUMES > gEfiWatchdogTimerArchProtocolGuid ## ALWAYS_PRODUCES >=20 > [Depex] > - TRUE > + gHardwareInterruptProtocolGuid > -- > 2.17.1