From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH 04/14] clk: samsung: Add clock description for basic CMU blocks Date: Wed, 27 Aug 2014 14:35:53 +0200 Message-ID: <53FDD0A9.3040109@samsung.com> References: <1409132889-2080-1-git-send-email-ch.naveen@samsung.com> <1409132889-2080-4-git-send-email-ch.naveen@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: Received: from mailout2.w1.samsung.com ([210.118.77.12]:14591 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933220AbaH0Mf6 (ORCPT ); Wed, 27 Aug 2014 08:35:58 -0400 Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout2.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0NAY00GIATSOVR40@mailout2.w1.samsung.com> for linux-samsung-soc@vger.kernel.org; Wed, 27 Aug 2014 13:38:48 +0100 (BST) In-reply-to: <1409132889-2080-4-git-send-email-ch.naveen@samsung.com> Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: Naveen Krishna Chatradhi Cc: naveenkrishna.ch@gmail.com, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, cpgs@samsung.com, Mike Turquette , Thomas Abraham On 27.08.2014 11:48, Naveen Krishna Chatradhi wrote: > This patch adds clock description for MUX, DIV, GATE and PLL > clocks available in TOPC, TOP0, TOP1, BUS0, BUS1, FSYS0, FSYS1, > CCORE, PERIC0, PERIC1, PERIS, DISP, G3D, MSCL and MFC blocks. [snip] > +Phy clocks: > + > +There are several clocks which are generated by specific PHYs. > +These clocks are fed into the clock controller and then routed to > +the hardware blocks. These clocks are defined as fixed clocks in the > +driver with following names: > + The names are missing? > +Required Properties for Clock Controller: > + > + - compatible: should be one of the following. > + 1) "samsung,exynos7-clock-topc" > + 2) "samsung,exynos7-clock-top0" > + 3) "samsung,exynos7-clock-top1" > + 4) "samsung,exynos7-clock-atlas" > + 5) "samsung,exynos7-clock-ccore" [snip] > + - clocks: list of clock identifiers which are fed as the input to > + the given clock controller. Please refer the next section to find > + the input clocks for a given controller. > + > + - clock-names: list of names of clocks which are fed as the input > + to the given clock controller. In the dtsi file added by patch 11/14, I don't see those being specified. > + > +Input clocks for topc clock controller: > + - fin_pll > + - fout_aud_pll [snip] > +/* PMS values for PLL 1451x */ > +static const struct samsung_pll_rate_table pll1451x_24mhz_tbl[] = { > + /* rate, m, p, s */ > + PLL_35XX_RATE(660000000, 165, 3, 1), This array needs to be sorted by output rate. However you might want to rebase this series onto series [1] to eliminate this requirement. Be aware that mentioned series might need a respin, though. Also the rates seem to be too nicely rounded. Are they the real values that can be obtained using the PLL equation (what is required by the PLL framework) or a copy paste from the documentation? Similarly, series [1] lessens the requirement a bit, due to recalculation of rates from specified coefficients and warning about incorrect entries. [1] https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg35188.html > + PLL_35XX_RATE(1800000000, 150, 2, 0), > + PLL_35XX_RATE(1700000000, 425, 6, 0), > + PLL_35XX_RATE(1600000000, 200, 3, 0), > + PLL_35XX_RATE(1500000000, 125, 2, 0), > + PLL_35XX_RATE(1400000000, 175, 3, 0), > + PLL_35XX_RATE(1380000000, 115, 2, 0), > + PLL_35XX_RATE(1300000000, 325, 6, 0), > + PLL_35XX_RATE(1200000000, 100, 2, 0), > + PLL_35XX_RATE(1180000000, 295, 6, 0), > + PLL_35XX_RATE(1104000000, 276, 6, 0), [snip] > + DIV(0, "dout_sclk_aud_pll", "mout_aud_pll_ctrl", DIV_TOPC3, 28, 3), > +}; > + > +static struct samsung_gate_clock topc_gate_clks[] __initdata = { > + GATE(ACLK_CCORE_532, "aclk_ccore_532", "dout_aclk_ccore_532", > + ENABLE_ACLK_TOPC0, 0, CLK_IGNORE_UNUSED, 0), Why CLK_IGNORE_UNUSED? (The same question for all the gate clocks defined with it in the driver.) Best regards, Tomasz From mboxrd@z Thu Jan 1 00:00:00 1970 From: t.figa@samsung.com (Tomasz Figa) Date: Wed, 27 Aug 2014 14:35:53 +0200 Subject: [PATCH 04/14] clk: samsung: Add clock description for basic CMU blocks In-Reply-To: <1409132889-2080-4-git-send-email-ch.naveen@samsung.com> References: <1409132889-2080-1-git-send-email-ch.naveen@samsung.com> <1409132889-2080-4-git-send-email-ch.naveen@samsung.com> Message-ID: <53FDD0A9.3040109@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 27.08.2014 11:48, Naveen Krishna Chatradhi wrote: > This patch adds clock description for MUX, DIV, GATE and PLL > clocks available in TOPC, TOP0, TOP1, BUS0, BUS1, FSYS0, FSYS1, > CCORE, PERIC0, PERIC1, PERIS, DISP, G3D, MSCL and MFC blocks. [snip] > +Phy clocks: > + > +There are several clocks which are generated by specific PHYs. > +These clocks are fed into the clock controller and then routed to > +the hardware blocks. These clocks are defined as fixed clocks in the > +driver with following names: > + The names are missing? > +Required Properties for Clock Controller: > + > + - compatible: should be one of the following. > + 1) "samsung,exynos7-clock-topc" > + 2) "samsung,exynos7-clock-top0" > + 3) "samsung,exynos7-clock-top1" > + 4) "samsung,exynos7-clock-atlas" > + 5) "samsung,exynos7-clock-ccore" [snip] > + - clocks: list of clock identifiers which are fed as the input to > + the given clock controller. Please refer the next section to find > + the input clocks for a given controller. > + > + - clock-names: list of names of clocks which are fed as the input > + to the given clock controller. In the dtsi file added by patch 11/14, I don't see those being specified. > + > +Input clocks for topc clock controller: > + - fin_pll > + - fout_aud_pll [snip] > +/* PMS values for PLL 1451x */ > +static const struct samsung_pll_rate_table pll1451x_24mhz_tbl[] = { > + /* rate, m, p, s */ > + PLL_35XX_RATE(660000000, 165, 3, 1), This array needs to be sorted by output rate. However you might want to rebase this series onto series [1] to eliminate this requirement. Be aware that mentioned series might need a respin, though. Also the rates seem to be too nicely rounded. Are they the real values that can be obtained using the PLL equation (what is required by the PLL framework) or a copy paste from the documentation? Similarly, series [1] lessens the requirement a bit, due to recalculation of rates from specified coefficients and warning about incorrect entries. [1] https://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg35188.html > + PLL_35XX_RATE(1800000000, 150, 2, 0), > + PLL_35XX_RATE(1700000000, 425, 6, 0), > + PLL_35XX_RATE(1600000000, 200, 3, 0), > + PLL_35XX_RATE(1500000000, 125, 2, 0), > + PLL_35XX_RATE(1400000000, 175, 3, 0), > + PLL_35XX_RATE(1380000000, 115, 2, 0), > + PLL_35XX_RATE(1300000000, 325, 6, 0), > + PLL_35XX_RATE(1200000000, 100, 2, 0), > + PLL_35XX_RATE(1180000000, 295, 6, 0), > + PLL_35XX_RATE(1104000000, 276, 6, 0), [snip] > + DIV(0, "dout_sclk_aud_pll", "mout_aud_pll_ctrl", DIV_TOPC3, 28, 3), > +}; > + > +static struct samsung_gate_clock topc_gate_clks[] __initdata = { > + GATE(ACLK_CCORE_532, "aclk_ccore_532", "dout_aclk_ccore_532", > + ENABLE_ACLK_TOPC0, 0, CLK_IGNORE_UNUSED, 0), Why CLK_IGNORE_UNUSED? (The same question for all the gate clocks defined with it in the driver.) Best regards, Tomasz