From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752422AbdDKRsj (ORCPT ); Tue, 11 Apr 2017 13:48:39 -0400 Received: from mail-cys01nam02on0089.outbound.protection.outlook.com ([104.47.37.89]:44409 "EHLO NAM02-CY1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751620AbdDKRsf (ORCPT ); Tue, 11 Apr 2017 13:48:35 -0400 Authentication-Results: spf=fail (sender IP is 192.88.168.50) smtp.mailfrom=nxp.com; nxp.com; dkim=none (message not signed) header.d=none;nxp.com; dmarc=fail action=none header.from=nxp.com; Message-ID: <1491932908.31718.33.camel@nxp.com> Subject: Re: [RFC PATCH 3/3] cpufreq: imx6q: refine clk operations From: Leonard Crestez To: Dong Aisheng , CC: , , , , , , , , , , , , , Date: Tue, 11 Apr 2017 20:48:28 +0300 In-Reply-To: <1491969809-20154-4-git-send-email-aisheng.dong@nxp.com> References: <1491969809-20154-1-git-send-email-aisheng.dong@nxp.com> <1491969809-20154-4-git-send-email-aisheng.dong@nxp.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2-0ubuntu3.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-EOPAttributedMessage: 0 X-Matching-Connectors: 131364065138036423;(91ab9b29-cfa4-454e-5278-08d120cd25b8);() X-Forefront-Antispam-Report: CIP:192.88.168.50;IPV:NLI;CTRY:US;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(336005)(39410400002)(39400400002)(39850400002)(39380400002)(39860400002)(39840400002)(39450400003)(2980300002)(1109001)(1110001)(339900001)(189002)(24454002)(377424004)(199003)(9170700003)(8656002)(76176999)(53936002)(54906002)(23676002)(50986999)(77096006)(104016004)(103116003)(229853002)(86362001)(6246003)(50226002)(8676002)(8936002)(81166006)(4326008)(47776003)(356003)(2870700001)(106466001)(7416002)(33646002)(189998001)(38730400002)(105606002)(5660300001)(5820100001)(2950100002)(305945005)(85426001)(36756003)(2906002)(50466002)(99106002)(32563001)(217873001);DIR:OUT;SFP:1101;SCL:1;SRVR:SN2PR03MB2287;H:tx30smr01.am.freescale.net;FPR:;SPF:Fail;MLV:ovrnspm;MX:1;A:1;PTR:InfoDomainNonexistent;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;BL2FFO11OLC006;1:p4Pjw1Wbg36RDcg0DJblnB5LBjMigBCUAoGxLdg9buIzV33xFsR3lC7xFrfzxB6mjDDkf8yq409XNrJ8+qG0ZOJfeV/cl7rm7+Saw1zitwRY2Z5+AMaTRPapOMzCWZet11TDctoeSJx1y58v13wk5NgAgsE9gOeOHjHpbk48mr85RpCSBBRO+DpLYXoj2+j51tFU/pkd9H3mRQsC2AKbCVbRs7nfPw7QyKJewQPUGi8iDO7jGdKeIDk8fJVPuyAG8u9Osc+4YmAEyeN8Nj/Gk4fxdBfrfOgtWhW4JzBQYfhl8PpNk7HTBlcFV+VEM2mbq5QX+ZJvP6cl5KLUcobLuDdcvlfZBgQ7WvPODPrAwXM1gwGr9cBCNvGkcc44qVE06iw1O6UIXhkSJr2RTFlMGWz41yoyJVkZ6fA+g3nY2cCjA0Vst16KiluF5EGNrTwF6iyl7rr5Lq3Dqsl/OCCwqnpPwNkqBMGN1V6C7J4X1mvk3lbFN/x5NyCfKTgUqYgzE1OVAUmfHf0Emmu2uYiowsEqUWfjFfmv/hhpXz1tgeYFQGUgj0ZeHN3fUvNLpzSxU1AZj4ldfFqQ66vay1TADe3tPeZmnQPxHvlhebA3cuZYQ5IE3EKW5uUOe1kBfeQLLkFfQ0c9HUjmEzZHfq2Ki5U+DRreNTcFW8fsMGN62J1JaZeNAye7jugC1ZjzBLiU X-MS-Office365-Filtering-Correlation-Id: e35dd260-5aec-4607-6d6a-08d48102f942 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(2017030255120)(201703131430075)(201703131517081);SRVR:SN2PR03MB2287; X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB2287;3:Sat6S3JZm50Yuq91CBuHkJxIoa5yH/9tYi0OQt63YSHxTNavvjunypcq4Q2jDxT1Z/Tx96ZPNVBE7O3a3T2BA+iYzmaANH3WPP/QDjqMZWmLJr9nJMLfQ4wxrZU6JjBXgiEoztLyvNhe7DIVrh03xeV1SFtJ0cN1j+0E6RnT3jPB1Wk5I9mUFcTxkkaUyXP7+r1vuPLPHeQrgo/OxaVphNmi7Wn8WHRviDK0ZBVVL1UuLspoWk80F08XF/Qq8BjEuqb5WLi1G4Zdlhf8ugRs7JYsczVLRnjFkPWYXHHnHPBTo1D1oNBhraEJYArE3SzMlvmt00w+qm2VTuMXPoQzqftLj2IhdOselP3XYczOREVQ8KLXB1AfaGwUDBd+SGARSwOzf0tyFO0LDH2XTJfAyeCgMCUQuNhoOShSgoIOgHC9KS+iHQfizWmnovbKNHZHgVE5HpV6C6pIHBZrZIje8g==;25:o19TifZ+H0VRLiQoVIgEM5FT+l/NRgEb8aT+hmj+yZ8s2ul26dCFxdUyShOJwZVKpgANQDxHZmDv22GjdvLm+7A7++bgBespAmtWblcTo51fsneMbRoTdq/AuUmkSBg8jJuC+qlfgbjiinp7MOfZy/Gy1LdOwat3gFjt+H1HRrRpyzCyCfpT0gHT8G1OMOHranpBR6dEm6aXCe/VYQqDoAKMGLulSpt6qxud09uHPQ8fy7YwfkC4isg+UcbAAz///KzKMaXWV53C/o/FXQ5CpFTvMXbM1V0k2pDf+Usn9AZoHihOLryDjglLzd2LnTwTcMhc+q3kjVuLvYzEPZiqMzeHa+JbSsLGOKLpgIU9Ull0DcjzTk5eZvYUMUXahHeB/TKiluGgc72r4nZQlMBTS5kawHbaP1zhKe2ZS6z0KV+rWY6IklDZM9LbUKxgKXM9vvs82Efruw4Wqw53qGgTlg== X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB2287;31:hPvmZgyaANuu4G2pmtaKiAZu65wrlAJHh6kM/1Lu7vh2o08494pRjGkg6316837skMyklsYgtLcYdbDpUsImLWUFpB7mglAm2mbHM3Klp6yUDZFWZMpMQNe/D84CJUSB+jDTL3wHhkrPmEKFXYbASO5IJnJNhM2w5X6Qi0QpFTPNZXhEEyz7LmohCQZI2zhBXvME71ueSsiOPYpkwLcer+Yt1N5w5ETiUX0+G9s9Ji7/A6cTYStR5Rl/SolX5W3Pa5CBGQ7uX0fQ/NyS1ct+hLrVA9dWhB7Aovc60fjN0Nw= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6095135)(601004)(2401047)(13017025)(13018025)(13024025)(13023025)(8121501046)(13015025)(5005006)(3002001)(10201501046)(93006095)(93001095)(6055026)(6096035)(201703131430075)(201703131441075)(201703131448075)(201703131433075)(201703161259075)(20161123556025)(20161123565025)(20161123561025)(20161123563025);SRVR:SN2PR03MB2287;BCL:0;PCL:0;RULEID:(400006);SRVR:SN2PR03MB2287; X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB2287;4:4qblpLZJWm5Zzty7Hoo4iZ32U06zpveOgBnOFbfmbUPnAwwDx9a0SmyTMeqkqqJN9F8ZFAohximdLbwHWHBufmgu0vHhMfvshV35cDwsTDxS82km32dDpVdsoYFGEy7t6b8Ub2WLarQmPGVd94M4uOSuOoac5d7arVAYsC/2Ek1h+kBuwGs/rd5CSUjWEI+8Fil/q9IghJtRxnCbfeQslzzKvQeTUy7qcBaLtrG+QIRE73PHS4qqY8XDvkF26pOYgtjKm1xgvvWh9tPuqRQaaSBa81AQNowTxjbGT/IxdTD7gxpiENgMN+0aDI5YyZT7//7VBG6FygPgwmHbunQG15hehOOI4XmPIZPDtZSlZSR14i7bJVAfKwMSYYmFFQLqHiAedpUnWJP2uCtOiFSwWBRcF9A8G4sjujFdMuAGK5Z44xVYe/o+DgnV4EWPLxZDvicPgqmp8HVauN7lbldDGzJDyXrb6x2pDcLbQ6G8S8EHNGaNcq+KefYK9ie7v/Lp9ni697V/SNWqR7IaUtKrdvcs1rXQeNrpgWCj2CNi5SLb0YioqZCpF8x26SXjXponJBbKBa0AqasXwxwZxzl3extBJSsEDs6Ixqc+34Io8rrKVE2xEBzQwx71nPH6ZMKjV8pdebzKR5K/G15aB9ua7/47bBCNar+5QpFt34lMMbbANpotayb9fIpkys0vUBmo1Xu/6uhtYtF8+ZnD1jWqj7xyTZ9viYHG1PR/qhiKXMmTMaPx7SSsd7/dmT1jIIu3zX1vlwQ//c1KZCYrbV9Q1dDOA24GhbXwP7o6H7L2Cm8hzwjAEB6NljMlgKbnNfsXxtI2Mt5bhanpLtGrrpQKlLiYGxnpwGQDC7u+H/UYb6+EpouPTsGU/E1S6rNQa/x0 X-Forefront-PRVS: 0274272F87 X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtTTjJQUjAzTUIyMjg3OzIzOjZicitTYlpxR0dUa3ZlcXhLMnZPWHQ1LzRX?= =?utf-8?B?dDY1ZFVHUU5SZEtWWTB3dTdBSjJ5bHBFWHN2VitUL3JVN29MVTcrbDdYVU03?= =?utf-8?B?RnNZbG5JYSsyVGR4OUtRSmkzZ0ZzQTRBTVJzb3V3TytuMUxvOTNjV0ZML1lD?= =?utf-8?B?cUNYQ28vRnRoVVpjM1NJN1FPVHZ1Tmptd0t6dy9qSkhSdjNLQlRmbGVrSy9z?= =?utf-8?B?LzIvZjJRRStGVTRQdW10UkhCRms1b1hYbUpCdDZhK3JVQjBMQ2lxQmptTHli?= =?utf-8?B?b0puaDBGQUJxUHhKZUpUTmNSODVSQ1RUYmJFc3poS0IzNWxRbDllaFZtdEpv?= =?utf-8?B?Z2NzRTdJWXdHMVpmcnRYYlRweGJ4cWZzMUpMcTdta0RPbFAxcHZoOHBmdjJp?= =?utf-8?B?QVhsc1dwYmpRNGtBNkxuODJpckozLy8rTitPcnIvbnZQeUl1SjFLanNSWFZV?= =?utf-8?B?VmF3Sm82aElOVHZ0U3hGZVlvTGNNeWMxM0lmeHorWEhwU2JFRktLVVBwRjN6?= =?utf-8?B?VWVkRy9WRVE1VlhCZi9yZlY2Y0wycFk5c0t5UXVwSVBjMmM3Z2lLblp6QnRx?= =?utf-8?B?Y0RlUElEYWp0NzI0YlZGK0JvU3VVVHNOZEU0d1BERGRyTFhjRGc4ZTQzc0Fp?= =?utf-8?B?RlVQM1ovSTFBY0FzMzV1L1FweG9jOTZyUkxXbTU1VWhpUWhWNmxrMENMdFJr?= =?utf-8?B?REpPWDdWMWN6STJJaWtyb1lCRFR4aE0wam5Fay9LT3dnak9tdnFuY0hRa1My?= =?utf-8?B?b1pTTHduN2xNSW1OY2dFQU5BT2hmbk1nZStVS0tiWFZuVS9qNWRjU0ZRNEVi?= =?utf-8?B?M084M2tTaWV5ODBwUGtLa3lGOVdhdFhvWTVjckpwZWpLcFZsK1lYUnlibmdG?= =?utf-8?B?M3YwY1NXMXV3TWtuVzgwZ0hmek9QeEFNMHI1NzhvT1QxUjhQQlIxSFRpc1hk?= =?utf-8?B?MUJITU5BbXQvaWhSelNxcUlJVEYwc3FFWmZReE1vVlYzNzVZWmdFYnBFYWlt?= =?utf-8?B?aS9SdmttV3BRT2dWWXU0c1c5d2dKQm5KQXJSQ1VnUHdsV005b2RvRDM2bWVR?= =?utf-8?B?Z0FkOTZ1VXVqU2owL242WE1wOFNZOWlKSG9hdlJQOVM2cGRIQmJreFpONkZO?= =?utf-8?B?TkpDbEpiZDBmSXd6WHI2dFh0ZFA0ZDBHWTJpaForN0RiK09LTG1DZ2FTWnJn?= =?utf-8?B?eU8zOTFQZ3djdXN2cHUvMWpkc21EQXBHSHg5c3gzL1ZNWjhoakIyTVhNREpi?= =?utf-8?B?bld4NlA5a0FEMHdoOTVWWTdIbkRLSktjbkdqU3dqS21kS2F3ZnBjZTRsNHVS?= =?utf-8?B?ejFnR0wwQnRySFhoQlB1RzBRV3lXSE1RRkJjK2U2cXZPQXJzYkNTc1FYYkxZ?= =?utf-8?B?MC9hQnhwOWdHZ0JPbEZvTUQyRVVzbW1udG5ycEpZWEJlMXpsNHRSOE0vUjEz?= =?utf-8?B?UmVpMmVGR28xWWZsL1FVS2h0dzBWYWFQRUJTdFU3Z0pMN1ZNVUQ1Y1VKaWZa?= =?utf-8?B?TnRJdXhwUjVoTG5adG5ONnV6WEFpNFlWNXJKbTBaQWcxY0VYQ2xMUytnK2pP?= =?utf-8?B?Z3kxMFdxKzNzK1R0YkdLUzJSSjViZEs1WVk2OU5jUkdCVjZPZWN4SDkveGpo?= =?utf-8?B?NndwZnlYajBOR1VrdUd2V1RwSitwcjFpRE9QZm45WDByZFBteUN4dnM0K3dq?= =?utf-8?B?K01wUDRPcUd1blRWVWFYVDFlWURtenNlUnRSd2hyMVBqWWtDYTBNNjJaTHM2?= =?utf-8?B?V3RGRFMyQy94ai9GeXpKTkxQVGs4bzAwRFVzekE4V2I5VzV1a0tvaEZCb244?= =?utf-8?B?czU4M3dWZDlVWVZReG5OeWtGSkZKdmVMaStDVjN6dHUzTXc9PQ==?= X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB2287;6:gFx1lA9cW3DxI/KdyecARPSwgoL3dwmDKihYfcBiHXXsY/mPNQCwBuCp8L57W/t4VAlYUwhMMO5u1VN1cvFvFkMW/I4L01z379wUxi8YHcZXoQH7rF+lY5Y3dRK8owEtf3UIO5eyCln9GQCZyhXkxecS6QR90iT0mx/cf6FOWSHqAU/F0rYbTUA7KMWOJaHE+qOMifa2EKQtI7TL5Mii0P92qMwqlcUsBlFRIczo5eopU/aQp+7N2HYpw7jEhLnkZ1hCOWblDnJ+WJ9tjbofUoa9FY4LGc9jSx3Mch9DUYI3Tq3t1gfFyBdYO3ygUs5wxtUnS7NUmbTZGLxCN6AaWikIYD2RReIxv41N91xIcXpRNEYTLJmFgSt7YMlJ1YB6iLpVBHCz8XJVZZz4IcO392spCQ0jUcV6BWp79CndRMvx9Mr/ahHdpwVvnJzbgLJIHsrGNSVBSp0mTehBBUZ+VQ==;5:q77wTAfUUsLJOqSA0mZ1KZN5x/jWlY0b6K+6ucnLM44JaOGKeF4mjJvdqz5N4sbNIkohkjG7hL1B0jICY2gxjDaF4LqDszpEk36+lLFODUy6biaN0VRwK9HKykaKdeA+RnFiowEiLqzj0kYh825xRw4OeBYj6KxaWtzf3J32SIhtU5Pf5eac3Tr+fLTmu0I0;24:/Xbb21qYYL4e7bgo85rX6oydWKtlkgkkCtch07Y85pTxXRRGedoSHPNEh/WE4gZSylEWD7PjTohMyaMzisbYwWh+WeLjAMiOenyst/33fN4= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;SN2PR03MB2287;7:XqxVbo9nVi7UyAfp112RWXbuFT0j6Cc+5GE7neim6iquWFuW4eND0nu9mAK4bWdrrSnM5rfJ99KiTott0QoHEs7n1BeBD9H6NWxoRCH0NsqsSwoAIjIUuFLEUW+QNPoF3myRtGtn3fdr7TlKl9hW8PwU6Ll7QD3aSlTAJQIFp5+iwK/cQ7OVJg+gnFkKw6GLQlzju21zMWF5EUvRg/5Q8NM7mlfcsN0NOQQeNJDqrWYWOU+f4+xvp3p0xzlNcbDP7mAoCAPXqXlGecZxXvDAjE1TE/rrRniArE9afqFjYRaxB7Ojwrd//y7/O4TgQKF4ndq2lSVwjAzgQvVOxPtbWA== X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2017 17:48:33.5384 (UTC) X-MS-Exchange-CrossTenant-Id: 5afe0b00-7697-4969-b663-5eab37d5f47e X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=5afe0b00-7697-4969-b663-5eab37d5f47e;Ip=[192.88.168.50];Helo=[tx30smr01.am.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN2PR03MB2287 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-04-12 at 12:03 +0800, Dong Aisheng wrote: > +static int num_clks; > +static struct clk_bulk_data clks[] = { > + { .id = "arm" }, > + { .id = "pll1_sys" }, > + { .id = "step" }, > + { .id = "pll1_sw" }, > + { .id = "pll2_pfd2_396m" }, > + { .id = "pll2_bus" }, > + { .id = "secondary_sel" }, > +}; The .id is only required for initialization, it seems strange to keep it around runtime data. It might be better for this API to work with an array of clk* and separate array of names (or clk_bulk_init_data if we need flags). Variable references would be shorter and it would allow more data to be const. > -put_clk: > - if (!IS_ERR(arm_clk)) > - clk_put(arm_clk); > - if (!IS_ERR(pll1_sys_clk)) > - clk_put(pll1_sys_clk); > - if (!IS_ERR(pll1_sw_clk)) > - clk_put(pll1_sw_clk); > - if (!IS_ERR(step_clk)) > - clk_put(step_clk); > - if (!IS_ERR(pll2_pfd2_396m_clk)) > - clk_put(pll2_pfd2_396m_clk); > - if (!IS_ERR(pll2_bus_clk)) > - clk_put(pll2_bus_clk); > - if (!IS_ERR(secondary_sel_clk)) > - clk_put(secondary_sel_clk); > + > + clk_bulk_put(num_clks, clks); > +put_node: >   of_node_put(np); > + >   return ret; >  } My subjective opinion is that a better way to clean this up would be to have a single imx6q_cpufreq_clean function that takes all resources and does stuff like: if (!IS_ERR(clk)) clk_put(clk); clk = NULL; That function can be called from both _remove and failed _probe without having to keep track of which resources have been allocated until then. Just free and NULL all clocks/regulators and simplify control flow. -- Regards, Leonard From mboxrd@z Thu Jan 1 00:00:00 1970 From: leonard.crestez@nxp.com (Leonard Crestez) Date: Tue, 11 Apr 2017 20:48:28 +0300 Subject: [RFC PATCH 3/3] cpufreq: imx6q: refine clk operations In-Reply-To: <1491969809-20154-4-git-send-email-aisheng.dong@nxp.com> References: <1491969809-20154-1-git-send-email-aisheng.dong@nxp.com> <1491969809-20154-4-git-send-email-aisheng.dong@nxp.com> Message-ID: <1491932908.31718.33.camel@nxp.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, 2017-04-12 at 12:03 +0800, Dong Aisheng wrote: > +static int num_clks; > +static struct clk_bulk_data clks[] = { > + { .id = "arm" }, > + { .id = "pll1_sys" }, > + { .id = "step" }, > + { .id = "pll1_sw" }, > + { .id = "pll2_pfd2_396m" }, > + { .id = "pll2_bus" }, > + { .id = "secondary_sel" }, > +}; The .id is only required for initialization, it seems strange to keep it around runtime data. It might be better for this API to work with an array of clk* and separate array of names (or clk_bulk_init_data if we need flags). Variable references would be shorter and it would allow more data to be const. > -put_clk: > - if (!IS_ERR(arm_clk)) > - clk_put(arm_clk); > - if (!IS_ERR(pll1_sys_clk)) > - clk_put(pll1_sys_clk); > - if (!IS_ERR(pll1_sw_clk)) > - clk_put(pll1_sw_clk); > - if (!IS_ERR(step_clk)) > - clk_put(step_clk); > - if (!IS_ERR(pll2_pfd2_396m_clk)) > - clk_put(pll2_pfd2_396m_clk); > - if (!IS_ERR(pll2_bus_clk)) > - clk_put(pll2_bus_clk); > - if (!IS_ERR(secondary_sel_clk)) > - clk_put(secondary_sel_clk); > + > + clk_bulk_put(num_clks, clks); > +put_node: > ? of_node_put(np); > + > ? return ret; > ?} My subjective opinion is that a better way to clean this up would be to have a single imx6q_cpufreq_clean function that takes all resources and does stuff like: if (!IS_ERR(clk)) clk_put(clk); clk = NULL; That function can be called from both _remove and failed _probe without having to keep track of which resources have been allocated until then. Just free and NULL all clocks/regulators and simplify control flow. -- Regards, Leonard