* [RFC v2 0/5] MIPS: ralink: peripheral clock gating driver
@ 2019-04-05 0:01 NOGUCHI Hiroshi
2019-04-05 0:01 ` [RFC v2 1/5] clk: mips: ralink: add Ralink MIPS gating clock driver NOGUCHI Hiroshi
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: NOGUCHI Hiroshi @ 2019-04-05 0:01 UTC (permalink / raw)
To: John Crispin
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
linux-mips, linux-clk, NOGUCHI Hiroshi
Changelog:
v2:
- moves ralink gating clock driver to drivers/clk/
- describes the resources of gating clocks in devicetree
and removes resource tables in source code
- changes gating clock provider to platform_driver
- changes PLL clock provider to platform_driver
- replaces clk_* API calls to clk_hw_* APIs
- uses BIT() macro
- uses IS_ENABLED(CONFIG_COMMON_CLK) macro
------
This series introduce Mediatek/Ralink SoC's clock gating driver and
rework PLL clks as clock provider.
The gating clocks are different at individual SoCs.
Their resources are described with devicetree.
NOGUCHI Hiroshi (5):
clk: ralink: add rt2880-clock driver
dt-bindings: clk: add document for ralink clock driver
mips: ralink: mt7620/76x8 use common clk framework
mips: ralink: mt76x8: add nodes for PLL clocks and gating clocks
mips: ralink: mt7620: add nodes for PLL clocks and gating clocks
.../bindings/clock/ralink,rt2880-clock.txt | 58 +++++
arch/mips/boot/dts/ralink/mt7620a.dtsi | 46 +++-
arch/mips/boot/dts/ralink/mt7628a.dtsi | 52 +++++
arch/mips/ralink/Kconfig | 2 +
arch/mips/ralink/clk.c | 33 +++
arch/mips/ralink/common.h | 4 +
arch/mips/ralink/mt7620.c | 82 +++++---
drivers/clk/Kconfig | 6 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-rt2880.c | 199 ++++++++++++++++++
include/dt-bindings/clock/mt7620-clk.h | 17 ++
11 files changed, 472 insertions(+), 28 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
create mode 100644 drivers/clk/clk-rt2880.c
create mode 100644 include/dt-bindings/clock/mt7620-clk.h
--
2.20.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC v2 1/5] clk: mips: ralink: add Ralink MIPS gating clock driver
2019-04-05 0:01 [RFC v2 0/5] MIPS: ralink: peripheral clock gating driver NOGUCHI Hiroshi
@ 2019-04-05 0:01 ` NOGUCHI Hiroshi
2019-04-25 19:27 ` Stephen Boyd
2019-04-05 0:01 ` [RFC v2 2/5] dt-bindings: clk: add document for ralink " NOGUCHI Hiroshi
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: NOGUCHI Hiroshi @ 2019-04-05 0:01 UTC (permalink / raw)
To: John Crispin
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
linux-mips, linux-clk, NOGUCHI Hiroshi
This patch adds clock gating driver for Ralink/Mediatek MIPS Socs.
Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
---
drivers/clk/Kconfig | 6 ++
drivers/clk/Makefile | 1 +
drivers/clk/clk-rt2880.c | 199 +++++++++++++++++++++++++++++++++++++++
3 files changed, 206 insertions(+)
create mode 100644 drivers/clk/clk-rt2880.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index f96c7f39ab7e..dbfdf1bcdc6c 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -290,6 +290,12 @@ config COMMON_CLK_BD718XX
This driver supports ROHM BD71837 and ROHM BD71847
PMICs clock gates.
+config COMMON_CLK_RT2880
+ bool "Clock driver for Mediatek/Ralink MIPS SoC Family"
+ depends on COMMON_CLK && OF
+ help
+ This driver support Mediatek/Ralink MIPS SoCs' clock gates.
+
config COMMON_CLK_FIXED_MMIO
bool "Clock driver for Memory Mapped Fixed values"
depends on COMMON_CLK && OF
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 1db133652f0c..b1c24b99e848 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o
obj-$(CONFIG_COMMON_CLK_PWM) += clk-pwm.o
obj-$(CONFIG_CLK_QORIQ) += clk-qoriq.o
obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o
+obj-$(CONFIG_COMMON_CLK_RT2880) += clk-rt2880.o
obj-$(CONFIG_COMMON_CLK_HI655X) += clk-hi655x.o
obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o
obj-$(CONFIG_COMMON_CLK_SCMI) += clk-scmi.o
diff --git a/drivers/clk/clk-rt2880.c b/drivers/clk/clk-rt2880.c
new file mode 100644
index 000000000000..bcdb4c1486d3
--- /dev/null
+++ b/drivers/clk/clk-rt2880.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 NOGUCHI Hiroshi <drvlabo@gmail.com>
+ */
+
+#include <linux/slab.h>
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/platform_device.h>
+
+
+/* clock configuration 1 */
+#define SYSC_REG_CLKCFG1 0x30
+
+#define GATE_CLK_NUM 32
+
+struct rt2880_gate {
+ struct clk_hw hw;
+ u8 shift;
+};
+
+#define to_rt2880_gate(_hw) container_of(_hw, struct rt2880_gate, hw)
+
+static struct clk_hw_onecell_data *clk_data;
+static struct regmap *syscon_regmap;
+
+static int rt2880_gate_enable(struct clk_hw *hw)
+{
+ struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
+ u32 val = BIT(clk_gate->shift);
+
+ return regmap_update_bits(syscon_regmap, SYSC_REG_CLKCFG1, val, val);
+}
+
+static void rt2880_gate_disable(struct clk_hw *hw)
+{
+ struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
+ u32 val = BIT(clk_gate->shift);
+
+ regmap_update_bits(syscon_regmap, SYSC_REG_CLKCFG1, val, 0);
+}
+
+static int rt2880_gate_is_enabled(struct clk_hw *hw)
+{
+ struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
+ unsigned int rdval;
+
+ if (regmap_read(syscon_regmap, SYSC_REG_CLKCFG1, &rdval))
+ return 0;
+
+ return rdval & BIT(clk_gate->shift);
+}
+
+static const struct clk_ops rt2880_gate_ops = {
+ .enable = rt2880_gate_enable,
+ .disable = rt2880_gate_disable,
+ .is_enabled = rt2880_gate_is_enabled,
+};
+
+static struct clk_hw * __init
+rt2880_register_gate(const char *name, const char *parent_name, u8 shift)
+{
+ struct rt2880_gate *clk_gate;
+ struct clk_init_data init;
+ int err;
+ const char *_parent_names[1] = { parent_name };
+
+ clk_gate = kzalloc(sizeof(*clk_gate), GFP_KERNEL);
+ if (!clk_gate)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &rt2880_gate_ops;
+ init.flags = 0;
+ init.parent_names = parent_name ? _parent_names : NULL;
+ init.num_parents = parent_name ? 1 : 0;
+
+ clk_gate->hw.init = &init;
+ clk_gate->shift = shift;
+
+ err = clk_hw_register(NULL, &clk_gate->hw);
+ if (err) {
+ kfree(clk_gate);
+ return ERR_PTR(err);
+ }
+
+ return &clk_gate->hw;
+}
+
+static struct clk_hw *
+rt2880_clk_hw_get(struct of_phandle_args *clkspec, void *data)
+{
+ struct clk_hw_onecell_data *hw_data = data;
+ unsigned int idx = clkspec->args[0];
+ int i;
+
+ if (idx >= GATE_CLK_NUM)
+ goto err;
+
+ for (i = 0; i < hw_data->num; i++)
+ if (idx == to_rt2880_gate(hw_data->hws[i])->shift)
+ break;
+ if (i >= hw_data->num)
+ goto err;
+
+ return hw_data->hws[i];
+
+err:
+ pr_err("%s: invalid index %u\n", __func__, idx);
+ return ERR_PTR(-EINVAL);
+}
+
+static int __init _rt2880_clkctrl_init_dt(struct device_node *np)
+{
+ struct clk_hw *clk_hw;
+ const char *name;
+ const char *parent_name;
+ u32 val;
+ int cnt;
+ int num;
+ int i;
+ int idx;
+
+ syscon_regmap = syscon_regmap_lookup_by_phandle(np, "ralink,sysctl");
+ if (IS_ERR(syscon_regmap)) {
+ pr_err("rt2880-clock: could not get syscon regmap.\n");
+ return PTR_ERR(syscon_regmap);
+ }
+
+ cnt = of_property_count_u32_elems(np, "clock-indices");
+ if (cnt < 0) {
+ pr_err("rt2880-clock: clock-indices property is invalid.\n");
+ return cnt;
+ }
+
+ num = 0;
+ for (i = 0; i < cnt; i++) {
+ if (of_property_read_u32_index(np, "clock-indices", i, &val))
+ break;
+ if (val < GATE_CLK_NUM)
+ num++;
+ }
+ if ((num <= 0) || (num > GATE_CLK_NUM)) {
+ pr_err("rt2880-clock: clock-indices property is invalid.\n");
+ return -EINVAL;
+ }
+
+ clk_data = kzalloc(struct_size(clk_data, hws, num), GFP_KERNEL);
+ if (!clk_data)
+ return -ENOMEM;
+ clk_data->num = num;
+
+ idx = 0;
+ for (i = 0; (i < cnt) && (idx < num); i++) {
+ if (of_property_read_u32_index(np, "clock-indices", i, &val))
+ break;
+ if ((int)val >= GATE_CLK_NUM)
+ continue;
+
+ if (of_property_read_string_index(np, "clock-output-names",
+ i, &name))
+ break;
+
+ parent_name = of_clk_get_parent_name(np, i);
+
+ clk_hw = rt2880_register_gate(name, parent_name, val);
+ if (IS_ERR_OR_NULL(clk_hw)) {
+ pr_err("rt2880-clock: could not register clock for %s\n",
+ name);
+ continue;
+ }
+ clk_data->hws[idx] = clk_hw;
+ idx++;
+ }
+
+ of_clk_add_hw_provider(np, rt2880_clk_hw_get, clk_data);
+
+ return 0;
+}
+
+static int rt2880_clkctrl_probe(struct platform_device *pdev)
+{
+ return _rt2880_clkctrl_init_dt(pdev->dev.of_node);
+}
+
+static const struct of_device_id rt2880_clkctrl_ids[] = {
+ { .compatible = "ralink,rt2880-clock" },
+ { }
+};
+
+static struct platform_driver rt2880_clkctrl_driver = {
+ .probe = rt2880_clkctrl_probe,
+ .driver = {
+ .name = "rt2880-clock",
+ .of_match_table = rt2880_clkctrl_ids,
+ },
+};
+builtin_platform_driver(rt2880_clkctrl_driver);
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC v2 2/5] dt-bindings: clk: add document for ralink clock driver
2019-04-05 0:01 [RFC v2 0/5] MIPS: ralink: peripheral clock gating driver NOGUCHI Hiroshi
2019-04-05 0:01 ` [RFC v2 1/5] clk: mips: ralink: add Ralink MIPS gating clock driver NOGUCHI Hiroshi
@ 2019-04-05 0:01 ` NOGUCHI Hiroshi
2019-04-25 19:29 ` Stephen Boyd
2019-04-05 0:01 ` [RFC v2 3/5] mips: ralink: mt7620/76x8 use common clk framework NOGUCHI Hiroshi
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: NOGUCHI Hiroshi @ 2019-04-05 0:01 UTC (permalink / raw)
To: John Crispin
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
linux-mips, linux-clk, NOGUCHI Hiroshi
Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
---
.../bindings/clock/ralink,rt2880-clock.txt | 58 +++++++++++++++++++
1 file changed, 58 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
diff --git a/Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt b/Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
new file mode 100644
index 000000000000..2fc0d622e01e
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
@@ -0,0 +1,58 @@
+* Clock bindings for Ralink/Mediatek MIPS based SoCs
+
+Required properties:
+ - compatible: must be "ralink,rt2880-clock"
+ - #clock-cells: must be 1
+ - ralink,sysctl: a phandle to a ralink syscon register region
+ - clock-indices: identifying number.
+ These must correspond to the bit number in CLKCFG1.
+ Clock consumers use one of them as #clock-cells index.
+ - clock-output-names: array of gating clocks' names
+ - clocks: array of phandles which points the parent clock
+ for gating clocks.
+ If gating clock does not need parent clock linkage,
+ we bind to dummy clock whose frequency is zero.
+
+
+Example:
+
+/* dummy parent clock node */
+dummy_ck: dummy_ck {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <0>;
+};
+
+clkctrl: clkctrl {
+ compatible = "ralink,rt2880-clock";
+ #clock-cells = <1>;
+ ralink,sysctl = <&sysc>;
+
+ clock-indices =
+ <12>,
+ <16>, <17>, <18>, <19>,
+ <20>,
+ <26>;
+ clock-output-names =
+ "uart0",
+ "i2c", "i2s", "spi", "uart1",
+ "uart2",
+ "pcie0";
+ clocks =
+ <&pll MT7620_CLK_PERIPH>,
+ <&pll MT7620_CLK_PERIPH>, <&pll MT7620_CLK_PCMI2S>, <&pll MT7620_CLK_SYS>, <&pll MT7620_CLK_PERIPH>,
+ <&pll MT7620_CLK_PERIPH>,
+ <&dummy_ck>;
+ };
+};
+
+/* consumer which refers "uart0" clock */
+uart0: uartlite@c00 {
+ compatible = "ns16550a";
+ reg = <0xc00 0x100>;
+
+ clocks = <&clkctrl 12>;
+ clock-names = "uart0";
+
+ ...
+};
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC v2 3/5] mips: ralink: mt7620/76x8 use common clk framework
2019-04-05 0:01 [RFC v2 0/5] MIPS: ralink: peripheral clock gating driver NOGUCHI Hiroshi
2019-04-05 0:01 ` [RFC v2 1/5] clk: mips: ralink: add Ralink MIPS gating clock driver NOGUCHI Hiroshi
2019-04-05 0:01 ` [RFC v2 2/5] dt-bindings: clk: add document for ralink " NOGUCHI Hiroshi
@ 2019-04-05 0:01 ` NOGUCHI Hiroshi
2019-04-25 19:18 ` Stephen Boyd
2019-04-05 0:01 ` [RFC v2 4/5] mips: ralink: mt76x8: add nodes for clocks NOGUCHI Hiroshi
2019-04-05 0:01 ` [RFC v2 5/5] mips: ralink: mt7620: " NOGUCHI Hiroshi
4 siblings, 1 reply; 12+ messages in thread
From: NOGUCHI Hiroshi @ 2019-04-05 0:01 UTC (permalink / raw)
To: John Crispin
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
linux-mips, linux-clk, NOGUCHI Hiroshi
This patch replaces PLL clks with fixed rate clock provider.
Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
---
arch/mips/ralink/Kconfig | 2 +
arch/mips/ralink/clk.c | 33 +++++++++++
arch/mips/ralink/common.h | 4 ++
arch/mips/ralink/mt7620.c | 82 ++++++++++++++++++--------
include/dt-bindings/clock/mt7620-clk.h | 17 ++++++
5 files changed, 113 insertions(+), 25 deletions(-)
create mode 100644 include/dt-bindings/clock/mt7620-clk.h
diff --git a/arch/mips/ralink/Kconfig b/arch/mips/ralink/Kconfig
index 49c22ddd9c41..afe0576f896f 100644
--- a/arch/mips/ralink/Kconfig
+++ b/arch/mips/ralink/Kconfig
@@ -40,6 +40,8 @@ choice
bool "MT7620/8"
select CPU_MIPSR2_IRQ_VI
select HAVE_PCI
+ select COMMON_CLK
+ select COMMON_CLK_RT2880
config SOC_MT7621
bool "MT7621"
diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c
index 1b7df115eb60..0fd2f0091ea4 100644
--- a/arch/mips/ralink/clk.c
+++ b/arch/mips/ralink/clk.c
@@ -15,8 +15,15 @@
#include <asm/time.h>
+#if IS_ENABLED(CONFIG_COMMON_CLK)
+#include <linux/clk-provider.h>
+#endif
+
#include "common.h"
+
+#if !IS_ENABLED(CONFIG_COMMON_CLK)
+
struct clk {
struct clk_lookup cl;
unsigned long rate;
@@ -72,6 +79,28 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
}
EXPORT_SYMBOL_GPL(clk_round_rate);
+#else /* CONFIG_COMMON_CLK */
+
+struct clk_hw * __init add_sys_clkdev(const char *id, unsigned long rate)
+{
+ struct clk_hw *clk_hw;
+ int err;
+
+ clk_hw = clk_hw_register_fixed_rate(NULL, id, NULL, 0, rate);
+ if (IS_ERR(clk_hw))
+ panic("failed to allocate %s clock structure", id);
+
+ err = clk_hw_register_clkdev(clk_hw, NULL, id);
+ if (err) {
+ pr_err("unable to register %s clock device\n", id);
+ clk_hw = ERR_PTR(err);
+ }
+
+ return clk_hw;
+}
+
+#endif /* CONFIG_COMMON_CLK */
+
void __init plat_time_init(void)
{
struct clk *clk;
@@ -79,6 +108,10 @@ void __init plat_time_init(void)
ralink_of_remap();
ralink_clk_init();
+
+ if (IS_ENABLED(CONFIG_COMMON_CLK))
+ of_clk_init(NULL);
+
clk = clk_get_sys("cpu", NULL);
if (IS_ERR(clk))
panic("unable to get CPU clock, err=%ld", PTR_ERR(clk));
diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
index b8245d0940d6..3c56a8d0517f 100644
--- a/arch/mips/ralink/common.h
+++ b/arch/mips/ralink/common.h
@@ -25,7 +25,11 @@ extern struct ralink_soc_info soc_info;
extern void ralink_of_remap(void);
extern void ralink_clk_init(void);
+#if IS_ENABLED(CONFIG_COMMON_CLK)
+extern struct clk_hw *add_sys_clkdev(const char *id, unsigned long rate);
+#else
extern void ralink_clk_add(const char *dev, unsigned long rate);
+#endif
extern void ralink_rst_init(void);
diff --git a/arch/mips/ralink/mt7620.c b/arch/mips/ralink/mt7620.c
index c1ce6f43642b..74aed8f7e502 100644
--- a/arch/mips/ralink/mt7620.c
+++ b/arch/mips/ralink/mt7620.c
@@ -10,9 +10,10 @@
* Copyright (C) 2013 John Crispin <john@phrozen.org>
*/
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/bug.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/clk-provider.h>
+#include <dt-bindings/clock/mt7620-clk.h>
#include <asm/mipsregs.h>
#include <asm/mach-ralink/ralink_regs.h>
@@ -504,6 +505,12 @@ mt7620_get_sys_rate(unsigned long cpu_rate)
return cpu_rate / div;
}
+static struct clk_hw_onecell_data *clk_data;
+
+#define RFMT(label) label ":%lu.%03luMHz "
+#define RINT(x) ((x) / 1000000)
+#define RFRAC(x) (((x) / 1000) % 1000)
+
void __init ralink_clk_init(void)
{
unsigned long xtal_rate;
@@ -517,10 +524,6 @@ void __init ralink_clk_init(void)
xtal_rate = mt7620_get_xtal_rate();
-#define RFMT(label) label ":%lu.%03luMHz "
-#define RINT(x) ((x) / 1000000)
-#define RFRAC(x) (((x) / 1000) % 1000)
-
if (is_mt76x8()) {
if (xtal_rate == MHZ(40))
cpu_rate = MHZ(580);
@@ -529,9 +532,6 @@ void __init ralink_clk_init(void)
dram_rate = sys_rate = cpu_rate / 3;
periph_rate = MHZ(40);
pcmi2s_rate = MHZ(480);
-
- ralink_clk_add("10000d00.uartlite", periph_rate);
- ralink_clk_add("10000e00.uartlite", periph_rate);
} else {
cpu_pll_rate = mt7620_get_cpu_pll_rate(xtal_rate);
pll_rate = mt7620_get_pll_rate(xtal_rate, cpu_pll_rate);
@@ -547,7 +547,6 @@ void __init ralink_clk_init(void)
RINT(cpu_pll_rate), RFRAC(cpu_pll_rate),
RINT(pll_rate), RFRAC(pll_rate));
- ralink_clk_add("10000500.uart", periph_rate);
}
pr_debug(RFMT("CPU") RFMT("DRAM") RFMT("SYS") RFMT("PERIPH"),
@@ -555,21 +554,29 @@ void __init ralink_clk_init(void)
RINT(dram_rate), RFRAC(dram_rate),
RINT(sys_rate), RFRAC(sys_rate),
RINT(periph_rate), RFRAC(periph_rate));
-#undef RFRAC
-#undef RINT
-#undef RFMT
- ralink_clk_add("cpu", cpu_rate);
- ralink_clk_add("10000100.timer", periph_rate);
- ralink_clk_add("10000120.watchdog", periph_rate);
- ralink_clk_add("10000900.i2c", periph_rate);
- ralink_clk_add("10000a00.i2s", pcmi2s_rate);
- ralink_clk_add("10000b00.spi", sys_rate);
- ralink_clk_add("10000b40.spi", sys_rate);
- ralink_clk_add("10000c00.uartlite", periph_rate);
- ralink_clk_add("10000d00.uart1", periph_rate);
- ralink_clk_add("10000e00.uart2", periph_rate);
- ralink_clk_add("10180000.wmac", xtal_rate);
+ clk_data = kzalloc(struct_size(clk_data, hws, MT7620_CLK_MAX),
+ GFP_KERNEL);
+ if (!clk_data)
+ return;
+ clk_data->num = MT7620_CLK_MAX;
+
+ /* system global */
+ clk_data->hws[MT7620_CLK_CPU] = add_sys_clkdev("cpu", cpu_rate);
+
+ /* parent reference clocks */
+ clk_data->hws[MT7620_CLK_SYS] =
+ clk_hw_register_fixed_rate(NULL, "sys", NULL,
+ 0, sys_rate);
+ clk_data->hws[MT7620_CLK_PERIPH] =
+ clk_hw_register_fixed_rate(NULL, "periph", NULL,
+ 0, periph_rate);
+ clk_data->hws[MT7620_CLK_PCMI2S] =
+ clk_hw_register_fixed_rate(NULL, "pcmi2s", NULL,
+ 0, pcmi2s_rate);
+ clk_data->hws[MT7620_CLK_XTAL] =
+ clk_hw_register_fixed_rate(NULL, "xtal", NULL,
+ 0, xtal_rate);
if (IS_ENABLED(CONFIG_USB) && !is_mt76x8()) {
/*
@@ -586,6 +593,31 @@ void __init ralink_clk_init(void)
}
}
+#undef RFRAC
+#undef RINT
+#undef RFMT
+
+static int mt7620_pll_probe(struct platform_device *pdev)
+{
+ of_clk_add_hw_provider(pdev->dev.of_node,
+ of_clk_hw_onecell_get, clk_data);
+ return 0;
+}
+
+static const struct of_device_id mt7620_pll_ids[] = {
+ { .compatible = "mediatek,mt7620-pll" },
+ { },
+};
+
+static struct platform_driver mt7620_pll_driver = {
+ .probe = mt7620_pll_probe,
+ .driver = {
+ .name = "mt7620-pll",
+ .of_match_table = mt7620_pll_ids,
+ },
+};
+builtin_platform_driver(mt7620_pll_driver);
+
void __init ralink_of_remap(void)
{
rt_sysc_membase = plat_of_remap_node("ralink,mt7620a-sysc");
diff --git a/include/dt-bindings/clock/mt7620-clk.h b/include/dt-bindings/clock/mt7620-clk.h
new file mode 100644
index 000000000000..54d3c582ca7f
--- /dev/null
+++ b/include/dt-bindings/clock/mt7620-clk.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 NOGUCHI Hiroshi <drvlabo@gmail.com>
+ */
+
+#ifndef __DT_BINDINGS_MT7620_CLK_H
+#define __DT_BINDINGS_MT7620_CLK_H
+
+#define MT7620_CLK_CPU 0
+#define MT7620_CLK_SYS 1
+#define MT7620_CLK_PERIPH 2
+#define MT7620_CLK_PCMI2S 3
+#define MT7620_CLK_XTAL 4
+
+#define MT7620_CLK_MAX 5
+
+#endif /* __DT_BINDINGS_MT7620_CLK_H */
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC v2 4/5] mips: ralink: mt76x8: add nodes for clocks
2019-04-05 0:01 [RFC v2 0/5] MIPS: ralink: peripheral clock gating driver NOGUCHI Hiroshi
` (2 preceding siblings ...)
2019-04-05 0:01 ` [RFC v2 3/5] mips: ralink: mt7620/76x8 use common clk framework NOGUCHI Hiroshi
@ 2019-04-05 0:01 ` NOGUCHI Hiroshi
2019-04-05 0:01 ` [RFC v2 5/5] mips: ralink: mt7620: " NOGUCHI Hiroshi
4 siblings, 0 replies; 12+ messages in thread
From: NOGUCHI Hiroshi @ 2019-04-05 0:01 UTC (permalink / raw)
To: John Crispin
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
linux-mips, linux-clk, NOGUCHI Hiroshi
Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
---
arch/mips/boot/dts/ralink/mt7628a.dtsi | 52 ++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/arch/mips/boot/dts/ralink/mt7628a.dtsi b/arch/mips/boot/dts/ralink/mt7628a.dtsi
index 9ff7e8faaecc..f7630b52c8d7 100644
--- a/arch/mips/boot/dts/ralink/mt7628a.dtsi
+++ b/arch/mips/boot/dts/ralink/mt7628a.dtsi
@@ -1,3 +1,5 @@
+#include <dt-bindings/clock/mt7620-clk.h>
+
/ {
#address-cells = <1>;
#size-cells = <1>;
@@ -26,6 +28,31 @@
compatible = "mti,cpu-interrupt-controller";
};
+ pll: pll {
+ compatible = "mediatek,mt7620-pll";
+ #clock-cells = <1>;
+ clock-output-names = "cpu", "sys", "periph", "pcmi2s", "xtal";
+ };
+
+ clkctrl: clkctrl {
+ compatible = "ralink,rt2880-clock";
+ #clock-cells = <1>;
+ ralink,sysctl = <&sysc>;
+
+ clock-indices =
+ <12>,
+ <16>, <17>, <18>, <19>,
+ <20>;
+ clock-output-names =
+ "uart0",
+ "i2c", "i2s", "spi", "uart1",
+ "uart2";
+ clocks =
+ <&pll MT7620_CLK_PERIPH>,
+ <&pll MT7620_CLK_PERIPH>, <&pll MT7620_CLK_PCMI2S>, <&pll MT7620_CLK_SYS>, <&pll MT7620_CLK_PERIPH>,
+ <&pll MT7620_CLK_PERIPH>;
+ };
+
palmbus@10000000 {
compatible = "palmbus";
reg = <0x10000000 0x200000>;
@@ -62,10 +89,29 @@
reg = <0x300 0x100>;
};
+ spi0: spi@b00 {
+ compatible = "ralink,mt7621-spi";
+ reg = <0xb00 0x100>;
+
+ clocks = <&clkctrl 18>;
+ clock-names = "spi";
+
+ resets = <&resetc 18>;
+ reset-names = "spi";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+ };
+
uart0: uartlite@c00 {
compatible = "ns16550a";
reg = <0xc00 0x100>;
+ clocks = <&clkctrl 12>;
+ clock-names = "uart0";
+
resets = <&resetc 12>;
reset-names = "uart0";
@@ -79,6 +125,9 @@
compatible = "ns16550a";
reg = <0xd00 0x100>;
+ clocks = <&clkctrl 19>;
+ clock-names = "uart1";
+
resets = <&resetc 19>;
reset-names = "uart1";
@@ -92,6 +141,9 @@
compatible = "ns16550a";
reg = <0xe00 0x100>;
+ clocks = <&clkctrl 20>;
+ clock-names = "uart2";
+
resets = <&resetc 20>;
reset-names = "uart2";
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC v2 5/5] mips: ralink: mt7620: add nodes for clocks
2019-04-05 0:01 [RFC v2 0/5] MIPS: ralink: peripheral clock gating driver NOGUCHI Hiroshi
` (3 preceding siblings ...)
2019-04-05 0:01 ` [RFC v2 4/5] mips: ralink: mt76x8: add nodes for clocks NOGUCHI Hiroshi
@ 2019-04-05 0:01 ` NOGUCHI Hiroshi
4 siblings, 0 replies; 12+ messages in thread
From: NOGUCHI Hiroshi @ 2019-04-05 0:01 UTC (permalink / raw)
To: John Crispin
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
linux-mips, linux-clk, NOGUCHI Hiroshi
Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
---
arch/mips/boot/dts/ralink/mt7620a.dtsi | 46 ++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 3 deletions(-)
diff --git a/arch/mips/boot/dts/ralink/mt7620a.dtsi b/arch/mips/boot/dts/ralink/mt7620a.dtsi
index 1f6e5320f486..7c98cb6bb5b9 100644
--- a/arch/mips/boot/dts/ralink/mt7620a.dtsi
+++ b/arch/mips/boot/dts/ralink/mt7620a.dtsi
@@ -1,15 +1,27 @@
// SPDX-License-Identifier: GPL-2.0
+#include <dt-bindings/clock/mt7620-clk.h>
+
/ {
#address-cells = <1>;
#size-cells = <1>;
compatible = "ralink,mtk7620a-soc";
cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
cpu@0 {
compatible = "mips,mips24KEc";
+ device_type = "cpu";
+ reg = <0>;
};
};
+ resetc: reset-controller {
+ compatible = "ralink,rt2880-reset";
+ #reset-cells = <1>;
+ };
+
cpuintc: cpuintc {
#address-cells = <0>;
#interrupt-cells = <1>;
@@ -17,6 +29,28 @@
compatible = "mti,cpu-interrupt-controller";
};
+ pll: pll {
+ compatible = "mediatek,mt7620-pll";
+ #clock-cells = <1>;
+ clock-output-names = "cpu", "sys", "periph", "pcmi2s", "xtal";
+ };
+
+ clkctrl: clkctrl {
+ compatible = "ralink,rt2880-clock";
+ #clock-cells = <1>;
+ ralink,sysctl = <&sysc>;
+
+ clock-indices =
+ <12>,
+ <16>, <17>, <18>, <19>;
+ clock-output-names =
+ "uart",
+ "i2c", "i2s", "spi", "uartl";
+ clocks =
+ <&pll MT7620_CLK_PERIPH>,
+ <&pll MT7620_CLK_PERIPH>, <&pll MT7620_CLK_PCMI2S>, <&pll MT7620_CLK_SYS>, <&pll MT7620_CLK_PERIPH>;
+ };
+
palmbus@10000000 {
compatible = "palmbus";
reg = <0x10000000 0x200000>;
@@ -25,8 +59,8 @@
#address-cells = <1>;
#size-cells = <1>;
- sysc@0 {
- compatible = "ralink,mt7620a-sysc";
+ sysc: sysc@0 {
+ compatible = "ralink,mt7620a-sysc", "syscon";
reg = <0x0 0x100>;
};
@@ -46,10 +80,16 @@
reg = <0x300 0x100>;
};
- uartlite@c00 {
+ uartlite: uartlite@c00 {
compatible = "ralink,mt7620a-uart", "ralink,rt2880-uart", "ns16550a";
reg = <0xc00 0x100>;
+ clocks = <&clkctrl 19>;
+ clock-names = "uartl";
+
+ resets = <&resetc 19>;
+ reset-names = "uartl";
+
interrupt-parent = <&intc>;
interrupts = <12>;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC v2 3/5] mips: ralink: mt7620/76x8 use common clk framework
2019-04-05 0:01 ` [RFC v2 3/5] mips: ralink: mt7620/76x8 use common clk framework NOGUCHI Hiroshi
@ 2019-04-25 19:18 ` Stephen Boyd
0 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2019-04-25 19:18 UTC (permalink / raw)
To: John Crispin, NOGUCHI Hiroshi
Cc: Michael Turquette, Rob Herring, Mark Rutland, linux-mips,
linux-clk, NOGUCHI Hiroshi
Quoting NOGUCHI Hiroshi (2019-04-04 17:01:27)
> diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c
> index 1b7df115eb60..0fd2f0091ea4 100644
> --- a/arch/mips/ralink/clk.c
> +++ b/arch/mips/ralink/clk.c
> @@ -15,8 +15,15 @@
>
> #include <asm/time.h>
>
> +#if IS_ENABLED(CONFIG_COMMON_CLK)
Please drop this unless you need it, and move the linux/ include above
any asm/ includes.
> +#include <linux/clk-provider.h>
> +#endif
> +
> #include "common.h"
>
> +
> +#if !IS_ENABLED(CONFIG_COMMON_CLK)
> +
> struct clk {
> struct clk_lookup cl;
> unsigned long rate;
> @@ -72,6 +79,28 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
> }
> EXPORT_SYMBOL_GPL(clk_round_rate);
>
> +#else /* CONFIG_COMMON_CLK */
> +
> +struct clk_hw * __init add_sys_clkdev(const char *id, unsigned long rate)
> +{
> + struct clk_hw *clk_hw;
> + int err;
> +
> + clk_hw = clk_hw_register_fixed_rate(NULL, id, NULL, 0, rate);
> + if (IS_ERR(clk_hw))
> + panic("failed to allocate %s clock structure", id);
This one panics but the other just continues. Why can't we continue from
here too? Is clkdev actually used by anything on this platform?
> +
> + err = clk_hw_register_clkdev(clk_hw, NULL, id);
> + if (err) {
> + pr_err("unable to register %s clock device\n", id);
> + clk_hw = ERR_PTR(err);
> + }
> +
> + return clk_hw;
> +}
> +
> +#endif /* CONFIG_COMMON_CLK */
> +
> void __init plat_time_init(void)
> {
> struct clk *clk;
> diff --git a/arch/mips/ralink/mt7620.c b/arch/mips/ralink/mt7620.c
> index c1ce6f43642b..74aed8f7e502 100644
> --- a/arch/mips/ralink/mt7620.c
> +++ b/arch/mips/ralink/mt7620.c
> @@ -504,6 +505,12 @@ mt7620_get_sys_rate(unsigned long cpu_rate)
> return cpu_rate / div;
> }
>
> +static struct clk_hw_onecell_data *clk_data;
> +
> +#define RFMT(label) label ":%lu.%03luMHz "
> +#define RINT(x) ((x) / 1000000)
> +#define RFRAC(x) (((x) / 1000) % 1000)
Is this necessary to move?
> +
> void __init ralink_clk_init(void)
> {
> unsigned long xtal_rate;
> @@ -586,6 +593,31 @@ void __init ralink_clk_init(void)
> }
> }
>
> +#undef RFRAC
> +#undef RINT
> +#undef RFMT
> +
> +static int mt7620_pll_probe(struct platform_device *pdev)
> +{
> + of_clk_add_hw_provider(pdev->dev.of_node,
> + of_clk_hw_onecell_get, clk_data);
> + return 0;
Why not return of_clk_add_hw_provider()?
> +}
> +
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC v2 1/5] clk: mips: ralink: add Ralink MIPS gating clock driver
2019-04-05 0:01 ` [RFC v2 1/5] clk: mips: ralink: add Ralink MIPS gating clock driver NOGUCHI Hiroshi
@ 2019-04-25 19:27 ` Stephen Boyd
2019-05-01 10:58 ` NOGUCHI Hiroshi
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2019-04-25 19:27 UTC (permalink / raw)
To: John Crispin, NOGUCHI Hiroshi
Cc: Michael Turquette, Rob Herring, Mark Rutland, linux-mips,
linux-clk, NOGUCHI Hiroshi
Quoting NOGUCHI Hiroshi (2019-04-04 17:01:25)
> This patch adds clock gating driver for Ralink/Mediatek MIPS Socs.
Please read Documentation/process/submitting-patches.rst and look at
this part of it in detail:
Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour.
>
> Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
> ---
> drivers/clk/Kconfig | 6 ++
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-rt2880.c | 199 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 206 insertions(+)
> create mode 100644 drivers/clk/clk-rt2880.c
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index f96c7f39ab7e..dbfdf1bcdc6c 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -290,6 +290,12 @@ config COMMON_CLK_BD718XX
> This driver supports ROHM BD71837 and ROHM BD71847
> PMICs clock gates.
>
> +config COMMON_CLK_RT2880
> + bool "Clock driver for Mediatek/Ralink MIPS SoC Family"
> + depends on COMMON_CLK && OF
Does it really depend on OF? Or just depends on it at runtime? If it's a
runtime requirement, perhaps make it just be depends on COMMON_CLK.
> + help
> + This driver support Mediatek/Ralink MIPS SoCs' clock gates.
> +
> config COMMON_CLK_FIXED_MMIO
> bool "Clock driver for Memory Mapped Fixed values"
> diff --git a/drivers/clk/clk-rt2880.c b/drivers/clk/clk-rt2880.c
> new file mode 100644
> index 000000000000..bcdb4c1486d3
> --- /dev/null
> +++ b/drivers/clk/clk-rt2880.c
> @@ -0,0 +1,199 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 NOGUCHI Hiroshi <drvlabo@gmail.com>
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/platform_device.h>
Sort these includes alphabetically?
> +
> +
> +/* clock configuration 1 */
Comment seems useless.
> +#define SYSC_REG_CLKCFG1 0x30
> +
> +#define GATE_CLK_NUM 32
> +
> +struct rt2880_gate {
> + struct clk_hw hw;
> + u8 shift;
> +};
> +
> +#define to_rt2880_gate(_hw) container_of(_hw, struct rt2880_gate, hw)
> +
> +static struct clk_hw_onecell_data *clk_data;
> +static struct regmap *syscon_regmap;
Can these be per some driver instance instead of globals? If they have
to stay global, please make a better name so that they're not so
generic.
> +
> +static int rt2880_gate_enable(struct clk_hw *hw)
> +{
> + struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
> + u32 val = BIT(clk_gate->shift);
> +
> + return regmap_update_bits(syscon_regmap, SYSC_REG_CLKCFG1, val, val);
> +}
> +
> +static void rt2880_gate_disable(struct clk_hw *hw)
> +{
> + struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
> + u32 val = BIT(clk_gate->shift);
> +
> + regmap_update_bits(syscon_regmap, SYSC_REG_CLKCFG1, val, 0);
> +}
> +
> +static int rt2880_gate_is_enabled(struct clk_hw *hw)
> +{
> + struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
> + unsigned int rdval;
> +
> + if (regmap_read(syscon_regmap, SYSC_REG_CLKCFG1, &rdval))
> + return 0;
> +
> + return rdval & BIT(clk_gate->shift);
> +}
> +
> +static const struct clk_ops rt2880_gate_ops = {
> + .enable = rt2880_gate_enable,
> + .disable = rt2880_gate_disable,
> + .is_enabled = rt2880_gate_is_enabled,
> +};
> +
> +static struct clk_hw * __init
> +rt2880_register_gate(const char *name, const char *parent_name, u8 shift)
> +{
> + struct rt2880_gate *clk_gate;
> + struct clk_init_data init;
> + int err;
> + const char *_parent_names[1] = { parent_name };
This isn't necessary.
> +
> + clk_gate = kzalloc(sizeof(*clk_gate), GFP_KERNEL);
> + if (!clk_gate)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &rt2880_gate_ops;
> + init.flags = 0;
> + init.parent_names = parent_name ? _parent_names : NULL;
Just use &parent_name instead.
> + init.num_parents = parent_name ? 1 : 0;
> +
> + clk_gate->hw.init = &init;
> + clk_gate->shift = shift;
> +
> + err = clk_hw_register(NULL, &clk_gate->hw);
Please pass in the device during registration.
> + if (err) {
> + kfree(clk_gate);
> + return ERR_PTR(err);
> + }
> +
> + return &clk_gate->hw;
> +}
> +
> +static struct clk_hw *
> +rt2880_clk_hw_get(struct of_phandle_args *clkspec, void *data)
> +{
> + struct clk_hw_onecell_data *hw_data = data;
> + unsigned int idx = clkspec->args[0];
> + int i;
> +
> + if (idx >= GATE_CLK_NUM)
> + goto err;
> +
> + for (i = 0; i < hw_data->num; i++)
> + if (idx == to_rt2880_gate(hw_data->hws[i])->shift)
> + break;
> + if (i >= hw_data->num)
> + goto err;
> +
> + return hw_data->hws[i];
> +
> +err:
> + pr_err("%s: invalid index %u\n", __func__, idx);
> + return ERR_PTR(-EINVAL);
> +}
> +
> +static int __init _rt2880_clkctrl_init_dt(struct device_node *np)
Drop __init, this is called from probe(). Also, please just collapse it
into the driver probe and use platform device APIs throughout this
function.
> +{
> + struct clk_hw *clk_hw;
> + const char *name;
> + const char *parent_name;
> + u32 val;
> + int cnt;
> + int num;
> + int i;
> + int idx;
> +
> + syscon_regmap = syscon_regmap_lookup_by_phandle(np, "ralink,sysctl");
> + if (IS_ERR(syscon_regmap)) {
> + pr_err("rt2880-clock: could not get syscon regmap.\n");
> + return PTR_ERR(syscon_regmap);
> + }
> +
> + cnt = of_property_count_u32_elems(np, "clock-indices");
> + if (cnt < 0) {
> + pr_err("rt2880-clock: clock-indices property is invalid.\n");
> + return cnt;
> + }
> +
> + num = 0;
> + for (i = 0; i < cnt; i++) {
> + if (of_property_read_u32_index(np, "clock-indices", i, &val))
I'm a little lost one what the indices are for? Why is the number space
being folded like this?
> + break;
> + if (val < GATE_CLK_NUM)
> + num++;
> + }
> + if ((num <= 0) || (num > GATE_CLK_NUM)) {
Please remove useless parenthesis.
> + pr_err("rt2880-clock: clock-indices property is invalid.\n");
Drop the full-stop on all error messages please.
> + return -EINVAL;
> + }
> +
> + clk_data = kzalloc(struct_size(clk_data, hws, num), GFP_KERNEL);
> + if (!clk_data)
> + return -ENOMEM;
> + clk_data->num = num;
> +
> + idx = 0;
> + for (i = 0; (i < cnt) && (idx < num); i++) {
Please drop useless parenthesis.
> + if (of_property_read_u32_index(np, "clock-indices", i, &val))
> + break;
> + if ((int)val >= GATE_CLK_NUM)
Why are we casting to int?
> + continue;
> +
> + if (of_property_read_string_index(np, "clock-output-names",
> + i, &name))
> + break;
> +
> + parent_name = of_clk_get_parent_name(np, i);
> +
> + clk_hw = rt2880_register_gate(name, parent_name, val);
> + if (IS_ERR_OR_NULL(clk_hw)) {
When does it return NULL? Should probably just be IS_ERR() check here.
> + pr_err("rt2880-clock: could not register clock for %s\n",
Consider dev_err() so that the prefix (rt2880-clock:) can be dropped.
> + name);
> + continue;
> + }
> + clk_data->hws[idx] = clk_hw;
> + idx++;
> + }
> +
> + of_clk_add_hw_provider(np, rt2880_clk_hw_get, clk_data);
Why not return of_clk_add_hw_provider()?
> +
> + return 0;
> +}
> +
> +static int rt2880_clkctrl_probe(struct platform_device *pdev)
> +{
> + return _rt2880_clkctrl_init_dt(pdev->dev.of_node);
> +}
> +
> +static const struct of_device_id rt2880_clkctrl_ids[] = {
> + { .compatible = "ralink,rt2880-clock" },
> + { }
> +};
> +
> +static struct platform_driver rt2880_clkctrl_driver = {
> + .probe = rt2880_clkctrl_probe,
> + .driver = {
> + .name = "rt2880-clock",
> + .of_match_table = rt2880_clkctrl_ids,
> + },
> +};
> +builtin_platform_driver(rt2880_clkctrl_driver);
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC v2 2/5] dt-bindings: clk: add document for ralink clock driver
2019-04-05 0:01 ` [RFC v2 2/5] dt-bindings: clk: add document for ralink " NOGUCHI Hiroshi
@ 2019-04-25 19:29 ` Stephen Boyd
2019-05-01 11:33 ` NOGUCHI Hiroshi
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2019-04-25 19:29 UTC (permalink / raw)
To: John Crispin, NOGUCHI Hiroshi
Cc: Michael Turquette, Rob Herring, Mark Rutland, linux-mips,
linux-clk, NOGUCHI Hiroshi
Quoting NOGUCHI Hiroshi (2019-04-04 17:01:26)
> Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
> ---
> .../bindings/clock/ralink,rt2880-clock.txt | 58 +++++++++++++++++++
> 1 file changed, 58 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt b/Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
> new file mode 100644
> index 000000000000..2fc0d622e01e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
> @@ -0,0 +1,58 @@
> +* Clock bindings for Ralink/Mediatek MIPS based SoCs
> +
> +Required properties:
> + - compatible: must be "ralink,rt2880-clock"
> + - #clock-cells: must be 1
> + - ralink,sysctl: a phandle to a ralink syscon register region
> + - clock-indices: identifying number.
> + These must correspond to the bit number in CLKCFG1.
These look like driver level details that we're putting in the DT so we
can compress the number space that consumers use. Is that right? If so,
I don't get it. Can we not use this property?
> + Clock consumers use one of them as #clock-cells index.
> + - clock-output-names: array of gating clocks' names
> + - clocks: array of phandles which points the parent clock
> + for gating clocks.
> + If gating clock does not need parent clock linkage,
> + we bind to dummy clock whose frequency is zero.
> +
> +
> +Example:
> +
> +/* dummy parent clock node */
> +dummy_ck: dummy_ck {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <0>;
> +};
Would this ever exist in practice? If not, please remove from the
example so we don't get the wrong idea.
> +
> +clkctrl: clkctrl {
> + compatible = "ralink,rt2880-clock";
> + #clock-cells = <1>;
> + ralink,sysctl = <&sysc>;
> +
> + clock-indices =
> + <12>,
> + <16>, <17>, <18>, <19>,
> + <20>,
> + <26>;
> + clock-output-names =
> + "uart0",
> + "i2c", "i2s", "spi", "uart1",
> + "uart2",
> + "pcie0";
> + clocks =
> + <&pll MT7620_CLK_PERIPH>,
> + <&pll MT7620_CLK_PERIPH>, <&pll MT7620_CLK_PCMI2S>, <&pll MT7620_CLK_SYS>, <&pll MT7620_CLK_PERIPH>,
> + <&pll MT7620_CLK_PERIPH>,
> + <&dummy_ck>;
> + };
> +};
> +
> +/* consumer which refers "uart0" clock */
> +uart0: uartlite@c00 {
> + compatible = "ns16550a";
> + reg = <0xc00 0x100>;
> +
> + clocks = <&clkctrl 12>;
So 12 matches in indices and then that is really "uart0" clk?
> + clock-names = "uart0";
> +
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC v2 1/5] clk: mips: ralink: add Ralink MIPS gating clock driver
2019-04-25 19:27 ` Stephen Boyd
@ 2019-05-01 10:58 ` NOGUCHI Hiroshi
0 siblings, 0 replies; 12+ messages in thread
From: NOGUCHI Hiroshi @ 2019-05-01 10:58 UTC (permalink / raw)
To: Stephen Boyd
Cc: John Crispin, Michael Turquette, Rob Herring, Mark Rutland,
linux-mips, linux-clk
Thanks for suggestions.
>> +{
>> + struct clk_hw *clk_hw;
>> + const char *name;
>> + const char *parent_name;
>> + u32 val;
>> + int cnt;
>> + int num;
>> + int i;
>> + int idx;
>> +
>> + syscon_regmap = syscon_regmap_lookup_by_phandle(np, "ralink,sysctl");
>> + if (IS_ERR(syscon_regmap)) {
>> + pr_err("rt2880-clock: could not get syscon regmap.\n");
>> + return PTR_ERR(syscon_regmap);
>> + }
>> +
>> + cnt = of_property_count_u32_elems(np, "clock-indices");
>> + if (cnt < 0) {
>> + pr_err("rt2880-clock: clock-indices property is invalid.\n");
>> + return cnt;
>> + }
>> +
>> + num = 0;
>> + for (i = 0; i < cnt; i++) {
>> + if (of_property_read_u32_index(np, "clock-indices", i, &val))
>
> I'm a little lost one what the indices are for? Why is the number space
> being folded like this?
I want to let the clock cell index match with the bit number in the
gate control register.
Is my "clock-indices" usage wrong ?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC v2 2/5] dt-bindings: clk: add document for ralink clock driver
2019-04-25 19:29 ` Stephen Boyd
@ 2019-05-01 11:33 ` NOGUCHI Hiroshi
2019-05-02 21:42 ` Stephen Boyd
0 siblings, 1 reply; 12+ messages in thread
From: NOGUCHI Hiroshi @ 2019-05-01 11:33 UTC (permalink / raw)
To: Stephen Boyd
Cc: John Crispin, Michael Turquette, Rob Herring, Mark Rutland,
linux-mips, linux-clk
On 2019/04/26 4:29, Stephen Boyd wrote:
>> +Required properties:
>> + - compatible: must be "ralink,rt2880-clock"
>> + - #clock-cells: must be 1
>> + - ralink,sysctl: a phandle to a ralink syscon register region
>> + - clock-indices: identifying number.
>> + These must correspond to the bit number in CLKCFG1.
>
> These look like driver level details that we're putting in the DT so we
> can compress the number space that consumers use. Is that right? If so,
> I don't get it. Can we not use this property?
I understand that the bit numbers in clock gating register are hardware
resource informations.
Therefore, it is not strange that they are described in DT, I think.
>> + Clock consumers use one of them as #clock-cells index.
>> + - clock-output-names: array of gating clocks' names
>> + - clocks: array of phandles which points the parent clock
>> + for gating clocks.
>> + If gating clock does not need parent clock linkage,
>> + we bind to dummy clock whose frequency is zero.
>> +
>> +
>> +Example:
>> +
>> +/* dummy parent clock node */
>> +dummy_ck: dummy_ck {
>> + #clock-cells = <0>;
>> + compatible = "fixed-clock";
>> + clock-frequency = <0>;
>> +};
>
> Would this ever exist in practice? If not, please remove from the
> example so we don't get the wrong idea.
I referred to arch/arm/boot/dts/.
omap24xx-clocks.dtsi : defines dummy_ck
omap2420-clocks.dtsi : refers dummy_ck
In practice, There is no problem in specifying another existing clock,
eg MT7620_CLK_PERIPH which is always active.
>> +
>> +clkctrl: clkctrl {
>> + compatible = "ralink,rt2880-clock";
>> + #clock-cells = <1>;
>> + ralink,sysctl = <&sysc>;
>> +
>> + clock-indices =
>> + <12>,
>> + <16>, <17>, <18>, <19>,
>> + <20>,
>> + <26>;
>> + clock-output-names =
>> + "uart0",
>> + "i2c", "i2s", "spi", "uart1",
>> + "uart2",
>> + "pcie0";
>> + clocks =
>> + <&pll MT7620_CLK_PERIPH>,
>> + <&pll MT7620_CLK_PERIPH>, <&pll MT7620_CLK_PCMI2S>, <&pll MT7620_CLK_SYS>, <&pll MT7620_CLK_PERIPH>,
>> + <&pll MT7620_CLK_PERIPH>,
>> + <&dummy_ck>;
>> + };
>> +};
>> +
>> +/* consumer which refers "uart0" clock */
>> +uart0: uartlite@c00 {
>> + compatible = "ns16550a";
>> + reg = <0xc00 0x100>;
>> +
>> + clocks = <&clkctrl 12>;
>
> So 12 matches in indices and then that is really "uart0" clk?
>
>> + clock-names = "uart0";
>> +
That is right.
rt2880-clock driver is implemented to let clock cell indices match
indcies in "clock-indices" property.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC v2 2/5] dt-bindings: clk: add document for ralink clock driver
2019-05-01 11:33 ` NOGUCHI Hiroshi
@ 2019-05-02 21:42 ` Stephen Boyd
0 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2019-05-02 21:42 UTC (permalink / raw)
To: NOGUCHI Hiroshi
Cc: John Crispin, Michael Turquette, Rob Herring, Mark Rutland,
linux-mips, linux-clk
Quoting NOGUCHI Hiroshi (2019-05-01 04:33:24)
>
>
> On 2019/04/26 4:29, Stephen Boyd wrote:
> >> +Required properties:
> >> + - compatible: must be "ralink,rt2880-clock"
> >> + - #clock-cells: must be 1
> >> + - ralink,sysctl: a phandle to a ralink syscon register region
> >> + - clock-indices: identifying number.
> >> + These must correspond to the bit number in CLKCFG1.
> >
> > These look like driver level details that we're putting in the DT so we
> > can compress the number space that consumers use. Is that right? If so,
> > I don't get it. Can we not use this property?
>
> I understand that the bit numbers in clock gating register are hardware
> resource informations.
> Therefore, it is not strange that they are described in DT, I think.
>
>
> >> + Clock consumers use one of them as #clock-cells index.
> >> + - clock-output-names: array of gating clocks' names
> >> + - clocks: array of phandles which points the parent clock
> >> + for gating clocks.
> >> + If gating clock does not need parent clock linkage,
> >> + we bind to dummy clock whose frequency is zero.
> >> +
> >> +
> >> +Example:
> >> +
> >> +/* dummy parent clock node */
> >> +dummy_ck: dummy_ck {
> >> + #clock-cells = <0>;
> >> + compatible = "fixed-clock";
> >> + clock-frequency = <0>;
> >> +};
> >
> > Would this ever exist in practice? If not, please remove from the
> > example so we don't get the wrong idea.
>
> I referred to arch/arm/boot/dts/.
> omap24xx-clocks.dtsi : defines dummy_ck
> omap2420-clocks.dtsi : refers dummy_ck
>
>
> In practice, There is no problem in specifying another existing clock,
> eg MT7620_CLK_PERIPH which is always active.
Ok. Please don't add things that don't exist into the example like dummy
clks. Sometimes people copy the examples directly and this can lead to
errors in the resulting DTBs.
>
>
> >> +
> >> +clkctrl: clkctrl {
> >> + compatible = "ralink,rt2880-clock";
> >> + #clock-cells = <1>;
> >> + ralink,sysctl = <&sysc>;
> >> +
> >> + clock-indices =
> >> + <12>,
> >> + <16>, <17>, <18>, <19>,
> >> + <20>,
> >> + <26>;
> >> + clock-output-names =
> >> + "uart0",
> >> + "i2c", "i2s", "spi", "uart1",
> >> + "uart2",
> >> + "pcie0";
> >> + clocks =
> >> + <&pll MT7620_CLK_PERIPH>,
> >> + <&pll MT7620_CLK_PERIPH>, <&pll MT7620_CLK_PCMI2S>, <&pll MT7620_CLK_SYS>, <&pll MT7620_CLK_PERIPH>,
> >> + <&pll MT7620_CLK_PERIPH>,
> >> + <&dummy_ck>;
> >> + };
> >> +};
> >> +
> >> +/* consumer which refers "uart0" clock */
> >> +uart0: uartlite@c00 {
> >> + compatible = "ns16550a";
> >> + reg = <0xc00 0x100>;
> >> +
> >> + clocks = <&clkctrl 12>;
> >
> > So 12 matches in indices and then that is really "uart0" clk?
> >
> >> + clock-names = "uart0";
> >> +
>
> That is right.
> rt2880-clock driver is implemented to let clock cell indices match
> indcies in "clock-indices" property.
Usually the binding has a bunch of #defines for the clks, instead of
using raw integers to indicate which clock it is. Then consumers point
to that clk via <&phandle DEFINE>, similar to your 'clocks' property
above in the clkctrl node. Then a clk provider driver will remap that
DEFINE to a clk_hw structure and the driver contains the register
offsets and bits to twiddle to control the clk.
It looks like here we put those register offsets and bits to twiddle in
DT as clock-indices, and then consumers are supposed to know what
clock-indices to match based on the register bits of the clk? That's a
novel approach that may work here but doesn't really scale. I guess it's
OK, but I'd prefer to see #defines even for the clock-indices like
RT2800_UART0 or RT2800_I2C.
After that, it seems risky to put the details of what bits to twiddle in
DT because it expresses driver level hardware details in a place where
we might not be able to as easily modify or fix those bits if certain
clks don't get tested. If we had only specified the provider/consumer
part of the binding (i.e. the numbers in the clk specifier) we wouldn't
need to worry as much because we could fix those driver details in the
driver.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-05-02 21:42 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-05 0:01 [RFC v2 0/5] MIPS: ralink: peripheral clock gating driver NOGUCHI Hiroshi
2019-04-05 0:01 ` [RFC v2 1/5] clk: mips: ralink: add Ralink MIPS gating clock driver NOGUCHI Hiroshi
2019-04-25 19:27 ` Stephen Boyd
2019-05-01 10:58 ` NOGUCHI Hiroshi
2019-04-05 0:01 ` [RFC v2 2/5] dt-bindings: clk: add document for ralink " NOGUCHI Hiroshi
2019-04-25 19:29 ` Stephen Boyd
2019-05-01 11:33 ` NOGUCHI Hiroshi
2019-05-02 21:42 ` Stephen Boyd
2019-04-05 0:01 ` [RFC v2 3/5] mips: ralink: mt7620/76x8 use common clk framework NOGUCHI Hiroshi
2019-04-25 19:18 ` Stephen Boyd
2019-04-05 0:01 ` [RFC v2 4/5] mips: ralink: mt76x8: add nodes for clocks NOGUCHI Hiroshi
2019-04-05 0:01 ` [RFC v2 5/5] mips: ralink: mt7620: " NOGUCHI Hiroshi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).