From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757152AbbIXQPX (ORCPT ); Thu, 24 Sep 2015 12:15:23 -0400 Received: from mail-bl2on0054.outbound.protection.outlook.com ([65.55.169.54]:28736 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756353AbbIXQPP (ORCPT ); Thu, 24 Sep 2015 12:15:15 -0400 Authentication-Results: spf=none (sender IP is 165.204.84.221) smtp.mailfrom=amd.com; alien8.de; dkim=none (message not signed) header.d=none;alien8.de; dmarc=permerror action=none header.from=amd.com; X-WSS-ID: 0NV6VTA-07-87R-02 X-M-MSG: Subject: Re: [PATCH 2/3] EDAC, amd64_edac: Extend scrub rate programmability feature for F15hM60h To: Borislav Petkov References: <1442436811-23382-1-git-send-email-Aravind.Gopalakrishnan@amd.com> <1442436811-23382-3-git-send-email-Aravind.Gopalakrishnan@amd.com> <20150924091841.GA3457@pd.tnic> CC: , , , From: Aravind Gopalakrishnan Message-ID: <5604218C.7020905@amd.com> Date: Thu, 24 Sep 2015 11:15:08 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150924091841.GA3457@pd.tnic> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.180.168.240] X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:165.204.84.221;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(428002)(199003)(164054003)(479174004)(189002)(377454003)(24454002)(54356999)(59896002)(36756003)(80316001)(99136001)(86362001)(33656002)(65956001)(64706001)(47776003)(65816999)(87936001)(101416001)(65806001)(46102003)(50986999)(50466002)(105586002)(92566002)(2950100001)(83506001)(11100500001)(64126003)(106466001)(110136002)(76176999)(189998001)(87266999)(5007970100001)(23676002)(77156002)(5001860100001)(68736005)(5001830100001)(97736004)(4001540100001)(77096005)(4001350100001)(5004730100002)(62966003)(4580500001);DIR:OUT;SFP:1101;SCL:1;SRVR:SN1PR12MB0717;H:atltwp01.amd.com;FPR:;SPF:None;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0717;2:9HAZL9s12vGHmgTQ2fsn7q1iEg4x6N1MwdMnaPwc0Gd6yZhW2DCppMvxc4VgNQIPfyWUfiWmQi4K/OcMUZtQdXV32ETfE8xSy3/Xg4U1NPNm+Oag/nrRU62iT8f5PF3rNcwoebGXbZLhfRb+gQpv6u5432TTKC9/qJxU63a57B8=;3:gd7I9Lb2NeH5xSxADiJjtKFJDvUANV9I6NVpkdkAo0a3qr1C0Sxq3NAcM8SezWT3j/PqOaenNnUzKYxWtObXKIzS01DGZYrKi0uYHwg1zWP/Ja9EfIpjFMWGpoBa7ARL58zVF6ZfCTxtN6E2VYQsTABjFD7ZsznfDyekQgLqoxKO0mOudXKiZ/NPhqd43/hMK02elpf76Ji2IxJW7SWPsQeTCk+H2N1dLxUuaWkpcBEXpBzwoa9ubya9evu2NvGA;25:wwwlM77/157tVZfbr2KgLatqBHYwjsrjRKnEzDYXAyrgQf2BgItPymsMdbZ1HXN7vLI3VwPRST274lTtXyzFhB34aU78YWxh9n6Lc/fLptWcAD/L4Si1izMv5pRI7CH9vLHHF6fcn4GYmVohcem2f99lln3yoYX3v/nUdnTfx5SNSQsbj8PAe5NeXYQmRsMR2VXr9SAs4UHtfK/ocLmccI1qkI+ZFLbhmp59w+iQKP7IQx6LzMOO0bO3tJ+SXzaYvet6svo3kvJA/kcep9sq3g== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0717; X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0717;20:IcmhuyGmjaQvDUWni0t0MWZT22mOZrD+W1bUHxWs0aL/PlZXlld/noiSCkbdSwHjoyLpHewk8+eKZcjT3mIwSprA8UZD5Cd0OcgzYa4/nWXuip1K2mORb0umvfMounJos850Dyc36BJYR/QlTR/20MW9oUcbiUrCZOYkhD53aRWpA5jp8OYLWN6fJNv+OhImGHO2qXpl+BNtZRf+w3EPKhAzTbigyMUCgw+AQHMr81vheyE6DJ20CKHvhjq+VjSg+3OqpmGUuVUma16VsaEczAQp9zmLyFsFBYnp7NfY+gMvZN3G9fcsW2/heNpJhj8DE/HYd2QkHxAEnoFnjmSd9U2NO/P+AMuZX2TCck3GPK9bc15d3NYjdYoe+ZA7ADhXzI/x8cUhcbU1DKG4SfFUgwsnT87WxyA8AoadFePukqZuKfiGJDcp5R3ce47A6qALww2J0mnGcgjZiU6WZAvxjCl/L5JbbvdMqlDV/qSwlRZea0VolCzb8dwLKX9qlpUq;4:sQuHM/WHKQdO87Bu+Hh7SaytudDfjX4fHtQ6D/NfDdaFDbWK/iRn4egWsBwcobzpgf6cXm6oLdii0X4xGEb69RwqXiiiWOtxrijNzCP5IQg5MDJ7vAyJJxunyvGrPDpt4YTcdJVzKzArWqW7/+2X0t9ERVGbdzodazrwvbvT9ozAmKzOxdepZgtOboX5G3ZQr+cjK3Ag852lP48pLfW+oJdawPbMQ1hnChDju3u5zwhHKNIFw1wRoptKZZ081ikb5mXpt69RsbPvVzVWFy3duTCLlw1hf/XUc8JliEE8uUQ6n22mu6Tjxba9JlkGd2w5ggGyNhXRtBtC21LMORqMtE9quimu9KPcVb04IidWkRI= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(520078)(5005006)(3002001);SRVR:SN1PR12MB0717;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0717; X-Forefront-PRVS: 070912876F X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtTTjFQUjEyTUIwNzE3OzIzOlpDV09ya1FjSHVVUUZTUkNLM1R4aW04ditC?= =?utf-8?B?OHhxOEpsMFc0MGtZcnM4c25XVHcxcGtPRDVlODEyNzdnRmJZZmE0emREa2R0?= =?utf-8?B?Zm9UWnVOYWMybUlYOWQrV0R4OHh0T28rZzM4R1ViQkFtMXorWjZuRys2eXI4?= =?utf-8?B?a1JBUVNNME1qUlJMOTV3Wi9vRGhvSlVZYjlhNG5IVlNxYWMwRis0cWgyU0gy?= =?utf-8?B?M3ExQm5rSWZnOVk3QVhBbzVBUmEyN3ZSQXFBT09GSGd0ZVErcERHY1p6V2Jr?= =?utf-8?B?cHk4Lzh0eDFqWXNjYU1mcmNJeVVFZ2dWd3g5Y3ZzNXYrWDcyWW5RZUdlcWF2?= =?utf-8?B?cGxCVy91M0tlWXY5NVNHSzVXZlBhM1VBai9WMVNQVlZtTHl1bGVRMURXUmFI?= =?utf-8?B?WE9TeE50d3liTDRGcnhlL29OOUhQK0hWZVFKVTQzZ0F0a2I0QXpaeTBxTmJv?= =?utf-8?B?VTNROThweWJsRmx1eFNTRy93RU84citjU3ZlU0x6aThUUWRLSGNCVFhVMTFK?= =?utf-8?B?cG8zNFBYT0dpalV2Rld0MS94SHZGTXlHd2todnRRRnFIcURpUS9ORFBVUUVF?= =?utf-8?B?SlJMc3EzODl3d1Z6aFFDZ0ZadTkwZFB6a0JzbW56dW54bVVxVnJEYndQSWJB?= =?utf-8?B?MU14bFN0WVA3ZDNGbEZwWEx5Y1pGajNkYWJCN210VU5qeTVMc1Q3bjErYkxD?= =?utf-8?B?d2xibFpma0Qya0d3eEFwbGpXM01MZUFCeWR4TXZuVytGMXlhYTVUblJHZVpL?= =?utf-8?B?NzJNOVUrT2lydUdQaThHZytlT1NqWlhzODZDS1VtTEV1S1I4Ky9BU2NKZWhk?= =?utf-8?B?MlRUVERDZXBub0x1TUt5cWVpTzNBSGIxdGZrSUhiSmhhT3I3Y0N4ZGNrQ0NZ?= =?utf-8?B?RHlnOTB4RWRRRUtEblhiYUJFOVBlM0FEQ01OZWNJTVllelFia1hYUEh5OWwr?= =?utf-8?B?Y3g1elNRRXl5L2Q4V0NmMWp5VjZlSGoyZWp2b2l1cHI5L2lmYjRTVERlQ3Bm?= =?utf-8?B?UlNyTWpud2hGWm1KbTJNTlNsM2VLbjEwdUFIWU9GOHQyV2Y2clJyL1lQUS80?= =?utf-8?B?bzFpWVRCbTkrZG80eWVQZ0g2bys3ak5kTFhLaTVCMnR1aTVnM28yMWlpSVVL?= =?utf-8?B?cXErcHpHalhlc3ZGRVY5QXR2cHUxWFNMWmtYa3l5UW1zc0M3WTR3eHk1L1ZW?= =?utf-8?B?MWxtbFN5d0dxRGh3Q0tOVUo2QjBla3NpRVNnaEp5RXAwbXA3aDYvKy80a2ZQ?= =?utf-8?B?UitRZE9va3JuYVdqSTBPMmFqSzd2M3FFSGFrWnFhUDM1RHFFYy93aXQvalNO?= =?utf-8?B?VVQ5elZ2VTZ4UzJnck1qOFpjUXJ4ZWgybDdYQmg5OXVkUmRjU0RobEtQWFN5?= =?utf-8?B?TExWdlhETGkvVlRHNmRxellFdldyMlZhYnBQVkJ3NXpCUDU2Rk5SVjlaUm9Q?= =?utf-8?B?RHRCZlZGNEljUlhzY0l5dFRWVmN2Qy9XV0MzK2pzbFQzTEJuZFNBQmdneVJz?= =?utf-8?B?QzZjMGtTRExDMXA4T2tWcWtJakhuL0ZlUnVIcE5vaUFNdGNTb291cFZXTlFM?= =?utf-8?B?bHNoZmlyVzhteGg2ZVJlNmtDU243SHVCOHJYeWdxVDF0KzYrcTVqNm84N1d0?= =?utf-8?B?eXFHUGtpNTdiaTNBMFJZTW4zUllObWpMUWFCcnEzc0huRWZ0VldWQjBQaGl6?= =?utf-8?B?eXNZclVMZFBZV1lFMDJ5ODRScUtZQnVTcGZzM1RtQmE2Y3UzTDhrNzBtUXAw?= =?utf-8?Q?11M6pO45Ac21Xbe7YLVSua04EsTHnGi5dqR1A=3D?= X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0717;5:qoy4+y6F0QFYx8KNlw7xfHE2NWC9wzpKEPIP9gc1QO4RWuc7xBRwlp2arG82jEJt3qQ4QHN1Rg9vtaBl50TNz162TovfExAEd/7Y3R81aiFPQgqNKCIOoywYFGwnhyW7ryPfdw6co6EjTgDkOEQbtQ==;24:Fz8/mfV8swSuBjHKvoXCyMkamGftFp875uzeImo2viW965BF7JnYwK/AJmwk7GTYWtWy68vc6od9YagKcPOar/bXUhcG2IXd09Pcvwawmpw=;20:OfRE09IwMUqN4DdsceyGcL8cptThrMfTsM1rMghYffgxLF4F+O4z5cfcAjWfdglP0GQbUIJkdvCvBKm3tVPh/g== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Sep 2015 16:15:12.0561 (UTC) X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.221];Helo=[atltwp01.amd.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB0717 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 9/24/2015 4:18 AM, Borislav Petkov wrote: > On Wed, Sep 16, 2015 at 03:53:30PM -0500, Aravind Gopalakrishnan wrote: >> -static int __set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate) >> +static u32 find_scrub_rate(u32 new_bw, u32 min_rate, u32 *scrub_bw) >> { >> u32 scrubval; >> int i; >> @@ -200,28 +200,52 @@ static int __set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate) >> } >> >> scrubval = scrubrates[i].scrubval; >> + *scrub_bw = scrubval ? scrubrates[i].bandwidth : 0; >> >> - pci_write_bits32(ctl, SCRCTRL, scrubval, 0x001F); >> + return scrubval; >> +} >> >> - if (scrubval) >> - return scrubrates[i].bandwidth; >> +static inline void __set_scrub_rate(struct pci_dev *ctl, int offset, >> + u32 scrubval) >> +{ >> + pci_write_bits32(ctl, offset, scrubval, SCRMASK); >> >> - return 0; >> } >> > What is all that churn good for? > > What's wrong with simply adding the model 0x60 check to > __set_scrub_rate() and doing the proper write there? I was thinking it's a little better from readability POV to separate out the for loop which does the job of finding the scrub value to program; And __set_scrub_rate() writes the value to the appropriate register. Maybe I am making it too modular? >> >> - amd64_read_pci_cfg(pvt->F3, SCRCTRL, &scrubval); >> + if (pvt->fam == 0x15 && pvt->model == 0x60) { >> + /* Since we mirror the same scrubrate value across >> + * both DCTs, it is enough to read the value off one of >> + * the DCT registers. >> + */ >> + f15h_select_dct(pvt, 0); > If it is enough, why do you select DCT 0? Just read the currently > selected one, whichever it is... Yeah, that's a good point! Will fix this. >> static int f16_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, >> - unsigned cs_mode, int cs_mask_nr) >> + unsigned cs_mode, int cs_mask_nr) >> { >> WARN_ON(cs_mode > 12); > Why is that hunk here? > >> @@ -1666,7 +1699,7 @@ static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range, >> } >> >> static int f15_m30h_match_to_this_node(struct amd64_pvt *pvt, unsigned range, >> - u64 sys_addr, int *chan_sel) >> + u64 sys_addr, int *chan_sel) >> { >> int cs_found = -EINVAL; >> int num_dcts_intlv = 0; > That one too? > I realize it's unrelated to the patch and it's not doing something useful; But I was thinking I'll fix the above indentation issues to keep indent rules consistent and since I was touching the file anyway. Can remove those in a V2.. Thanks, -Aravind.