From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=203.18.50.4; helo=nat-hk.nvidia.com; envelope-from=jbrasen@nvidia.com; receiver=edk2-devel@lists.01.org Received: from nat-hk.nvidia.com (nat-hk.nvidia.com [203.18.50.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7C6342117B545 for ; Fri, 9 Nov 2018 09:04:23 -0800 (PST) Received: from hkpgpgate101.nvidia.com (Not Verified[10.18.92.9]) by nat-hk.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Sat, 10 Nov 2018 01:04:21 +0800 Received: from HKMAIL101.nvidia.com ([10.18.16.10]) by hkpgpgate101.nvidia.com (PGP Universal service); Fri, 09 Nov 2018 09:04:21 -0800 X-PGP-Universal: processed; by hkpgpgate101.nvidia.com on Fri, 09 Nov 2018 09:04:21 -0800 Received: from DRBGMAIL101.nvidia.com (10.18.16.20) by HKMAIL101.nvidia.com (10.18.16.10) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Fri, 9 Nov 2018 17:04:19 +0000 Received: from HKMAIL104.nvidia.com (10.18.16.13) by DRBGMAIL101.nvidia.com (10.18.16.20) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Fri, 9 Nov 2018 17:04:17 +0000 Received: from NAM01-SN1-obe.outbound.protection.outlook.com (207.46.163.120) by HKMAIL104.nvidia.com (10.18.16.13) with Microsoft SMTP Server (TLS) id 15.0.1395.4 via Frontend Transport; Fri, 9 Nov 2018 17:04:17 +0000 Received: from DM5PR12MB2439.namprd12.prod.outlook.com (52.132.141.32) by DM5PR12MB1804.namprd12.prod.outlook.com (10.175.91.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1294.31; Fri, 9 Nov 2018 17:03:54 +0000 Received: from DM5PR12MB2439.namprd12.prod.outlook.com ([fe80::d0a8:220b:4ebc:35e2]) by DM5PR12MB2439.namprd12.prod.outlook.com ([fe80::d0a8:220b:4ebc:35e2%4]) with mapi id 15.20.1294.034; Fri, 9 Nov 2018 17:03:53 +0000 From: Jeff Brasen To: Leif Lindholm CC: "edk2-devel@lists.01.org" , Ard Biesheuvel , Grish Pathak , "Sami Mujawar" Thread-Topic: [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function Thread-Index: AQHUd4uhcqcHw3MLLUeQSNcll5pwwKVHfKuAgAAvC5k= Date: Fri, 9 Nov 2018 17:03:53 +0000 Message-ID: References: <6dbe50db1293266db7588630fc97328276e82843.1541699412.git.jbrasen@nvidia.com>, <20181109140951.jcworswoffwot2q4@bivouac.eciton.net> In-Reply-To: <20181109140951.jcworswoffwot2q4@bivouac.eciton.net> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=jbrasen@nvidia.com; x-originating-ip: [216.228.112.22] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DM5PR12MB1804; 6:vO6CKcIEc84DlHCnaPOecSad352AqYMwNEFNOyhetB3qrfL3/LkEmMkQPBW+efLMBn0keKHfTf3V5sYH9tq3VIVCTnX8Ku5CZ+p7+er2A/63QZv84gJJ5w4sYB7wELLkKgRFtNcyxrdXbEUQ4qEXXaXzvSdW0ZZdP4yFWODnnNf05PAeLA+2U9cOQQZl7dy4xYJchLoDXPNMtUtR7tUT3uWUjPmHZKfSUF9Is7kzu/+z2TF9UcdRAEvkIUC8ObQD+VlqhzRYPSQU7H6uBawC1Bp36gMS/snr5pmADvJG13ZlXqKx5jeZZkaWlHBuY8QofK17IY8CDB3UNObybcGQtHutLZyvgAwLt81XUmE9J7m7EvE4OBWhhPird2aEpKzoIKUjdXmiyCgKnp28KFg2vVVB3D7Ro1EeboJIrtTiU/zN22vXwT/zURHUH+Rxts9JyKDFNXVwasRHH2Ffzm0mww==; 5:frCYiZRcGx41vjiRItfzELSqJzWZkjWFbgSIAR4PZGVM8qge8jpmgyMMthYx/gvTaEpHjN8sUXilB9LLcMzvuiIavtul6JLMvM3tIH75j+6uI2gIqSro1OKvkCIsq6Bf1fVwKmMPbCw6WvEsuMz3Rsp9GMMEUFnc+f7QBhOtyAs=; 7:Fq94wQEu8l2P65NTe9vpSHW/P/5XTny7FfdsZJC21N1iKigGywHwIbKzxGAA58bdyoxEr3JQvqalmjitLsw0Igi/YD4ygI6MSGbtKO+h9IMmUO4CY+KigxQlhI2y7Dd5/EK/VEM+tSfs9Xxpeu6fMQ== x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: b01aa002-2319-40fa-e4ee-08d64665544f x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(5600074)(711020)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:DM5PR12MB1804; x-ms-traffictypediagnostic: DM5PR12MB1804: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(162533806227266)(18589796830644)(180628864354917)(166708455590820); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040522)(2401047)(8121501046)(5005006)(93006095)(93001095)(10201501046)(3002001)(3231382)(944501410)(52105095)(148016)(149066)(150057)(6041310)(20161123564045)(20161123560045)(20161123562045)(20161123558120)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991095); SRVR:DM5PR12MB1804; BCL:0; PCL:0; RULEID:; SRVR:DM5PR12MB1804; x-forefront-prvs: 08512C5403 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(396003)(39860400002)(136003)(376002)(366004)(189003)(199004)(486006)(97736004)(8936002)(14454004)(54896002)(68736007)(966005)(2900100001)(6436002)(33656002)(55016002)(71200400001)(106356001)(6916009)(71190400001)(606006)(54906003)(6306002)(229853002)(8676002)(81156014)(575784001)(105586002)(316002)(19627405001)(86362001)(81166006)(476003)(76176011)(446003)(7696005)(6606003)(5660300001)(256004)(14444005)(53936002)(19627235002)(11346002)(2906002)(25786009)(186003)(53546011)(74316002)(236005)(478600001)(26005)(6506007)(66066001)(9686003)(102836004)(3846002)(1015004)(4326008)(7736002)(6246003)(6116002)(99286004); DIR:OUT; SFP:1101; SCL:1; SRVR:DM5PR12MB1804; H:DM5PR12MB2439.namprd12.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: nvidia.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: B7Br1dckf8Q96rWOe+u3fe2OGivaomyjC1DN1/84W7rzEAFjOj0Ygq4TVhQvYFf0eJOnxJr6T214BMGqMg6xByNKGPnXIFtCfAcKxj7BH1piygHtY1FLZUoG4kVAA1wQdc9KbRfH+xbJpjgCksEFYum+/x7B2EgDwlYGuahsvas8v7nqcJmwp0VZaOREspk/TVz8LG0KjbMe0ybxpGif4cMBmwnvKjeBhy15Sq4YgCJvMC9aE1iacVeq0HKboXJX+xdR6+ciUoedU/JHDxqagsYM6EwSlnp/y9pm5xD1g+nhpMQNb1OTZlBeSkYOv33INbcF0ewJF9+RcsHoZBOwyZmj+9/0vz6CyvwIhDTBzr8= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: b01aa002-2319-40fa-e4ee-08d64665544f X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Nov 2018 17:03:53.8104 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1804 X-OriginatorOrg: Nvidia.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1541783061; bh=d/FWzrm/YLuPOb5/omkJu0b/zvaerwJAaJoYOLmi/L4=; h=X-PGP-Universal:From:To:CC:Subject:Thread-Topic:Thread-Index:Date: Message-ID:References:In-Reply-To:Accept-Language:X-MS-Has-Attach: X-MS-TNEF-Correlator:authentication-results:x-originating-ip: x-ms-publictraffictype:x-microsoft-exchange-diagnostics: x-ms-exchange-antispam-srfa-diagnostics: x-ms-office365-filtering-correlation-id:x-microsoft-antispam: x-ms-traffictypediagnostic:x-microsoft-antispam-prvs: x-exchange-antispam-report-test:x-ms-exchange-senderadcheck: x-exchange-antispam-report-cfa-test:x-forefront-prvs: x-forefront-antispam-report:received-spf: x-microsoft-antispam-message-info:spamdiagnosticoutput: spamdiagnosticmetadata:MIME-Version: X-MS-Exchange-CrossTenant-Network-Message-Id: X-MS-Exchange-CrossTenant-originalarrivaltime: X-MS-Exchange-CrossTenant-fromentityheader: X-MS-Exchange-CrossTenant-id: X-MS-Exchange-Transport-CrossTenantHeadersStamped:X-OriginatorOrg: Content-Language:Content-Type; b=k7FoF+V3dk/h6aI58G+WcsJ6abwIUCN5QzzHk+JeC3S+tN/tOU+cckk21QROGk62/ o/u510lVNndJotMLMZEaFTmI6otNpZXRUATjaPHFJKS05JocFGF+5q7nC/+9dBrNTU o9n0Uht1Cra/FMwKIwb6Xa5ys0te8rBkHd+cplZ3Gnie3ATPPBNoLngTZGgVAti5k6 jAfHLqV8tIFDHAkoX73gkZBI+R+dzt/syq5vrEJaByPGa8wfLb9J72i+Z/B5v8c9oC HBhhPxfZLpyb806tBEY8KQ/0+Hrnak0e+A7h3XKRMs89n2uNQI7r/ac1K5SXSaPG6o UyqxcS7Y++VtA== X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function 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: Fri, 09 Nov 2018 17:04:24 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable ________________________________ From: Leif Lindholm Sent: Friday, November 9, 2018 7:09 AM To: Jeff Brasen Cc: edk2-devel@lists.01.org; Ard Biesheuvel; Grish Pathak; Sami Mujawar Subject: Re: [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function Hi Jeff, On Thu, Nov 08, 2018 at 10:50:44AM -0700, Jeff Brasen wrote: > Add function to allow enabling and disabling of the clock using the SCM= I > interface. Update the protocol GUID as the protocol interface has > changed. Changing a protocol GUID for an interface update feels a bit un-idiomatic for tianocore. (Generally, a new version of the protocol is added.) My main concern is that I can't see how removing the ability to discover the protocol by the old GUID could ever have a desirable outcome. Girish, Sami, what's your take? [JB] I was trying to prevent new dynamic apps from crashing on old platfo= rms. I figured this was ok as this is a fairly new non-specification base= d protocol. I do see the concern with this though. Of the following what = is your preference? =20 1. Add new ArmScmiClock2Protocol.h + guid =20 2. Add new guid and document that you have to use that guid for the = enable function =20 3. Add new guid under old name and have the old guid installed on th= e handle as well, this would keep things fairly clean but would have a gu= id that wouldn't map to a protocol in header form. =20 4. Just change the protocol without changing the guid, this has the = reverse issue of the change I made (except errors can result in a crash) = (don't really like this option) > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jeff Brasen > Cc: Ard Biesheuvel > Cc: Leif Lindholm > Cc: Grish Pathak > --- > ArmPkg/ArmPkg.dec | 2 +- > .../ArmScmiDxe/ArmScmiClockProtocolPrivate.h | 7 +++ > ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c | 50 ++++++++++++++= +++++++- > ArmPkg/Include/Protocol/ArmScmiClockProtocol.h | 21 ++++++++- > 4 files changed, 77 insertions(+), 3 deletions(-) Could you make sure you follow https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-gi= t-guide-for-edk2-contributors-and-maintainers#contrib-23 when generating patches (or let os know if you are, and we have more git breakage)? / =20 Leif > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec > index 84e57a0..a7b55a1 100644 > --- a/ArmPkg/ArmPkg.dec > +++ b/ArmPkg/ArmPkg.dec > @@ -57,7 +57,7 @@ > > ## Arm System Control and Management Interface(SCMI) Clock managemen= t protocol > ## ArmPkg/Include/Protocol/ArmScmiClockProtocol.h > - gArmScmiClockProtocolGuid =3D { 0x91ce67a8, 0xe0aa, 0x4012, { 0xb9, = 0x9f, 0xb6, 0xfc, 0xf3, 0x4, 0x8e, 0xaa } } > + gArmScmiClockProtocolGuid =3D { 0xb8d8caf2, 0x9e94, 0x462c, { 0xa8, = 0x34, 0x6c, 0x99, 0xfc, 0x05, 0xef, 0xcf } } > > ## Arm System Control and Management Interface(SCMI) Clock managemen= t protocol > ## ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h > diff --git a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h b/= ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h > index 0d1ec6f..c135bac 100644 > --- a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h > +++ b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h > @@ -59,6 +59,13 @@ typedef struct { > CLOCK_RATE_DWORD Rate; > } CLOCK_RATE_SET_ATTRIBUTES; > > + > +// Message parameters for CLOCK_CONFIG_SET command. > +typedef struct { > + UINT32 ClockId; > + UINT32 Attributes; > +} CLOCK_CONFIG_SET_ATTRIBUTES; > + > // if ClockAttr Bit[0] is set then clock device is enabled. > #define CLOCK_ENABLE_MASK 0x1 > #define CLOCK_ENABLED(ClockAttr) ((ClockAttr & CLOCK_ENABLE_MASK) =3D= =3D 1) > diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c b/ArmPkg/Dri= vers/ArmScmiDxe/ScmiClockProtocol.c > index 64d2afa..493eb45 100644 > --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c > +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c > @@ -388,6 +388,53 @@ ClockRateSet ( > return Status; > } > > +/** Enable/Disable specified clock. > + > + @param[in] This A Pointer to SCMI_CLOCK_PROTOCOL Instance. > + @param[in] ClockId Identifier for the clock device. > + @param[in] Enable TRUE to enable, FALSE to disable. > + > + @retval EFI_SUCCESS Clock enable/disable successful. > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > + @retval !(EFI_SUCCESS) Other errors. > +**/ > +STATIC > +EFI_STATUS > +ClockEnable ( > + IN SCMI_CLOCK_PROTOCOL *This, > + IN UINT32 ClockId, > + IN BOOLEAN Enable > + ) > +{ > + EFI_STATUS Status; > + CLOCK_CONFIG_SET_ATTRIBUTES *ClockConfigSetAttributes; > + SCMI_COMMAND Cmd; > + UINT32 PayloadLength; > + > + Status =3D ScmiCommandGetPayload ((UINT32**)&ClockConfigSetAttribute= s); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + // Fill arguments for clock protocol command. > + ClockConfigSetAttributes->ClockId =3D ClockId; > + ClockConfigSetAttributes->Attributes =3D Enable ? BIT0 : 0; > + > + Cmd.ProtocolId =3D SCMI_PROTOCOL_ID_CLOCK; > + Cmd.MessageId =3D SCMI_MESSAGE_ID_CLOCK_CONFIG_SET; > + > + PayloadLength =3D sizeof (CLOCK_CONFIG_SET_ATTRIBUTES); > + > + // Execute and wait for response on a SCMI channel. > + Status =3D ScmiCommandExecute ( > + &Cmd, > + &PayloadLength, > + NULL > + ); > + > + return Status; > +} > + > // Instance of the SCMI clock management protocol. > STATIC CONST SCMI_CLOCK_PROTOCOL ScmiClockProtocol =3D { > ClockGetVersion, > @@ -395,7 +442,8 @@ STATIC CONST SCMI_CLOCK_PROTOCOL ScmiClockProtocol = =3D { > ClockGetClockAttributes, > ClockDescribeRates, > ClockRateGet, > - ClockRateSet > + ClockRateSet, > + ClockEnable > }; > > /** Initialize clock management protocol and install protocol on a giv= en handle. > diff --git a/ArmPkg/Include/Protocol/ArmScmiClockProtocol.h b/ArmPkg/In= clude/Protocol/ArmScmiClockProtocol.h > index 3db26cb..d367f37 100644 > --- a/ArmPkg/Include/Protocol/ArmScmiClockProtocol.h > +++ b/ArmPkg/Include/Protocol/ArmScmiClockProtocol.h > @@ -21,7 +21,7 @@ > #include > > #define ARM_SCMI_CLOCK_PROTOCOL_GUID { \ > - 0x91ce67a8, 0xe0aa, 0x4012, {0xb9, 0x9f, 0xb6, 0xfc, 0xf3, 0x4, 0x8e= , 0xaa} \ > + 0xb8d8caf2, 0x9e94, 0x462c, { 0xa8, 0x34, 0x6c, 0x99, 0xfc, 0x05, 0x= ef, 0xcf } \ > } > > extern EFI_GUID gArmScmiClockProtocolGuid; > @@ -205,6 +205,24 @@ EFI_STATUS > IN UINT64 Rate > ); > > +/** Enable/Disable specified clock. > + > + @param[in] This A Pointer to SCMI_CLOCK_PROTOCOL Instance. > + @param[in] ClockId Identifier for the clock device. > + @param[in] Enable TRUE to enable, FALSE to disable. > + > + @retval EFI_SUCCESS Clock enable/disable successful. > + @retval EFI_DEVICE_ERROR SCP returns an SCMI error. > + @retval !(EFI_SUCCESS) Other errors. > +**/ > +typedef > +EFI_STATUS > +(EFIAPI *SCMI_CLOCK_ENABLE) ( > + IN SCMI_CLOCK_PROTOCOL *This, > + IN UINT32 ClockId, > + IN BOOLEAN Enable > + ); > + > typedef struct _SCMI_CLOCK_PROTOCOL { > SCMI_CLOCK_GET_VERSION GetVersion; > SCMI_CLOCK_GET_TOTAL_CLOCKS GetTotalClocks; > @@ -212,6 +230,7 @@ typedef struct _SCMI_CLOCK_PROTOCOL { > SCMI_CLOCK_DESCRIBE_RATES DescribeRates; > SCMI_CLOCK_RATE_GET RateGet; > SCMI_CLOCK_RATE_SET RateSet; > + SCMI_CLOCK_ENABLE Enable; > } SCMI_CLOCK_PROTOCOL; > > #endif /* ARM_SCMI_CLOCK_PROTOCOL_H_ */ > -- > 2.7.4 > Thanks, Jeff Thanks, Jeff -------------------------------------------------------------------------= ---------- This email message is for the sole use of the intended recipient(s) and m= ay contain confidential information. Any unauthorized review, use, disclosure or di= stribution is prohibited. If you are not the intended recipient, please contact the= =20sender by reply email and destroy all copies of the original message. -------------------------------------------------------------------------= ----------