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: 17866 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-vc0-f199.google.com (mail-vc0-f199.google.com [209.85.220.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 130D625DF1 for ; Thu, 13 Jun 2013 07:02:10 +0000 (UTC) Received: by mail-vc0-f199.google.com with SMTP id gf11sf2803564vcb.10 for ; Thu, 13 Jun 2013 00:02:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-beenthere:x-forwarded-to:x-forwarded-for:delivered-to :mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:x-original-sender:x-original-authentication-results:precedence :mailing-list:list-id:x-google-group-id:list-post:list-help :list-archive:list-unsubscribe:content-type; bh=EdTIjfEHHvT/920uEi3g09C/5w1z+2LsbaB0JfpwwbI=; b=k8FKlXXUaVx4GgqDKxArQjKYkU4lyR/zEu54y/ToQW6/gdYRgLtPy/ci3xIYIHqZd8 1RlQ449Ym343N9Ndieq8XAiaSbnHqRT73nthcH5xAOS6/7iO+FC++BofG5sAJ0qtAWJ4 5XMhlbb1eXfUK5J0qDqSQ9+DeDZRGwCGDDgjtK58XVKZ+oAzXbvQzka/HxD7zFLCbrKV mDecCwoffPE74/1B35IJh91ApO56OapgUpO3sjvVCOXujbGmexYMFJEHgIvMD9HYSab2 PY1EQGmwFQzVZ0yCRYGNafWZHeugdVnG9wlXWZgZc3mDVIBC0UB2enyGCB4Ws0NLvXot N/PA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-beenthere:x-forwarded-to:x-forwarded-for:delivered-to :mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:x-gm-message-state:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :x-google-group-id:list-post:list-help:list-archive:list-unsubscribe :content-type; bh=EdTIjfEHHvT/920uEi3g09C/5w1z+2LsbaB0JfpwwbI=; b=LDPEl1Du6el1AkVXS1bL0zfK3EgH9CYk+jXvzmrtzH8Rp9UF3iDEkCub+FPMHarZAD C9rV8XQ/mhRGmM63PRLMGaj9MPgCY0GRfDpmkmiLvI1REur7161KQNioPtKgIa4kNZW+ a3CJAQsc1ulkabS6iXzrpdkQDuRCdQVWJg05f9LmaEdeMjWgNzMkzZu6nWbX5+M87RPE xiRuJMGhh8Q0Ma/lTblkPJBjD1WfgRqwqwAA3VMCbyVwcY6ErpTdY6rnUXgUt3StbzZM IRNhTPn5SJFvANToVHoIoXOsyL/1dytqx4oHQo/qQGxmL5mE3R+WK/WGCoqxMT7vU2s/ jVlA== X-Received: by 10.224.59.205 with SMTP id m13mr925932qah.7.1371106929438; Thu, 13 Jun 2013 00:02:09 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.4.138 with SMTP id k10ls202399qek.15.gmail; Thu, 13 Jun 2013 00:02:09 -0700 (PDT) X-Received: by 10.58.173.36 with SMTP id bh4mr11099178vec.9.1371106929241; Thu, 13 Jun 2013 00:02:09 -0700 (PDT) Received: from mail-vc0-f176.google.com (mail-vc0-f176.google.com [209.85.220.176]) by mx.google.com with ESMTPS id xb6si9948963vdb.133.2013.06.13.00.02.09 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 13 Jun 2013 00:02:09 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.176 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.176; Received: by mail-vc0-f176.google.com with SMTP id ha12so3408647vcb.35 for ; Thu, 13 Jun 2013 00:02:09 -0700 (PDT) X-Received: by 10.52.170.148 with SMTP id am20mr9385495vdc.75.1371106929083; Thu, 13 Jun 2013 00:02:09 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.58.191.99 with SMTP id gx3csp10155vec; Thu, 13 Jun 2013 00:02:08 -0700 (PDT) X-Received: by 10.60.138.134 with SMTP id qq6mr15805956oeb.41.1371106927658; Thu, 13 Jun 2013 00:02:07 -0700 (PDT) Received: from mail-ie0-x22c.google.com (mail-ie0-x22c.google.com [2607:f8b0:4001:c03::22c]) by mx.google.com with ESMTPS id u6si18976910oes.38.2013.06.13.00.02.05 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 13 Jun 2013 00:02:07 -0700 (PDT) Received-SPF: pass (google.com: domain of yadi.brar01@gmail.com designates 2607:f8b0:4001:c03::22c as permitted sender) client-ip=2607:f8b0:4001:c03::22c; Received: by mail-ie0-f172.google.com with SMTP id 16so3726751iea.31 for ; Thu, 13 Jun 2013 00:02:05 -0700 (PDT) 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 X-Gm-Message-State: ALoCoQmzOEEJ/gSuttYBnMDtoTak2hW0zYEteqqVVc8wkNhuCUzUfsZ0GnvBP0jn/zlpX03jbzmM X-Original-Sender: yadi.brar01@gmail.com X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.220.176 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@gmail.com Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , 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<-------------------------------------------------------------------------- --- 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