[PATCHv2 1/2] ARM: socfpga: Enable soft reset

[PATCHv2 1/2] ARM: socfpga: Enable soft reset

[dinguyen at altera.com]

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

[PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree

[dinguyen at altera.com]

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", &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

[PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree

[Mike Turquette]

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

[PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree

[Dinh Nguyen]

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
> 

[PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree

[Mike Turquette]

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
> >

[PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree

[Dinh Nguyen]

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
> > >
> 

[PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree

[Arnd Bergmann]

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

[PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree

[Pavel Machek]

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

[PATCHv2 2/2] ARM: socfpga: Add clock entries into device tree

[Pavel Machek]

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

[PATCHv2 1/2] ARM: socfpga: Enable soft reset

[Pavel Machek]

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

[PATCHv2 1/2] ARM: socfpga: Enable soft reset

[Russell King - ARM Linux]

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.

[PATCHv2 1/2] ARM: socfpga: Enable soft reset

[Olof Johansson]

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

[PATCHv2 1/2] ARM: socfpga: Enable soft reset

[Pavel Machek]

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

[PATCHv2 1/2] ARM: socfpga: Enable soft reset

[Olof Johansson]

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

[PATCHv2 1/2] ARM: socfpga: Enable soft reset

[Dinh Nguyen]

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
>