From patchwork Wed May 15 13:58:04 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Machek X-Patchwork-Id: 2572711 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) by patchwork2.kernel.org (Postfix) with ESMTP id 3A0FFDF2A2 for ; Wed, 15 May 2013 13:59:42 +0000 (UTC) Received: from merlin.infradead.org ([205.233.59.134]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UccEL-0002uL-Nx; Wed, 15 May 2013 13:58:58 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UccDv-0001CL-Rl; Wed, 15 May 2013 13:58:31 +0000 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UccDr-0001Bu-HA for linux-arm-kernel@lists.infradead.org; Wed, 15 May 2013 13:58:29 +0000 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 512) id 6CF828181D; Wed, 15 May 2013 15:58:06 +0200 (CEST) Date: Wed, 15 May 2013 15:58:04 +0200 From: Pavel Machek To: dinguyen@altera.com Subject: Re: [PATCH 3/5] ARM: socfpga: Add support to gate peripheral clocks Message-ID: <20130515135804.GE9066@amd.pavel.ucw.cz> References: <1368571955-6652-1-git-send-email-dinguyen@altera.com> <1368571955-6652-3-git-send-email-dinguyen@altera.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1368571955-6652-3-git-send-email-dinguyen@altera.com> User-Agent: Mutt/1.5.20 (2009-06-14) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130515_095827_787235_23BC4D61 X-CRM114-Status: GOOD ( 25.40 ) X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [195.113.26.193 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Mike Turquette , wd@denx.de, Arnd Bergmann , linux@arm.linux.org.uk, Olof Johansson , dinh.linux@gmail.com, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Hi! > Add support to gate the clocks that directly feed peripherals. For clocks > with multiple parents, add the ability to determine the correct parent, > and also set parents. Also add support to calculate and set the clocks' > rate. > > Signed-off-by: Dinh Nguyen > CC: Mike Turquette > CC: Arnd Bergmann > CC: Olof Johansson > CC: Pavel Machek > CC: > @@ -132,8 +147,9 @@ static __init struct clk *socfpga_clk_init(struct device_node *node, > > socfpga_clk->hw.hw.init = &init; > > - if (strcmp(clk_name, "main_pll") || strcmp(clk_name, "periph_pll") || > - strcmp(clk_name, "sdram_pll")) { > + if (strcmp(clk_name, "main_pll") == 0 || > + strcmp(clk_name, "periph_pll") == 0 || > + strcmp(clk_name, "sdram_pll") == 0) { > socfpga_clk->hw.bit_idx = SOCFPGA_PLL_EXT_ENA; > clk_pll_ops.enable = clk_gate_ops.enable; > clk_pll_ops.disable = clk_gate_ops.disable; Perhaps do #define streq(a, b) (strcmp((a), (b)) == 0) and use that to simplify the conditions? Actually, here are suggested cleanups. Note that there are two FIXMEs -- I could not understand why the code is safe, and that the BUG_ON()s are commented on. AFAICT I got them correct, but for some reason they seem to trigger. Thanks, Pavel diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c index a0551f2..ddb340d 100644 --- a/drivers/clk/socfpga/clk.c +++ b/drivers/clk/socfpga/clk.c @@ -24,17 +24,17 @@ #include /* Clock Manager offsets */ -#define CLKMGR_CTRL 0x0 -#define CLKMGR_BYPASS 0x4 +#define CLKMGR_CTRL 0x0 +#define CLKMGR_BYPASS 0x4 #define CLKMGR_L4SRC 0x70 #define CLKMGR_PERPLL_SRC 0xAC /* Clock bypass bits */ -#define MAINPLL_BYPASS (1<<0) -#define SDRAMPLL_BYPASS (1<<1) -#define SDRAMPLL_SRC_BYPASS (1<<2) -#define PERPLL_BYPASS (1<<3) -#define PERPLL_SRC_BYPASS (1<<4) +#define MAINPLL_BYPASS (1<<0) +#define SDRAMPLL_BYPASS (1<<1) +#define SDRAMPLL_SRC_BYPASS (1<<2) +#define PERPLL_BYPASS (1<<3) +#define PERPLL_SRC_BYPASS (1<<4) #define SOCFPGA_PLL_BG_PWRDWN 0 #define SOCFPGA_PLL_EXT_ENA 1 @@ -45,14 +45,16 @@ #define SOCFPGA_PLL_DIVQ_SHIFT 16 #define SOCFGPA_MAX_PARENTS 3 -#define SOCFPGA_L4_MP_CLK "l4_mp_clk" -#define SOCFPGA_L4_SP_CLK "l4_sp_clk" +#define SOCFPGA_L4_MP_CLK "l4_mp_clk" +#define SOCFPGA_L4_SP_CLK "l4_sp_clk" #define SOCFPGA_NAND_CLK "nand_clk" #define SOCFPGA_NAND_X_CLK "nand_x_clk" #define SOCFPGA_MMC_CLK "mmc_clk" #define SOCFPGA_DB_CLK "gpio_db_clk" +#define SOCFPGA_QSPI_CLK "qspi_clk" #define div_mask(width) ((1 << (width)) - 1) +#define streq(a, b) (strcmp((a), (b)) == 0) extern void __iomem *clk_mgr_base_addr; @@ -62,8 +64,8 @@ struct socfpga_clk { char *clk_name; u32 fixed_div; void __iomem *div_reg; - u32 width; - u32 shift; + u32 width; /* only valid if div_reg != 0 */ + u32 shift; /* only valid if div_reg != 0 */ }; #define to_socfpga_clk(p) container_of(p, struct socfpga_clk, hw.hw) @@ -145,11 +147,12 @@ static __init struct clk *socfpga_clk_init(struct device_node *node, init.parent_names = &parent_name; init.num_parents = 1; + // FIXME: assigning automatic variable ptr into malloced structure? socfpga_clk->hw.hw.init = &init; - if (strcmp(clk_name, "main_pll") == 0 || - strcmp(clk_name, "periph_pll") == 0 || - strcmp(clk_name, "sdram_pll") == 0) { + if (streq(clk_name, "main_pll") || + streq(clk_name, "periph_pll") || + streq(clk_name, "sdram_pll")) { socfpga_clk->hw.bit_idx = SOCFPGA_PLL_EXT_ENA; clk_pll_ops.enable = clk_gate_ops.enable; clk_pll_ops.disable = clk_gate_ops.disable; @@ -168,61 +171,57 @@ static u8 socfpga_clk_get_parent(struct clk_hw *hwclk) { u32 l4_src; u32 perpll_src; - u8 parent; - if (strcmp(hwclk->init->name, SOCFPGA_L4_MP_CLK) == 0) { + if (streq(hwclk->init->name, SOCFPGA_L4_MP_CLK)) { l4_src = readl(clk_mgr_base_addr + CLKMGR_L4SRC); - l4_src &= 0x1; - parent = l4_src; - } else if (strcmp(hwclk->init->name, SOCFPGA_L4_SP_CLK) == 0) { + return l4_src & 1; + } + if (streq(hwclk->init->name, SOCFPGA_L4_SP_CLK)) { l4_src = readl(clk_mgr_base_addr + CLKMGR_L4SRC); - l4_src = ((l4_src & 0x2) >> 1); - parent = l4_src; - } else { - perpll_src = readl(clk_mgr_base_addr + CLKMGR_PERPLL_SRC); - if (strcmp(hwclk->init->name, SOCFPGA_MMC_CLK) == 0) - perpll_src &= 0x3; - else if (strcmp(hwclk->init->name, SOCFPGA_NAND_CLK) == 0 || - strcmp(hwclk->init->name, SOCFPGA_NAND_X_CLK) == 0) - perpll_src = ((perpll_src & 0xC) >> 2); - else /*QSPI clock */ - perpll_src = ((perpll_src & 0x30) >> 4); - parent = perpll_src; + return !!(l4_src & 2); } + perpll_src = readl(clk_mgr_base_addr + CLKMGR_PERPLL_SRC); + if (streq(hwclk->init->name, SOCFPGA_MMC_CLK)) + return perpll_src & 3; + if (streq(hwclk->init->name, SOCFPGA_NAND_CLK) || + streq(hwclk->init->name, SOCFPGA_NAND_X_CLK)) + return (perpll_src >> 2) & 3; + + /* else QSPI clock */ +// BUG_ON(!streq(hwclk->init->name, SOCFPGA_QSPI_CLK)); + return (perpll_src >> 4) & 3; +} - return parent; +static int clrsetbits(u32 adr, u32 clr, u32 set) +{ + u32 reg; + + reg = readl(adr); + reg &= ~clr; + reg |= set; + writel(reg, adr); + return 0; } static int socfpga_clk_set_parent(struct clk_hw *hwclk, u8 parent) { - u32 src_reg; - - if (strcmp(hwclk->init->name, SOCFPGA_L4_MP_CLK) == 0) { - src_reg = readl(clk_mgr_base_addr + CLKMGR_L4SRC); - src_reg &= ~0x1; - src_reg |= parent; - writel(src_reg, clk_mgr_base_addr + CLKMGR_L4SRC); - } else if (strcmp(hwclk->init->name, SOCFPGA_L4_SP_CLK) == 0) { - src_reg = readl(clk_mgr_base_addr + CLKMGR_L4SRC); - src_reg &= ~0x2; - src_reg |= (parent << 1); - writel(src_reg, clk_mgr_base_addr + CLKMGR_L4SRC); - } else { - src_reg = readl(clk_mgr_base_addr + CLKMGR_PERPLL_SRC); - if (strcmp(hwclk->init->name, SOCFPGA_MMC_CLK) == 0) { - src_reg &= ~0x3; - src_reg |= parent; - } else if (strcmp(hwclk->init->name, SOCFPGA_NAND_CLK) == 0 || - strcmp(hwclk->init->name, SOCFPGA_NAND_X_CLK) == 0) { - src_reg &= ~0xC; - src_reg |= (parent << 2); - } else {/*QSPI clock */ - src_reg &= ~0x30; - src_reg |= (parent << 4); - } - writel(src_reg, clk_mgr_base_addr + CLKMGR_PERPLL_SRC); - } +// BUG_ON(parent & ~3); + + if (streq(hwclk->init->name, SOCFPGA_L4_MP_CLK)) + return clrsetbits(clk_mgr_base_addr + CLKMGR_L4SRC, 1, parent); + if (streq(hwclk->init->name, SOCFPGA_L4_SP_CLK)) + return clrsetbits(clk_mgr_base_addr + CLKMGR_L4SRC, 2, parent << 1); + if (streq(hwclk->init->name, SOCFPGA_MMC_CLK)) + return clrsetbits(clk_mgr_base_addr + CLKMGR_PERPLL_SRC, 3, parent); + if (streq(hwclk->init->name, SOCFPGA_NAND_CLK) || + streq(hwclk->init->name, SOCFPGA_NAND_X_CLK)) + return clrsetbits(clk_mgr_base_addr + CLKMGR_PERPLL_SRC, 0xC, parent << 2); + + /* else QSPI clock */ +// BUG_ON(!streq(hwclk->init->name, SOCFPGA_QSPI_CLK)); + + clrsetbits(clk_mgr_base_addr + CLKMGR_PERPLL_SRC, 0x30, parent << 4); return 0; } @@ -237,7 +236,7 @@ static unsigned long socfpga_clk_recalc_rate(struct clk_hw *hwclk, else if (socfpgaclk->div_reg) { val = readl(socfpgaclk->div_reg) >> socfpgaclk->shift; val &= div_mask(socfpgaclk->width); - if (strcmp(hwclk->init->name, SOCFPGA_DB_CLK) == 0) + if (streq(hwclk->init->name, SOCFPGA_DB_CLK)) div = val + 1; else div = (1 << val); @@ -273,13 +272,6 @@ static void __init socfpga_gate_clk_init(struct device_node *node, rc = of_property_read_u32_array(node, "clk-gate", clk_gate, 2); if (rc) clk_gate[0] = 0; - - rc = of_property_read_u32(node, "fixed-divider", &fixed_div); - if (rc) - socfpga_clk->fixed_div = 0; - else - socfpga_clk->fixed_div = fixed_div; - if (clk_gate[0]) { socfpga_clk->hw.reg = clk_mgr_base_addr + clk_gate[0]; socfpga_clk->hw.bit_idx = clk_gate[1]; @@ -288,6 +280,12 @@ static void __init socfpga_gate_clk_init(struct device_node *node, gateclk_ops.disable = clk_gate_ops.disable; } + rc = of_property_read_u32(node, "fixed-divider", &fixed_div); + if (rc) + socfpga_clk->fixed_div = 0; + else + socfpga_clk->fixed_div = fixed_div; + rc = of_property_read_u32_array(node, "div-reg", div_reg, 3); if (!rc) { socfpga_clk->div_reg = clk_mgr_base_addr + div_reg[0]; @@ -302,12 +300,17 @@ static void __init socfpga_gate_clk_init(struct device_node *node, init.name = clk_name; init.ops = ops; init.flags = 0; - while (i < SOCFGPA_MAX_PARENTS && (parent_name[i] = - of_clk_get_parent_name(node, i)) != NULL) + while (i < SOCFGPA_MAX_PARENTS) { + char *name = of_clk_get_parent_name(node, i); + parent_name[i] = name; + if (name == NULL) + break; i++; + } init.parent_names = parent_name; init.num_parents = i; + // FIXME: assigning automatic variable ptr into malloced structure? socfpga_clk->hw.hw.init = &init; clk = clk_register(NULL, &socfpga_clk->hw.hw);