From patchwork Thu Jun 13 07:02:05 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yadwinder Singh Brar X-Patchwork-Id: 2714071 Return-Path: X-Original-To: patchwork-linux-samsung-soc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 15AE29F3DD for ; Thu, 13 Jun 2013 07:02:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 884772021D for ; Thu, 13 Jun 2013 07:02:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2330A20157 for ; Thu, 13 Jun 2013 07:02:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754592Ab3FMHCH (ORCPT ); Thu, 13 Jun 2013 03:02:07 -0400 Received: from mail-ie0-f180.google.com ([209.85.223.180]:58028 "EHLO mail-ie0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752082Ab3FMHCG (ORCPT ); Thu, 13 Jun 2013 03:02:06 -0400 Received: by mail-ie0-f180.google.com with SMTP id f4so20523819iea.39 for ; Thu, 13 Jun 2013 00:02:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=EdTIjfEHHvT/920uEi3g09C/5w1z+2LsbaB0JfpwwbI=; b=avlpOxKwL4Q53deO4uwf61aErSnuwJOttPq10e4sZYF0xR5oZX/EgtQMsbVcloZA1W sb25wupd4ErymgK1s/rv/zysm4wCOmeOUhKErqLyzw+eYJjJQ0ldchVcBYjW9ab9TuLj 0D9z/ZGcw110EqL4fkw8jiCHxREc6k9x4Fkv9IR2uXfKBXpfEae6GGxo0n2DCSkWbFyO 1v0YcUiM2T/cRQUUvWpyi6fbXZ8Ybez7PqQi8CpJwKhaDI9GjOquEQx4cWVk6HQ0p/OR IN8yioKAbRD3Ub/qZMZOiQ1XIVf5Xr7QDntZtbm75OQZT+rnd3yG7LiFmutZpjCAXaaY MWSw== MIME-Version: 1.0 X-Received: by 10.50.17.166 with SMTP id p6mr5113328igd.12.1371106925410; Thu, 13 Jun 2013 00:02:05 -0700 (PDT) Received: by 10.50.25.193 with HTTP; Thu, 13 Jun 2013 00:02:05 -0700 (PDT) In-Reply-To: References: <1370272196-4346-1-git-send-email-yadi.brar@samsung.com> <1370272196-4346-2-git-send-email-yadi.brar@samsung.com> <1501124.EVQA8KO0ic@flatron> Date: Thu, 13 Jun 2013 12:32:05 +0530 Message-ID: Subject: Re: [PATCH v4 1/6] clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx From: Yadwinder Singh Brar To: Andrew Bresticker Cc: Doug Anderson , Tomasz Figa , Yadwinder Singh Brar , linux-samsung-soc , "linux-arm-kernel@lists.infradead.org" , Kukjin Kim , Mike Turquette , Thomas Abraham , Tomasz Figa , Vikas Sajjan , Patch Tracking Sender: linux-samsung-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-samsung-soc@vger.kernel.org X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Jun 13, 2013 at 3:32 AM, Andrew Bresticker wrote: > Doug, > >>> Hmm, if done properly, it could simplify PLL registration in SoC clock >>> initialization code a lot. >>> >>> I'm not sure if this is really the best solution (feel free to suggest >>> anything better), but we could put PLLs in an array, like other clocks, >>> e.g. >>> >>> ... exynos4210_pll_clks[] = { >>> CLK_PLL45XX(...), >>> CLK_PLL45XX(...), >>> CLK_PLL46XX(...), >>> CLK_PLL46XX(...), >>> }; >>> >>> and then just call a helper like >>> >>> samsung_clk_register_pll(exynos4210_pll_clks, >>> ARRAY_SIZE(exynos4210_pll_clks)); >> >> Something like that looks like what I was thinking. I'd have to see >> it actually coded up to see if there's something I'm missing that >> would prevent us from doing that, but I don't see anything. > > The only issue I see with this is that we may only want to register a > rate table with a PLL only if fin_pll is running at a certain rate. > On 5250 and 5420, for example, we have EPLL and VPLL rate tables that > should only be registered if fin_pll is 24Mhz. We may have to > register those separately, but this approach seems fine otherwise. > As Andrew Bresticker said, we will face problem with different table, and it will give some pain while handling such cases but I think overall code may look better. Similar thoughts were their in my mind also, but i didn't want to disturb this series :). Anyways, I think we can do it now only rather going for incremental patches after this series. I was thinking to make samsung_clk_register_pllxxxx itself little generic instead of using helper, as we are almost duplicating code for most PLLs. A rough picture in my mind was, After implementing generic samung_clk_register_pll(), code may look like below. Its just an idea, please feel free to correct it. Later we can factor out other common clk.ops for PLLs also. this diff is over this series. Assuming a generic samung_clk_register_pll() is their(which i think is not impossible) 8<-------------------------------------------------------------------------- --- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/drivers/clk/samsung/clk-exynos5250.c +++ b/drivers/clk/samsung/clk-exynos5250.c @@ -493,6 +493,20 @@ static __initdata struct samsung_pll_rate_table epll_24mhz_tbl[] = { PLL_36XX_RATE(32768000, 131, 3, 5, 4719), }; +struct __initdata samsung_pll_init_data samsung_plls[] = { + PLL(pll_3500, "fout_apll", "fin_pll", APLL_LOCK, APLL_CON0, NULL), + PLL(pll_3500, "fout_mpll", "fin_pll", MPLL_LOCK, MPLL_CON0, NULL), + PLL(pll_3500, "fout_bpll", "fin_pll",BPLL_LOCK, BPLL_CON0, NULL), + PLL(pll_3500, "fout_gpll", "fin_pll",GPLL_LOCK, GPLL_CON0, NULL), + PLL(pll_3500, "fout_cpll", "fin_pll",BPLL_LOCK, CPLL_CON0, NULL), +}; + +struct __initdata samsung_pll_init_data epll_init_data = + PLL(pll_3600, "fout_epll", "fin_pll", EPLL_LOCK, EPLL_CON0, NULL); + +struct __initdata samsung_pll_init_data vpll_init_data = + PLL(pll_3600, "fout_epll", "fin_pll", VPLL_LOCK, VPLL_CON0, NULL); + /* register exynox5250 clocks */ void __init exynos5250_clk_init(struct device_node *np) { @@ -519,44 +533,22 @@ void __init exynos5250_clk_init(struct device_node *np) samsung_clk_register_mux(exynos5250_pll_pmux_clks, ARRAY_SIZE(exynos5250_pll_pmux_clks)); - fin_pll_rate = _get_rate("fin_pll"); + samsung_clk_register_pll(samsung_plls, ARRAY_SIZE(samsung_plls)); + vpllsrc = __clk_lookup("mout_vpllsrc"); if (vpllsrc) mout_vpllsrc_rate = clk_get_rate(vpllsrc); - apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll", - reg_base, NULL, 0); - mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll", - reg_base + 0x4000, NULL, 0); - bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll", - reg_base + 0x20010, NULL, 0); - gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll", - reg_base + 0x10050, NULL, 0); - cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll", - reg_base + 0x10020, NULL, 0); - + fin_pll_rate = _get_rate("fin_pll"); if (fin_pll_rate == (24 * MHZ)) { - epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll", - reg_base + 0x10030, epll_24mhz_tbl, - ARRAY_SIZE(epll_24mhz_tbl)); - } else { - pr_warn("%s: valid epll rate_table missing for\n" - "parent fin_pll:%lu hz\n", __func__, fin_pll_rate); - epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll", - reg_base + 0x10030, NULL, 0); + epll_init_data.rate_table = epll_24mhz_tb; } + samsung_clk_register_pll(&fout_epll_data, 1); if (mout_vpllsrc_rate == (24 * MHZ)) { - vpll = samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc" - , reg_base + 0x10040, vpll_24mhz_tbl, - ARRAY_SIZE(vpll_24mhz_tbl)); - } else { - pr_warn("%s: valid vpll rate_table missing for\n" - "parent mout_vpllsrc_rate:%lu hz\n", __func__, - mout_vpllsrc_rate); - samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc", - reg_base + 0x10040, NULL, 0); + vpll_init_data.rate_table = epll_24mhz_tb; } + samsung_clk_register_pll(&fout_epll_data, 1); samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks, ARRAY_SIZE(exynos5250_fixed_rate_clks)); diff --git a/drivers/clk/samsung/clk-pll.h b/drivers/clk/samsung/clk-pll.h index 4780e6c..3b02dc5 100644 --- a/drivers/clk/samsung/clk-pll.h +++ b/drivers/clk/samsung/clk-pll.h @@ -39,6 +39,39 @@ struct samsung_pll_rate_table { unsigned int kdiv; }; +#define PLL(_type, _name, _pname, _lock_reg, _con_reg, _rate_table) \ + { \ + .type = _type, \ + .name = _name, \ + .parent_name = _pname, \ + .flags = CLK_GET_RATE_NOCACHE, \ + .rate_table = _rate_table, \ + .con_reg = _con_reg, \ + .lock_reg = _lock_reg, \ + } + +enum samsung_pll_type { + pll_3500, + pll_45xx, + pll_2550, + pll_3600, + pll_46xx, + pll_2660, +}; + + +struct samsung_pll_init_data { + enum samsung_pll_type type; + struct samsung_pll_rate_table *rate_table; + const char *name; + const char *parent_name; + unsigned long flags; + const void __iomem *con_reg; + const void __iomem *lock_reg; +}; + +extern int __init samsung_clk_register_pll(struct samsung_pll_init_data *data, + unsigned int nr_pll); + Regards, Yadwinder