From mboxrd@z Thu Jan 1 00:00:00 1970 From: Naveen Krishna Ch Subject: Re: [PATCH 04/14] clk: samsung: Add clock description for basic CMU blocks Date: Wed, 3 Sep 2014 13:10:00 +0530 Message-ID: References: <1409132889-2080-1-git-send-email-ch.naveen@samsung.com> <1409132889-2080-4-git-send-email-ch.naveen@samsung.com> <53FDD0A9.3040109@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Received: from mail-qc0-f171.google.com ([209.85.216.171]:34351 "EHLO mail-qc0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750833AbaICHkV (ORCPT ); Wed, 3 Sep 2014 03:40:21 -0400 Received: by mail-qc0-f171.google.com with SMTP id x3so8301319qcv.30 for ; Wed, 03 Sep 2014 00:40:20 -0700 (PDT) In-Reply-To: <53FDD0A9.3040109@samsung.com> Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: Tomasz Figa Cc: Naveen Krishna Chatradhi , "linux-arm-kernel@lists.infradead.org" , "linux-samsung-soc@vger.kernel.org" , cpgs@samsung.com, Mike Turquette , Thomas Abraham Hi Tomasz, On 27 August 2014 18:05, Tomasz Figa wrote: > 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? Ok. Will fix. > >> +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. Ok. > >> + >> +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. Ok. Thanks for the pointer. > > 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. These rates are from the user manual. I will check in detail about these rates. > > [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.) Ok. Fixed. Thanks. > > Best regards, > Tomasz -- Shine bright, (: Nav :) From mboxrd@z Thu Jan 1 00:00:00 1970 From: naveenkrishna.ch@gmail.com (Naveen Krishna Ch) Date: Wed, 3 Sep 2014 13:10:00 +0530 Subject: [PATCH 04/14] clk: samsung: Add clock description for basic CMU blocks In-Reply-To: <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> <53FDD0A9.3040109@samsung.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Tomasz, On 27 August 2014 18:05, Tomasz Figa wrote: > 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? Ok. Will fix. > >> +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. Ok. > >> + >> +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. Ok. Thanks for the pointer. > > 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. These rates are from the user manual. I will check in detail about these rates. > > [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.) Ok. Fixed. Thanks. > > Best regards, > Tomasz -- Shine bright, (: Nav :)