Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755377AbbESQaU (ORCPT ); Tue, 19 May 2015 12:30:20 -0400 Received: from mail-bn1bbn0100.outbound.protection.outlook.com ([157.56.111.100]:54336 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752429AbbESQaH (ORCPT ); Tue, 19 May 2015 12:30:07 -0400 Authentication-Results: spf=fail (sender IP is 66.35.236.227) smtp.mailfrom=opensource.altera.com; vger.kernel.org; dkim=none (message not signed) header.d=none; Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=dinguyen@opensource.altera.com; Message-ID: <555B64FE.40104@opensource.altera.com> Date: Tue, 19 May 2015 11:29:50 -0500 From: Dinh Nguyen User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Stephen Boyd CC: , , , , , , , , Subject: Re: [PATCHv3 2/4] clk: socfpga: add a clock driver for the Arria 10 platform References: <1431011523-10049-1-git-send-email-dinguyen@opensource.altera.com> <1431011523-10049-3-git-send-email-dinguyen@opensource.altera.com> <20150516005235.GP31753@codeaurora.org> In-Reply-To: <20150516005235.GP31753@codeaurora.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [99.103.66.154] X-ClientProxiedBy: BLUPR08CA0052.namprd08.prod.outlook.com (10.141.200.32) To BY1PR03MB1372.namprd03.prod.outlook.com (25.162.109.30) X-Microsoft-Exchange-Diagnostics-untrusted: 1;BY1PR03MB1372;2:zVIgi0UidQUdfC0gVl6eBy02ORg/9vMkjMatPp162ybheLge55n53vbGxTFaEBHa;2:CjLe9jUUk8lli4f5D0qnTYqOwb036qd7Xxk+r0TLuONKyVdVk/AgYZllmp4V/lkKuourpRInjPLS3xfBSx8Pcjplw6IbWYVnwMg0HSbKfFEDPwHpvBlANh5EUfyeLzsgr4DkXJ2gyqcROlF2U26gIw==;6:YsEoC857hUEf+VKa2m2OsWCIB5hg8As+pfsOzco9MXj0JQcGdKbH1qx3xlkSaUIW88up0H6AQLRtysj1NFwpqkP+bGOBsaG72wFE+Izn/tqnV7vfd6b/ljq3saIDXPB/IAfCFKhj6cgxmzuJZvKOEQ==;3:iJ3OJrTNUMSQ+SEY9epvzhS03cxAW5FgRG9KQ7HanunbBjRq1a8y6Tnejv9w2e81M20HymTT8lHPZ4oUoiI9M2Rx/c5OjLER3rfQ5Jg+lXsMDjY6WTzggdezBamZgi3AVlNyPsVgD7CtgomQwOCFjQWrhPw0BlvNkkqf+Z072On5yPDZsanXSDOI/JBM11t8gFwqrU4Dt1cdw5kQwAPbqm7+aZf5TSV7kbxCOudceBqAXSqSVFPbnGOIocwWULsFmtP6FLHvRG22qRNBy8v7+uDt66eh44AoxbEZ1Sj/anI= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY1PR03MB1372;UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN2PR03MB046; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:;UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5005006)(3002001);SRVR:BY1PR03MB1372;BCL:0;PCL:0;RULEID:;SRVR:BY1PR03MB1372;BCL:0;PCL:0;RULEID:(601004)(5005006)(3002001);SRVR:SN2PR03MB046;BCL:0;PCL:0;RULEID:;SRVR:SN2PR03MB046; X-Forefront-PRVS: 0581B5AB35 X-Forefront-Antispam-Report-Untrusted: SFV:NSPM;SFS:(10009020)(6009001)(199003)(51704005)(24454002)(479174004)(377454003)(189002)(52604005)(50466002)(19580405001)(40100003)(106356001)(19580395003)(83506001)(33656002)(122386002)(87976001)(64126003)(5001830100001)(76176999)(50986999)(62966003)(77156002)(54356999)(5001960100002)(5001860100001)(110136002)(101416001)(189998001)(65816999)(46102003)(65806001)(65956001)(66066001)(47776003)(64706001)(68736005)(77096005)(2950100001)(42186005)(81156007)(4001350100001)(23746002)(97736004)(92566002)(86362001)(4001540100001)(105586002);DIR:OUT;SFP:1101;SCL:1;SRVR:BY1PR03MB1372;H:Dinhs-MacBook-Pro.local;FPR:;SPF:None;PTR:InfoNoRecords;A:0;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics-untrusted: =?Windows-1252?Q?1;BY1PR03MB1372;9:qjgq50VD6Xnac2mQr+s9A+NQa1gUMeXvdFVvHp?= =?Windows-1252?Q?RmPAeCTRWcrP08DveY6zF91B4N6f4FfqXAtWTelA7hpFeMl2FOe4NEpl?= =?Windows-1252?Q?Tm7xA5jzABw77nWlBZm8PAUNntiViKoNuD0m9r5lhvFx+FI1eclxepc+?= =?Windows-1252?Q?8bDIpzeBbxct1qhWXjzHYgJ+XvaQ6H9VBVz2+wopY7aTGOFmz2IWHPTn?= =?Windows-1252?Q?yy+n1FWmGgzgsiwVy/44UHBl3PYdMApKq5s1OiobBa95ckE4+j9wNH1f?= =?Windows-1252?Q?CdRkUN3TPUtmwjgbInhhRKQ6fJkatDEnLguVtQgJUX4ytt5TWRXbKFBS?= =?Windows-1252?Q?Q8jWksHBrTLGRrdm+b+82KSA0nAoR+2l/E217bCNitizKlBBWCBt/CFX?= =?Windows-1252?Q?a7/uFGNlfzGxSjQZmIAUx5QYlCC6gG1ciNj6Pp84KX+ixMtbH/rdv2aT?= =?Windows-1252?Q?Vm9axGuyEAb61GcaLa/1oMWke+lhRerprhphbrdYCphBqtZFDZcgm6+l?= =?Windows-1252?Q?yOM3K7Ex3yxvPUh8tXxmFgH7x1K0FSHHaWqU/iq/VpUfDM5+eVZxTwCd?= =?Windows-1252?Q?ljEbEWdeNVGzpw3yxPnvi2YFVh0GD7FKJ637vwlStumcqI/JClN4VV5J?= =?Windows-1252?Q?nxWI9D7Nxc2nfb0WsQT4v4uPqVpr+8aVGwV8UZtUK6ZypFBAhNV04mOF?= =?Windows-1252?Q?jEPP92/9HnLh+zzHoCunerwzRilQDj9ndVUpU9+tmgdOxu9Dyp9rlWQa?= =?Windows-1252?Q?fzFZyfFgTzJqCrIO0oVt/+Oe9yCZOELqavbMA/03bTNv0HmZ5iT/+arE?= =?Windows-1252?Q?kMuvnmV2/EYmzX83XlgPbGb95+ak3pSokTkepfsJXayD8yoi6a/hAmV8?= =?Windows-1252?Q?+bJxPcd1HgsQysHi5TyF16sybGwB2FSeE3o7mPJK3SHJBhYsHLUJfF/g?= =?Windows-1252?Q?4Kh8k1QeDcYRKGdFRWcPe0Z+wNs55YsIyRv9DMuEnL/qJG66gLqgxnT5?= =?Windows-1252?Q?yKcsmalizRQFdXHrkEoPQpU0DcTeJxF3Y8hH2u5zD5t1svLFMdxXJxyl?= =?Windows-1252?Q?LFdcON8iB/1UOI+j4B4YF7M3DzPUxmuibamsnl7dRTWNnOlXxmGvoBE7?= =?Windows-1252?Q?CusWuz0084+IkE7SrJ68r8szDJaQ3Jm8NKCssf6G2EypGV9E5arUrQ55?= =?Windows-1252?Q?Nwr/qN8q116xb1tIUVDtfaktJWT1I3rPblx4N+abBhDLFXcWnvHbguP2?= =?Windows-1252?Q?NUPhkgY4ROP8eF411NgXrqNH1ytq4gg+TCCg663cMEbqphA3inNU+Ntb?= =?Windows-1252?Q?McZ7jVn4kbgSnoiqK3aya+2fRvBdqrkCKZmlRQfTqVeJw=3D?= X-Microsoft-Exchange-Diagnostics-untrusted: 1;BY1PR03MB1372;3:g8BPKr/dZjinTyfKL+D8gmP5eytl//4K+r5qSlRFwFztohS2A8jrVVBCKMSih6LWGbCfE+5LNByge0yhHW53umaWDe7EzIH6g4MLdrsCwWR9cHdbTylIlGbgyTmO+YbUe3fB8umnFr8YiANXmzHp3g==;10:MGWiKXuD8scftjvxbR3BiunUwP10y7blS73Aq5o3lgxG2J5VYuVtHkplvxv09cYvnzc+JbNzDk+eBnr8xh91tkWALqsCnBsa6cfW3XdputM=;6:4TXlnvFxZsy3l0GuPuT6GGMWxaB0qzHYYiZg1J2uxHDgPkolzeaUpJ8/xFi8u6J7 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR03MB1372 X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: BY2FFO11FD052.protection.gbl X-Microsoft-Exchange-Diagnostics: 1;BY2FFO11FD052;1:p5QjQC7suYCshMN3ucrNgpASGhuTUzz6FZejlyNEDKyKXdoKvsBphj6PyIy9qPI7flWB8omzhR4hljs1OVujPqCFSIyVCGPkfw6J8DqpE6cCkKdnTlpDHdzZvbc8CjVjkpVvZiPGxUXbmz9xG4aUA05p1YnLhZoNW4KGWRLrQ6gugkuAi2Ca8DYsyGeaqAm+VK8M7cKCkOOWr7M/fyWl1yTu+88wMAYNo8KGPmVQf9jM269BoSd6CDqAxmxdpzcc0u+ORogmTD1UD9JPC5D6+ZBDVT1sZtMwGgQy9ql3UGHCWbLb+e9lt15PUcK54s1w X-Forefront-Antispam-Report: CIP:66.35.236.227;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(339900001)(52604005)(51704005)(479174004)(24454002)(377454003)(199003)(189002)(65806001)(65956001)(66066001)(76176999)(5001960100002)(4001540100001)(65816999)(54356999)(81156007)(50466002)(6806004)(64706001)(47776003)(50986999)(87936001)(40100003)(6070500001)(68736005)(110136002)(2950100001)(5001860100001)(189998001)(5001830100001)(122386002)(97736004)(46102003)(4001350100001)(77156002)(62966003)(77096005)(23746002)(86362001)(33656002)(19580395003)(19580405001)(105606002)(83506001)(106466001)(85426001)(16796002)(92566002)(64126003)(7099028);DIR:OUT;SFP:1101;SCL:1;SRVR:SN2PR03MB046;H:sj-itexedge03.altera.priv.altera.com;FPR:;SPF:Fail;PTR:InfoDomainNonexistent;A:0;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB046;2:5HPzJ0iG+9alEMNBW4LOB1JB4kL/6s2X1LnMY4XY6kjWcxLqLAHDC2WyRt5vNWj8;2:UvJ76qk8wVUwlrsyZmHlyUfZOV/k7w8j+s52xE3w54JtdQxifY4Fhqnn5HHIsxO3PDHCn3XnyFOrX1Dz0i3LeEOYJnYQy0gW9isHULml2ReU4NAgHEVk3qimIxQbhxlBE8yva3uNpnLa11z+9MQHagqHL9Sdh0cdpa8p/hy8eQ/iqy9uDaP8CbmTuq4soPagRYlAS1a+8D2Z+/8kEVpggw2REv44mc2Yob8OErpWBlc=;6:uZNmG3CfY9Y14POiKiPnQxbrwJBzWUn/y+0FDAS3DCEJukZqWBZsyVHx9OjAyaFQm3eWI3dkNg4reecJE4+FYc3vcAuFDcFPBG67rO5qRG2ExEszuJgzVDVxNjC+hcp/vYPo/QoaHrjn5y1K9qGTyw==;3:HmSd5gHuhO+/uqlHrrfQPU29bHcRcufWl2b5aPI1wIJjpSWCwMvHXusb6wwS+975RFFQNDm9GY79/oxzAoKO4IbSFCJRKDyj5TOSN7owKIln7hnMLD1UnlqJZ5b+ejLWoHA+bQ0w+z8yTYhE36C0UJrHDQHRPJAMmzS+vwoVTE0GD/5qiy8Y1Kcxon6ZxoWiy17f2ms6fv/taDCdjquoZrhICmI2wjdv+HAINEZqAeponnMAvcN8mXavGwHvl3JgbLtc4XzfRtr/aGcoqXdRFlZkqI3aIxqi3C7gnBkraM8= X-Forefront-PRVS: 0581B5AB35 X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;SN2PR03MB046;9:Ek4CoahiQ+d7POUC4QljxKiigyBfi49h6y7VfZE?= =?Windows-1252?Q?sVVFs9d6M91m/ubI3T5YUnW3uWh7OtPvNg7qp7B1QujcPKjKtmvhqPe5?= =?Windows-1252?Q?1w1Tttgxw4dD54tlhmdVZ6JLDGHRlUGYJMVRNpznotL6+F/O5misIU66?= =?Windows-1252?Q?a15rMm5XOM9aU+JTRMxdBjJNqeBpcNeagunPYMtSdRES/XTYS3TyhIlh?= =?Windows-1252?Q?/VPJjZzMGxHuHUswCgdLLUseAFS+SyX63HXS1cZPuKdv+Qn20x8XJ1Cy?= =?Windows-1252?Q?iO5qh0BxXz9zsPhD6K9wQk5ksz4eBnGTtBGSOaCDCOjmWxSsv8fyIqVJ?= =?Windows-1252?Q?LprJxIYPsYzrCrwY1XnVgpfwXeVr9b1SwHebvUTNemQmm6uv6yz1n0RG?= =?Windows-1252?Q?zoXnOGRz+cAFcqobnP7SMpkbdUOcjcW3uNq24crMFBUugP3I7g2orYu7?= =?Windows-1252?Q?IkKI0GliFpB9Kb3UZdWsdDKjBLdu2w7RH05RsKd5D3q+oN0EatoNy/rf?= =?Windows-1252?Q?ydIzEW0t38S9btrEW/IlOFtxNPfz7knWdCXys9xoNbcjptM7pU0PD1i7?= =?Windows-1252?Q?KOQSp2OfWDFMAdF2Eb71R/nJ0gkhpY/W2vz3xRk0okGQNZc0kUmNeK3a?= =?Windows-1252?Q?5mKvwD/tTxk5S6wFrBjERAppeE9JHR+Cc9fWOWq8vvTTe25nm9ieHNw3?= =?Windows-1252?Q?1SaTCDXLejOyugg2ad2G5HvRugLujbB4k2vViwH+QARqJkhuWNFiBRkA?= =?Windows-1252?Q?RQMNivgqjhEm6TGlr3UIxOYkOY2j337xlc9SZCWhrALhzkvNRLIOxKI1?= =?Windows-1252?Q?0ImAncmnM6seG0oE00oeBaWSDpZWzePTNSKmwJFszA+yKRocw4Go3pcA?= =?Windows-1252?Q?wBy6K/p2aFiV+mLIJVLISTKWR5gDOv2nb72Y47UuQOZzOCqRxhgteIBX?= =?Windows-1252?Q?yPHBgSUhrTlJgTiXXDFG/zFx7hhk/zwcGbTpocNS9KwZY5DVgFEpS9s+?= =?Windows-1252?Q?t0tVOgwSG7v7+XiMcVKF9ynJUeOOooMMG11CzWr7FnEqMTGZ189Iux+v?= =?Windows-1252?Q?AG5a4glgWDwFkKhFEOaqo+JOd02ORH/2McKTjcuQL8iEhpFwEgkF92Re?= =?Windows-1252?Q?h6CFGgJmpq+slX5GERroGsYr1BK3k0S+ZdcBKkGNJfARDcWtXMzLuRz8?= =?Windows-1252?Q?+CaDWCfGC4lPxf27mViOssmFQVcAgzuZmA2foxKc002Z3vSv/8BJIUAb?= =?Windows-1252?Q?2Zws53Y7YK371x74PYmSX/AmIlxM4MvON/NSpGZXmnFS1gI9GjrGyY10?= =?Windows-1252?Q?ZJWltj5fWVG0nJ8oxN9ZHRNxxS8lMEEvW2t2Tg8PVgYECJYiEx45OnZD?= =?Windows-1252?Q?YRxdPr8qvUwc4T7PoT3pgJnfPSMhrGjYTTLJofevyOKCrk9Jgn+iu+nt?= =?Windows-1252?Q?B74OS4hGoib8BSTQL?= X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB046;3:dKz3oCzvidrGciUgSzkjg47IYHVDyBLSxixtcQN2/SIr5No9CowkAvhzOMEAKh7KLC/tdBNi6ieH1jmakkB58ClX9C+j4EMS6C5K6kxGEaYFWQ+T1F3Lqfgn++zAc4abEt800KkbiqMFWNacPAQVNg==;10:G7ZM4UTf/LkxtDhuVMOp07vfCBh+Gt4QnEjfdIscaRGLYfadQ4yiHCmQDdRoW+9r6SunLn8YJuAHMyjx2t7kjoelzGSCidjHauHWLmZEr9Y=;6:l/anmlNglEnBTg2mWykQOjZJ2leS5iJNCfSRTRPVdsbINg4kyEIitYzvJ7zbpC+h X-OriginatorOrg: opensource.altera.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 May 2015 16:30:02.4765 (UTC) X-MS-Exchange-CrossTenant-Id: fbd72e03-d4a5-4110-adce-614d51f2077a X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=fbd72e03-d4a5-4110-adce-614d51f2077a;Ip=[66.35.236.227];Helo=[sj-itexedge03.altera.priv.altera.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN2PR03MB046 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7172 Lines: 269 On 5/15/15 7:52 PM, Stephen Boyd wrote: > On 05/07, dinguyen@opensource.altera.com wrote: >> diff --git a/drivers/clk/socfpga/clk-gate-a10.c b/drivers/clk/socfpga/clk-gate-a10.c >> new file mode 100644 >> index 0000000..fadf6f7 >> --- /dev/null >> +++ b/drivers/clk/socfpga/clk-gate-a10.c >> @@ -0,0 +1,187 @@ > [...] >> + >> +static int socfpga_clk_prepare(struct clk_hw *hwclk) >> +{ >> + struct socfpga_gate_clk *socfpgaclk = to_socfpga_gate_clk(hwclk); >> + struct regmap *sys_mgr_base_addr; >> + int i; >> + u32 hs_timing; >> + u32 clk_phase[2]; >> + >> + if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) { >> + sys_mgr_base_addr = syscon_regmap_lookup_by_compatible("altr,sys-mgr"); >> + if (IS_ERR(sys_mgr_base_addr)) { > > Is there a reason the syscon is grabbed lazily in prepare? Why > not get it before registering this clock? This syscon node is only associated with clocks that have a clk-phase property, which on the SoCFPGA platform, is the SD/MMC clocks. The way to implement this went through quite a few rounds of discussion for the Cyclone5/Arria5 platform before settling to this method. The reason why syscon is grabbed here is that the setting of the clock phase must be done before enabling of the clock, so it seem that prepare was a good place. Should this be move moved to the socfpga_gate_init() instead? > >> + pr_err("%s: failed to find altr,sys-mgr regmap!\n", __func__); >> + return -EINVAL; >> + } >> + >> + for (i = 0; i < 2; i++) { > > i < ARRAY_SIZE(clk_phase) ? > Will fix. >> + switch (socfpgaclk->clk_phase[i]) { >> + case 0: >> + clk_phase[i] = 0; >> + break; >> + case 45: >> + clk_phase[i] = 1; >> + break; >> + case 90: >> + clk_phase[i] = 2; >> + break; >> + case 135: >> + clk_phase[i] = 3; >> + break; >> + case 180: >> + clk_phase[i] = 4; >> + break; >> + case 225: >> + clk_phase[i] = 5; >> + break; >> + case 270: >> + clk_phase[i] = 6; >> + break; >> + case 315: >> + clk_phase[i] = 7; >> + break; >> + default: >> + clk_phase[i] = 0; >> + break; >> + } >> + } >> + >> + hs_timing = SYSMGR_SDMMC_CTRL_SET(clk_phase[0], clk_phase[1]); >> + regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET, >> + hs_timing); >> + } >> + return 0; >> +} >> + >> +static struct clk_ops gateclk_ops = { > > const? > I cannot make this a const as I am assigning the .enable/.disable to use the common clk_gate_ops. >> + .prepare = socfpga_clk_prepare, >> + .recalc_rate = socfpga_gate_clk_recalc_rate, >> +}; >> + >> diff --git a/drivers/clk/socfpga/clk-periph-a10.c b/drivers/clk/socfpga/clk-periph-a10.c >> new file mode 100644 >> index 0000000..81b9274 >> --- /dev/null >> +++ b/drivers/clk/socfpga/clk-periph-a10.c >> @@ -0,0 +1,131 @@ > [...] >> + */ >> +#include > > Are you using this include? > >> +#include > > Are you using this include? > > Applies to every file added in this patch. Removed... > >> +#include >> +#include >> +#include >> + >> +#include "clk.h" >> + >> +#define CLK_MGR_FREE_SHIFT 16 >> +#define CLK_MGR_FREE_MASK 0x7 >> + >> +#define SOCFPGA_MPU_FREE_CLK "mpu_free_clk" >> +#define SOCFPGA_NOC_FREE_CLK "noc_free_clk" >> +#define SOCFPGA_SDMMC_FREE_CLK "sdmmc_free_clk" > [..] >> + >> +static __init void __socfpga_periph_init(struct device_node *node, >> + const struct clk_ops *ops) >> +{ > [..] >> + init.name = clk_name; >> + init.ops = ops; >> + init.flags = 0; >> + >> + parent_name = of_clk_get_parent_name(node, 0); >> + init.num_parents = 1; >> + init.parent_names = &parent_name; >> + >> + periph_clk->hw.hw.init = &init; >> + >> + clk = clk_register(NULL, &periph_clk->hw.hw); >> + if (WARN_ON(IS_ERR(clk))) { >> + kfree(periph_clk); >> + return; >> + } >> + rc = of_clk_add_provider(node, of_clk_src_simple_get, clk); > > Why not check the return value? Added check... > >> + >> diff --git a/drivers/clk/socfpga/clk-pll-a10.c b/drivers/clk/socfpga/clk-pll-a10.c >> new file mode 100644 >> index 0000000..2adc2f5 >> --- /dev/null >> +++ b/drivers/clk/socfpga/clk-pll-a10.c > [..] >> + >> +static u8 clk_pll_get_parent(struct clk_hw *hwclk) >> +{ >> + struct socfpga_pll *socfpgaclk = to_socfpga_clk(hwclk); >> + u32 pll_src; >> + >> + pll_src = readl(socfpgaclk->hw.reg); >> + >> + return (pll_src >> CLK_MGR_PLL_CLK_SRC_SHIFT) & >> + CLK_MGR_PLL_CLK_SRC_MASK; >> +} >> + >> + > > Nitpick: Single newline please. > Cleaned up... >> +static struct clk_ops clk_pll_ops = { > > const? > Same comment applies here as I'm using clk_get_ops for .enable/.disable. >> + .recalc_rate = clk_pll_recalc_rate, >> + .get_parent = clk_pll_get_parent, >> +}; >> + >> +static __init struct clk *__socfpga_pll_init(struct device_node *node, > > __init goes after the return type, doesn't it? > >> + const struct clk_ops *ops) >> +{ >> + u32 reg; >> + struct clk *clk; >> + struct socfpga_pll *pll_clk; >> + const char *clk_name = node->name; >> + const char *parent_name[SOCFGPA_MAX_PARENTS]; >> + struct clk_init_data init; >> + struct device_node *clkmgr_np; >> + int rc; >> + int i = 0; >> + >> + of_property_read_u32(node, "reg", ®); >> + >> + pll_clk = kzalloc(sizeof(*pll_clk), GFP_KERNEL); >> + if (WARN_ON(!pll_clk)) >> + return NULL; >> + >> + clkmgr_np = of_find_compatible_node(NULL, NULL, "altr,clk-mgr"); > > I haven't looked at the binding, but I would expect there to be > some phandle to this node somewhere so that we don't have to > search the whole tree? > I'm putting all of the clocks under the clk-mgr node as much of this clock driver was derived from clk-highbank. >> + clk_mgr_a10_base_addr = of_iomap(clkmgr_np, 0); >> + BUG_ON(!clk_mgr_a10_base_addr); >> + pll_clk->hw.reg = clk_mgr_a10_base_addr + reg; >> + >> + of_property_read_string(node, "clock-output-names", &clk_name); >> + >> + init.name = clk_name; >> + init.ops = ops; > [..] >> diff --git a/drivers/clk/socfpga/clk.h b/drivers/clk/socfpga/clk.h >> index b09a5d5..6989847 100644 >> --- a/drivers/clk/socfpga/clk.h >> +++ b/drivers/clk/socfpga/clk.h >> @@ -34,10 +34,14 @@ >> ((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0)) >> >> extern void __iomem *clk_mgr_base_addr; >> +extern void __iomem *clk_mgr_a10_base_addr; >> >> void __init socfpga_pll_init(struct device_node *node); >> void __init socfpga_periph_init(struct device_node *node); >> void __init socfpga_gate_init(struct device_node *node); >> +void __init socfpga_a10_pll_init(struct device_node *node); >> +void __init socfpga_a10_periph_init(struct device_node *node); >> +void __init socfpga_a10_gate_init(struct device_node *node); > > __init is useless on prototypes. It's not your fault for copying > previous code, but it would be good to avoid adding more and to > clean this up in some other patch. > Will clean up. Thanks for reviewing this patch. Dinh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/