From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753990AbcAMQ1P (ORCPT ); Wed, 13 Jan 2016 11:27:15 -0500 Received: from hqemgate16.nvidia.com ([216.228.121.65]:19828 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753686AbcAMQ1M (ORCPT ); Wed, 13 Jan 2016 11:27:12 -0500 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Wed, 13 Jan 2016 08:22:42 -0800 Subject: Re: [PATCH 9/9] clk: tegra210: Initialize PLL_D2 to a sane rate To: Thierry Reding References: <1452278714-10751-1-git-send-email-rklein@nvidia.com> <1452278714-10751-10-git-send-email-rklein@nvidia.com> <20160113140309.GB15782@ulmo> CC: Peter De Schrijver , Mike Turquette , Stephen Warren , Stephen Boyd , Alexandre Courbot , Bill Huang , Jim Lin , Benson Leung , , , From: Rhyland Klein Message-ID: <56967ADD.8090904@nvidia.com> Date: Wed, 13 Jan 2016 11:27:09 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160113140309.GB15782@ulmo> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 1/13/2016 9:03 AM, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Fri, Jan 08, 2016 at 01:45:14PM -0500, Rhyland Klein wrote: >> Initialize PLL_D2 to a sane rate at the start of the day. >> >> Signed-off-by: Rhyland Klein >> --- >> drivers/clk/tegra/clk-tegra210.c | 1 + >> 1 file changed, 1 insertion(+) > > There are a lot of assumptions in this commit message. I'm asking myself > why does it need to be initialized to any rate at all? Isn't it up to > the user driver to set the PLL to whatever it knows to be a sane rate? > Why is 594 MHz a sane rate? > > A good commit message should answer those questions. > Thanks. This patch came about in response to some work with suspend/resume logic in the ChromeOS kernel. In the suspend/resume patch there, it reads all clk rates and caches them in suspend, then re-sets them on resume. We hadn't been using pll_d2 at all, and its rate (when read in suspend()) came back as 0. Then in resume, we would tr to set it, clk_set_rate(pll_d2, 0) and would generate a warning/error that 0 wasn't a valid rate. As for the specific rate I chose, I simply took one off the table of supported frequencies in the clk-tegra210 code, so that it was non-zero. Since we don't support suspend/resume right now in the clk drivers, I don't think its urgent to merge this patch. I can drop in in the next rev if thats easier. -rhyland -- nvpublic