* [PATCHv2 1/2] ARM: socfpga: Enable soft reset @ 2013-03-19 15:45 dinguyen at altera.com 2013-03-19 15:45 ` [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree dinguyen at altera.com 2013-03-20 15:29 ` [PATCHv2 1/2] ARM: socfpga: Enable soft reset Pavel Machek 0 siblings, 2 replies; 15+ messages in thread From: dinguyen at altera.com @ 2013-03-19 15:45 UTC (permalink / raw) To: linux-arm-kernel From: Dinh Nguyen <dinguyen@altera.com> Enable a cold or warm reset to the HW from userspace. Also fix a few sparse errors: warning: symbol 'sys_manager_base_addr' was not declared. Should it be static? warning: symbol 'rst_manager_base_addr' was not declared. Should it be static? Signed-off-by: Dinh Nguyen <dinguyen@altera.com> v2: - Remove hotplug support while investigating on how implement PSCI --- arch/arm/mach-socfpga/core.h | 11 +++++++++++ arch/arm/mach-socfpga/platsmp.c | 3 --- arch/arm/mach-socfpga/socfpga.c | 10 +++++++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h index 315edff..572b8f7 100644 --- a/arch/arm/mach-socfpga/core.h +++ b/arch/arm/mach-socfpga/core.h @@ -20,12 +20,23 @@ #ifndef __MACH_CORE_H #define __MACH_CORE_H +#define SOCFPGA_RSTMGR_CTRL 0x04 +#define SOCFPGA_RSTMGR_MODPERRST 0x14 +#define SOCFPGA_RSTMGR_BRGMODRST 0x1c + +/* System Manager bits */ +#define RSTMGR_CTRL_SWCOLDRSTREQ 0x1 /* Cold Reset */ +#define RSTMGR_CTRL_SWWARMRSTREQ 0x2 /* Warm Reset */ + extern void socfpga_secondary_startup(void); extern void __iomem *socfpga_scu_base_addr; extern void socfpga_init_clocks(void); extern void socfpga_sysmgr_init(void); +extern void __iomem *sys_manager_base_addr; +extern void __iomem *rst_manager_base_addr; + extern struct smp_operations socfpga_smp_ops; extern char secondary_trampoline, secondary_trampoline_end; diff --git a/arch/arm/mach-socfpga/platsmp.c b/arch/arm/mach-socfpga/platsmp.c index 84c60fa..b907fb9 100644 --- a/arch/arm/mach-socfpga/platsmp.c +++ b/arch/arm/mach-socfpga/platsmp.c @@ -30,9 +30,6 @@ #include "core.h" -extern void __iomem *sys_manager_base_addr; -extern void __iomem *rst_manager_base_addr; - static void __cpuinit socfpga_secondary_init(unsigned int cpu) { /* diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c index 1042c02..b41a945 100644 --- a/arch/arm/mach-socfpga/socfpga.c +++ b/arch/arm/mach-socfpga/socfpga.c @@ -87,7 +87,15 @@ static void __init socfpga_init_irq(void) static void socfpga_cyclone5_restart(char mode, const char *cmd) { - /* TODO: */ + u32 temp; + + temp = __raw_readl(rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL); + + if (mode == 'h') + temp |= RSTMGR_CTRL_SWCOLDRSTREQ; + else + temp |= RSTMGR_CTRL_SWWARMRSTREQ; + __raw_writel(temp, rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL); } static void __init socfpga_cyclone5_init(void) -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree 2013-03-19 15:45 [PATCHv2 1/2] ARM: socfpga: Enable soft reset dinguyen at altera.com @ 2013-03-19 15:45 ` dinguyen at altera.com 2013-03-19 16:46 ` Mike Turquette 2013-03-20 13:46 ` Pavel Machek 2013-03-20 15:29 ` [PATCHv2 1/2] ARM: socfpga: Enable soft reset Pavel Machek 1 sibling, 2 replies; 15+ messages in thread From: dinguyen at altera.com @ 2013-03-19 15:45 UTC (permalink / raw) To: linux-arm-kernel From: Dinh Nguyen <dinguyen@altera.com> Adds the main PLL clock groups for SOCFPGA into device tree file so that the clock framework to query the clock and clock rates appropriately. $cat /sys/kernel/debug/clk/clk_summary clock enable_cnt prepare_cnt rate --------------------------------------------------------------------- osc1 2 2 25000000 sdram_pll 0 0 400000000 s2f_usr2_clk 0 0 66666666 ddr_dq_clk 0 0 200000000 ddr_2x_dqs_clk 0 0 400000000 ddr_dqs_clk 0 0 200000000 periph_pll 2 2 500000000 s2f_usr1_clk 0 0 50000000 per_base_clk 4 4 100000000 per_nand_mmc_clk 0 0 25000000 per_qsi_clk 0 0 250000000 emac1_clk 1 1 125000000 emac0_clk 0 0 125000000 main_pll 1 1 1600000000 cfg_s2f_usr0_clk 0 0 100000000 main_nand_sdmmc_clk 0 0 100000000 main_qspi_clk 0 0 400000000 dbg_base_clk 0 0 400000000 mainclk 0 0 400000000 mpuclk 1 1 800000000 smp_twd 1 1 200000000 Signed-off-by: Dinh Nguyen <dinguyen@altera.com> v2: - Add copyright from Calxeda for clk.c - Add device tree "fixed-divider" for clocks with fixed dividers - Fix space and tab issues --- .../bindings/arm/altera/socfpga-clk-manager.txt | 11 ++ .../devicetree/bindings/clock/altr_socfpga.txt | 18 ++ arch/arm/boot/dts/socfpga.dtsi | 157 +++++++++++++++++ arch/arm/boot/dts/socfpga_cyclone5.dts | 8 + arch/arm/boot/dts/socfpga_vt.dts | 8 + arch/arm/mach-socfpga/socfpga.c | 6 + drivers/clk/socfpga/clk.c | 179 +++++++++++++++++--- 7 files changed, 366 insertions(+), 21 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.txt create mode 100644 Documentation/devicetree/bindings/clock/altr_socfpga.txt diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.txt new file mode 100644 index 0000000..2c28f1d --- /dev/null +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.txt @@ -0,0 +1,11 @@ +Altera SOCFPGA Clock Manager + +Required properties: +- compatible : "altr,clk-mgr" +- reg : Should contain base address and length for Clock Manager + +Example: + clkmgr at ffd04000 { + compatible = "altr,clk-mgr"; + reg = <0xffd04000 0x1000>; + }; diff --git a/Documentation/devicetree/bindings/clock/altr_socfpga.txt b/Documentation/devicetree/bindings/clock/altr_socfpga.txt new file mode 100644 index 0000000..bd0c841 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/altr_socfpga.txt @@ -0,0 +1,18 @@ +Device Tree Clock bindings for Altera's SoCFPGA platform + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of the following: + "altr,socfpga-pll-clock" - for a PLL clock + "altr,socfpga-perip-clock" - The peripheral clock divided from the + PLL clock. +- reg : shall be the control register offset from CLOCK_MANAGER's base for the clock. +- clocks : shall be the input parent clock phandle for the clock. This is + either an oscillator or a pll output. +- #clock-cells : from common clock binding, shall be set to 0. + +Optional properties: +- fixed-divider : If clocks have a fixed divider value, use this property. diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi index 7e8769b..16a6e13 100644 --- a/arch/arm/boot/dts/socfpga.dtsi +++ b/arch/arm/boot/dts/socfpga.dtsi @@ -81,6 +81,163 @@ }; }; + clkmgr at ffd04000 { + compatible = "altr,clk-mgr"; + reg = <0xffd04000 0x1000>; + + clocks { + #address-cells = <1>; + #size-cells = <0>; + + osc: osc1 { + #clock-cells = <0>; + compatible = "fixed-clock"; + }; + + main_pll: main_pll { + #address-cells = <1>; + #size-cells = <0>; + #clock-cells = <0>; + compatible = "altr,socfpga-pll-clock"; + clocks = <&osc>; + reg = <0x40>; + + mpuclk: mpuclk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&main_pll>; + fixed-divider = <2>; + reg = <0x48>; + }; + + mainclk: mainclk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&main_pll>; + fixed-divider = <4>; + reg = <0x4C>; + }; + + dbg_base_clk: dbg_base_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&main_pll>; + fixed-divider = <4>; + reg = <0x50>; + }; + + main_qspi_clk: main_qspi_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&main_pll>; + reg = <0x54>; + }; + + main_nand_sdmmc_clk: main_nand_sdmmc_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&main_pll>; + reg = <0x58>; + }; + + cfg_s2f_usr0_clk: cfg_s2f_usr0_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&main_pll>; + reg = <0x5C>; + }; + }; + + periph_pll: periph_pll { + #address-cells = <1>; + #size-cells = <0>; + #clock-cells = <0>; + compatible = "altr,socfpga-pll-clock"; + clocks = <&osc>; + reg = <0x80>; + + emac0_clk: emac0_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&periph_pll>; + reg = <0x88>; + }; + + emac1_clk: emac1_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&periph_pll>; + reg = <0x8C>; + }; + + per_qspi_clk: per_qsi_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&periph_pll>; + reg = <0x90>; + }; + + per_nand_mmc_clk: per_nand_mmc_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&periph_pll>; + reg = <0x94>; + }; + + per_base_clk: per_base_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&periph_pll>; + reg = <0x98>; + }; + + s2f_usr1_clk: s2f_usr1_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&periph_pll>; + reg = <0x9C>; + }; + }; + + sdram_pll: sdram_pll { + #address-cells = <1>; + #size-cells = <0>; + #clock-cells = <0>; + compatible = "altr,socfpga-pll-clock"; + clocks = <&osc>; + reg = <0xC0>; + + ddr_dqs_clk: ddr_dqs_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&sdram_pll>; + reg = <0xC8>; + }; + + ddr_2x_dqs_clk: ddr_2x_dqs_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&sdram_pll>; + reg = <0xCC>; + }; + + ddr_dq_clk: ddr_dq_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&sdram_pll>; + reg = <0xD0>; + }; + + s2f_usr2_clk: s2f_usr2_clk { + #clock-cells = <0>; + compatible = "altr,socfpga-perip-clk"; + clocks = <&sdram_pll>; + reg = <0xD4>; + }; + }; + }; + }; + gmac0: stmmac at ff700000 { compatible = "altr,socfpga-stmmac", "snps,dwmac-3.70a", "snps,dwmac"; reg = <0xff700000 0x2000>; diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dts b/arch/arm/boot/dts/socfpga_cyclone5.dts index 3ae8a83..2495958 100644 --- a/arch/arm/boot/dts/socfpga_cyclone5.dts +++ b/arch/arm/boot/dts/socfpga_cyclone5.dts @@ -33,6 +33,14 @@ }; soc { + clkmgr at ffd04000 { + clocks { + osc1 { + clock-frequency = <25000000>; + }; + }; + }; + timer0 at ffc08000 { clock-frequency = <100000000>; }; diff --git a/arch/arm/boot/dts/socfpga_vt.dts b/arch/arm/boot/dts/socfpga_vt.dts index 1036eba..0bf035d 100644 --- a/arch/arm/boot/dts/socfpga_vt.dts +++ b/arch/arm/boot/dts/socfpga_vt.dts @@ -33,6 +33,14 @@ }; soc { + clkmgr at ffd04000 { + clocks { + osc1 { + clock-frequency = <10000000>; + }; + }; + }; + timer0 at ffc08000 { clock-frequency = <7000000>; }; diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c index b41a945..856625a 100644 --- a/arch/arm/mach-socfpga/socfpga.c +++ b/arch/arm/mach-socfpga/socfpga.c @@ -15,6 +15,7 @@ * along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include <linux/dw_apb_timer.h> +#include <linux/clk-provider.h> #include <linux/irqchip.h> #include <linux/of_address.h> #include <linux/of_irq.h> @@ -29,6 +30,7 @@ void __iomem *socfpga_scu_base_addr = ((void __iomem *)(SOCFPGA_SCU_VIRT_BASE)); void __iomem *sys_manager_base_addr; void __iomem *rst_manager_base_addr; +void __iomem *clk_mgr_base_addr; unsigned long cpu1start_addr; static struct map_desc scu_io_desc __initdata = { @@ -77,6 +79,9 @@ void __init socfpga_sysmgr_init(void) np = of_find_compatible_node(NULL, NULL, "altr,rst-mgr"); rst_manager_base_addr = of_iomap(np, 0); + + np = of_find_compatible_node(NULL, NULL, "altr,clk-mgr"); + clk_mgr_base_addr = of_iomap(np, 0); } static void __init socfpga_init_irq(void) @@ -102,6 +107,7 @@ static void __init socfpga_cyclone5_init(void) { l2x0_of_init(0, ~0UL); of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); + of_clk_init(NULL); socfpga_init_clocks(); } diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c index 2c855a6..da6b461 100644 --- a/drivers/clk/socfpga/clk.c +++ b/drivers/clk/socfpga/clk.c @@ -1,5 +1,6 @@ /* - * Copyright (C) 2012 Altera Corporation <www.altera.com> + * Copyright 2011-2012 Calxeda, Inc. + * Copyright (C) 2012-2013 Altera Corporation <www.altera.com> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -11,41 +12,177 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * + * Based from clk-highbank.c + * * You should have received a copy of the GNU General Public License * along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include <linux/clk.h> #include <linux/clkdev.h> #include <linux/clk-provider.h> +#include <linux/io.h> +#include <linux/of.h> -#define SOCFPGA_OSC1_CLK 10000000 -#define SOCFPGA_MPU_CLK 800000000 -#define SOCFPGA_MAIN_QSPI_CLK 432000000 -#define SOCFPGA_MAIN_NAND_SDMMC_CLK 250000000 -#define SOCFPGA_S2F_USR_CLK 125000000 +/* Clock Manager offsets */ +#define CLKMGR_CTRL 0x0 +#define CLKMGR_BYPASS 0x4 -void __init socfpga_init_clocks(void) +/* 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 SOCFPGA_PLL_BG_PWRDWN 0x00000001 +#define SOCFPGA_PLL_EXT_ENA 0x00000002 +#define SOCFPGA_PLL_PWR_DOWN 0x00000004 +#define SOCFPGA_PLL_DIVF_MASK 0x0000FFF8 +#define SOCFPGA_PLL_DIVF_SHIFT 3 +#define SOCFPGA_PLL_DIVQ_MASK 0x003F0000 +#define SOCFPGA_PLL_DIVQ_SHIFT 15 + +extern void __iomem *clk_mgr_base_addr; + +struct socfpga_clk { + struct clk_hw hw; + void __iomem *reg; + char *parent_name; + char *clk_name; +}; +#define to_socfpga_clk(p) container_of(p, struct socfpga_clk, hw) + +static int clk_pll_enable(struct clk_hw *hwclk) +{ + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); + u32 reg; + + reg = readl(socfpgaclk->reg); + reg |= SOCFPGA_PLL_EXT_ENA; + writel(reg, socfpgaclk->reg); + + return 0; +} + +static void clk_pll_disable(struct clk_hw *hwclk) { + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); + u32 reg; + + reg = readl(socfpgaclk->reg); + reg &= ~SOCFPGA_PLL_EXT_ENA; + writel(reg, socfpgaclk->reg); +} + +static unsigned long clk_pll_recalc_rate(struct clk_hw *hwclk, + unsigned long parent_rate) +{ + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); + unsigned long divf, divq, vco_freq, reg; + unsigned long bypass; + + reg = readl(socfpgaclk->reg); + bypass = readl(clk_mgr_base_addr + CLKMGR_BYPASS); + if (bypass & MAINPLL_BYPASS) + return parent_rate; + + divf = (reg & SOCFPGA_PLL_DIVF_MASK) >> SOCFPGA_PLL_DIVF_SHIFT; + divq = (reg & SOCFPGA_PLL_DIVQ_MASK) >> SOCFPGA_PLL_DIVQ_SHIFT; + vco_freq = parent_rate * (divf + 1); + return vco_freq / (1 << divq); +} + + +static const struct clk_ops clk_pll_ops = { + .enable = clk_pll_enable, + .disable = clk_pll_disable, + .recalc_rate = clk_pll_recalc_rate, +}; + +static unsigned long clk_periclk_recalc_rate(struct clk_hw *hwclk, + unsigned long parent_rate) +{ + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); + u32 div; + + if (socfpgaclk->fixed_div) + div = socfpgaclk->fixed_div; + else + div = ((readl(socfpgaclk->reg) & 0x1ff) + 1); + + return parent_rate / div; +} + +static const struct clk_ops periclk_ops = { + .recalc_rate = clk_periclk_recalc_rate, +}; + +static __init struct clk *socfpga_clk_init(struct device_node *node, const struct clk_ops *ops) +{ + u32 reg; struct clk *clk; + struct socfpga_clk *socfpga_clk; + const char *clk_name = node->name; + const char *parent_name; + struct clk_init_data init; + int rc; + u32 fixed_div; + + rc = of_property_read_u32(node, "reg", ®); + if (WARN_ON(rc)) + return NULL; + + socfpga_clk = kzalloc(sizeof(*socfpga_clk), GFP_KERNEL); + if (WARN_ON(!socfpga_clk)) + return NULL; + + socfpga_clk->reg = clk_mgr_base_addr + reg; - clk = clk_register_fixed_rate(NULL, "osc1_clk", NULL, CLK_IS_ROOT, SOCFPGA_OSC1_CLK); - clk_register_clkdev(clk, "osc1_clk", NULL); + rc = of_property_read_u32(node, "fixed-divider", &fixed_div); + if (rc) + socfpga_clk->fixed_div = 0; + else + socfpga_clk->fixed_div = fixed_div; - clk = clk_register_fixed_rate(NULL, "mpu_clk", NULL, CLK_IS_ROOT, SOCFPGA_MPU_CLK); - clk_register_clkdev(clk, "mpu_clk", NULL); + of_property_read_string(node, "clock-output-names", &clk_name); - clk = clk_register_fixed_rate(NULL, "main_clk", NULL, CLK_IS_ROOT, SOCFPGA_MPU_CLK/2); - clk_register_clkdev(clk, "main_clk", NULL); + init.name = clk_name; + init.ops = ops; + init.flags = 0; + parent_name = of_clk_get_parent_name(node, 0); + init.parent_names = &parent_name; + init.num_parents = 1; - clk = clk_register_fixed_rate(NULL, "dbg_base_clk", NULL, CLK_IS_ROOT, SOCFPGA_MPU_CLK/2); - clk_register_clkdev(clk, "dbg_base_clk", NULL); + socfpga_clk->hw.init = &init; - clk = clk_register_fixed_rate(NULL, "main_qspi_clk", NULL, CLK_IS_ROOT, SOCFPGA_MAIN_QSPI_CLK); - clk_register_clkdev(clk, "main_qspi_clk", NULL); + clk = clk_register(NULL, &socfpga_clk->hw); + if (WARN_ON(IS_ERR(clk))) { + kfree(socfpga_clk); + return NULL; + } + rc = of_clk_add_provider(node, of_clk_src_simple_get, clk); + return clk; +} + +static void __init socfpga_pll_init(struct device_node *node) +{ + socfpga_clk_init(node, &clk_pll_ops); +} +CLK_OF_DECLARE(socfpga_pll, "altr,socfpga-pll-clock", socfpga_pll_init); + +static void __init socfpga_periph_init(struct device_node *node) +{ + socfpga_clk_init(node, &periclk_ops); +} +CLK_OF_DECLARE(socfpga_periph, "altr,socfpga-perip-clk", socfpga_periph_init); - clk = clk_register_fixed_rate(NULL, "main_nand_sdmmc_clk", NULL, CLK_IS_ROOT, SOCFPGA_MAIN_NAND_SDMMC_CLK); - clk_register_clkdev(clk, "main_nand_sdmmc_clk", NULL); +void __init socfpga_init_clocks(void) +{ + struct clk *clk; + int ret; - clk = clk_register_fixed_rate(NULL, "s2f_usr_clk", NULL, CLK_IS_ROOT, SOCFPGA_S2F_USR_CLK); - clk_register_clkdev(clk, "s2f_usr_clk", NULL); + clk = clk_register_fixed_factor(NULL, "smp_twd", "mpuclk", 0, 1, 4); + ret = clk_register_clkdev(clk, NULL, "smp_twd"); + if (ret) + pr_err("smp_twd alias not registered\n"); } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree 2013-03-19 15:45 ` [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree dinguyen at altera.com @ 2013-03-19 16:46 ` Mike Turquette 2013-03-19 18:45 ` Dinh Nguyen 2013-03-20 13:46 ` Pavel Machek 1 sibling, 1 reply; 15+ messages in thread From: Mike Turquette @ 2013-03-19 16:46 UTC (permalink / raw) To: linux-arm-kernel Quoting dinguyen at altera.com (2013-03-19 08:45:36) > From: Dinh Nguyen <dinguyen@altera.com> > > Adds the main PLL clock groups for SOCFPGA into device tree file > so that the clock framework to query the clock and clock rates > appropriately. > > $cat /sys/kernel/debug/clk/clk_summary > clock enable_cnt prepare_cnt rate > --------------------------------------------------------------------- > osc1 2 2 25000000 > sdram_pll 0 0 400000000 > s2f_usr2_clk 0 0 66666666 > ddr_dq_clk 0 0 200000000 > ddr_2x_dqs_clk 0 0 400000000 > ddr_dqs_clk 0 0 200000000 > periph_pll 2 2 500000000 > s2f_usr1_clk 0 0 50000000 > per_base_clk 4 4 100000000 > per_nand_mmc_clk 0 0 25000000 > per_qsi_clk 0 0 250000000 > emac1_clk 1 1 125000000 > emac0_clk 0 0 125000000 > main_pll 1 1 1600000000 > cfg_s2f_usr0_clk 0 0 100000000 > main_nand_sdmmc_clk 0 0 100000000 > main_qspi_clk 0 0 400000000 > dbg_base_clk 0 0 400000000 > mainclk 0 0 400000000 > mpuclk 1 1 800000000 > smp_twd 1 1 200000000 > > Signed-off-by: Dinh Nguyen <dinguyen@altera.com> There is a lot going on in this one patch. I would prefer to see the clock driver broken out separately. <snip> > diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c > index 2c855a6..da6b461 100644 > --- a/drivers/clk/socfpga/clk.c > +++ b/drivers/clk/socfpga/clk.c <snip> > +static int clk_pll_enable(struct clk_hw *hwclk) > +{ > + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); > + u32 reg; > + > + reg = readl(socfpgaclk->reg); > + reg |= SOCFPGA_PLL_EXT_ENA; > + writel(reg, socfpgaclk->reg); > + > + return 0; > +} > + > +static void clk_pll_disable(struct clk_hw *hwclk) > { > + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); > + u32 reg; > + > + reg = readl(socfpgaclk->reg); > + reg &= ~SOCFPGA_PLL_EXT_ENA; > + writel(reg, socfpgaclk->reg); > +} > + For a simple enable which just sets a bit, you might want to re-use the basic gate clock type. This can be done similar to the composite clock patches (currently on the list) by stuffing a clk_gate structure into your custom socfpga_clk type. Regards, Mike ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree 2013-03-19 16:46 ` Mike Turquette @ 2013-03-19 18:45 ` Dinh Nguyen 2013-03-19 22:12 ` Mike Turquette 0 siblings, 1 reply; 15+ messages in thread From: Dinh Nguyen @ 2013-03-19 18:45 UTC (permalink / raw) To: linux-arm-kernel Hi Mike, On Tue, 2013-03-19 at 09:46 -0700, Mike Turquette wrote: > Quoting dinguyen at altera.com (2013-03-19 08:45:36) > > From: Dinh Nguyen <dinguyen@altera.com> > > > > Adds the main PLL clock groups for SOCFPGA into device tree file > > so that the clock framework to query the clock and clock rates > > appropriately. > > > > $cat /sys/kernel/debug/clk/clk_summary > > clock enable_cnt prepare_cnt rate > > --------------------------------------------------------------------- > > osc1 2 2 25000000 > > sdram_pll 0 0 400000000 > > s2f_usr2_clk 0 0 66666666 > > ddr_dq_clk 0 0 200000000 > > ddr_2x_dqs_clk 0 0 400000000 > > ddr_dqs_clk 0 0 200000000 > > periph_pll 2 2 500000000 > > s2f_usr1_clk 0 0 50000000 > > per_base_clk 4 4 100000000 > > per_nand_mmc_clk 0 0 25000000 > > per_qsi_clk 0 0 250000000 > > emac1_clk 1 1 125000000 > > emac0_clk 0 0 125000000 > > main_pll 1 1 1600000000 > > cfg_s2f_usr0_clk 0 0 100000000 > > main_nand_sdmmc_clk 0 0 100000000 > > main_qspi_clk 0 0 400000000 > > dbg_base_clk 0 0 400000000 > > mainclk 0 0 400000000 > > mpuclk 1 1 800000000 > > smp_twd 1 1 200000000 > > > > Signed-off-by: Dinh Nguyen <dinguyen@altera.com> > > There is a lot going on in this one patch. I would prefer to see the > clock driver broken out separately. Not sure what you mean by breaking out the clock driver separately. The patch is only touching the clocks for mach-socfpga. The patch is 7 files changed, 366 insertions(+), 21 deletions(-) while the patch to enable clk-highbank was: 8 files changed, 463 insertions(+), 64 deletions(-) > <snip> > > > diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c > > index 2c855a6..da6b461 100644 > > --- a/drivers/clk/socfpga/clk.c > > +++ b/drivers/clk/socfpga/clk.c > <snip> > > +static int clk_pll_enable(struct clk_hw *hwclk) > > +{ > > + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); > > + u32 reg; > > + > > + reg = readl(socfpgaclk->reg); > > + reg |= SOCFPGA_PLL_EXT_ENA; > > + writel(reg, socfpgaclk->reg); > > + > > + return 0; > > +} > > + > > +static void clk_pll_disable(struct clk_hw *hwclk) > > { > > + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); > > + u32 reg; > > + > > + reg = readl(socfpgaclk->reg); > > + reg &= ~SOCFPGA_PLL_EXT_ENA; > > + writel(reg, socfpgaclk->reg); > > +} > > + > > For a simple enable which just sets a bit, you might want to re-use the > basic gate clock type. This can be done similar to the composite clock > patches (currently on the list) by stuffing a clk_gate structure into > your custom socfpga_clk type. I'll take a look at the list about this. Thanks for the review. Dinh > > Regards, > Mike > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree 2013-03-19 18:45 ` Dinh Nguyen @ 2013-03-19 22:12 ` Mike Turquette 2013-03-20 12:24 ` Dinh Nguyen 0 siblings, 1 reply; 15+ messages in thread From: Mike Turquette @ 2013-03-19 22:12 UTC (permalink / raw) To: linux-arm-kernel Quoting Dinh Nguyen (2013-03-19 11:45:50) > Hi Mike, > > On Tue, 2013-03-19 at 09:46 -0700, Mike Turquette wrote: > > Quoting dinguyen at altera.com (2013-03-19 08:45:36) > > > From: Dinh Nguyen <dinguyen@altera.com> > > > > > > Adds the main PLL clock groups for SOCFPGA into device tree file > > > so that the clock framework to query the clock and clock rates > > > appropriately. > > > > > > $cat /sys/kernel/debug/clk/clk_summary > > > clock enable_cnt prepare_cnt rate > > > --------------------------------------------------------------------- > > > osc1 2 2 25000000 > > > sdram_pll 0 0 400000000 > > > s2f_usr2_clk 0 0 66666666 > > > ddr_dq_clk 0 0 200000000 > > > ddr_2x_dqs_clk 0 0 400000000 > > > ddr_dqs_clk 0 0 200000000 > > > periph_pll 2 2 500000000 > > > s2f_usr1_clk 0 0 50000000 > > > per_base_clk 4 4 100000000 > > > per_nand_mmc_clk 0 0 25000000 > > > per_qsi_clk 0 0 250000000 > > > emac1_clk 1 1 125000000 > > > emac0_clk 0 0 125000000 > > > main_pll 1 1 1600000000 > > > cfg_s2f_usr0_clk 0 0 100000000 > > > main_nand_sdmmc_clk 0 0 100000000 > > > main_qspi_clk 0 0 400000000 > > > dbg_base_clk 0 0 400000000 > > > mainclk 0 0 400000000 > > > mpuclk 1 1 800000000 > > > smp_twd 1 1 200000000 > > > > > > Signed-off-by: Dinh Nguyen <dinguyen@altera.com> > > > > There is a lot going on in this one patch. I would prefer to see the > > clock driver broken out separately. > > Not sure what you mean by breaking out the clock driver separately. The > patch is only touching the clocks for mach-socfpga. > I mean breaking the change to drivers/clk/socfpga/clk.c out into a separate patch. > The patch is > 7 files changed, 366 insertions(+), 21 deletions(-) > > while the patch to enable clk-highbank was: > 8 files changed, 463 insertions(+), 64 deletions(-) > That is a fair comparison. However I still prefer to see data (e.g. dts changes) separated from logic (clk.c changes). I think it makes for a cleaner git history and makes patches more readable too. Regards, Mike > > <snip> > > > > > diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c > > > index 2c855a6..da6b461 100644 > > > --- a/drivers/clk/socfpga/clk.c > > > +++ b/drivers/clk/socfpga/clk.c > > <snip> > > > +static int clk_pll_enable(struct clk_hw *hwclk) > > > +{ > > > + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); > > > + u32 reg; > > > + > > > + reg = readl(socfpgaclk->reg); > > > + reg |= SOCFPGA_PLL_EXT_ENA; > > > + writel(reg, socfpgaclk->reg); > > > + > > > + return 0; > > > +} > > > + > > > +static void clk_pll_disable(struct clk_hw *hwclk) > > > { > > > + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); > > > + u32 reg; > > > + > > > + reg = readl(socfpgaclk->reg); > > > + reg &= ~SOCFPGA_PLL_EXT_ENA; > > > + writel(reg, socfpgaclk->reg); > > > +} > > > + > > > > For a simple enable which just sets a bit, you might want to re-use the > > basic gate clock type. This can be done similar to the composite clock > > patches (currently on the list) by stuffing a clk_gate structure into > > your custom socfpga_clk type. > > I'll take a look at the list about this. > > Thanks for the review. > > Dinh > > > > Regards, > > Mike > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree 2013-03-19 22:12 ` Mike Turquette @ 2013-03-20 12:24 ` Dinh Nguyen 2013-03-20 12:31 ` Arnd Bergmann 0 siblings, 1 reply; 15+ messages in thread From: Dinh Nguyen @ 2013-03-20 12:24 UTC (permalink / raw) To: linux-arm-kernel Hi Mike, On Tue, 2013-03-19 at 15:12 -0700, Mike Turquette wrote: > Quoting Dinh Nguyen (2013-03-19 11:45:50) > > Hi Mike, > > > > On Tue, 2013-03-19 at 09:46 -0700, Mike Turquette wrote: > > > Quoting dinguyen at altera.com (2013-03-19 08:45:36) > > > > From: Dinh Nguyen <dinguyen@altera.com> > > > > > > > > Adds the main PLL clock groups for SOCFPGA into device tree file > > > > so that the clock framework to query the clock and clock rates > > > > appropriately. > > > > > <snip> > > > > > > There is a lot going on in this one patch. I would prefer to see the > > > clock driver broken out separately. > > > > Not sure what you mean by breaking out the clock driver separately. The > > patch is only touching the clocks for mach-socfpga. > > > > I mean breaking the change to drivers/clk/socfpga/clk.c out into a > separate patch. > > > The patch is > > 7 files changed, 366 insertions(+), 21 deletions(-) > > > > while the patch to enable clk-highbank was: > > 8 files changed, 463 insertions(+), 64 deletions(-) > > > > That is a fair comparison. However I still prefer to see data (e.g. dts > changes) separated from logic (clk.c changes). I think it makes for a > cleaner git history and makes patches more readable too. I agree that it makes things alot cleaner to split DTS and code into separate patches, but at the same time the code is pretty much useless without the DTS entries. I apologize if there has already been a similar discussion on the list about this. I just want to make sure that I know to split up patches in the same manner in the future? > > Regards, > Mike > > > > <snip> > > > > > > > diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c > > > > index 2c855a6..da6b461 100644 > > > > --- a/drivers/clk/socfpga/clk.c > > > > +++ b/drivers/clk/socfpga/clk.c > > > <snip> > > > > +static int clk_pll_enable(struct clk_hw *hwclk) > > > > +{ > > > > + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); > > > > + u32 reg; > > > > + > > > > + reg = readl(socfpgaclk->reg); > > > > + reg |= SOCFPGA_PLL_EXT_ENA; > > > > + writel(reg, socfpgaclk->reg); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static void clk_pll_disable(struct clk_hw *hwclk) > > > > { > > > > + struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk); > > > > + u32 reg; > > > > + > > > > + reg = readl(socfpgaclk->reg); > > > > + reg &= ~SOCFPGA_PLL_EXT_ENA; > > > > + writel(reg, socfpgaclk->reg); > > > > +} > > > > + > > > > > > For a simple enable which just sets a bit, you might want to re-use the > > > basic gate clock type. This can be done similar to the composite clock > > > patches (currently on the list) by stuffing a clk_gate structure into > > > your custom socfpga_clk type. > > > > I'll take a look at the list about this. I think mvebu/clk-gating-ctrl.c is doing the same thing you're suggesting right? Thanks, Dinh > > > > Thanks for the review. > > > > Dinh > > > > > > Regards, > > > Mike > > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree 2013-03-20 12:24 ` Dinh Nguyen @ 2013-03-20 12:31 ` Arnd Bergmann 0 siblings, 0 replies; 15+ messages in thread From: Arnd Bergmann @ 2013-03-20 12:31 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 20 March 2013, Dinh Nguyen wrote: > > That is a fair comparison. However I still prefer to see data (e.g. dts > > changes) separated from logic (clk.c changes). I think it makes for a > > cleaner git history and makes patches more readable too. > > I agree that it makes things alot cleaner to split DTS and code into > separate patches, but at the same time the code is pretty much useless > without the DTS entries. I apologize if there has already been a similar > discussion on the list about this. I just want to make sure that I know > to split up patches in the same manner in the future? In particular, it would be helpful to merge the dts changes through the arm-soc tree and the clk changes through Mike's clock tree. Ideally those two should be completely independent of one another, although you obviously need both to get the new feature. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree 2013-03-19 15:45 ` [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree dinguyen at altera.com 2013-03-19 16:46 ` Mike Turquette @ 2013-03-20 13:46 ` Pavel Machek 2013-03-20 14:00 ` Pavel Machek 1 sibling, 1 reply; 15+ messages in thread From: Pavel Machek @ 2013-03-20 13:46 UTC (permalink / raw) To: linux-arm-kernel Hi! > Adds the main PLL clock groups for SOCFPGA into device tree file > so that the clock framework to query the clock and clock rates > appropriately. Is there an easy way to test it? I was keeping modified clk.c with hardcoded clock to have working ethernet; I reverted that patch and applied 2/2, but now NFS root refuses to mount (similar symptoms to missing clk.c modifications). Also I need following patch to get it to compile: commit f85232eec5330e64984facd300ef864c53a326f5 Author: Pavel Machek <pavel@pollux.denx.de> Date: Wed Mar 20 14:41:59 2013 +0100 Patch needed to get clk.c to compile. Signed-off-by: Pavel Machek <pavel@denx.de> diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c index da6b461..3504dbf 100644 --- a/drivers/clk/socfpga/clk.c +++ b/drivers/clk/socfpga/clk.c @@ -49,6 +49,7 @@ struct socfpga_clk { void __iomem *reg; char *parent_name; char *clk_name; + int fixed_div; }; #define to_socfpga_clk(p) container_of(p, struct socfpga_clk, hw) Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree 2013-03-20 13:46 ` Pavel Machek @ 2013-03-20 14:00 ` Pavel Machek 0 siblings, 0 replies; 15+ messages in thread From: Pavel Machek @ 2013-03-20 14:00 UTC (permalink / raw) To: linux-arm-kernel Hi! > > Adds the main PLL clock groups for SOCFPGA into device tree file > > so that the clock framework to query the clock and clock rates > > appropriately. > > Is there an easy way to test it? Please consider cleanups below. If compiles and NFS root fails the same way it failed below. Thanks, Pavel commit 82158c4cf015e3b8c3db1b5ba84722fe1a480dce Author: Pavel Machek <pavel@pollux.denx.de> Date: Wed Mar 20 14:56:56 2013 +0100 Convert clock_manager_base_addr to structure, to get type checking and nicer code. Improve constants alignment a bit. Signed-off-by: Pavel Machek <pavel@denx.de> diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c index 856625a..b88b992 100644 --- a/arch/arm/mach-socfpga/socfpga.c +++ b/arch/arm/mach-socfpga/socfpga.c @@ -30,7 +30,7 @@ void __iomem *socfpga_scu_base_addr = ((void __iomem *)(SOCFPGA_SCU_VIRT_BASE)); void __iomem *sys_manager_base_addr; void __iomem *rst_manager_base_addr; -void __iomem *clk_mgr_base_addr; +struct clock_manager_hw __iomem *clk_mgr_base_addr; unsigned long cpu1start_addr; static struct map_desc scu_io_desc __initdata = { diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c index 3504dbf..0239ff5 100644 --- a/drivers/clk/socfpga/clk.c +++ b/drivers/clk/socfpga/clk.c @@ -17,16 +17,13 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see <http://www.gnu.org/licenses/>. */ + #include <linux/clk.h> #include <linux/clkdev.h> #include <linux/clk-provider.h> #include <linux/io.h> #include <linux/of.h> -/* Clock Manager offsets */ -#define CLKMGR_CTRL 0x0 -#define CLKMGR_BYPASS 0x4 - /* Clock bypass bits */ #define MAINPLL_BYPASS (1<<0) #define SDRAMPLL_BYPASS (1<<1) @@ -37,12 +34,17 @@ #define SOCFPGA_PLL_BG_PWRDWN 0x00000001 #define SOCFPGA_PLL_EXT_ENA 0x00000002 #define SOCFPGA_PLL_PWR_DOWN 0x00000004 -#define SOCFPGA_PLL_DIVF_MASK 0x0000FFF8 +#define SOCFPGA_PLL_DIVF_MASK 0x0000FFF8 #define SOCFPGA_PLL_DIVF_SHIFT 3 -#define SOCFPGA_PLL_DIVQ_MASK 0x003F0000 +#define SOCFPGA_PLL_DIVQ_MASK 0x003F0000 #define SOCFPGA_PLL_DIVQ_SHIFT 15 -extern void __iomem *clk_mgr_base_addr; +struct clock_manager_hw { + u32 ctrl; + u32 bypass; +}; + +extern struct clock_manager_hw __iomem *clk_mgr_base_addr; struct socfpga_clk { struct clk_hw hw; @@ -83,7 +85,7 @@ static unsigned long clk_pll_recalc_rate(struct clk_hw *hwclk, unsigned long bypass; reg = readl(socfpgaclk->reg); - bypass = readl(clk_mgr_base_addr + CLKMGR_BYPASS); + bypass = readl(&clk_mgr_base_addr->bypass); if (bypass & MAINPLL_BYPASS) return parent_rate; -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCHv2 1/2] ARM: socfpga: Enable soft reset 2013-03-19 15:45 [PATCHv2 1/2] ARM: socfpga: Enable soft reset dinguyen at altera.com 2013-03-19 15:45 ` [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree dinguyen at altera.com @ 2013-03-20 15:29 ` Pavel Machek 2013-03-20 17:44 ` Russell King - ARM Linux 2013-04-03 18:52 ` Olof Johansson 1 sibling, 2 replies; 15+ messages in thread From: Pavel Machek @ 2013-03-20 15:29 UTC (permalink / raw) To: linux-arm-kernel Hi! > From: Dinh Nguyen <dinguyen@altera.com> > > Enable a cold or warm reset to the HW from userspace. > > Also fix a few sparse errors: > > warning: symbol 'sys_manager_base_addr' was not declared. Should it be static? > warning: symbol 'rst_manager_base_addr' was not declared. Should it be static? > > Signed-off-by: Dinh Nguyen <dinguyen@altera.com> Tested-by: Pavel Machek <pavel@denx.de> Would it make sense to apply something like this? Struct looks cleaner than offset defines... Thanks, Pavel Switch reset manager to using struct (not defines), cleanups. Convert SMP code to use the struct instead of open-coded numbers. Also none of the code is time-critical, so it does not make sense to use __raw_writel variants. Signed-off-by: Pavel Machek <pavel@denx.de> diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h index d2a251f..f4b6048 100644 --- a/arch/arm/mach-socfpga/core.h +++ b/arch/arm/mach-socfpga/core.h @@ -20,19 +20,21 @@ #ifndef __MACH_CORE_H #define __MACH_CORE_H -#define SOCFPGA_RSTMGR_CTRL 0x04 -#define SOCFPGA_RSTMGR_MODPERRST 0x14 -#define SOCFPGA_RSTMGR_BRGMODRST 0x1c - -/* System Manager bits */ -#define RSTMGR_CTRL_SWCOLDRSTREQ 0x1 /* Cold Reset */ -#define RSTMGR_CTRL_SWWARMRSTREQ 0x2 /* Warm Reset */ -/*MPU Module Reset Register */ -#define RSTMGR_MPUMODRST_CPU0 0x1 /*CPU0 Reset*/ -#define RSTMGR_MPUMODRST_CPU1 0x2 /*CPU1 Reset*/ -#define RSTMGR_MPUMODRST_WDS 0x4 /*Watchdog Reset*/ -#define RSTMGR_MPUMODRST_SCUPER 0x8 /*SCU and periphs reset*/ -#define RSTMGR_MPUMODRST_L2 0x10 /*L2 Cache reset*/ +struct socfpga_rstmgr_hw { + u32 unk; + u32 ctrl; /* 0x04 */ + u32 unk2, unk3; +/* MPU Module Reset Register */ +#define RSTMGR_MPUMODRST_CPU0 0x1 /* CPU0 Reset */ +#define RSTMGR_MPUMODRST_CPU1 0x2 /* CPU1 Reset */ +#define RSTMGR_MPUMODRST_WDS 0x4 /* Watchdog Reset */ +#define RSTMGR_MPUMODRST_SCUPER 0x8 /* SCU and periphs reset */ +#define RSTMGR_MPUMODRST_L2 0x10 /* L2 Cache reset */ + u32 mpumodrst; /* 0x10 */ + u32 modperrst; /* 0x14 */ + u32 unk5; + u32 bgrmodrst; /* 0x1c */ +}; extern void socfpga_secondary_startup(void); extern void __iomem *socfpga_scu_base_addr; @@ -41,7 +43,7 @@ extern void socfpga_init_clocks(void); extern void socfpga_sysmgr_init(void); extern void __iomem *sys_manager_base_addr; -extern void __iomem *rst_manager_base_addr; +extern struct socfpga_rstmgr_hw __iomem *rst_manager_base_addr; extern struct smp_operations socfpga_smp_ops; extern char secondary_trampoline, secondary_trampoline_end; diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c index b41a945..81b8f1e 100644 --- a/arch/arm/mach-socfpga/socfpga.c +++ b/arch/arm/mach-socfpga/socfpga.c @@ -28,7 +28,7 @@ void __iomem *socfpga_scu_base_addr = ((void __iomem *)(SOCFPGA_SCU_VIRT_BASE)); void __iomem *sys_manager_base_addr; -void __iomem *rst_manager_base_addr; +struct socfpga_rstmgr_hw __iomem *rst_manager_base_addr; unsigned long cpu1start_addr; static struct map_desc scu_io_desc __initdata = { @@ -89,13 +89,13 @@ static void socfpga_cyclone5_restart(char mode, const char *cmd) { u32 temp; - temp = __raw_readl(rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL); + temp = readl(&rst_manager_base_addr->ctrl); if (mode == 'h') - temp |= RSTMGR_CTRL_SWCOLDRSTREQ; + temp |= 1; /* RSTMGR_CTRL_SWCOLDRSTREQ, cold reset */ else - temp |= RSTMGR_CTRL_SWWARMRSTREQ; - __raw_writel(temp, rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL); + temp |= 2; /* RSTMGR_CTRL_SWWARMRSTREQ, warm reset */ + writel(temp, &rst_manager_base_addr->ctrl); } static void __init socfpga_cyclone5_init(void) diff --git a/arch/arm/mach-socfpga/platsmp.c b/arch/arm/mach-socfpga/platsmp.c index c75c33d..822a93e 100644 --- a/arch/arm/mach-socfpga/platsmp.c +++ b/arch/arm/mach-socfpga/platsmp.c @@ -47,7 +47,7 @@ static int __cpuinit socfpga_boot_secondary(unsigned int cpu, struct task_struct if (cpu1start_addr) { memcpy(phys_to_virt(0), &secondary_trampoline, trampoline_size); - __raw_writel(virt_to_phys(socfpga_secondary_startup), + writel(virt_to_phys(socfpga_secondary_startup), (sys_manager_base_addr + (cpu1start_addr & 0x000000ff))); flush_cache_all(); @@ -55,7 +55,7 @@ static int __cpuinit socfpga_boot_secondary(unsigned int cpu, struct task_struct outer_clean_range(0, trampoline_size); /* This will release CPU #1 out of reset.*/ - __raw_writel(0, rst_manager_base_addr + 0x10); + writel(0, &rst_manager_base_addr->mpumodrst); } return 0; @@ -101,7 +101,7 @@ static void socfpga_cpu_die(unsigned int cpu) flush_cache_all(); /* This will put CPU1 into reset.*/ - __raw_writel(RSTMGR_MPUMODRST_CPU1, rst_manager_base_addr + 0x10); + writel(RSTMGR_MPUMODRST_CPU1, &rst_manager_base_addr->mpumodrst); cpu_do_idle(); -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCHv2 1/2] ARM: socfpga: Enable soft reset 2013-03-20 15:29 ` [PATCHv2 1/2] ARM: socfpga: Enable soft reset Pavel Machek @ 2013-03-20 17:44 ` Russell King - ARM Linux 2013-04-03 18:52 ` Olof Johansson 1 sibling, 0 replies; 15+ messages in thread From: Russell King - ARM Linux @ 2013-03-20 17:44 UTC (permalink / raw) To: linux-arm-kernel On Wed, Mar 20, 2013 at 04:29:15PM +0100, Pavel Machek wrote: > Hi! > > > From: Dinh Nguyen <dinguyen@altera.com> > > > > Enable a cold or warm reset to the HW from userspace. > > > > Also fix a few sparse errors: > > > > warning: symbol 'sys_manager_base_addr' was not declared. Should it be static? > > warning: symbol 'rst_manager_base_addr' was not declared. Should it be static? > > > > Signed-off-by: Dinh Nguyen <dinguyen@altera.com> > > Tested-by: Pavel Machek <pavel@denx.de> > > Would it make sense to apply something like this? Struct looks cleaner > than offset defines... Structs used to define offsets is technically dodgy according to the C standards, so I personally really do not like this practice, and I will not use it ever myself where offsets matter. Your use is fine though, but only because we know how the compiler behaves on ARM - and that's a very important point with doing this. We're reliant on that compiler behaviour if we go down this path. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 1/2] ARM: socfpga: Enable soft reset 2013-03-20 15:29 ` [PATCHv2 1/2] ARM: socfpga: Enable soft reset Pavel Machek 2013-03-20 17:44 ` Russell King - ARM Linux @ 2013-04-03 18:52 ` Olof Johansson 2013-04-03 20:32 ` Pavel Machek 1 sibling, 1 reply; 15+ messages in thread From: Olof Johansson @ 2013-04-03 18:52 UTC (permalink / raw) To: linux-arm-kernel On Wed, Mar 20, 2013 at 04:29:15PM +0100, Pavel Machek wrote: > Hi! > > > From: Dinh Nguyen <dinguyen@altera.com> > > > > Enable a cold or warm reset to the HW from userspace. > > > > Also fix a few sparse errors: > > > > warning: symbol 'sys_manager_base_addr' was not declared. Should it be static? > > warning: symbol 'rst_manager_base_addr' was not declared. Should it be static? > > > > Signed-off-by: Dinh Nguyen <dinguyen@altera.com> > > Tested-by: Pavel Machek <pavel@denx.de> > > Would it make sense to apply something like this? Struct looks cleaner > than offset defines... > > Thanks, > Pavel > > Switch reset manager to using struct (not defines), cleanups. > > Convert SMP code to use the struct instead of open-coded numbers. > > Also none of the code is time-critical, so it > does not make sense to use __raw_writel variants. > > Signed-off-by: Pavel Machek <pavel@denx.de> > > diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h > index d2a251f..f4b6048 100644 > --- a/arch/arm/mach-socfpga/core.h > +++ b/arch/arm/mach-socfpga/core.h > @@ -20,19 +20,21 @@ > #ifndef __MACH_CORE_H > #define __MACH_CORE_H > > -#define SOCFPGA_RSTMGR_CTRL 0x04 > -#define SOCFPGA_RSTMGR_MODPERRST 0x14 > -#define SOCFPGA_RSTMGR_BRGMODRST 0x1c > - > -/* System Manager bits */ > -#define RSTMGR_CTRL_SWCOLDRSTREQ 0x1 /* Cold Reset */ > -#define RSTMGR_CTRL_SWWARMRSTREQ 0x2 /* Warm Reset */ > -/*MPU Module Reset Register */ > -#define RSTMGR_MPUMODRST_CPU0 0x1 /*CPU0 Reset*/ > -#define RSTMGR_MPUMODRST_CPU1 0x2 /*CPU1 Reset*/ > -#define RSTMGR_MPUMODRST_WDS 0x4 /*Watchdog Reset*/ > -#define RSTMGR_MPUMODRST_SCUPER 0x8 /*SCU and periphs reset*/ > -#define RSTMGR_MPUMODRST_L2 0x10 /*L2 Cache reset*/ > +struct socfpga_rstmgr_hw { > + u32 unk; > + u32 ctrl; /* 0x04 */ > + u32 unk2, unk3; > +/* MPU Module Reset Register */ > +#define RSTMGR_MPUMODRST_CPU0 0x1 /* CPU0 Reset */ > +#define RSTMGR_MPUMODRST_CPU1 0x2 /* CPU1 Reset */ > +#define RSTMGR_MPUMODRST_WDS 0x4 /* Watchdog Reset */ > +#define RSTMGR_MPUMODRST_SCUPER 0x8 /* SCU and periphs reset */ > +#define RSTMGR_MPUMODRST_L2 0x10 /* L2 Cache reset */ > + u32 mpumodrst; /* 0x10 */ > + u32 modperrst; /* 0x14 */ > + u32 unk5; > + u32 bgrmodrst; /* 0x1c */ > +}; As Russell already replied, struct used to represent register layout is normally frowned upon in drivers. If you want to tighten up the code in this case, make a local helper function that just takes the register offset instead, i.e.: > - temp = __raw_readl(rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL); > + temp = readl(&rst_manager_base_addr->ctrl); temp = rst_readl(SOCFPGA_RSTMGR_CTRL); (and then have rst_readl/rst_writel do the math based on base address). -Olof ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 1/2] ARM: socfpga: Enable soft reset 2013-04-03 18:52 ` Olof Johansson @ 2013-04-03 20:32 ` Pavel Machek 2013-04-03 21:12 ` Olof Johansson 0 siblings, 1 reply; 15+ messages in thread From: Pavel Machek @ 2013-04-03 20:32 UTC (permalink / raw) To: linux-arm-kernel Hi! > > +struct socfpga_rstmgr_hw { > > + u32 unk; > > + u32 ctrl; /* 0x04 */ > > + u32 unk2, unk3; > > +/* MPU Module Reset Register */ > > +#define RSTMGR_MPUMODRST_CPU0 0x1 /* CPU0 Reset */ > > +#define RSTMGR_MPUMODRST_CPU1 0x2 /* CPU1 Reset */ > > +#define RSTMGR_MPUMODRST_WDS 0x4 /* Watchdog Reset */ > > +#define RSTMGR_MPUMODRST_SCUPER 0x8 /* SCU and periphs reset */ > > +#define RSTMGR_MPUMODRST_L2 0x10 /* L2 Cache reset */ > > + u32 mpumodrst; /* 0x10 */ > > + u32 modperrst; /* 0x14 */ > > + u32 unk5; > > + u32 bgrmodrst; /* 0x1c */ > > +}; > > As Russell already replied, struct used to represent register layout is > normally frowned upon in drivers. Well... as Russell also said, this is arch-specific code, so it is actually ok. > If you want to tighten up the code in this case, make a local helper > function that just takes the register offset instead, i.e.: > > > - temp = __raw_readl(rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL); > > + temp = readl(&rst_manager_base_addr->ctrl); > > temp = rst_readl(SOCFPGA_RSTMGR_CTRL); > > (and then have rst_readl/rst_writel do the math based on base > address). That does not prevent mistake such as rst_readl(SOCFPGA_SYSMGR_CTRL)... and with number of different subsystems on socfpga (each with separate base address), I fear that might be an issue. Unfortunately, C does not check type of enums, so they can't be used to solve that. Any other ideas? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 1/2] ARM: socfpga: Enable soft reset 2013-04-03 20:32 ` Pavel Machek @ 2013-04-03 21:12 ` Olof Johansson 2013-04-04 16:08 ` Dinh Nguyen 0 siblings, 1 reply; 15+ messages in thread From: Olof Johansson @ 2013-04-03 21:12 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 3, 2013 at 1:32 PM, Pavel Machek <pavel@denx.de> wrote: > Hi! > >> > +struct socfpga_rstmgr_hw { >> > + u32 unk; >> > + u32 ctrl; /* 0x04 */ >> > + u32 unk2, unk3; >> > +/* MPU Module Reset Register */ >> > +#define RSTMGR_MPUMODRST_CPU0 0x1 /* CPU0 Reset */ >> > +#define RSTMGR_MPUMODRST_CPU1 0x2 /* CPU1 Reset */ >> > +#define RSTMGR_MPUMODRST_WDS 0x4 /* Watchdog Reset */ >> > +#define RSTMGR_MPUMODRST_SCUPER 0x8 /* SCU and periphs reset */ >> > +#define RSTMGR_MPUMODRST_L2 0x10 /* L2 Cache reset */ >> > + u32 mpumodrst; /* 0x10 */ >> > + u32 modperrst; /* 0x14 */ >> > + u32 unk5; >> > + u32 bgrmodrst; /* 0x1c */ >> > +}; >> >> As Russell already replied, struct used to represent register layout is >> normally frowned upon in drivers. > > Well... as Russell also said, this is arch-specific code, so it is > actually ok. Which is why I said "normally frowned upon" instead of "always frowned upon". >> If you want to tighten up the code in this case, make a local helper >> function that just takes the register offset instead, i.e.: >> >> > - temp = __raw_readl(rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL); >> > + temp = readl(&rst_manager_base_addr->ctrl); >> >> temp = rst_readl(SOCFPGA_RSTMGR_CTRL); >> >> (and then have rst_readl/rst_writel do the math based on base >> address). > > That does not prevent mistake such as > rst_readl(SOCFPGA_SYSMGR_CTRL)... and with number of different > subsystems on socfpga (each with separate base address), I fear that > might be an issue. > > Unfortunately, C does not check type of enums, so they can't be used > to solve that. That's no different from accidentally doing readl(&sys_manager_base_addr->ctrl) instead of rst_manager_base_addr->ctrl. -Olof ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv2 1/2] ARM: socfpga: Enable soft reset 2013-04-03 21:12 ` Olof Johansson @ 2013-04-04 16:08 ` Dinh Nguyen 0 siblings, 0 replies; 15+ messages in thread From: Dinh Nguyen @ 2013-04-04 16:08 UTC (permalink / raw) To: linux-arm-kernel Hi Pavel, On Wed, 2013-04-03 at 14:12 -0700, Olof Johansson wrote: > On Wed, Apr 3, 2013 at 1:32 PM, Pavel Machek <pavel@denx.de> wrote: > > Hi! > > > >> > +struct socfpga_rstmgr_hw { > >> > + u32 unk; > >> > + u32 ctrl; /* 0x04 */ > >> > + u32 unk2, unk3; > >> > +/* MPU Module Reset Register */ > >> > +#define RSTMGR_MPUMODRST_CPU0 0x1 /* CPU0 Reset */ > >> > +#define RSTMGR_MPUMODRST_CPU1 0x2 /* CPU1 Reset */ > >> > +#define RSTMGR_MPUMODRST_WDS 0x4 /* Watchdog Reset */ > >> > +#define RSTMGR_MPUMODRST_SCUPER 0x8 /* SCU and periphs reset */ > >> > +#define RSTMGR_MPUMODRST_L2 0x10 /* L2 Cache reset */ > >> > + u32 mpumodrst; /* 0x10 */ > >> > + u32 modperrst; /* 0x14 */ > >> > + u32 unk5; > >> > + u32 bgrmodrst; /* 0x1c */ > >> > +}; > >> > >> As Russell already replied, struct used to represent register layout is > >> normally frowned upon in drivers. > > > > Well... as Russell also said, this is arch-specific code, so it is > > actually ok. > > Which is why I said "normally frowned upon" instead of "always frowned upon". > > >> If you want to tighten up the code in this case, make a local helper > >> function that just takes the register offset instead, i.e.: > >> > >> > - temp = __raw_readl(rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL); > >> > + temp = readl(&rst_manager_base_addr->ctrl); > >> > >> temp = rst_readl(SOCFPGA_RSTMGR_CTRL); > >> > >> (and then have rst_readl/rst_writel do the math based on base > >> address). > > > > That does not prevent mistake such as > > rst_readl(SOCFPGA_SYSMGR_CTRL)... and with number of different > > subsystems on socfpga (each with separate base address), I fear that > > might be an issue. > > > > Unfortunately, C does not check type of enums, so they can't be used > > to solve that. > > That's no different from accidentally doing > readl(&sys_manager_base_addr->ctrl) instead of > rst_manager_base_addr->ctrl. > I also don't really like to use structs for register offsets. So unless, there's some standard to use them in machine code, I'm not going to use them in mach-socfpga. I apologize for not getting to a v3 for this patch series with Mike's recommended change for using composite clocks as quickly as I would have like. I hope to have a v3 by next week. Dinh > > -Olof > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-04-04 16:08 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-03-19 15:45 [PATCHv2 1/2] ARM: socfpga: Enable soft reset dinguyen at altera.com 2013-03-19 15:45 ` [PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree dinguyen at altera.com 2013-03-19 16:46 ` Mike Turquette 2013-03-19 18:45 ` Dinh Nguyen 2013-03-19 22:12 ` Mike Turquette 2013-03-20 12:24 ` Dinh Nguyen 2013-03-20 12:31 ` Arnd Bergmann 2013-03-20 13:46 ` Pavel Machek 2013-03-20 14:00 ` Pavel Machek 2013-03-20 15:29 ` [PATCHv2 1/2] ARM: socfpga: Enable soft reset Pavel Machek 2013-03-20 17:44 ` Russell King - ARM Linux 2013-04-03 18:52 ` Olof Johansson 2013-04-03 20:32 ` Pavel Machek 2013-04-03 21:12 ` Olof Johansson 2013-04-04 16:08 ` Dinh Nguyen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.