On Thu 04 Apr 2024 at 18:59, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:
On 04/04/2024 10:13, Jerome Brunet wrote:
On Wed 03 Apr 2024 at 09:46, Neil Armstrong <neil.armstrong@xxxxxxxxxx>
wrote:
The VCLK and VCLK_DIV clocks have supplementary bits.The checkpatch warning is still there. Is it a choice or a mistake ?
The VCLK gate has a "SOFT RESET" bit to toggle after the whole
VCLK sub-tree rate has been set, this is implemented in
the gate enable callback.
The VCLK_DIV clocks as enable and reset bits used to disable
and reset the divider, associated with CLK_SET_RATE_GATE it ensures
the rate is set while the divider is disabled and in reset mode.
The VCLK_DIV enable bit isn't implemented as a gate since it's part
of the divider logic and vendor does this exact sequence to ensure
the divider is correctly set.
Documentation says "GPL v2" exists for historic reason which seems to
hint "GPL" is preferred, and I suppose this is why checkpatch warns for
it.
Well I didn't see this warning, this is what I fixed:
$ scripts/checkpatch.pl --strict drivers/clk/meson/vclk.c
CHECK: Alignment should match open parenthesis
#63: FILE: drivers/clk/meson/vclk.c:63:
+static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
+ unsigned long prate)
CHECK: Alignment should match open parenthesis
#73: FILE: drivers/clk/meson/vclk.c:73:
+static int meson_vclk_div_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
CHECK: Alignment should match open parenthesis
#83: FILE: drivers/clk/meson/vclk.c:83:
+static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
I would not ask a respin solely for this. It's nice to fix it but I was
mostly after the warning TBH.
<snip>
It seems that checking a commit triggers an extra check....
$ scripts/checkpatch.pl --strict -G 1bac9f6aa3c3
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#58:
new file mode 100644
<snip>
WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity")
#203: FILE: drivers/clk/meson/vclk.c:141:
+MODULE_LICENSE("GPL v2");
Hum, I'm running checkpatch against the mail itself, not the commit. I
still get the warning
<snip>
Neil
Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
---
drivers/clk/meson/Kconfig | 4 ++
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/vclk.c | 141 +++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/meson/vclk.h | 51 ++++++++++++++++
4 files changed, 197 insertions(+)
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 29ffd14d267b..8a9823789fa3 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
tristate
select COMMON_CLK_MESON_REGMAP
+config COMMON_CLK_MESON_VCLK
+ tristate
+ select COMMON_CLK_MESON_REGMAP
+
config COMMON_CLK_MESON_CLKC_UTILS
tristate
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 9ee4b954c896..9ba43fe7a07a 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
+obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
# Amlogic Clock controllers
diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
new file mode 100644
index 000000000000..45dc216941ea
--- /dev/null
+++ b/drivers/clk/meson/vclk.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2024 Neil Armstrong <neil.armstrong@xxxxxxxxxx>
+ */
+
+#include <linux/module.h>
+#include "vclk.h"
+
+/* The VCLK gate has a supplementary reset bit to pulse after ungating */
+
+static inline struct meson_vclk_gate_data *
+clk_get_meson_vclk_gate_data(struct clk_regmap *clk)
+{
+ return (struct meson_vclk_gate_data *)clk->data;
+}
+
+static int meson_vclk_gate_enable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
+
+ meson_parm_write(clk->map, &vclk->enable, 1);
+
+ /* Do a reset pulse */
+ meson_parm_write(clk->map, &vclk->reset, 1);
+ meson_parm_write(clk->map, &vclk->reset, 0);
+
+ return 0;
+}
+
+static void meson_vclk_gate_disable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
+
+ meson_parm_write(clk->map, &vclk->enable, 0);
+}
+
+static int meson_vclk_gate_is_enabled(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
+
+ return meson_parm_read(clk->map, &vclk->enable);
+}
+
+const struct clk_ops meson_vclk_gate_ops = {
+ .enable = meson_vclk_gate_enable,
+ .disable = meson_vclk_gate_disable,
+ .is_enabled = meson_vclk_gate_is_enabled,
+};
+EXPORT_SYMBOL_GPL(meson_vclk_gate_ops);
+
+/* The VCLK Divider has supplementary reset & enable bits */
+
+static inline struct meson_vclk_div_data *
+clk_get_meson_vclk_div_data(struct clk_regmap *clk)
+{
+ return (struct meson_vclk_div_data *)clk->data;
+}
+
+static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
+ unsigned long prate)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ return divider_recalc_rate(hw, prate, meson_parm_read(clk->map, &vclk->div),
+ vclk->table, vclk->flags, vclk->div.width);
+}
+
+static int meson_vclk_div_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ return divider_determine_rate(hw, req, vclk->table, vclk->div.width,
+ vclk->flags);
+}
+
+static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+ int ret;
+
+ ret = divider_get_val(rate, parent_rate, vclk->table, vclk->div.width,
+ vclk->flags);
+ if (ret < 0)
+ return ret;
+
+ meson_parm_write(clk->map, &vclk->div, ret);
+
+ return 0;
+};
+
+static int meson_vclk_div_enable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ /* Unreset the divider when ungating */
+ meson_parm_write(clk->map, &vclk->reset, 0);
+ meson_parm_write(clk->map, &vclk->enable, 1);
+
+ return 0;
+}
+
+static void meson_vclk_div_disable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ /* Reset the divider when gating */
+ meson_parm_write(clk->map, &vclk->enable, 0);
+ meson_parm_write(clk->map, &vclk->reset, 1);
+}
+
+static int meson_vclk_div_is_enabled(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ return meson_parm_read(clk->map, &vclk->enable);
+}
+
+const struct clk_ops meson_vclk_div_ops = {
+ .recalc_rate = meson_vclk_div_recalc_rate,
+ .determine_rate = meson_vclk_div_determine_rate,
+ .set_rate = meson_vclk_div_set_rate,
+ .enable = meson_vclk_div_enable,
+ .disable = meson_vclk_div_disable,
+ .is_enabled = meson_vclk_div_is_enabled,
+};
+EXPORT_SYMBOL_GPL(meson_vclk_div_ops);
+
+MODULE_DESCRIPTION("Amlogic vclk clock driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@xxxxxxxxxx>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h
new file mode 100644
index 000000000000..20b0b181db09
--- /dev/null
+++ b/drivers/clk/meson/vclk.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2024 Neil Armstrong <neil.armstrong@xxxxxxxxxx>
+ */
+
+#ifndef __VCLK_H
+#define __VCLK_H
+
+#include "clk-regmap.h"
+#include "parm.h"
+
+/**
+ * struct meson_vclk_gate_data - vclk_gate regmap backed specific data
+ *
+ * @enable: vclk enable field
+ * @reset: vclk reset field
+ * @flags: hardware-specific flags
+ *
+ * Flags:
+ * Same as clk_gate except CLK_GATE_HIWORD_MASK which is ignored
+ */
+struct meson_vclk_gate_data {
+ struct parm enable;
+ struct parm reset;
+ u8 flags;
+};
+
+extern const struct clk_ops meson_vclk_gate_ops;
+
+/**
+ * struct meson_vclk_div_data - vclk_div regmap back specific data
+ *
+ * @div: divider field
+ * @enable: vclk divider enable field
+ * @reset: vclk divider reset field
+ * @table: array of value/divider pairs, last entry should have div = 0
+ *
+ * Flags:
+ * Same as clk_divider except CLK_DIVIDER_HIWORD_MASK which is ignored
+ */
+struct meson_vclk_div_data {
+ struct parm div;
+ struct parm enable;
+ struct parm reset;
+ const struct clk_div_table *table;
+ u8 flags;
+};
+
+extern const struct clk_ops meson_vclk_div_ops;
+
+#endif /* __VCLK_H */