Hi Stephen,
thanks for the review.
Am 2019-12-24 09:05, schrieb Stephen Boyd:
Quoting Michael Walle (2019-12-09 15:33:05)
diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
new file mode 100644
index 000000000000..b92054d15ab1
--- /dev/null
+++ b/drivers/clk/clk-fsl-sai.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Freescale SAI BCLK as a generic clock driver
+ *
+ * Copyright 2019 Kontron Europe GmbH
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#define I2S_CSR 0x00
+#define I2S_CR2 0x08
+#define CSR_BCE_BIT 28
+#define CR2_BCD BIT(24)
+#define CR2_DIV_SHIFT 0
+#define CR2_DIV_WIDTH 8
+
+struct fsl_sai_clk {
+ struct clk_divider div;
+ struct clk_gate gate;
+ spinlock_t lock;
+};
+
+static void __init fsl_sai_clk_setup(struct device_node *node)
+{
+ const char *clk_name = node->name;
+ struct fsl_sai_clk *sai_clk;
+ unsigned int num_parents;
+ const char *parent_name;
+ void __iomem *base;
+ struct clk_hw *hw;
+
+ num_parents = of_clk_get_parent_count(node);
+ if (!num_parents) {
+ pr_err("%s: no parent found", clk_name);
+ return;
+ }
+
+ parent_name = of_clk_get_parent_name(node, 0);
Could this use the new way of specifying clk parents so that we don't
have to query DT for parent names and just let the core framework do it
whenever it needs to?
you mean specifying parent_data with .index = 0? Seems like
clk_composite
does not support this. The parent can only be specified by supplying the
clock names.
I could add that in a separate patch. What do you think about the
following new functions, where a driver can use parent_data instead
of parent_names.
+struct clk *clk_register_composite_pdata(struct device *dev, const char
*name,
+ const struct clk_parent_data *parent_data,
+ struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+ struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
+ struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+ unsigned long flags);
+struct clk_hw *clk_hw_register_composite_pdata(struct device *dev,
+ const char *name, const struct clk_parent_data
*parent_data,
+ struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+ struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
+ struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+ unsigned long flags);
+
+ sai_clk = kzalloc(sizeof(*sai_clk), GFP_KERNEL);
+ if (!sai_clk)
+ return;
+
+ base = of_iomap(node, 0);
+ if (base == NULL) {
+ pr_err("%s: failed to map register space", clk_name);
+ goto err;
+ }
+
+ spin_lock_init(&sai_clk->lock);
+
+ sai_clk->gate.reg = base + I2S_CSR;
+ sai_clk->gate.bit_idx = CSR_BCE_BIT;
+ sai_clk->gate.lock = &sai_clk->lock;
+
+ sai_clk->div.reg = base + I2S_CR2;
+ sai_clk->div.shift = CR2_DIV_SHIFT;
+ sai_clk->div.width = CR2_DIV_WIDTH;
+ sai_clk->div.lock = &sai_clk->lock;
+
+ /* set clock direction, we are the BCLK master */
Should this configuration come from DT somehow?
No, we are always master, because as a slave, there would be no clock
output ;)
+ writel(CR2_BCD, base + I2S_CR2);
+
+ hw = clk_hw_register_composite(NULL, clk_name, &parent_name,
1,
+ NULL, NULL,
+ &sai_clk->div.hw,
&clk_divider_ops,
+ &sai_clk->gate.hw,
&clk_gate_ops,
+ CLK_SET_RATE_GATE);
+ if (IS_ERR(hw))
+ goto err;
+
+ of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw);
+
+ return;
+
+err:
+ kfree(sai_clk);
+}
+
+CLK_OF_DECLARE(fsl_sai_clk, "fsl,vf610-sai-clock",
fsl_sai_clk_setup);
Is there a reason this can't be a platform device driver?
I don't think so, the user will be a sound codec for now. I'll convert
it
to a platform device, in that case I could also use the devm_ variants.
-michael
